Skip to content

Stale-read detectors for the surviving pools: ASan poisoning + make jit-checked (#1665) - #1705

Merged
InauguralPhysicist merged 3 commits into
mainfrom
detect-1665
Oct 10, 2026
Merged

InauguralPhysicist merged 3 commits into
mainfrom
detect-1665

Conversation

@InauguralPhysicist

Copy link
Copy Markdown
Collaborator

Part of #1665: the detector items for the two recycling layers that stay (the NUM freelist and call-env recycling).

What it adds

  • ASan whole-object poisoning (compiled only under __SANITIZE_ADDRESS__):
    • Parked NUMs are poisoned whole except the 8-byte freelist link. Under ASan only, the link moves from data+0 to data+8, so a stale read of the number is caught.
    • Parked call envs: values, names, assign_counts and the hash arrays are poisoned, plus the Env struct except parent, which vm_take_call_env reads to match the closure.
    • Poison is lifted on reuse, on take, by the collector walk (and put back afterwards) and in chunk_free.
    • All 29 release objects are byte-identical to the base build, checked with cmp.
  • make jit-checked (EIGS_JIT_CHECKED):
    • Emitted GET_LOCAL, SET_LOCAL and LOCAL_DOT_GET/SET check that the cached values base still matches the frame env and that slot < count.
    • The EnvIC hit paths of GET_NAME and SET_NAME check slot < count.
    • A failed check aborts and names the read site.
    • Default emission is unchanged. A CI step runs the JIT-tier sections under this build.
  • Section [0fg]: three planted stale reads (a parked num, a parked env slot, a parked env field) and a clean control.

Evidence

  • Plants:
    • main's ASan: all three undetected (rc 0);
    • this build: all three reported as use-after-poison at the planted address;
    • the control passes clean.
  • Mutation train: eight mutants, each removing one poison or unpoison site (GC unpoison/re-poison, take, pop, park, NUM park, chunk_free, link-word position). Each turns [0fg] red.
  • Heap use-after-free: under-arity call leaves an unbound param slot that a GC cull frees (v0.45.0, minimal repro) #1661 revert under jit-checked: EIGS_JIT_CHECKED: local slot out of range (1 vs 1) at GET_LOCAL offset 5 in chunk 'g' line 6, rc 134, on all four reproducers at both JIT tiers. The release build with the same revert prints ok.
  • Gates:
    • jit-smoke passes;
    • jit_diff passes on release and on jit-checked (256 programs);
    • JIT sections under jit-checked: 29/29;
    • ASan with leak detection: 448/448 over 34 changed sections plus 31/31 core sections, with no poison report and no leaks;
    • TSan: 57/57;
    • freestanding-check, werror_switch_check, suite labels and the changelog pass.
  • CI runs the full release and ASan suites. The builder did not run them; it worked to a long-lane budget.

Caveats

Review

Built by a Claude builder after three Codex Cloud attempts ended in ERROR with no output. The builder's self-check (plants, mutation train and byte-identical objects) is the evidence above.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF

xxxxxxxxxxxxx and others added 3 commits October 10, 2026 09:44
The two recycling pools #1665 kept keep a parked object "allocated" to
ASan, so a stale read of one was invisible (#1661). Under
__SANITIZE_ADDRESS__ only:

- a NUM on the per-thread freelist is poisoned whole except its link
  word; under ASan the link moves from data+0 (the number itself) to
  data+8, so a stale NUMBER read is reported too. Unpoisoned on pop.
- a call env parked on chunk->env_cache has values[], names[],
  assign_counts[], the three hash arrays and the Env struct poisoned,
  except `parent` (read by vm_take_call_env). Unpoisoned on take; the
  cycle collector lifts the poison when it walks chunk -> env_cache and
  restores it for surviving chunks; chunk_free lifts it before the drop.

Every other build expands the hooks to nothing: all 29 release objects
are byte-identical to the base (cmp).

tests/test_pool_poison.c plants a stale read of a parked NUM's number,
a parked env's values[0] and a parked Env field; suite [0ff] requires
each to be reported as use-after-poison at the planted address, and a
control (freelist pop, call-env take, a collection over a parked env,
chunk_free of a parked env) to stay sanitizer-clean with leaks on.
Release builds announce a skip.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF
…ccess

-DEIGS_JIT_CHECKED (the new jit-checked variant) makes the emitter call a
witness before every direct env->values[slot] access: GET_LOCAL,
SET_LOCAL, LOCAL_DOT_GET/SET (the %r12 values base) check that the
cached base is still the current frame's fn_env->values and that
slot < fn_env->count, the interpreter's range check; the inline EnvIC
hit paths of GET_NAME and SET_NAME* check slot_idx < target->count. A
violation prints the opcode, offset, chunk and line and aborts. The
scanner charges the extra bytes to the size estimate.

Nothing is emitted, declared or defined in the default build: release
jit.o and vm.o are byte-identical to the base. CI runs the JIT-tier
sections (82 100 129 130 0fd 0fe) against the checked build in the
jit-differential job.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-10T14:52:42.429589Z 9206f2a PR opened
🔒 Security Review ✅ Completed 2026-10-10T14:56:28.150748Z 9206f2a PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@codspeed

codspeed Bot commented Oct 10, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 13 untouched benchmarks


Comparing detect-1665 (9206f2a) with main (515b151)

Open in CodSpeed

@InauguralPhysicist
InauguralPhysicist added this pull request to the merge queue Oct 10, 2026
Merged via the queue into main with commit 7b1a902 Oct 10, 2026
32 checks passed
@InauguralPhysicist
InauguralPhysicist deleted the detect-1665 branch October 10, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants