Skip to content

Commit 2c88921

Browse files
proggeramlugRalph
andauthored
fix(runtime): #5844 — Proxy trap dispatch fixes across getOwnPropertyDescriptor, has, set, and setPrototypeOf (#5882)
* fix(runtime): #5844 — Proxy trap dispatch fixes across getOwnPropertyDescriptor, has, set, and setPrototypeOf Fixes 12 of 33 test262 built-ins/Proxy failures, zero regressions (verified against built-ins/Proxy, built-ins/Object, built-ins/Reflect, language/expressions/in — 219/219/3132/101/15 pass respectively). Root causes, all variations on "a Proxy is a small registered id, not a real heap pointer, and several generic object-machinery code paths either mis-treated it as one or never checked for it at all": - js_reflect_get_own_property_descriptor: the missing-trap fallback read the target's descriptor via the raw ObjectHeader path even when the target was itself a Proxy; now recurses through the Reflect entry point so a chain of trap-less proxies forwards correctly. - ordinary_has_property (the `in` operator's prototype-chain walk) and its array-fast-path counterpart didn't recognize a Proxy sitting in the recorded `[[Prototype]]` chain, silently returning false (or reading garbage) instead of dispatching the proxy's `has` trap. - ordinary_set_with_receiver's prototype-chain walk had the same gap on the write side, and `js_proxy_set`/`Reflect.set` didn't thread a distinct `receiver` through to the trap when a Proxy was reached via a prototype hop rather than as the direct assignment target. - Object.create(proto) and Object.setPrototypeOf(obj, proto) rejected a Proxy `proto` argument outright (their "is this an object" checks don't recognize the small registered id), and Object.create had no path to record a Proxy prototype at all — it can't be modeled via the synthetic-class-id machinery used for real prototype objects, so route it through the same static-prototype side table setPrototypeOf uses. - Object.setPrototypeOf's cycle-detection walk called `[[GetPrototypeOf]]` unconditionally while probing the candidate prototype's chain, including on a Proxy — invoking its trap as an unrelated side effect of cycle-safety bookkeeping (OrdinarySetPrototypeOf step 7.b.ii.1 says to stop the walk there instead). Also fixed, found while debugging the above with a fresh (Proxy-free) repro: the cycle-detection loop's Floyd's tortoise-and-hare walk only guarded `tortoise` against an already-null position before advancing; `hare` (which steps twice per iteration) had no equivalent guard, so a 2-hop-to-null prototype chain re-advanced an already-null `hare` and threw "Cannot convert undefined or null to object" on a plain, entirely ordinary `Object.setPrototypeOf({}, {foo: 1})`. Refs #5844. * refactor(runtime): move js_proxy_set/proxy_set_with_receiver into proxy/put_value.rs proxy.rs grew to 2004 lines after rebasing onto main (which independently added ~30 lines), tripping the 2000-line file-size CI gate. Relocate the Proxy [[Set]] entry points into the sibling put_value.rs submodule (already the natural home for PutValue/[[Set]] dispatch) rather than trim comments to fit — same pattern as the file's other topical splits. Pure code motion, no behavior change (re-verified: built-ins/Proxy still 219 pass / 21 runtime-fail). --------- Co-authored-by: Ralph <ralph@skelpo.com>
1 parent 9a1dd0e commit 2c88921

7 files changed

Lines changed: 205 additions & 104 deletions

File tree

