-
-
Notifications
You must be signed in to change notification settings - Fork 161
fix(runtime): materialize Object.prototype.__proto__ as a real accessor #10647
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,4 @@ | ||
| ### Fixed | ||
|
|
||
| - `Object.prototype` now has a real, spec-shaped `__proto__` accessor (`{ get, set, enumerable: false, configurable: true }`), so `hasOwnProperty`, `Object.hasOwn`, `Object.getOwnPropertyNames`, `Object.getOwnPropertyDescriptor`, `Reflect.ownKeys`, and `"__proto__" in obj` all agree with Node about it — closing a prototype-pollution guard bypass in libraries (e.g. `qs`) that use `hasOwnProperty.call(Object.prototype, key)` to reject `"__proto__"` as a key. | ||
| - Fixed a related bug the new accessor exposed: reading `.__proto__` on a `Number`/`String` primitive via a dynamic property access (`(5).__proto__`) could invoke an accessor inherited from `Object.prototype` with `this` bound to the intermediate builtin prototype (`Number.prototype`) instead of the original primitive, answering `Object.prototype` instead of `Number.prototype`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,6 +67,109 @@ fn install_array_iterator_symbol(proto_obj: *mut ObjectHeader, value: f64) { | |
| ); | ||
| } | ||
|
|
||
| /// #10482: install `Object.prototype.__proto__` as a REAL accessor | ||
| /// descriptor — `{ get, set, enumerable: false, configurable: true }`, per | ||
| /// ECMA-262 Annex B §B.3.1 — instead of the purely behavioral special-casing | ||
| /// Perry had before (reads/writes worked through `__proto__` as a magic key | ||
| /// name in several call sites, but nothing on `Object.prototype` reflected | ||
| /// it). `hasOwnProperty`/`Object.hasOwn`/`getOwnPropertyNames`/ | ||
| /// `getOwnPropertyDescriptor`/`"__proto__" in {}`/`Reflect.ownKeys` all read | ||
| /// `ACCESSOR_DESCRIPTORS`/`PROPERTY_DESCRIPTORS` unconditionally, so a real | ||
| /// entry here is what makes them agree with Node. | ||
| /// | ||
| /// Uses `set_builtin_accessor_descriptor` (gate-neutral): it does not flip | ||
| /// `GLOBAL_DESCRIPTORS_IN_USE` / `ACCESSORS_IN_USE`, so ordinary property | ||
| /// read/write fast paths are unaffected for every OTHER key. `__proto__` | ||
| /// itself was already treated as unconditionally interceptable by | ||
| /// `object_proto_may_intercept_key` / `plain_custom_prototype_may_intercept` | ||
| /// (see `object/descriptor_state.rs`) before this change, so installing a | ||
| /// real descriptor for it changes no hot-path gate this key didn't already | ||
| /// trip — only what reflection sees. | ||
| /// | ||
| /// The getter delegates to `js_object_get_prototype_of`, which already | ||
| /// implements the getter's exact spec shape (ToObject-style wrapper | ||
| /// resolution for primitives, Proxy/Temporal/handle receivers, and a throw | ||
| /// on `null`/`undefined`). The setter delegates to | ||
| /// `proxy::legacy_dunder_proto_set`, the same Annex-B logic `proxy.rs`'s | ||
| /// `ordinary_set_with_receiver` used to inline for this one key (#6828) — | ||
| /// now shared so both call sites can never drift apart. Once this | ||
| /// descriptor exists, `own_set_descriptor` finds it and dispatches through | ||
| /// the ordinary accessor-setter path before that inlined special case is | ||
| /// ever reached (see the comment there). | ||
| fn install_object_prototype_dunder_proto(proto_obj: *mut ObjectHeader) { | ||
| if proto_obj.is_null() { | ||
| return; | ||
| } | ||
| let getter = crate::closure::js_closure_alloc( | ||
| object_prototype_dunder_proto_getter_thunk as *const u8, | ||
| 0, | ||
| ); | ||
| let setter = crate::closure::js_closure_alloc( | ||
| object_prototype_dunder_proto_setter_thunk as *const u8, | ||
| 0, | ||
| ); | ||
| if getter.is_null() || setter.is_null() { | ||
| return; | ||
| } | ||
| crate::closure::js_register_closure_arity( | ||
| object_prototype_dunder_proto_getter_thunk as *const u8, | ||
| 0, | ||
| ); | ||
| crate::closure::js_register_closure_arity( | ||
| object_prototype_dunder_proto_setter_thunk as *const u8, | ||
| 1, | ||
| ); | ||
| super::super::native_module::set_bound_native_closure_name(getter, "get __proto__"); | ||
| super::super::native_module::set_bound_native_closure_name(setter, "set __proto__"); | ||
| super::super::native_module::set_builtin_closure_length(getter as usize, 0); | ||
| super::super::native_module::set_builtin_closure_length(setter as usize, 1); | ||
| super::super::native_module::set_builtin_closure_non_constructable(getter as usize); | ||
| super::super::native_module::set_builtin_closure_non_constructable(setter as usize); | ||
| let get_bits = crate::value::js_nanbox_pointer(getter as i64).to_bits(); | ||
| let set_bits = crate::value::js_nanbox_pointer(setter as i64).to_bits(); | ||
| // A descriptor alone doesn't make the name enumerable by | ||
| // `getOwnPropertyNames`/`hasOwnProperty`/`Object.hasOwn`/ | ||
| // `Reflect.ownKeys` — those walk the object's OWN KEYS ARRAY, which | ||
| // `set_builtin_accessor_descriptor` (deliberately gate-neutral) never | ||
| // touches. Write an ordinary placeholder field first, exactly like | ||
| // `perf_hooks::install_perf_getter`: this appends `"__proto__"` to the | ||
| // keys array via the ordinary field-set path, and the accessor | ||
| // descriptor installed right after takes over every actual read/write — | ||
| // the placeholder `undefined` is never observed. | ||
| let key = crate::string::js_string_from_bytes(b"__proto__".as_ptr(), 9); | ||
| js_object_set_field_by_name(proto_obj, key, f64::from_bits(crate::value::TAG_UNDEFINED)); | ||
|
Comment on lines
+103
to
+140
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. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '60,155p' crates/perry-runtime/src/object/global_this/proto_methods.rs
sed -n '190,320p' crates/perry-runtime/src/error_subclass_stack.rs
rg -n 'RuntimeHandleScope|root_nanbox|root_ptr|js_closure_alloc|set_builtin_accessor_descriptor' crates/perry-runtime/src/object/global_this crates/perry-runtime/src/error_subclass_stack.rs | head -120Repository: PerryTS/perry Length of output: 23115 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- GC handle scope definitions ---'
rg -n -A100 -B20 'struct RuntimeHandleScope|impl RuntimeHandleScope|root_nanbox_f64|root_ptr|across_nanbox|refreshed_nanbox' crates/perry-runtime/src
printf '%s\n' '--- closure allocation and metadata operations ---'
rg -n -A35 -B15 'fn js_closure_alloc|pub.*js_closure_alloc|set_bound_native_closure_name|set_builtin_closure_length|set_builtin_closure_non_constructable|set_builtin_accessor_descriptor' crates/perry-runtime/src/object crates/perry-runtime/src/closure.rs crates/perry-runtime/src
printf '%s\n' '--- key insertion and field-set definitions ---'
rg -n -A80 -B20 'fn ensure_key_in_keys_array|pub.*ensure_key_in_keys_array|fn js_object_set_field_by_name|pub.*js_object_set_field_by_name' crates/perry-runtime/src
printf '%s\n' '--- analogous accessor installers ---'
sed -n '700,760p' crates/perry-runtime/src/object/global_this/populate.rs
sed -n '930,990p' crates/perry-runtime/src/object/global_this/populate.rs
sed -n '320,430p' crates/perry-runtime/src/object/global_this/proto_methods.rs
sed -n '1080,1145p' crates/perry-runtime/src/object/global_this/proto_methods.rsRepository: PerryTS/perry Length of output: 50370 🏁 Script executed: sed -n '1,220p' crates/perry-runtime/src/gc.rs; rg -n -A80 -B20 'set_builtin_accessor_descriptor|ensure_key_in_keys_array|js_object_set_field_by_name' crates/perry-runtime/src/object crates/perry-runtime/src; sed -n '700,760p' crates/perry-runtime/src/object/global_this/populate.rs; sed -n '930,990p' crates/perry-runtime/src/object/global_this/populate.rsRepository: PerryTS/perry Length of output: 50370 🏁 Script executed: #!/bin/bash
set -e
cat -n crates/perry-runtime/src/object/global_this/proto_methods.rs | sed -n '70,155p;1100,1145p'
rg -n -A70 -B15 'install_object_prototype_dunder_proto\(' crates/perry-runtime/src
rg -n -A80 -B20 'pub.*fn set_builtin_accessor_descriptor|fn set_builtin_accessor_descriptor|ensure_key_in_keys_array|js_object_set_field_by_name' crates/perry-runtime/src/object
rg -n -A80 -B20 'pub.*fn js_closure_alloc|fn js_closure_alloc' crates/perry-runtime/src/closure.rs crates/perry-runtime/src
rg -n -A100 -B15 'struct RuntimeHandleScope|impl RuntimeHandleScope' crates/perry-runtime/src/gc.rs crates/perry-runtime/src/gcRepository: PerryTS/perry Length of output: 50370 🏁 Script executed: #!/bin/bash
python3 - <<'PY'
from pathlib import Path
roots = [Path("crates/perry-runtime/src")]
needles = [
"pub struct RuntimeHandleScope",
"struct RuntimeHandleScope",
"impl RuntimeHandleScope",
"pub unsafe extern \"C\" fn js_closure_alloc",
"pub extern \"C\" fn js_closure_alloc",
"fn js_closure_alloc",
"pub unsafe fn js_object_set_field_by_name",
"pub fn js_object_set_field_by_name",
"fn js_object_set_field_by_name",
"fn ensure_key_in_keys_array",
"pub fn ensure_key_in_keys_array",
"fn set_builtin_accessor_descriptor",
"pub(crate) fn set_builtin_accessor_descriptor",
"install_object_prototype_dunder_proto(",
]
for needle in needles:
found = []
for root in roots:
for path in root.rglob("*.rs"):
try:
lines = path.read_text(errors="replace").splitlines()
except OSError:
continue
for i, line in enumerate(lines):
if needle in line:
found.append((path, i, lines))
for path, i, lines in found:
print(f"\n--- {needle} @ {path}:{i+1} ---")
lo, hi = max(0, i-8), min(len(lines), i+45)
for n in range(lo, hi):
print(f"{path}:{n+1}:{lines[n]}")
PYRepository: PerryTS/perry Length of output: 50370 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- helper files ---'
rg -l 'ensure_key_in_keys_array|set_builtin_accessor_descriptor|js_object_set_field_by_name' crates/perry-runtime/src/object | sort
printf '%s\n' '--- installer call and local function ---'
rg -n -A8 -B8 'install_object_prototype_dunder_proto' crates/perry-runtime/src/object/global_this/proto_methods.rs crates/perry-runtime/src/object/global_this
printf '%s\n' '--- relevant helper definitions ---'
for f in $(rg -l 'ensure_key_in_keys_array|set_builtin_accessor_descriptor|js_object_set_field_by_name' crates/perry-runtime/src/object | sort | head -20); do
rg -n -A45 -B10 'fn ensure_key_in_keys_array|pub.*ensure_key_in_keys_array|fn set_builtin_accessor_descriptor|pub.*set_builtin_accessor_descriptor|fn js_object_set_field_by_name|pub.*js_object_set_field_by_name' "$f" || true
doneRepository: PerryTS/perry Length of output: 15595 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- key-array helper ---'
rg -n -A80 -B20 'ensure_key_in_keys_array' crates/perry-runtime/src/object/object_ops/keys_array.rs crates/perry-runtime/src/object/object_ops.rs crates/perry-runtime/src/object/mod.rs
printf '%s\n' '--- field-set entry and key insertion path ---'
rg -n -A100 -B25 'js_object_set_field_by_name' crates/perry-runtime/src/object/field_set_by_name.rs crates/perry-runtime/src/object/field_set_by_name crates/perry-runtime/src/object/object_ops/keys_array.rs
printf '%s\n' '--- Object prototype installer caller ---'
sed -n '330,475p' crates/perry-runtime/src/object/global_this/proto_methods.rsRepository: PerryTS/perry Length of output: 50370 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- key-array allocation and append tail ---'
sed -n '80,220p' crates/perry-runtime/src/object/object_ops/keys_array.rs
printf '%s\n' '--- field-set entry/tail references ---'
rg -n -A35 -B15 'ensure_key_in_keys_array|js_array_push|keys_array' crates/perry-runtime/src/object/field_set_by_name/tail.rs crates/perry-runtime/src/object/field_set_by_name/write_helpers.rs crates/perry-runtime/src/object/field_set_by_name/fast_paths.rs | head -240
printf '%s\n' '--- enclosing installer ---'
sed -n '330,475p' crates/perry-runtime/src/object/global_this/proto_methods.rsRepository: PerryTS/perry Length of output: 39746 Root the accessor installation state across allocations.
Create a 🤖 Prompt for AI Agents |
||
| super::super::set_builtin_accessor_descriptor( | ||
| proto_obj as usize, | ||
| "__proto__".to_string(), | ||
| super::super::AccessorDescriptor { | ||
| get: get_bits, | ||
| set: set_bits, | ||
| }, | ||
| crate::object::PropertyAttrs::new(true, false, true), | ||
| ); | ||
| } | ||
|
|
||
| extern "C" fn object_prototype_dunder_proto_getter_thunk( | ||
| _closure: *const crate::closure::ClosureHeader, | ||
| ) -> f64 { | ||
| // Spec (Annex B §B.3.1 `get __proto__`): `ToObject(this).[[GetPrototypeOf]]()`. | ||
| // `js_object_get_prototype_of` already implements exactly this shape — | ||
| // wrapper-prototype resolution for primitives, Proxy/Temporal/handle | ||
| // receivers, and a throw on `null`/`undefined` (the `ToObject` failure | ||
| // case) — so the getter is a direct delegation, not a reimplementation. | ||
| let receiver = crate::object::js_implicit_this_get(); | ||
| crate::object::js_object_get_prototype_of(receiver) | ||
| } | ||
|
|
||
| extern "C" fn object_prototype_dunder_proto_setter_thunk( | ||
| _closure: *const crate::closure::ClosureHeader, | ||
| value: f64, | ||
| ) -> f64 { | ||
| let receiver = crate::object::js_implicit_this_get(); | ||
| crate::proxy::legacy_dunder_proto_set(receiver, value); | ||
| f64::from_bits(crate::value::TAG_UNDEFINED) | ||
| } | ||
|
|
||
| pub(crate) fn populate_builtin_prototype_methods(builtin_name: &str, proto_obj: *mut ObjectHeader) { | ||
| if proto_obj.is_null() { | ||
| return; | ||
|
|
@@ -360,6 +463,7 @@ pub(crate) fn populate_builtin_prototype_methods(builtin_name: &str, proto_obj: | |
| object_prototype_property_is_enumerable_thunk as *const u8, | ||
| 1, | ||
| ); | ||
| install_object_prototype_dunder_proto(proto_obj); | ||
| } | ||
| "Function" => { | ||
| // `Function.prototype` has own `length` (0) and `name` ("") data | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -767,6 +767,30 @@ fn reflect_value_is_symbol(value: f64) -> bool { | |
| && unsafe { crate::symbol::js_is_symbol(value) != 0 } | ||
| } | ||
|
|
||
| /// #6828/#10482: Annex B §B.3.1 `set __proto__` semantics — shared by the | ||
| /// real accessor descriptor installed on `Object.prototype` | ||
| /// (`object/global_this/proto_methods.rs`'s setter closure, reached via | ||
| /// `own_set_descriptor` + `call_setter_with_receiver` above) and this | ||
| /// function's own caller (the walk's defensive fallback for the same key). | ||
| /// Both must behave identically, so both call this one implementation | ||
| /// instead of keeping the logic written out twice. | ||
| /// | ||
| /// Per spec: a non-object/non-null `value` or a non-object `receiver` is | ||
| /// silently ignored (no throw, unlike `Object.setPrototypeOf`); a genuine | ||
| /// `[[SetPrototypeOf]]` failure (cyclic / non-extensible) still throws via | ||
| /// `js_object_set_prototype_of` itself, matching `Object.setPrototypeOf`'s | ||
| /// failure behavior for that case. | ||
| pub(crate) fn legacy_dunder_proto_set(receiver: f64, value: f64) { | ||
| let value_bits = value.to_bits(); | ||
| let valid_proto = value_bits == TAG_NULL | ||
| || lookup(value).is_some() | ||
| || crate::object::class_ref_id(value).is_some() | ||
| || unsafe { crate::object::value_is_object_like(value) }; | ||
| if valid_proto && reflect_value_is_object(receiver) { | ||
| crate::object::js_object_set_prototype_of(receiver, value); | ||
|
Comment on lines
+783
to
+790
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 Throw for null and undefined reflected setter receivers. The direct reflected setter path passes an explicitly supplied Add the null and undefined receiver check before 🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
|
|
||
| /// Is `value` a Reflect-acceptable object? Heap objects, class refs (callable | ||
| /// constructors), and proxies all count. Primitives / null / undefined do not. | ||
| pub(crate) fn reflect_value_is_object(value: f64) -> bool { | ||
|
|
@@ -2207,31 +2231,28 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) | |
| } | ||
| }; | ||
| } | ||
| // #6828: `%Object.prototype%.__proto__` is a legacy accessor whose | ||
| // setter performs `SetPrototypeOf(Receiver, value)`. Perry exposes the | ||
| // getter intrinsically but does not materialize the built-in accessor | ||
| // in the ordinary descriptor table, so model it at the exact point in | ||
| // the [[Set]] walk where that descriptor would be found. | ||
| // #6828/#10482: `%Object.prototype%.__proto__` is a legacy accessor | ||
| // whose setter performs `SetPrototypeOf(Receiver, value)`. | ||
| // `object/global_this/proto_methods.rs` now materializes it as a | ||
| // REAL accessor descriptor on `Object.prototype` (#10482), so | ||
| // `own_set_descriptor` just above finds it and dispatches through | ||
| // `call_setter_with_receiver` before the walk ever reaches here — | ||
| // this arm is kept as a fallback for a walk that reaches | ||
| // `Object.prototype` without ever consulting the descriptor table | ||
| // (defensive; not known to be reachable). Both arms must behave | ||
| // identically, so both call the one shared implementation. | ||
| // | ||
| // Keep this AFTER `own_set_descriptor`: a user-installed own | ||
| // `__proto__` data/accessor property on an object earlier in the chain | ||
| // must win. A null-prototype receiver never reaches the canonical | ||
| // Object.prototype and therefore still creates an ordinary own data | ||
| // property. Per Annex B, a primitive RHS is ignored rather than | ||
| // throwing (unlike `Object.setPrototypeOf`). | ||
| // property. | ||
| let current_addr = extract_pointer(current.to_bits()) as usize; | ||
| if current_addr != 0 | ||
| && current_addr == crate::array::object_prototype_addr() | ||
| && key_to_rust_string(key).as_deref() == Some("__proto__") | ||
| { | ||
| let value_bits = value.to_bits(); | ||
| let valid_proto = value_bits == TAG_NULL | ||
| || lookup(value).is_some() | ||
| || crate::object::class_ref_id(value).is_some() | ||
| || unsafe { crate::object::value_is_object_like(value) }; | ||
| if valid_proto && reflect_value_is_object(receiver) { | ||
| crate::object::js_object_set_prototype_of(receiver, value); | ||
| } | ||
| legacy_dunder_proto_set(receiver, value); | ||
| return true; | ||
| } | ||
| if crate::closure::is_closure_ptr(extract_pointer(current.to_bits()) as usize) { | ||
|
|
||
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.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 8706
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 14613
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 28374
Root the displaced accessor override during the recursive lookup.
accessor_receiver_override_beginreturns the existing outer receiver from the thread-local override cell. That receiver can be a GC-managed pointer.js_object_get_field_by_namecan invoke a getter, which can trigger moving collection. The GC updates the rooted cell, but not the rawprev_overridelocal. Restoring that local can publish a stale pointer.📝 Committable suggestion
🤖 Prompt for AI Agents