-
-
Notifications
You must be signed in to change notification settings - Fork 161
fix(runtime): #9192 — an array with a non-array [[Prototype]] inherited nothing (47 divergences from node → 19) #9219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| ### fix(runtime): an array retargeted to a non-array prototype inherits from it | ||
|
|
||
| `Object.setPrototypeOf(arr, someObject)` recorded the new `[[Prototype]]` — | ||
| and paid the process-wide array-index deoptimisation for it — but the lookup | ||
| paths then declined to consult it: the index probe accepted a recorded | ||
| prototype only when the prototype was itself an array, and the named-property | ||
| fallback hardcoded `Array.prototype`. A retargeted array therefore inherited | ||
| nothing from its new prototype while still inheriting everything from the old | ||
| one, with no error: `a[7]` and `a.foo` were `undefined`, `7 in a` was `false`, | ||
| and `typeof a.map` was still `"function"`. | ||
|
|
||
| Index reads/`in`/writes, named reads, `in` on a name, `arr.constructor`, | ||
| `arr.__proto__`, symbol-keyed reads, and method dispatch (`arr.first()`, the | ||
| ES5 `MyList.prototype = Object.create(Array.prototype)` idiom) now all resolve | ||
| through the recorded chain, with the array bound as the receiver so a | ||
| prototype accessor observes the right `this`. `Object.setPrototypeOf(arr, | ||
| null)` correctly inherits nothing at all. Named properties on an *array* | ||
| prototype, which never resolved either, are fixed by the same change. Arrays | ||
| with the default prototype are untouched. Fixes #9192. |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| //! Test-only bounds and receiver checks split out of `indexing.rs` to keep | ||
| //! it under the 2000-line cap. The modules are unchanged; only their home | ||
| //! file moved. | ||
|
|
||
| #[cfg(test)] | ||
| mod keys_len_cap_tests { | ||
| use crate::array::js_array_length; | ||
| use crate::array::keys_array_len_capped_to_capacity; | ||
|
|
||
| #[test] | ||
| fn keys_len_capped_bounds_bogus_length_to_capacity() { | ||
| // Freshly-allocated array: well-formed (length 0 <= capacity), so the | ||
| // cap is a no-op and returns the real length. | ||
| let arr = crate::array::js_array_alloc(8); | ||
| let capacity = unsafe { (*arr).capacity } as usize; | ||
| assert!(capacity >= 8); | ||
| assert_eq!(unsafe { keys_array_len_capped_to_capacity(arr) }, 0); | ||
|
|
||
| // Simulate a malformed keys array whose length field reports a bogus, | ||
| // pointer-sized value — the pathology the object property walks guard | ||
| // against. Un-capped, callers would iterate/allocate ~645M slots. | ||
| unsafe { | ||
| (*arr).length = 645_115_168; | ||
| } | ||
| assert_eq!( | ||
| js_array_length(arr) as usize, | ||
| 645_115_168, | ||
| "sanity: js_array_length reflects the forged length" | ||
| ); | ||
| assert_eq!( | ||
| unsafe { keys_array_len_capped_to_capacity(arr) }, | ||
| capacity, | ||
| "cap must bound a bogus oversized length to the array's capacity" | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod claimed_array_string_receiver_tests { | ||
| use crate::array::indexing::array_get_property_by_key; | ||
|
|
||
| #[test] | ||
| fn numeric_string_key_reads_a_heap_string_before_by_name_fallback() { | ||
| let receiver = crate::string::js_string_from_bytes(b"ss".as_ptr(), 2); | ||
| let zero = crate::string::js_string_from_bytes(b"0".as_ptr(), 1); | ||
| let indexed = array_get_property_by_key(receiver.cast(), zero); | ||
| assert_eq!( | ||
| crate::builtins::jsvalue_string_content(indexed).as_deref(), | ||
| Some("s") | ||
| ); | ||
|
|
||
| let length = crate::string::js_string_from_bytes(b"length".as_ptr(), 6); | ||
| assert_eq!(array_get_property_by_key(receiver.cast(), length), 2.0); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| //! Test-only strict-dense store helpers, split out of `indexing.rs` to keep | ||
| //! it under the 2000-line cap. `#![cfg(test)]` at module level, so the | ||
| //! per-item `#[cfg(test)]` attributes the originals carried are dropped. | ||
|
|
||
| #![cfg(test)] | ||
|
|
||
| use super::indexing::{try_strict_dense_number_store, try_strict_dense_pointer_overwrite}; | ||
| use super::*; | ||
|
|
||
| pub(crate) fn test_strict_dense_pointer_overwrite( | ||
| arr: *mut ArrayHeader, | ||
| index: u32, | ||
| value: f64, | ||
| ) -> bool { | ||
| unsafe { try_strict_dense_pointer_overwrite(arr, index, value) }.is_some() | ||
| } | ||
|
|
||
| pub(crate) fn test_strict_dense_number_store( | ||
| arr: *mut ArrayHeader, | ||
| index: u32, | ||
| value: f64, | ||
| ) -> bool { | ||
| unsafe { try_strict_dense_number_store(arr, index, value) }.is_some() | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| //! #9192: array property slots that a retargeted `[[Prototype]]` changes. | ||
| //! | ||
| //! Split out of `get_field_by_name_tail.rs`, which is at the 2000-line cap. | ||
| //! Both slots answer off the static-prototype side table rather than the | ||
| //! implicit array chain, because a recorded custom `[[Prototype]]` replaces | ||
| //! that chain entirely. | ||
|
|
||
| use super::accessors::array_prototype_property_value; | ||
| use crate::value::JSValue; | ||
|
|
||
| /// `arr.__proto__` IS the array's `[[Prototype]]` — the spec models it as an | ||
| /// `Object.prototype` accessor returning `[[GetPrototypeOf]](this)`. Without | ||
| /// this a retargeted array reported the WRONG object while | ||
| /// `Object.getPrototypeOf(arr)` reported the right one. | ||
| pub(super) fn array_proto_slot(obj: *const crate::object::ObjectHeader) -> JSValue { | ||
| // `__proto__` itself lives on `Object.prototype`, so an array whose chain | ||
| // no longer reaches it (an explicit null prototype) has no such property. | ||
| if crate::object::prototype_chain::object_static_prototype(obj as usize) | ||
| == Some(crate::value::TAG_NULL) | ||
| { | ||
| return JSValue::undefined(); | ||
| } | ||
| let receiver = crate::value::js_nanbox_pointer(obj as i64); | ||
| let proto = | ||
| crate::object::object_ops::js_object_get_prototype_of(f64::from_bits(receiver.to_bits())); | ||
| JSValue::from_bits(proto.to_bits()) | ||
| } | ||
|
|
||
| /// A recorded custom `[[Prototype]]` replaces the whole implicit chain, so | ||
| /// `constructor` must resolve through it — a plain `{}` prototype answers | ||
| /// `Object`, not `Array` — rather than short-circuiting to the global `Array`. | ||
| /// `None` means no retarget was recorded and the caller keeps its fast path. | ||
| pub(super) fn array_constructor_slot(obj: *const crate::object::ObjectHeader) -> Option<JSValue> { | ||
| crate::object::prototype_chain::object_static_prototype(obj as usize)?; | ||
| // SAFETY: the caller has already established `obj` as a live array | ||
| // header; this reads the recorded prototype's own properties. | ||
| Some( | ||
| unsafe { array_prototype_property_value("constructor", obj as usize) } | ||
| .unwrap_or_else(JSValue::undefined), | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -279,6 +279,34 @@ unsafe fn req_handle_symbol_fallback(obj_f64: f64, sym_f64: f64) -> Option<f64> | |
| } | ||
|
|
||
| unsafe fn object_header_ptr_from_value_bits(bits: u64) -> Option<usize> { | ||
| let (raw, obj_type) = heap_ptr_and_type_from_value_bits(bits)?; | ||
| if obj_type == crate::gc::GC_TYPE_OBJECT { | ||
| Some(raw) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
|
Comment on lines
+282
to
+288
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Continue symbol lookup through array prototype hops.
Allow arrays and lazy arrays as chain owners. Keep 🤖 Prompt for AI Agents |
||
|
|
||
| /// #9192: [`object_header_ptr_from_value_bits`] for the RECEIVER position, | ||
| /// which may be a real array (`Object.setPrototypeOf(arr, {[S]: v})`). Only the | ||
| /// ADDRESS is used here — as the key of the recorded-`[[Prototype]]` lookup — | ||
| /// so an `ArrayHeader` is safe, while the chain HOPS still require a genuine | ||
| /// `GC_TYPE_OBJECT` before anything is dereferenced as one. | ||
| unsafe fn receiver_ptr_from_value_bits(bits: u64) -> Option<usize> { | ||
| let (raw, obj_type) = heap_ptr_and_type_from_value_bits(bits)?; | ||
| if obj_type == crate::gc::GC_TYPE_OBJECT | ||
| || obj_type == crate::gc::GC_TYPE_ARRAY | ||
| || obj_type == crate::gc::GC_TYPE_LAZY_ARRAY | ||
| { | ||
| Some(raw) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
|
|
||
| /// Validate a value's bits as a live tracked heap allocation and report its | ||
| /// address together with its GC type byte. | ||
| unsafe fn heap_ptr_and_type_from_value_bits(bits: u64) -> Option<(usize, u8)> { | ||
| let top16 = bits >> 48; | ||
| let raw = if top16 == 0x7FFD { | ||
| (bits & POINTER_MASK) as usize | ||
|
|
@@ -304,11 +332,7 @@ unsafe fn object_header_ptr_from_value_bits(bits: u64) -> Option<usize> { | |
| if !tracked_malloc && !(arena_payload && arena_header) { | ||
| return None; | ||
| } | ||
| if (*gc_header).obj_type == crate::gc::GC_TYPE_OBJECT { | ||
| Some(raw) | ||
| } else { | ||
| None | ||
| } | ||
| Some((raw, (*gc_header).obj_type)) | ||
| } | ||
|
|
||
| /// Walk the explicit static prototype chain to find an inherited symbol property. | ||
|
|
@@ -320,7 +344,11 @@ pub(crate) unsafe fn inherited_symbol_property(obj_f64: f64, sym_f64: f64) -> Op | |
|
|
||
| unsafe fn resolve_explicit_object_prototype_symbol(obj_f64: f64, sym_f64: f64) -> Option<f64> { | ||
| const TAG_NULL: u64 = 0x7FFC_0000_0000_0002; | ||
| let mut owner = object_header_ptr_from_value_bits(obj_f64.to_bits())?; | ||
| // #9192: the receiver may be a real ARRAY with a retargeted `[[Prototype]]` | ||
| // (`Object.setPrototypeOf(arr, {[S]: v})`). Its address is only a lookup | ||
| // key here, so accept it; every chain HOP below still demands a real | ||
| // `GC_TYPE_OBJECT` before dereferencing. | ||
| let mut owner = receiver_ptr_from_value_bits(obj_f64.to_bits())?; | ||
| let mut visited_buf = [0usize; 16]; | ||
| let mut visited_len = 0usize; | ||
| let mut visited_overflow: Option<std::collections::HashSet<usize>> = None; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not fall through after a non-callable prototype match.
If a custom prototype defines
map: 1,dispatch_handle_proto_methodresolves that property but returnsNonebecause it is not a closure. The subsequent built-inmaparm then runs instead of reporting a non-callable invocation.Distinguish a missing property from a resolved non-callable property. Preserve the resolved value so the normal call validation can report the required
TypeError.🤖 Prompt for AI Agents
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Root the array receiver during custom method dispatch.
dispatch_handle_proto_methodallocates the key and can run user code inresolve_inherited_field, but it retainshandle_id,object, the key, the resolved closure, and the saved implicitthisas bare values. A moving collection can relocate those values before reuse. This new array path can then resolve with a stale address or restore stalethis.Create a
RuntimeHandleScopeinsidedispatch_handle_proto_method. Root and reload the receiver, key, resolved value, and savedthisaround every allocating or user-code call.Based on learnings, root a NaN-boxed value before an operation that can evacuate its object and reload it from its handle before reuse.
🤖 Prompt for AI Agents
Source: Learnings