crates/perry-runtime/src/object/descriptors.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,6 +1368,7 @@ pub extern "C" fn js_object_create_with_props(proto_value: f64, props_value: f64
13681368
let proto_jv = crate::value::JSValue::from_bits(proto_value.to_bits());
13691369
let proto_is_symbol = unsafe { crate::symbol::js_is_symbol(proto_value) != 0 };
13701370
let proto_ok = proto_jv.is_null()
1371+
|| crate::proxy::js_proxy_is_proxy(proto_value) != 0
13711372
|| (!proto_is_symbol
13721373
&& (unsafe { value_is_object_like(proto_value) }
13731374
|| super::class_ref_id(proto_value).is_some()));

crates/perry-runtime/src/object/field_get_set/has_property.rs

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,18 @@ pub extern "C" fn js_object_has_property(obj: f64, key: f64) -> f64 {
295295
// / `arr.hasOwnProperty(i)` stay correct after `arr.length = N`.
296296
let arr = crate::array::clean_arr_ptr(obj_ptr as *const crate::array::ArrayHeader);
297297
let length = (*arr).length;
298+
// A Proxy installed as the array's `[[Prototype]]`
299+
// (`Object.setPrototypeOf(arr, proxy)`) — `array_spec_has_index`
300+
// only recognizes a *real array* custom prototype, so a Proxy
301+
// hop is silently treated as absent. Recover it here so the
302+
// idx/string-key misses below can fall back to the proxy's
303+
// `[[HasProperty]]` instead of a bare `false` (ECMA-262 10.1.7.1
304+
// step 5).
305+
let proxy_proto =
306+
super::super::prototype_chain::object_static_prototype(obj_ptr as usize)
307+
.filter(|&b| (b >> 48) == 0x7FFD)
308+
.map(f64::from_bits)
309+
.filter(|&v| crate::proxy::js_proxy_is_proxy(v) != 0);
298310
// Numeric key: extract the index. Accept both NaN-boxed i32
299311
// and plain f64 (e.g. literal `1`) provided it's a
300312
// non-negative integer in range.
@@ -329,6 +341,24 @@ pub extern "C" fn js_object_has_property(obj: f64, key: f64) -> f64 {
329341
if crate::array::object_prototype_has_index_prop(idx) {
330342
return nanbox_true;
331343
}
344+
if let Some(proxy) = proxy_proto {
345+
let idx_str = idx.to_string();
346+
let key_ptr = crate::string::js_string_from_bytes(
347+
idx_str.as_ptr(),
348+
idx_str.len() as u32,
349+
);
350+
let key_val = f64::from_bits(
351+
crate::value::js_nanbox_string(key_ptr as i64).to_bits(),
352+
);
353+
return if crate::value::js_is_truthy(crate::proxy::js_proxy_has(
354+
proxy, key_val,
355+
)) != 0
356+
{
357+
nanbox_true
358+
} else {
359+
nanbox_false
360+
};
361+
}
332362
return nanbox_false;
333363
}
334364
if key_val.is_any_string() {
@@ -353,12 +383,21 @@ pub extern "C" fn js_object_has_property(obj: f64, key: f64) -> f64 {
353383
{
354384
return nanbox_true;
355385
}
356-
return nanbox_false;
357-
}
358-
if array_prototype_property_value(key_name, obj_ptr as usize).is_some()
386+
} else if array_prototype_property_value(key_name, obj_ptr as usize)
387+
.is_some()
359388
{
360389
return nanbox_true;
361390
}
391+
if let Some(proxy) = proxy_proto {
392+
return if crate::value::js_is_truthy(crate::proxy::js_proxy_has(
393+
proxy, key,
394+
)) != 0
395+
{
396+
nanbox_true
397+
} else {
398+
nanbox_false
399+
};
400+
}
362401
}
363402
}
364403
}
@@ -497,6 +536,23 @@ unsafe fn ordinary_has_property(
497536
Some(b) if b == TAG_NULL => return false,
498537
Some(b) => {
499538
let top16 = b >> 48;
539+
// A Proxy prototype hop (ECMA-262 10.1.7.1 step 5: `Return ?
540+
// parent.[[HasProperty]](P)`) — the small registered proxy id is
541+
// NOT a real heap pointer, so continuing the raw-pointer walk
542+
// below would misread garbage (or crash). Dispatch through the
543+
// proxy's own `[[HasProperty]]` (trap, or its trap-less forward
544+
// through further proxy targets / the eventual real target) and
545+
// use its boolean result directly — that call already resolves
546+
// the rest of the chain.
547+
if top16 == 0x7FFD {
548+
let proto_val = f64::from_bits(b);
549+
if crate::proxy::js_proxy_is_proxy(proto_val) != 0 {
550+
let key_val =
551+
f64::from_bits(crate::value::js_nanbox_string(key as i64).to_bits());
552+
let result = crate::proxy::js_proxy_has(proto_val, key_val);
553+
return crate::value::js_is_truthy(result) != 0;
554+
}
555+
}
500556
let p = if top16 == 0x7FFD {
501557
(b & crate::value::POINTER_MASK) as usize
502558
} else if top16 == 0 && b > 0x10000 {

crates/perry-runtime/src/object/object_ops/define_properties.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ pub extern "C" fn js_object_set_prototype_of(obj_value: f64, proto: f64) -> f64
167167
let proto_is_null = proto_bits == TAG_NULL;
168168
let proto_is_symbol = unsafe { crate::symbol::js_is_symbol(proto) != 0 };
169169
let proto_ok = proto_is_null
170+
|| crate::proxy::js_proxy_is_proxy(proto) != 0
170171
|| (!proto_is_symbol
171172
&& (unsafe { value_is_object_like(proto) }
172173
|| super::super::class_ref_id(proto).is_some()));
@@ -209,6 +210,17 @@ pub extern "C" fn js_object_set_prototype_of(obj_value: f64, proto: f64) -> f64
209210
const TAG_UNDEFINED_U64: u64 = 0x7FFC_0000_0000_0001;
210211
let advance = |bits: u64| -> u64 {
211212
let val = f64::from_bits(bits);
213+
// OrdinarySetPrototypeOf step 7.b.ii.1: if `p`'s [[GetPrototypeOf]]
214+
// is not the ordinary internal method (a Proxy's is exotic — it may
215+
// run arbitrary trap code), the walk stops here without invoking it.
216+
// Without this guard the cycle-detection walk called the target's
217+
// `getPrototypeOf` trap as a side effect of unrelated cycle-safety
218+
// bookkeeping (test262 has/call-in-prototype-index.js,
219+
// set/call-parameters-prototype-index.js observe a `getPrototypeOf`
220+
// trap the test handler never installs).
221+
if crate::proxy::js_proxy_is_proxy(val) != 0 {
222+
return TAG_NULL_U64;
223+
}
212224
let next = js_object_get_prototype_of(val);
213225
let nb = next.to_bits();
214226
// Treat undefined as chain-end like null: `js_object_get_prototype_of`
@@ -236,11 +248,7 @@ pub extern "C" fn js_object_set_prototype_of(obj_value: f64, proto: f64) -> f64
236248
if tortoise == TAG_NULL_U64 {
237249
break;
238250
}
239-
// Advance tortoise one step, hare two steps. Guard the second
240-
// advance: if the first step lands on null, calling advance(null)
241-
// would invoke js_object_get_prototype_of(null) which throws
242-
// "Cannot convert undefined or null to object" (test262
243-
// setPrototypeOf/success.js — plain object proto chain ends at null).
251+
// Advance tortoise one step, hare two steps.
244252
tortoise = advance(tortoise);
245253
// The hare reaches the chain end (null) before the tortoise on any
246254
// acyclic chain longer than one link (e.g. a function proto:

crates/perry-runtime/src/object/object_ops/prototype.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,27 @@ pub extern "C" fn js_object_create(proto_value: f64) -> f64 {
5151
// Set/Map/Regex source Perry can't model as a prototype) falls back
5252
// to the original behavior: a plain prototype-less object.
5353
const POINTER_TAG: u64 = 0x7FFD_0000_0000_0000;
54+
55+
// `Object.create(proxy)` — a Proxy is a small registered id, not a real
56+
// heap pointer, so the synthetic-class-id modeling below (which stores a
57+
// REAL prototype pointer) can't represent it, and the `is_valid_obj_ptr`
58+
// check would reject it outright (falling back to a plain, prototype-less
59+
// object — wrong: reads/writes/`in` on the result must still route through
60+
// the proxy). Record it in the SAME observable `[[Prototype]]` side table
61+
// `Object.setPrototypeOf` uses instead: a plain (class_id 0, non-null-proto)
62+
// object whose prototype hop the generic chain walks (`ordinary_has_property`,
63+
// `own_set_descriptor`'s `prototype_of_for_set`, field-get) already resolve
64+
// through the proxy's traps. (test262 has/call-in-prototype.js,
65+
// has/call-object-create.js, set/call-parameters-prototype.js.)
66+
if crate::proxy::js_proxy_is_proxy(proto_value) != 0 {
67+
let obj = js_object_alloc(0, 0);
68+
crate::object::prototype_chain::object_set_static_prototype(
69+
obj as usize,
70+
proto_value.to_bits(),
71+
);
72+
return f64::from_bits((obj as u64) | POINTER_TAG);
73+
}
74+
5475
let mut class_id: u32 = 0;
5576
let proto_bits = proto_value.to_bits();
5677
if (proto_bits & 0xFFFF_0000_0000_0000) == POINTER_TAG {

crates/perry-runtime/src/proxy.rs

Lines changed: 11 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ use crate::closure::{js_closure_call0, js_closure_call1, js_closure_call2, js_cl
2222

2323
mod invariants;
2424
mod put_value;
25-
pub use put_value::js_put_value_set;
25+
pub(crate) use put_value::proxy_set_with_receiver;
26+
pub use put_value::{js_proxy_set, js_put_value_set};
2627
mod json;
2728
mod metadata;
2829
mod own_keys;
@@ -705,84 +706,6 @@ fn target_get(target: f64, key: f64) -> f64 {
705706
)
706707
}
707708

708-
/// `proxy[key] = value` — if handler.set exists, call it with
709-
/// (target, key, value) and return TAG_TRUE (the trap's return value is
710-
/// ignored by the default test semantics since we echo `value`). Otherwise
711-
/// forward to the target directly.
712-
#[no_mangle]
713-
pub extern "C" fn js_proxy_set(proxy_boxed: f64, key: f64, value: f64) -> f64 {
714-
let id = match lookup(proxy_boxed) {
715-
Some(id) => id,
716-
None => return f64::from_bits(TAG_FALSE),
717-
};
718-
let (target, handler, revoked) = PROXIES.with(|p| {
719-
p.borrow()
720-
.get(id as usize)
721-
.and_then(|o| o.as_ref())
722-
.map(|e| (e.target, e.handler, e.revoked))
723-
.unwrap_or((
724-
f64::from_bits(TAG_UNDEFINED),
725-
f64::from_bits(TAG_UNDEFINED),
726-
false,
727-
))
728-
});
729-
if revoked {
730-
return revoked_return();
731-
}
732-
let trap = handler_trap(handler, "set");
733-
if is_callable(trap) {
734-
// #2756: the `set` trap's boolean result is observable through
735-
// `Reflect.set(proxy, …)` (and strict-mode assignment). Coerce and
736-
// return it rather than discarding it. The trap receives the spec
737-
// argument list `(target, key, value, receiver)` with `this` bound to
738-
// the handler.
739-
let scope = crate::gc::RuntimeHandleScope::new();
740-
let target_h = scope.root_nanbox_f64(target);
741-
let key_h = scope.root_nanbox_f64(key);
742-
let value_h = scope.root_nanbox_f64(value);
743-
let trap_result = call_trap(
744-
handler,
745-
trap,
746-
&[
747-
target_h.get_nanbox_f64(),
748-
key_h.get_nanbox_f64(),
749-
value_h.get_nanbox_f64(),
750-
proxy_boxed,
751-
],
752-
);
753-
// A falsy trap result means the assignment failed; no invariant check.
754-
if crate::value::js_is_truthy(trap_result) == 0 {
755-
return nanbox_bool(false);
756-
}
757-
invariants::enforce_set_invariant(
758-
target_h.get_nanbox_f64(),
759-
key_h.get_nanbox_f64(),
760-
value_h.get_nanbox_f64(),
761-
);
762-
return nanbox_bool(true);
763-
}
764-
// No set trap — forward to the target's `[[Set]]`. When the target is
765-
// itself a Proxy, recurse through the proxy dispatch (its own trap or
766-
// target) rather than `ordinary_set`, which would deref the fake pointer.
767-
if lookup(target).is_some() {
768-
return js_proxy_set(target, key, value);
769-
}
770-
reflect_ordinary_set(target, key, value)
771-
}
772-
773-
/// Perform an ordinary (non-proxy) `[[Set]]` and report success as a NaN-boxed
774-
/// boolean, without throwing on a non-writable / non-extensible target the way
775-
/// strict-mode assignment does (#2756 / #615). Returns `false` when the write
776-
/// cannot be applied.
777-
fn reflect_ordinary_set_property_key(target: f64, property_key: f64, value: f64) -> f64 {
778-
nanbox_bool(ordinary_set_with_receiver(
779-
target,
780-
property_key,
781-
value,
782-
target,
783-
))
784-
}
785-
786709
/// `Reflect.set` with an explicit receiver: OrdinarySet(target, P, V,
787710
/// receiver), boolean result NaN-boxed.
788711
pub(crate) fn reflect_ordinary_set_with_receiver(
@@ -799,20 +722,6 @@ pub(crate) fn reflect_ordinary_set_with_receiver(
799722
))
800723
}
801724

802-
fn reflect_ordinary_set(target: f64, key: f64, value: f64) -> f64 {
803-
let scope = crate::gc::RuntimeHandleScope::new();
804-
let target_handle = scope.root_nanbox_f64(target);
805-
let key_handle = scope.root_nanbox_f64(key);
806-
let value_handle = scope.root_nanbox_f64(value);
807-
let property_key_handle = scope
808-
.root_nanbox_f64(unsafe { crate::object::js_to_property_key(key_handle.get_nanbox_f64()) });
809-
reflect_ordinary_set_property_key(
810-
target_handle.get_nanbox_f64(),
811-
property_key_handle.get_nanbox_f64(),
812-
value_handle.get_nanbox_f64(),
813-
)
814-
}
815-
816725
fn target_set(target: f64, key: f64, value: f64) {
817726
let property_key = unsafe { crate::object::js_to_property_key(key) };
818727
if unsafe { crate::symbol::js_is_symbol(property_key) } != 0 {
@@ -1331,6 +1240,15 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64)
13311240

13321241
let mut current = target;
13331242
for _ in 0..64 {
1243+
// A Proxy hop in the prototype chain: `OrdinarySetWithOwnDescriptor`
1244+
// step 2.a-b dispatches the full `[[Set]]` on the parent with the
1245+
// ORIGINAL `receiver`, not the raw own-descriptor walk below (which
1246+
// would misread the small proxy id as a heap pointer).
1247+
if lookup(current).is_some() {
1248+
return crate::value::js_is_truthy(proxy_set_with_receiver(
1249+
current, key, value, receiver,
1250+
)) != 0;
1251+
}
13341252
// Integer-Indexed exotic [[Set]] (§10.4.5.5): a typed array in the
13351253
// chain intercepts a canonical numeric index key — the prototype
13361254
// chain is NEVER consulted for it. `SameValue(O, Receiver)` writes

crates/perry-runtime/src/proxy/put_value.rs

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,97 @@
55
66
use super::*;
77

8+
/// `proxy[key] = value` — if handler.set exists, call it with
9+
/// (target, key, value) and return TAG_TRUE (the trap's return value is
10+
/// ignored by the default test semantics since we echo `value`). Otherwise
11+
/// forward to the target directly.
12+
#[no_mangle]
13+
pub extern "C" fn js_proxy_set(proxy_boxed: f64, key: f64, value: f64) -> f64 {
14+
proxy_set_with_receiver(proxy_boxed, key, value, proxy_boxed)
15+
}
16+
17+
/// Proxy `[[Set]]` (ECMA-262 §10.5.9) with an explicit `Receiver`, distinct
18+
/// from `proxy_boxed` itself — reached when a Proxy sits partway up another
19+
/// object's `[[Prototype]]` chain (`OrdinarySetWithOwnDescriptor` forwards to
20+
/// `parent.[[Set]](P, V, Receiver)` with the ORIGINAL receiver, not `parent`).
21+
pub(crate) fn proxy_set_with_receiver(
22+
proxy_boxed: f64,
23+
key: f64,
24+
value: f64,
25+
receiver: f64,
26+
) -> f64 {
27+
let id = match lookup(proxy_boxed) {
28+
Some(id) => id,
29+
None => return f64::from_bits(TAG_FALSE),
30+
};
31+
let (target, handler, revoked) = PROXIES.with(|p| {
32+
p.borrow()
33+
.get(id as usize)
34+
.and_then(|o| o.as_ref())
35+
.map(|e| (e.target, e.handler, e.revoked))
36+
.unwrap_or((
37+
f64::from_bits(TAG_UNDEFINED),
38+
f64::from_bits(TAG_UNDEFINED),
39+
false,
40+
))
41+
});
42+
if revoked {
43+
return revoked_return();
44+
}
45+
let trap = handler_trap(handler, "set");
46+
if is_callable(trap) {
47+
// #2756: the `set` trap's boolean result is observable through
48+
// `Reflect.set(proxy, …)` (and strict-mode assignment). Coerce and
49+
// return it rather than discarding it. The trap receives the spec
50+
// argument list `(target, key, value, receiver)` with `this` bound to
51+
// the handler.
52+
let scope = crate::gc::RuntimeHandleScope::new();
53+
let target_h = scope.root_nanbox_f64(target);
54+
let key_h = scope.root_nanbox_f64(key);
55+
let value_h = scope.root_nanbox_f64(value);
56+
let receiver_h = scope.root_nanbox_f64(receiver);
57+
let trap_result = call_trap(
58+
handler,
59+
trap,
60+
&[
61+
target_h.get_nanbox_f64(),
62+
key_h.get_nanbox_f64(),
63+
value_h.get_nanbox_f64(),
64+
receiver_h.get_nanbox_f64(),
65+
],
66+
);
67+
// A falsy trap result means the assignment failed; no invariant check.
68+
if crate::value::js_is_truthy(trap_result) == 0 {
69+
return nanbox_bool(false);
70+
}
71+
invariants::enforce_set_invariant(
72+
target_h.get_nanbox_f64(),
73+
key_h.get_nanbox_f64(),
74+
value_h.get_nanbox_f64(),
75+
);
76+
return nanbox_bool(true);
77+
}
78+
// No set trap — forward to the target's `[[Set]]`. When the target is
79+
// itself a Proxy, recurse through the proxy dispatch (its own trap or
80+
// target) rather than `ordinary_set`, which would deref the fake pointer.
81+
if lookup(target).is_some() {
82+
return proxy_set_with_receiver(target, key, value, receiver);
83+
}
84+
let scope = crate::gc::RuntimeHandleScope::new();
85+
let target_handle = scope.root_nanbox_f64(target);
86+
let key_handle = scope.root_nanbox_f64(key);
87+
let value_handle = scope.root_nanbox_f64(value);
88+
let receiver_handle = scope.root_nanbox_f64(receiver);
89+
let property_key_handle = scope
90+
.root_nanbox_f64(unsafe { crate::object::js_to_property_key(key_handle.get_nanbox_f64()) });
91+
reflect_ordinary_set_with_receiver(
92+
target_handle.get_nanbox_f64(),
93+
property_key_handle.get_nanbox_f64(),
94+
value_handle.get_nanbox_f64(),
95+
receiver_handle.get_nanbox_f64(),
96+
)
97+
}
98+
899
/// Assignment PutValue for a property reference. Returns the assigned RHS value
9100
/// on success or sloppy failure, and throws TypeError when strict code attempts
10101
/// a failed [[Set]].

0 commit comments

Comments
 (0)