You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up to #6935 / #6941, which fixed the js_to_property_key sites. While sweeping for those I found the same shape in a second, adjacent family that #6941 deliberately left alone: entry points that use js_string_coerce(key)as the property-key coercion, without an earlier ToPropertyKey.
js_string_coerce on an object argument runs the user toString / valueOf, and allocates the stringified form for every primitive — so it is GC-capable and can evacuate exactly like js_to_property_key. Where the receiver (and, on the define/set paths, the descriptor or value) is a raw Rust local across it, the same corruption applies: a stale receiver drops the write onto a forwarding stub, a stale stored value plants a dangling pointer inside a live object.
Sites, all on main @ 0bb03e8, none of them preceded by a js_to_property_key call:
crates/perry-runtime/src/object/object_ops/define_property.rs:606, 790, 893 — Object.defineProperty(obj, key, desc); obj / obj_value and the descriptor object are held across js_string_coerce(key_value).
crates/perry-runtime/src/object/reflect_support.rs:59, 77, 110 — e.g. the typed-array and array arms hold obj / obj_addr across js_string_coerce(key) and then dereference them.
The fix is mechanical and identical to #6941: crate::gc::RuntimeHandleScope + root_heap_word_u64 / root_raw_mut_ptr / root_nanbox_f64 around the coercion, re-reading the receiver and any stored operand through their handles afterwards, with the property_key_coercion_is_inert fast path (added in #6941, object/property_key.rs) keeping the common already-a-heap-string key on the pre-fix path.
Two things worth deciding while doing it:
Several of these are spec-level ToPropertyKey(P) steps (Object.defineProperty step 2, Reflect.*), so they arguably want js_to_property_key rather than a bare js_string_coerce — that would also fix Symbol-key handling on those paths. Worth checking against test262 before choosing.
Follow-up to #6935 / #6941, which fixed the
js_to_property_keysites. While sweeping for those I found the same shape in a second, adjacent family that #6941 deliberately left alone: entry points that usejs_string_coerce(key)as the property-key coercion, without an earlierToPropertyKey.js_string_coerceon an object argument runs the usertoString/valueOf, and allocates the stringified form for every primitive — so it is GC-capable and can evacuate exactly likejs_to_property_key. Where the receiver (and, on the define/set paths, the descriptor or value) is a raw Rust local across it, the same corruption applies: a stale receiver drops the write onto a forwarding stub, a stale stored value plants a dangling pointer inside a live object.Sites, all on
main@ 0bb03e8, none of them preceded by ajs_to_property_keycall:crates/perry-runtime/src/object/object_ops/define_property.rs:606, 790, 893—Object.defineProperty(obj, key, desc);obj/obj_valueand the descriptor object are held acrossjs_string_coerce(key_value).crates/perry-runtime/src/object/descriptors.rs:196, 271, 471, 615, 950crates/perry-runtime/src/object/descriptor_state.rs:733crates/perry-runtime/src/object/reflect_support.rs:59, 77, 110— e.g. the typed-array and array arms holdobj/obj_addracrossjs_string_coerce(key)and then dereference them.crates/perry-runtime/src/object/array_object_ops.rs:266crates/perry-runtime/src/object/typed_array_define.rs:137crates/perry-runtime/src/proxy.rs:1385The fix is mechanical and identical to #6941:
crate::gc::RuntimeHandleScope+root_heap_word_u64/root_raw_mut_ptr/root_nanbox_f64around the coercion, re-reading the receiver and any stored operand through their handles afterwards, with theproperty_key_coercion_is_inertfast path (added in #6941,object/property_key.rs) keeping the common already-a-heap-string key on the pre-fix path.Two things worth deciding while doing it:
ToPropertyKey(P)steps (Object.definePropertystep 2,Reflect.*), so they arguably wantjs_to_property_keyrather than a barejs_string_coerce— that would also fix Symbol-key handling on those paths. Worth checking against test262 before choosing.