diff --git a/changelog.d/8141-buffer-receiver-own-key.md b/changelog.d/8141-buffer-receiver-own-key.md new file mode 100644 index 0000000000..37414f7a45 --- /dev/null +++ b/changelog.d/8141-buffer-receiver-own-key.md @@ -0,0 +1,11 @@ +### Fixed + +- **`buf.method = fn` through the dynamic `[[Set]]` segfaulted on Linux** (#8117). `obj_value_has_own_key` has arms for a registry typed array, a `GC_TYPE_ARRAY`/`GC_TYPE_LAZY_ARRAY`, a closure and a native-module namespace, but none for a Buffer / ArrayBuffer / DataView — so one fell through to the ordinary `ObjectHeader` arm. 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`, with nothing between the two but a `< 0x10000` magnitude floor that payload bytes clear routinely. + + Four lines reproduce it, 10/10 on Linux — `const b: any = Buffer.alloc(8); b.readUInt8 = function () {}; const k = "readUInt8"; b[k](0)` — through `js_put_value_set_dyn_ic_miss` → `proxy::ordinary_set_with_receiver` → `proxy::own_set_descriptor` → `obj_value_has_own_key` → `js_array_length`. It is the two `pass -> crash` gap regressions in #8117 (`test_gap_buffer_own_props`, `test_gap_6386_dataview_concat_regex_fastpaths`), and the same "ask the receiver question before the generic path claims it" shape as #8090/#8109/#8119/#8120. + + 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. Canonical integer indices are deliberately not folded in — the byte-index `[[Set]]` is routed upstream of this call. The `keys_array` guard also becomes `addr_class::is_plausible_heap_addr` rather than a bare magnitude floor, as defence in depth: a receiver kind with no arm here should give a wrong answer, not a SIGSEGV. + + macOS's 2 TB heap floor made the same garbage read as null, so the call silently answered "no own key" for a property the buffer really owns instead of crashing — which is why 30 clean runs per fixture under every GC instrument proved nothing, and why the CI-log bisect landed on #7314 (whose Linux `initialize_stack_maps` reads the whole executable at `js_gc_init`, moving the heap enough to make a pre-existing garbage read fatal). The new test asserts the ANSWER, so it fails on both platforms. + + Verified end-to-end on Linux, before → after: the 4-line repro 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 byte-identical to node v26.5.1 afterwards. diff --git a/crates/perry-runtime/src/object/reflect_support.rs b/crates/perry-runtime/src/object/reflect_support.rs index 448831e752..f52af05d1f 100644 --- a/crates/perry-runtime/src/object/reflect_support.rs +++ b/crates/perry-runtime/src/object/reflect_support.rs @@ -73,6 +73,47 @@ pub(crate) fn obj_value_has_own_key(value: f64, key: f64) -> bool { key_str, ); } + // Buffer / ArrayBuffer / DataView NEXT, and for the same reason + // (#8117). These receivers are `BufferHeader`s, not `ObjectHeader`s: + // they have no `class_id` and no `keys_array`. Nothing below rejected + // them, so the ordinary arm read `(*obj).keys_array` out of the bytes + // that follow a buffer header, and handed that to `js_array_length` — + // which dereferences `addr - 8` for its lazy-array probe. The only + // thing between the two was a `< 0x10000` magnitude floor, which + // arbitrary payload bytes clear routinely: + // + // const b: any = Buffer.alloc(8); + // b.readUInt8 = function () { return "shadowed"; }; + // const k = "readUInt8"; + // b[k](0); // SIGSEGV, 10/10 on Linux + // + // reached through `js_put_value_set_dyn_ic_miss` -> + // `proxy::ordinary_set_with_receiver` -> `proxy::own_set_descriptor`. + // It is the same "ask the receiver question before the generic walk + // claims it" shape as #8090/#8109/#8119/#8120, on the has-own-key path. + // + // A buffer's OWN string keys are exactly its expando table (#6406). + // Prototype methods (`readUInt8`, `subarray`, …) are inherited, not + // own, so they must answer false — that is what lets `b.readUInt8 = fn` + // install a shadowing own property instead of being 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. + if crate::buffer::is_registered_buffer(obj_addr) { + // `key_to_rust_string` runs `js_string_coerce`, which allocates and + // can therefore evacuate. The buffer's address is the side-table + // KEY, so carry it across the call on a handle rather than binding + // the pre-call value (#6943). + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); + let (key_name, obj) = + obj_handle.across_mut::(|| key_to_rust_string(key)); + let Some(key_name) = key_name else { + return false; + }; + return crate::buffer::buffer_has_own_prop(obj as usize, &key_name); + } if obj_addr >= crate::gc::GC_HEADER_SIZE + 0x1000 { let gc = gc_header_for(obj); if (*gc).obj_type == crate::gc::GC_TYPE_ARRAY @@ -146,7 +187,14 @@ pub(crate) fn obj_value_has_own_key(value: f64, key: f64) -> bool { let keys_handle = scope.root_raw_mut_ptr((*obj).keys_array); let key_handle = scope.root_string_ptr(key_str); let ((), mut keys) = keys_handle.across_mut::(|| ()); - if keys.is_null() || (keys as usize) < 0x10000 { + // Defence in depth for the class the buffer arm above closes by + // routing: `keys_array` is only an `ArrayHeader` when `obj` really is + // an `ObjectHeader`, and a receiver kind with no arm here reaches this + // line holding payload bytes. A bare magnitude floor does not catch + // that — use the canonical predicate, which rejects the handle band and + // anything outside the heap before `js_array_length` dereferences + // `keys - 8`. A missing arm should be a wrong answer, not a SIGSEGV. + if keys.is_null() || !crate::value::addr_class::is_plausible_heap_addr(keys as usize) { return false; } let key_count = crate::array::js_array_length(keys) as usize; diff --git a/crates/perry-runtime/src/object/tests.rs b/crates/perry-runtime/src/object/tests.rs index 3f4089449d..576cfb82ca 100644 --- a/crates/perry-runtime/src/object/tests.rs +++ b/crates/perry-runtime/src/object/tests.rs @@ -1586,3 +1586,55 @@ fn constructor_ref_method_value_resolves_static_over_instance_method() { the instance `lex`" ); } + +/// #8117: a `Buffer` / `DataView` receiver must not reach the ordinary +/// `ObjectHeader` walk in `obj_value_has_own_key`. +/// +/// A buffer is a `BufferHeader` — no `class_id`, no `keys_array`. With no arm +/// of its own it fell through to the ordinary arm, which 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 two platforms fail differently, which is why this test asserts the +/// ANSWER rather than merely "did not crash": +/// +/// * Linux: the payload bytes clear the old `< 0x10000` magnitude floor and the +/// dereference is a SIGSEGV. `b.readUInt8 = fn` reached through the dynamic +/// `[[Set]]` (`js_put_value_set_dyn_ic_miss` -> `proxy::ordinary_set_with_ +/// receiver` -> `proxy::own_set_descriptor`) crashed 10/10. +/// * macOS: the heap floor is high enough that the garbage usually reads as +/// null, so it silently answered "no own key" for a property the buffer +/// really owns. +/// +/// The first assertion below fails on BOTH. +#[test] +fn buffer_own_key_comes_from_the_expando_table_not_the_object_walk() { + let addr = crate::buffer::buffer_alloc(8) as usize; + crate::buffer::buffer_set_own_prop(addr, "myFlag", 42.0); + let receiver = crate::value::js_nanbox_pointer(addr as i64); + + let present = crate::string::js_string_from_bytes(b"myFlag".as_ptr(), 6); + let present_key = crate::value::js_nanbox_string(present as i64); + assert!( + obj_value_has_own_key(receiver, present_key), + "a buffer's own expando property must be reported as an own key" + ); + + // A `Buffer.prototype` method is INHERITED, not own. That is what lets + // `buf.readUInt8 = fn` install a shadowing own property instead of + // being treated as the redefinition of an existing one. + let inherited = crate::string::js_string_from_bytes(b"readUInt8".as_ptr(), 9); + let inherited_key = crate::value::js_nanbox_string(inherited as i64); + assert!( + !obj_value_has_own_key(receiver, inherited_key), + "a Buffer.prototype method is inherited, not an own key" + ); + + // And a key the buffer has never seen. + let absent = crate::string::js_string_from_bytes(b"nope".as_ptr(), 4); + let absent_key = crate::value::js_nanbox_string(absent as i64); + assert!( + !obj_value_has_own_key(receiver, absent_key), + "an unknown key is not an own key" + ); +}