fix(object): a Buffer/DataView receiver must not reach the ordinary object walk (#8117) - #8141
Conversation
…bject walk (#8117) `obj_value_has_own_key` has arms for a registry typed array, a GC_TYPE_ARRAY/LAZY_ARRAY, a closure, and a native-module namespace. It had none for a Buffer / ArrayBuffer / DataView, so one fell through to the ordinary `ObjectHeader` arm — and a buffer is a `BufferHeader`: no `class_id`, no `keys_array`. The walk read `(*obj).keys_array` out of the bytes that follow a buffer header and handed that to `js_array_length`, whose lazy-array probe dereferences `addr - 8`. The only thing in between was a `< 0x10000` magnitude floor, which arbitrary payload bytes clear routinely. Four lines reproduce it, and it is the two `pass -> crash` entries of #8117: const b: any = Buffer.alloc(8); b.readUInt8 = function () { return "shadowed"; }; const k = "readUInt8"; b[k](0); #0 js_array_length <- SIGSEGV #1 perry_runtime::object::reflect_support::obj_value_has_own_key #2 perry_runtime::proxy::own_set_descriptor #3 perry_runtime::proxy::ordinary_set_with_receiver #4 js_put_value_set #5 js_put_value_set_dyn_ic_miss with `x0 = 0x12b00003aa1f03e2` — payload bytes, not an address. It is the same "ask the receiver question before the generic path claims it" shape as #8090/#8109/#8119/#8120, on the has-own-key / `[[Set]]` path. A buffer's own string keys are exactly its expando table (#6406). Prototype methods are inherited, not own, which is what lets `buf.readUInt8 = fn` install a shadowing own property rather than be treated as a redefinition. Canonical integer indices are deliberately not folded in: the byte-index `[[Set]]` is routed upstream of this call, and answering "own" for one would divert it into the ordinary data-property store. Second, smaller change: the `keys_array` guard becomes `addr_class::is_plausible_heap_addr` instead of the bare `< 0x10000` floor. That is defence in depth for the class this fix closes by routing — a receiver kind with no arm here should get a wrong answer, not a SIGSEGV. Why it was invisible on macOS, and why it looked twelve days old: the garbage `keys_array` has to clear the floor AND land unmapped. macOS's 2 TB heap floor means it usually reads as null, so the same call silently answered "no own key" for a property the buffer really owns. That is what the new test asserts, so it fails on both platforms. Testing - `object::tests::buffer_own_key_comes_from_the_expando_table_not_the_object_walk`, watched fail with the buffer arm removed: "a buffer's own expando property must be reported as an own key". Also asserts a prototype method and an unknown key are NOT own, so the arm cannot pass by answering true. - `cargo test -p perry-runtime --lib`: 2390 passed, 0 failed, 4 ignored (baseline 2389 + this test), exit 0; `Compiling perry-runtime v` = 1. - End-to-end on Linux (ubuntu 24.04 aarch64 container, release, `PERRY_NO_AUTO_OPTIMIZE=1`, `PERRY_RUNTIME_DIR` pinned), before -> after: mini repro above 10/10 SIGSEGV -> 20/20 exit 0 test_gap_6386_dataview_concat_regex_fastpaths 25/25 SIGSEGV -> 20/20 exit 0 test_gap_buffer_own_props SIGSEGV -> 20/20 exit 0 Both gap fixtures are byte-identical to node v26.5.1 after the fix. - The x86-64 side is confirmed independently: on ubuntu-latest, `test_gap_buffer_own_props` segfaults standalone at base fa83eca. - rustfmt, `scripts/check_file_size.sh` and all sixteen `lint` gate scripts clean (`raw_handle_debt` included — the new arm carries its address across the GC-capable coercion with `across_mut`, not a bare handle read). Claude-Session: https://claude.ai/code/session_01MsfDzkTEnuS2nh7ygsYkoi
|
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 (3)
📝 WalkthroughWalkthrough
ChangesBuffer own-key handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized change fixes Buffer and DataView own-property handling with targeted regression coverage; no actionable merge-blocking risk remains after normal checks and review. 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 |
|
Control run, to keep the two halves of this PR honestly labelled. Rebuilt in the Linux container with only the buffer arm — the Both clean, 15/15. So the arm alone fixes the crash and the |
Closes the two
pass -> crashentries of #8117:test_gap_buffer_own_propsandtest_gap_6386_dataview_concat_regex_fastpaths. Both are SIGSEGV, both reproduce standalone on Linux, and both are this one missing receiver arm.The bug
obj_value_has_own_key(crates/perry-runtime/src/object/reflect_support.rs) has arms for a registry typed array, aGC_TYPE_ARRAY/GC_TYPE_LAZY_ARRAY, a closure, and a native-module namespace. It had none for a Buffer / ArrayBuffer / DataView, so one fell through to the ordinaryObjectHeaderarm.A buffer is a
BufferHeader: noclass_id, nokeys_array. The walk read(*obj).keys_arrayout of the bytes that follow a buffer header and handed that tojs_array_length, whose lazy-array probe dereferencesaddr - 8. The only thing between the two was a< 0x10000magnitude floor, which arbitrary payload bytes clear routinely.Four lines reproduce it — 10/10 on Linux:
x0is payload bytes, not an address. The faulting instruction isldurb w9, [x20, #-8]followed bycmp w9, #0x9and a compare against0x4C5A5841—GC_TYPE_LAZY_ARRAYandLAZY_ARRAY_MAGIC.This is the same "ask the receiver question before the generic path claims it" shape as #8090 / #8109 / #8119 / #8120, on the has-own-key /
[[Set]]path instead of the element path.The fix
A registered-buffer arm, placed beside the typed-array arm and for the same stated reason (it must precede the GC-header read, because small slab buffers carry no
GcHeaderat all).A buffer's own string keys are exactly its expando table (#6406). Prototype methods are inherited, not own — that is what lets
buf.readUInt8 = fninstall a shadowing own property instead of being treated as the redefinition of an existing one. Canonical integer indices are deliberately not folded in: the byte-index[[Set]]is routed upstream of this call, and answering "own" for one would divert it into the ordinary data-property store.Second, smaller change: the
keys_arrayguard becomesaddr_class::is_plausible_heap_addrinstead of the bare< 0x10000floor — defence in depth for the class this fix closes by routing. A receiver kind with no arm here should produce a wrong answer, not a SIGSEGV.Why it read as twelve days old and macOS-only
The garbage
keys_arrayhas to clear the floor and land unmapped. macOS's 2 TB heap floor means it usually reads as null, so the same call silently answered "no own key" for a property the buffer really owns — wrong, but not fatal. That is why 30 runs each on macOS/arm64 were clean under every GC instrument (PERRY_GC_ZEAL,PERRY_GC_ZEAL_ALLOC_KB=0, forced evacuation, from-space protect + scan,PERRY_GC_SCHEDULE_RATE=1,PERRY_GEN_GC=0) while CI segfaulted.It is also why the CI-log bisect landed on #7314 (2026-08-03): that commit added
initialize_stack_maps()tojs_gc_init, whose Linux armstd::fs::reads the whole executable at startup — a Linux-only multi-megabyte alloc/free before any user code, which moved the heap enough to make the pre-existing garbage read fatal. The defect is older than #7314; #7314 only made it land. Details in #8117 (comment).The new test asserts the ANSWER, not "did not crash", so it fails on macOS too.
Testing
object::tests::buffer_own_key_comes_from_the_expando_table_not_the_object_walk— watched fail with the buffer arm removed:It also asserts a
Buffer.prototypemethod and an unknown key are not own, so the arm cannot pass by answeringtrueunconditionally.cargo test -p perry-runtime --lib→ 2390 passed, 0 failed, 4 ignored (repo baseline 2389 + this test), exit 0.grep -c "Compiling perry-runtime v"on the restore build = 1.End-to-end on Linux (ubuntu 24.04 aarch64 container,
--release,PERRY_NO_AUTO_OPTIMIZE=1,PERRY_RUNTIME_DIRpinned), before → after:test_gap_6386_dataview_concat_regex_fastpathstest_gap_buffer_own_propsBoth gap fixtures are byte-identical to node v26.5.1 after the fix (
diffclean;node --versionchecked against.node-version).The x86-64 side is confirmed independently: on
ubuntu-latestat basefa83ecab2,test_gap_buffer_own_propssegfaults standalone, outside the gap harness — so this is not a test-isolation or ordering artefact.rustfmt,
scripts/check_file_size.sh, and all sixteenlintgate scripts run individually, all exit 0.raw_handle_debtincluded: the new arm carries its address across the GC-capable key coercion withacross_mutrather than a bare handle read, so the file stays at its ceiling of 3.Not in this PR
js_class_method_bindsites that still bind a movable heap string's interior (Fix a crash when reading a method off a Buffer without calling it #7747's siblings).Summary by CodeRabbit
Bug Fixes
Tests