fix(gc/codegen): epoch-gate read-PIC pointer tokens against GC address recycling (#6080) - #7434
Merged
Merged
Conversation
added 2 commits
August 5, 2026 12:32
…s recycling (#6080) #6080(a) residual: class instances (and any receiver without a #6804 ShapeId stamp) still prime the RAW keys-array pointer into their per-site @perry_ic_N cache. Those globals are invisible to every GC scanner, and GC_FLAG_SHAPE_SHARED keeps a keys array LIVE but not address-STABLE - the copying minor moves it like anything else and the vacated from-space address is recycled, so a stale prime can pointer-match a different-shape keys array and the inline hit path loads the wrong slot, silently. Fix (the epoch design from the #6080 thread, narrowed to pointer tokens): - runtime: process-global PERRY_IC_EPOCH (starts at 1 so a zeroinitializer cache can never match), bumped in GcStats::record_collection - the single per-collection funnel - and again at budgeted-sweep ENTRY, because budgeted sweep slices interleave with the mutator before the end-of-cycle funnel runs. - js_object_get_field_ic_miss snapshots the live epoch into cache[2] at prime time (cache widens [i64;2] -> [i64;3]; the emitted global was already [8 x i64], so no layout change). - codegen: the inline monomorphic hit predicate requires cache[2] == @PERRY_IC_EPOCH before trusting a pointer token. Shape-ID tokens (bit 62) skip the check - ids are never reused - so the hot stamped-plain-object population never re-primes after GC. Coverage: IR contract test asserts the emitted guard (epoch-slot gep + @PERRY_IC_EPOCH load); runtime tests assert the funnel bumps the epoch and that a pointer-token prime goes stale on bump (the guard's exact inputs, so the instrument provably CAN fail). Probe run on Windows: 200k class-instance reads through one monomorphic site under copying minors (copied_objects>0 confirmed via PERRY_GC_DIAG) and under PERRY_GEN_GC=0, all values correct; defineProperty-after-prime (#6254 half) still honored. Sibling not covered here: the #6812 3-way dynamic-key WRITE IC compares the same discriminated shape token and packs 4 ways into its 8-slot global (no free epoch slot); noted on #6080.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe change adds a process-wide GC epoch to property-read PICs. Pointer-token hits require a matching cached epoch, while shape-ID tokens bypass validation. GC collection and sweep paths advance the epoch, and regression tests cover cache refresh and generated validation. ChangesRead PIC epoch invalidation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GC
participant PERRY_IC_EPOCH
participant GenericPropertyGet
participant js_object_get_field_ic_miss
GC->>PERRY_IC_EPOCH: bump epoch during sweep or collection
GenericPropertyGet->>PERRY_IC_EPOCH: load live epoch
GenericPropertyGet->>GenericPropertyGet: compare pointer-token cache[2]
GenericPropertyGet->>js_object_get_field_ic_miss: route stale token to miss path
js_object_get_field_ic_miss->>PERRY_IC_EPOCH: snapshot current epoch
js_object_get_field_ic_miss-->>GenericPropertyGet: re-prime cache[2]
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
proggeramlug
pushed a commit
that referenced
this pull request
Aug 5, 2026
cargo fmt --all -- --check failed on main. lint is a REQUIRED context, so while it is red every merge bypasses a required gate. Two hunks, six lines: an assert! in gc/tests/telemetry_verifier.rs over rustfmt's default fn_call_width of 60, and a pub(crate) use in object/field_get_set.rs added below the pub use block where reorder_imports wants it above. Not a toolchain artifact: CI uses dtolnay/rust-toolchain@stable and the repo has no rust-toolchain.toml or rustfmt.toml.
proggeramlug
added a commit
that referenced
this pull request
Aug 5, 2026
cargo fmt --all -- --check failed on main. lint is a REQUIRED context, so while it is red every merge bypasses a required gate. Two hunks, six lines: an assert! in gc/tests/telemetry_verifier.rs over rustfmt's default fn_call_width of 60, and a pub(crate) use in object/field_get_set.rs added below the pub use block where reorder_imports wants it above. Not a toolchain artifact: CI uses dtolnay/rust-toolchain@stable and the repo has no rust-toolchain.toml or rustfmt.toml. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the remaining half of #6080 — (a) ABA staleness of the read-PIC keys-pointer token. ((b) was fixed by #6254.)
What was still broken
Since #6803/#6807, plain stamped objects prime never-reused ShapeId tokens (ABA-immune), and raw keys pointers are only primed for
GC_FLAG_SHAPE_SHAREDarrays. But shape-shared means rooted, not address-stable: the copying minor moves shape-shared arrays like anything else (move_youngmerely preserves the flag), every rooted reference is rewritten — except the@perry_ic_Nglobals, which no GC scanner knows about. After the from-space flip the vacated address is recycled; a different-shape keys array landing there makes a primed site falsely pointer-match, and the inline hit path loads the wrong slot. Silent wrong value, exactly the class #6006 fixed in the sibling transition cache. Class instances (class_id != 0) are the main exposed population.The fix (the epoch design from the issue thread, narrowed to pointer tokens)
PERRY_IC_EPOCH(AtomicU64, starts at 1 so azeroinitializercache can never match). Bumped inGcStats::record_collection— the documented single per-collection funnel, covering the copying fast path, the minor fallback, full mark-sweep, and budgeted cycles — and a second time at budgeted-sweep entry, because budgeted sweep slices interleave with the mutator before the end-of-cycle funnel runs (primes taken after that bump reference marked arrays, which later slices of the same sweep never free). Double-bump is harmless: one extra re-prime.js_object_get_field_ic_misssnapshots the live epoch intocache[2]at prime time ([i64;2]→[i64;3]; the emitted global was already[8 x i64]).lower_generic_property_get): the inline hit predicate ANDs inis_stamp || cache[2] == @PERRY_IC_EPOCH. Pointer tokens re-prime once per collection; ShapeId tokens skip the check entirely (ids are never reused), so the hot stamped population pays two loads + icmp + or that fold into the existinghitcond_br and never re-primes after GC. Any epoch change happens inside a call (a collection), so LLVM hoisting of the epoch load stays sound.The full-outline path (
js_object_get_field_ic) already re-validates every read through the miss handler; it inherits the epoch stamping unchanged.Why not the alternatives the thread ruled out
Pinning shape-shared arrays trades the silent wrong value for un-traced key strings collected under a pinned array (pinned objects are not traced) and would need a new root scanner + cap. The epoch is one atomic add per collection and two loads per pointer-token hit.
Coverage (each gate can fail)
generic_property_get_hit_path_is_epoch_gated): emitted IR must carry thecache[2]gep and the@PERRY_IC_EPOCHload — deleting the guard turns it red.test_record_collection_bumps_read_pic_epoch: the funnel must advance the epoch.pointer_token_prime_stamps_epoch_and_goes_stale_on_bump: a class-instance prime stores the raw keys pointer + live epoch, a bump strands it (the guard's exact inputs), and re-priming heals it.Validation
cargo test -p perry-runtime --lib c3c_pic_tests/telemetry— green (Windows,--test-threads=1).cargo test -p perry-codegen --lib property_get— green.any-typed site with dynamic-shape churn;PERRY_GC_DIAG=1confirms copying minors actually ran (copied_objects=6912, 102 diag lines) — sum exact,defineProperty-after-prime still returns the getter value (the fix(codegen): read-PIC honors descriptors installed after priming (#6080) #6254 half), mixed stamped/class population exact. Same results underPERRY_GEN_GC=0.Out of scope, noted for follow-up
The #6812 3-way dynamic-key write IC compares the same discriminated shape token and packs 4
(token, slot)ways into its 8-slot global (no free epoch slot), so its pointer-token ways carry the same exposure; needs its own treatment (per-way epoch packing or a keys-content check like the #6006 transition validation). Commenting on #6080 with this.No version bump per external-contribution flow — maintainer bumps at merge.
Summary by CodeRabbit
Bug Fixes
Tests