Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions changelog.d/8141-buffer-receiver-own-key.md
Original file line number Diff line number Diff line change
@@ -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.
50 changes: 49 additions & 1 deletion crates/perry-runtime/src/object/reflect_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<super::ObjectHeader, _>(|| 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
Expand Down Expand Up @@ -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::<crate::array::ArrayHeader, _>(|| ());
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;
Expand Down
52 changes: 52 additions & 0 deletions crates/perry-runtime/src/object/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}
Loading