test(gc): the POINTER_FREE trace-skip hazard probe that faults (#7635) - #7644
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change adds GC regression tests for the ChangesPOINTER_FREE hazard regression coverage
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
#7635 reported that forcing POINTER_FREE on a pointer-bearing object strands nothing under any instrument. The instruments were never the problem -- the probes were: JSON.parse of a non-tiny blob is LAZY by default (#7499's tape), so a probe that parses, churns, and only then reads the records back materializes the whole cohort AFTER the collections ran. There was nothing to strand. The same shape with one pre-churn touch loop (defeating the tape) faults immediately: SIGSEGV under default cycles, and a precise [gc-fromspace-protect] report under ZEAL=1 PROTECT_FROMSPACE=1. This test plants the hazard directly at unit level, no env knob: an object built through the materialiser's exact deferred-store path whose only reference to a young string sits in a field slot, finalized with a LYING saw_pointer=false. The copying minor moves the object, honors the state, skips the payload -- and the test asserts BOTH halves of the strand (slot bits survive unrewritten AND the child sits in poisoned from-space), plus subject-liveness (the object must have moved) so neither arm can pass vacuously. The green twin asserts the truthful finalize evacuates the child and rewrites the slot. Also settles the #7633 evidence question: the "zeal arm identical" line there was indeed non-discriminating for parse-only shapes, and any future parse-cohort probe must defeat the lazy tape or say it did not.
4fb66a8 to
e268592
Compare
Audit — merged as v0.5.1366Two agents worked #7635 independently and landed on the same root cause without contact: the probes were vacuous because Kept alongside #7643 rather than closed as a duplicate, because the two observe different layers:
#7643 asks the collector what it intends to visit; this asks what actually happened after it ran. The second is the one that would survive a refactor that fixed the enumeration and broke the rewrite. Sabotage-verified two-sided, which the red-control design earns: inverting the plant's Two design details I checked specifically:
Gates: 20/20 from the The standing rule in your writeup is the durable part and I've taken it into CLAUDE.md's orbit: any parse-cohort GC probe must defeat the lazy tape by touching the cohort before the collections, or state explicitly that it is exercising the lazy path. #7647 tracks the end-to-end CI gate — still open and still wanted, since both merged tests are unit-level and neither would catch a new materialiser path that forgets to finalize at all. |
#7643 measured that PERRY_JSON_TAPE=0 + PERRY_GC_FROMSPACE_SCAN=1 over a parse-then-churn workload is a known-good end-to-end detector for the layout-state family (#7630/#7633/#7635/#7644), but nothing ran it in CI. Adds the fixture, a driver script, and a Python checker whose verdict requires correctness (dangling=0, byte-exact read-back after the churn), liveness (a copying minor actually relocated objects), and eagerness (the scan's own object census reached the record count, so the cohort was not still on the lazy tape) before it can say PASS -- and a --self-test proving the checker can say no on all three, in both directions. Building the fixture surfaced a real, general false-positive source in PERRY_GC_FROMSPACE_SCAN: string allocations were not zero-filling their 0-7 byte 8-byte-alignment pad, so leftover arena bytes there could occasionally decode as a plausible pointer and report a false "dangling" hit -- harmless to every string API (all bounded by byte_len/capacity, never GcHeader.size) but not to a scan that trusts GcHeader.size as the payload's true extent. Fixed at string_storage_alloc's single choke point, mirroring the existing TAG_HOLE fix for array-growth slack. Not wired into branch protection's required contexts here -- that is a maintainer action for after this job's first green run on main.
… (#7711) * gate(gc): promote the parse-then-churn layout-state check to CI (#7647) #7643 measured that PERRY_JSON_TAPE=0 + PERRY_GC_FROMSPACE_SCAN=1 over a parse-then-churn workload is a known-good end-to-end detector for the layout-state family (#7630/#7633/#7635/#7644), but nothing ran it in CI. Adds the fixture, a driver script, and a Python checker whose verdict requires correctness (dangling=0, byte-exact read-back after the churn), liveness (a copying minor actually relocated objects), and eagerness (the scan's own object census reached the record count, so the cohort was not still on the lazy tape) before it can say PASS -- and a --self-test proving the checker can say no on all three, in both directions. Building the fixture surfaced a real, general false-positive source in PERRY_GC_FROMSPACE_SCAN: string allocations were not zero-filling their 0-7 byte 8-byte-alignment pad, so leftover arena bytes there could occasionally decode as a plausible pointer and report a false "dangling" hit -- harmless to every string API (all bounded by byte_len/capacity, never GcHeader.size) but not to a scan that trusts GcHeader.size as the payload's true extent. Fixed at string_storage_alloc's single choke point, mirroring the existing TAG_HOLE fix for array-growth slack. Not wired into branch protection's required contexts here -- that is a maintainer action for after this job's first green run on main. * docs(changelog): add fragment for #7711 * chore: bump version to 0.5.1412 Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
Fixes #7635 — the probe that faults, and the reason nobody could build one.
Root cause: the probes were vacuous, not the instruments
JSON.parseof a non-tiny blob is lazy by default (#7499's tape). Every probe in this investigation — the audit's two shapes and my reproduction — parsed, churned, and only then read the records back. The materialisation happens at read-back, after the collections ran: the cohort did not exist while the collectors were being tested, so no sabotage could strand it and no instrument could catch anything. The instruments were fine the entire time.Demonstrated end-to-end on an instrumented build: the identical TS shape with one pre-churn touch loop (defeating the tape) faults immediately under the sabotage —
ZEAL=1 PROTECT_FROMSPACE=1[gc-fromspace-protect] FAULT: signal 10 at 0x47ab075bbd0— precise, named addressA supporting measurement that sealed it: under sabotage with the lazy probe, a trace-time counter saw only ~5 pointer-bearing PF selections across dozens of cycles × 4,000 records — the records simply weren't there.
The committed regression test
gc/tests/layout_pointer_free_hazard.rs, in thefromspace_protectplanted-hazard convention, no env knobs:saw_pointer=false. The copying minor moves the object, honors the state, skips the payload — asserted on both halves of the strand: the slot bits survive unrewritten (the skip was live), and the child sits in poisoned from-space (QUARANTINE_POISON_WORD).Consequences for the record
POINTER_FREEtrace-skip is real, observable, and load-bearing — outcome (3) of the issue's trichotomy, with the missing shape identified.perry-runtimesuite: 1,904 passed / 0 failed (both new tests green).cargo fmt --checkandcheck_file_size.shclean. No production code changed — this PR is the test alone.Summary by CodeRabbit
Bug Fixes
Documentation