fix(gc): root Object.defineProperty's receiver, key and descriptor fields across its own allocating calls - #7978
Merged
Merged
Conversation
added 3 commits
August 12, 2026 17:57
…elds `js_object_define_property` resolved the receiver's `ObjectHeader` and coerced the key to a `StringHeader` once, near the top, then carried both -- plus `obj_value` / `descriptor_value` / `key_value` -- as bare Rust locals to the end of the function, past a dozen calls that can allocate and therefore evacuate. `desc_has_field` / `desc_read_field` additionally run USER JS when a descriptor field is an accessor. A raw Rust local is neither a shadow slot nor a temp root nor reachable from any registered scanner, and the static IR checker cannot see it. `obj as usize` is also the OWNER KEY of the descriptor side tables, so a stale receiver files attributes and accessors under a dead address. Root all five in one scope and make `across!` the only way to name them across a call. Root `DescView`'s six field values as runtime handles (the stale word was being STORED into the receiver). Root `validate_nonconfigurable_redefine`'s descriptor / current value / accessor bits. Root `obj_value_has_own_key`'s keys-array walk across the lazy-array materialization in `js_array_get`. Closes #7963.
Comparing each arm inline (observed() === expected()) leaves the left operand an SSA temporary live across a call that collects, which faults in js_jsvalue_equals <- js_eq <- main on a pristine build AND on this branch: a separate codegen root-dominance defect, filed on its own. Bind both sides first so this program isolates the defineProperty window, where it now exits 138 on pristine (arm 3, the descriptor-getter arm) and 0 here with 301 copying minors and 110,912 objects moved.
📝 WalkthroughWalkthrough
ChangesObject.defineProperty GC rooting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant js_object_define_property
participant RuntimeHandleScope
participant GC
participant DescriptorStorage
Caller->>js_object_define_property: define property
js_object_define_property->>RuntimeHandleScope: root receiver, key, descriptor, and accessors
js_object_define_property->>GC: perform allocation-capable operation
GC-->>RuntimeHandleScope: relocate and refresh rooted values
js_object_define_property->>DescriptorStorage: install refreshed property data
DescriptorStorage-->>Caller: return defined property result
Possibly related issues
Possibly related PRs
🚥 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
marked this pull request as ready for review
August 12, 2026 16:35
This was referenced Aug 12, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
changelog.d/7978-define-property-rooting.md (1)
1-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the changeset focused on shipped behavior.
This fragment includes incident details, probe transcripts, raw-handle accounting, and separate
#7964/#7803investigations. Keep the changeset as one concise release-note entry for theObject.definePropertyrooting fix. Move the audit history and compiler investigations togc-handoff/DEFPROP-NOTES.md.Based on learnings: “For PerryTS/perry changelog fragments in
changelog.d/, describe the final shipped behavior as one coherent release-note entry. Do not include separate development-slice narratives that may contradict one another when the release notes are assembled.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@changelog.d/7978-define-property-rooting.md` around lines 1 - 76, Condense the changelog fragment into one concise release-note entry describing the shipped Object.defineProperty GC-rooting fix and its user-visible behavior. Remove incident details, test/probe transcripts, raw-handle accounting, proof and investigation narratives from the changelog, and move the audit history and compiler investigations to gc-handoff/DEFPROP-NOTES.md.Source: Learnings
crates/perry-runtime/src/object/object_ops/descriptor_helpers.rs (1)
677-689: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the now-unused
accbinding with a presence test.The accessor comparisons read
acc_get_handleandacc_set_handle, so theaccbinding has no remaining use. Line 689 silences the warning withlet _ = acc;. Bind nothing instead, so a future reader cannot reintroduce the stale pre-call copy.♻️ Proposed refactor
- if let Some(acc) = cur_accessor { + if cur_accessor.is_some() {- let _ = acc; if desc_has_get {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/object/object_ops/descriptor_helpers.rs` around lines 677 - 689, Replace the `if let Some(acc) = cur_accessor` binding with a presence-only check, such as testing whether `cur_accessor` is `Some`, and remove `let _ = acc;`. Keep the existing accessor comparison logic using `acc_get_handle` and `acc_set_handle` unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@changelog.d/7978-define-property-rooting.md`:
- Around line 74-76: Correct the raw-handle debt summary in the changelog so it
states that the debt falls to 5, or falls by 2 to 5, matching the listed count
changes; leave the recorded baseline unchanged.
In `@crates/perry-runtime/src/gc/tests/mod.rs`:
- Line 38: Update all repository documentation and scripts that invoke
perry-runtime tests, including targeted test commands, to prefix or otherwise
set RUST_TEST_THREADS=1. Ensure every cargo test -p perry-runtime invocation
consistently runs single-threaded.
In `@gc-handoff/DEFPROP-NOTES.md`:
- Around line 249-255: Remove the machine-specific absolute worktree path from
the collision note in DEFPROP-NOTES.md, replacing it with a repository-relative
description or omitting the collision details while preserving any useful
reproducible information.
- Around line 167-175: Update the ambiguous “Both arms” wording in the probe
description to “all three arms,” since it refers to the three listed test cases;
reserve “both builds” only for comparisons between pristine and fixed builds.
In `@test-files/test_gap_gc_define_property_descriptor_rooting.ts`:
- Around line 35-41: Register test_gap_gc_define_property_descriptor_rooting.ts
in test-parity/gc_repsel_corpus.txt and update its loop_polls witness
configuration in gc-moving-witnesses.yml to clear inherited GC knobs before
applying the specified schedule seed/rate and from-space protection settings,
including depth 800.
---
Nitpick comments:
In `@changelog.d/7978-define-property-rooting.md`:
- Around line 1-76: Condense the changelog fragment into one concise
release-note entry describing the shipped Object.defineProperty GC-rooting fix
and its user-visible behavior. Remove incident details, test/probe transcripts,
raw-handle accounting, proof and investigation narratives from the changelog,
and move the audit history and compiler investigations to
gc-handoff/DEFPROP-NOTES.md.
In `@crates/perry-runtime/src/object/object_ops/descriptor_helpers.rs`:
- Around line 677-689: Replace the `if let Some(acc) = cur_accessor` binding
with a presence-only check, such as testing whether `cur_accessor` is `Some`,
and remove `let _ = acc;`. Keep the existing accessor comparison logic using
`acc_get_handle` and `acc_set_handle` unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f3ccdebd-5a30-45b2-826a-a55c0a246613
📒 Files selected for processing (8)
changelog.d/7978-define-property-rooting.mdcrates/perry-runtime/src/gc/tests/mod.rscrates/perry-runtime/src/gc/tests/rooted_define_property.rscrates/perry-runtime/src/object/object_ops/define_property.rscrates/perry-runtime/src/object/object_ops/descriptor_helpers.rscrates/perry-runtime/src/object/reflect_support.rsgc-handoff/DEFPROP-NOTES.mdtest-files/test_gap_gc_define_property_descriptor_rooting.ts
Comment on lines
+74
to
+76
| `scripts/raw_handle_debt.py` falls by 5 (`define_property.rs` 3 → 2, | ||
| `reflect_support.rs` 4 → 3); the recorded baseline is deliberately left | ||
| unchanged so parallel debt-paying PRs do not collide. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the raw-handle debt wording.
Line 74 says the debt “falls by 5”. The listed counts change from 7 to 5, so the debt falls by 2 and ends at 5. Change the text to “falls to 5” or “falls by 2, to 5”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@changelog.d/7978-define-property-rooting.md` around lines 74 - 76, Correct
the raw-handle debt summary in the changelog so it states that the debt falls to
5, or falls by 2 to 5, matching the listed count changes; leave the recorded
baseline unchanged.
| mod promote_in_place; | ||
| mod root_words; | ||
| mod rooted_container_values; | ||
| mod rooted_define_property; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for dir in .github scripts; do
if [[ -d "$dir" ]]; then
rg -n -C 4 'RUST_TEST_THREADS|cargo test.*perry-runtime|perry-runtime.*test' "$dir"
fi
doneRepository: PerryTS/perry
Length of output: 39280
Keep perry-runtime test commands single-threaded.
CI runs the crate tests with RUST_TEST_THREADS=1, but repository scripts also invoke cargo test -p perry-runtime without this setting. Apply RUST_TEST_THREADS=1 to every documented or scripted perry-runtime test command, including targeted tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/gc/tests/mod.rs` at line 38, Update all repository
documentation and scripts that invoke perry-runtime tests, including targeted
test commands, to prefix or otherwise set RUST_TEST_THREADS=1. Ensure every
cargo test -p perry-runtime invocation consistently runs single-threaded.
Source: Coding guidelines
Comment on lines
+167
to
+175
| `test-files/test_gap_gc_define_property_descriptor_rooting.ts`, three arms: an | ||
| allocating `Object.groupBy` first arm (to retire from-space blocks), a | ||
| hand-written `Object.defineProperty` loop, and a loop whose descriptor bag | ||
| carries three allocating accessor getters (so `desc_read_field` runs user JS | ||
| mid-define). | ||
|
|
||
| Both arms compiled with `PERRY_NO_AUTO_OPTIMIZE=1` and `PERRY_RUNTIME_DIR` | ||
| pinned to their own `.a` pair; the fixed pair's mtimes were confirmed to have | ||
| moved after the edit. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a consistent probe count.
Lines 167-171 list three probe arms, but Line 173 says “Both arms”. Replace it with “all three arms” if it refers to probe arms, or “both builds” if it refers to the pristine and fixed builds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gc-handoff/DEFPROP-NOTES.md` around lines 167 - 175, Update the ambiguous
“Both arms” wording in the probe description to “all three arms,” since it
refers to the three listed test cases; reserve “both builds” only for
comparisons between pristine and fixed builds.
Comment on lines
+249
to
+255
| **Collision:** `/Users/amlug/projects/perry/wt-codex-7964` is another agent's | ||
| worktree on branch `fix/7964-zod-star-reexports`, already carrying uncommitted | ||
| edits to `perry-hir/src/lower/module_decl.rs`, `perry-hir/src/dynamic_import.rs` | ||
| and `perry-codegen/src/codegen/helpers.rs`, plus fixtures | ||
| `test-files/test_gap_export_star_variable_reexport.ts` and | ||
| `test-files/_helpers/issue_7964_{leaf,barrel,bridge,top}.ts` — the same four-file | ||
| shape I arrived at independently. I stopped at the verdict rather than shipping a |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the machine-specific worktree path.
The committed audit note contains /Users/amlug/projects/perry/wt-codex-7964. Replace it with a repository-relative description or omit the collision details. The absolute path is not portable and adds no reproducible evidence.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gc-handoff/DEFPROP-NOTES.md` around lines 249 - 255, Remove the
machine-specific absolute worktree path from the collision note in
DEFPROP-NOTES.md, replacing it with a repository-relative description or
omitting the collision details while preserving any useful reproducible
information.
Comment on lines
+35
to
+41
| // PERRY_GC_SCHEDULE_SEED=1 PERRY_GC_SCHEDULE_RATE=1 \ | ||
| // PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800 | ||
| // | ||
| // Before the fix this exits 138 with `[gc-fromspace-protect] FAULT` naming a | ||
| // retired from-space address during arm 3 (`obj_type=2`, a receiver | ||
| // `ObjectHeader`); after it the program exits 0 with 301 copying minors and | ||
| // ~110k objects moved, byte-identical to node in every configuration. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for dir in .github scripts crates; do
if [[ -d "$dir" ]]; then
rg -n -C 5 \
'test_gap_gc_define_property_descriptor_rooting|test_gap_gc_|PERRY_GC_SCHEDULE_SEED|PERRY_GC_SCHEDULE_RATE|PERRY_GC_PROTECT_FROMSPACE|PERRY_GC_PROTECT_FROMSPACE_DEPTH' \
"$dir"
fi
doneRepository: PerryTS/perry
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- workflow relevant sections ---'
sed -n '245,325p' .github/workflows/gc-moving-witnesses.yml
sed -n '1,75p' .github/workflows/gc-moving-witnesses.yml
echo '--- matrix runner arm and invocation sections ---'
rg -n -C 8 \
'loop_polls|SCHEDULE_SEED|SCHEDULE_RATE|PROTECT_FROMSPACE|PROTECT_FROMSPACE_DEPTH|FILTER|filter|env -u|gc_repsel_corpus' \
scripts/gc_repsel_matrix.sh | head -240
echo '--- exact witness registration ---'
rg -n -C 4 \
'test_gap_gc_define_property_descriptor_rooting' \
test-parity .github scripts test-filesRepository: PerryTS/perry
Length of output: 19660
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- file presence ---'
git ls-files --stage -- test-files/test_gap_gc_define_property_descriptor_rooting.ts
if [[ -f test-files/test_gap_gc_define_property_descriptor_rooting.ts ]]; then
wc -l test-files/test_gap_gc_define_property_descriptor_rooting.ts
sed -n '1,80p' test-files/test_gap_gc_define_property_descriptor_rooting.ts
else
echo 'file is not present in the working tree'
fi
echo '--- GC witness files and registry entries ---'
find test-files -maxdepth 1 -type f -name 'test_gap_gc_*.ts' -printf '%f\n' | sort | head -200
rg -n -C 2 \
'define_property|descriptor_root|test_gap_gc_' \
test-parity/gc_repsel_corpus.txt test-parity/gc_repsel_triage.txt 2>/dev/null | head -200
echo '--- matrix environment construction and execution ---'
sed -n '430,650p' scripts/gc_repsel_matrix.shRepository: PerryTS/perry
Length of output: 35232
Register and run test_gap_gc_define_property_descriptor_rooting.ts with its witness configuration.
The file is not registered in test-parity/gc_repsel_corpus.txt, so gc-moving-witnesses.yml does not execute it. The loop_polls arm also omits PERRY_GC_SCHEDULE_SEED=1, PERRY_GC_SCHEDULE_RATE=1, PERRY_GC_PROTECT_FROMSPACE=1, and PERRY_GC_PROTECT_FROMSPACE_DEPTH=800. Add the registry entry and apply these settings while clearing inherited GC knobs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test-files/test_gap_gc_define_property_descriptor_rooting.ts` around lines 35
- 41, Register test_gap_gc_define_property_descriptor_rooting.ts in
test-parity/gc_repsel_corpus.txt and update its loop_polls witness configuration
in gc-moving-witnesses.yml to clear inherited GC knobs before applying the
specified schedule seed/rate and from-space protection settings, including depth
800.
proggeramlug
pushed a commit
that referenced
this pull request
Aug 12, 2026
proggeramlug
added a commit
that referenced
this pull request
Aug 12, 2026
* docs(gc-handoff): triage #7803 on a corpus that links again The zod dep-corpus links on main again (#7980), so #7803 is runnable for the first time since it was filed. Record what running it says: - #7803's own reproducer (seed 1, rate 1, quarantine off) no longer fails, but the class it reports does — 3 of 16 seeds fail, one with the same "Cannot read properties of undefined" shape. - The candidate cause on record (#7962/#7978, Object.defineProperti(es) rooting) is refuted by a sabotage A/B: reverting both fixes underneath current main does not bring the failure back. - Every failure is intermittent; a fixed seed does not replay. The reportable figure is the rate, not the seed. - Seed 15 is a separate, self-detecting bug: the #7645 pin latch aborts on a pinned young Map, and the FATAL's own suggested remediation (scripts/gc_pin_sites.py) reports OK. - Aside: PERRY_GC_DIAG=0 enables diagnostics (var_os(...).is_some()). * docs(changelog): fragment for #7989 * docs(gc-handoff): mark the protected-arm sweep as in-flight, not concluded * docs(gc-handoff): correct the seed-1 run count to 4/4 and qualify section 1 * docs(gc-handoff): final sweep counts and the #7990 cross-reference --------- 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 #7963.
The window
js_object_define_propertyresolved the receiver'sObjectHeaderand coerced the key to aStringHeaderonce, near the top, and then carried both — plus the three NaN-boxed wordsobj_value/descriptor_value/key_value— as bare Rust locals to the end of the function. Between there and the last use it runs a dozen calls that can allocate and therefore evacuate:define_array_property,enforce_define_property_invariants,obj_value_has_own_key,ensure_key_in_keys_array,clone_closure_rebind_this,define_property_force_store_value, and everydesc_has_field/desc_read_field— the last two allocate a field-name string per probe and, when a descriptor field is an accessor, run user JS mid-define.A raw Rust local is neither a shadow slot nor a temp root nor reachable from any registered scanner, so an evacuating minor could neither keep those objects alive nor rewrite the local.
scripts/gc_root_dominance_check.pyreads emitted LLVM IR, so it is structurally blind to this class — the runtime instruments are the only detector.The stale receiver is the worse half:
obj as usizeis the OWNER KEY of the per-property descriptor side tables, so a define that lands after a collection files its attributes and accessors under a dead address, where the matching read can never find them. That is a silent wrong answer, not a crash.This is the window #6949's scope note names and defers, and the one #7949/#7962 deliberately left open.
What changed
object_ops/define_property.rs— all five values are rooted in one scope, and anacross!macro is now the only way to name any of them across a call: it runs the call first and rebinds all five from their roots afterwards, so a pre-collection address is never nameable. Also rooted: the descriptor'sget/setfield values (live acrossensure_key_in_keys_arrayand the first of two closure clones), the existing accessor's closure bits (written back into the GC-scanned accessor table when the redefining descriptor omits a field), and the class-prototype mirror's method value. The three per-armRuntimeHandleScopes collapse into one — an inner scope dropped while an outer one is still taking handles truncates the outer container's newest entries, the hazard documented ongc::RootedValues.object_ops/descriptor_helpers.rs—DescView's six field values were rawJSValues read at decode time and handed back a dozen statements later; the stale word was then stored into the receiver bydefine_property_force_store_value. Each present field is now aRuntimeHandle, soreadreturns the post-collection address.validate_nonconfigurable_redefine's per-field arm likewise roots the descriptor, the current value and the current accessor bits, and re-resolvesdesc_ptrafter the allocation that precedes each read.object/reflect_support.rs—obj_value_has_own_key's final keys-array walk heldkeysandkey_stracrossjs_array_get, which materializes a lazy array and therefore allocates. Both are rooted and re-read per iteration.No new bare
get_raw_*_ptrsites:RuntimeHandle::across_{mut,const}is what thescripts/raw_handle_debt.pyratchet asks for. The count falls by 5 (define_property.rs3 → 2,reflect_support.rs4 → 3); the recorded baseline is deliberately left unchanged so parallel debt-paying PRs do not collide.How it is proven
crates/perry-runtime/src/gc/tests/rooted_define_property.rs, three tests, all underCopyingNurseryTestGuard+suppress_automatic_triggers:define_property_lands_on_the_receiver_a_descriptor_getter_moved— end-to-end through the real#[no_mangle]entry point, with a descriptor whosevaluefield is an accessor whose getter forces a copying minor. Asserts, in order:copied_objects > 0; the receiver's address changed; the key string's address changed; the property reads back the getter's payload bytes; andget_property_attrsfinds the entry at the live address (the assertion that catches a stale receiver, since the table is keyed by address).desc_view_field_values_are_rooted— theDescViewhalf: decode, force a copying minor, assert the field's address changed and it still reads the original bytes.unrooted_receiver_copy_still_names_from_space— the sabotage arm. The same address in a plain Rustusize— exactly what pre-fixjs_object_define_propertycarried — keeps naming its pre-collection value in the same cycle in which the rooted handle moves. This is what makes (1) and (2) non-vacuous.Sabotage verification, with the fix committed first. Reverting
define_property.rs+descriptor_helpers.rstoorigin/mainmakes (1) abort the harness (the property reads backundefined, so the byte check dereferences a masked non-pointer). Restoring the files and instead makingDescView::readreturn its decode-time copy makes (2) fail with "the descriptor'svaluewas not relocated". Both restored: 3/3 pass.Compiled probe — the A/B
test-files/test_gap_gc_define_property_descriptor_rooting.ts: an allocatingObject.groupByarm (to retire from-space blocks), a hand-writtenObject.definePropertyloop, and a loop whose descriptor bag carries three allocating accessor getters. Both arms compiled withPERRY_NO_AUTO_OPTIMIZE=1andPERRY_RUNTIME_DIRpinned to their own.apair, whose mtimes were confirmed to have moved after the edit.SCHEDULE_SEED=1 SCHEDULE_RATE=1 PROTECT_FROMSPACE=1 DEPTH=800origin/main(a769faf)[gc-fromspace-protect] FAULTatblock+2106,retired_by_minor=#135,obj_type=2(a receiverObjectHeader); stdout stops after arm 2, i.e. it dies in the descriptor-getter arm[gc-schedule] done: safepoints=301 scheduled_collections=301 copying_minors=301 moved_objects=110912 loop_polls=8175Instrument liveness is reported rather than assumed: the pristine arm retired 135 from-space page-sets before it faulted; the fixed arm ran 301 copying minors moving 110,912 objects.
A second defect this turned up (filed separately)
The probe's first draft compared each arm inline (
observed() === expected()). That faults under the same witness configuration on a pristine build and on this branch, injs_jsvalue_equals←js_eq←main(symbolicated against an unstrippedperry-devruntime withPERRY_DEBUG_SYMBOLS=1): the left operand is an SSA temporary live across a call that collects. That is a codegen root-dominance defect, unrelated toObject.defineProperty; the probe now binds both sides toconstfirst so it witnesses one defect. See the linked issue.Worth recording as method: the census line (
obj_type,size) is a hint, not an attribution — the first draft's fault wasobj_type=3atuser_ptr + 4, which really isStringHeader::byte_len, and reading that as "the defineProperty key" would have been wrong. Only the symbolicated backtrace settled it.Gates
cargo fmt --check,scripts/check_file_size.sh,scripts/gc_runtime_root_holders.py(no new root holder —RuntimeHandleborrows the already-registered handle stack),scripts/raw_handle_debt.py(5 below baseline): all clean.Working notes:
gc-handoff/DEFPROP-NOTES.md.Summary by CodeRabbit
Bug Fixes
Object.definePropertyreliability during garbage collection, including cases involving moved objects, property keys, descriptors, and accessor functions.Tests
Documentation