diff --git a/changelog.d/11711-strict-arguments-descriptors.md b/changelog.d/11711-strict-arguments-descriptors.md new file mode 100644 index 0000000000..7b85c1b73f --- /dev/null +++ b/changelog.d/11711-strict-arguments-descriptors.md @@ -0,0 +1,4 @@ +Strict escaping `arguments` objects now keep their `length` and restricted +`callee` attributes in the shared key layout, with the thrower accessor in the +object's own slot. Construction no longer installs per-object entries in the +address-keyed property and accessor descriptor tables. diff --git a/crates/perry-runtime/src/gc/tests/arguments_objects.rs b/crates/perry-runtime/src/gc/tests/arguments_objects.rs index e8985835b3..e953cb8581 100644 --- a/crates/perry-runtime/src/gc/tests/arguments_objects.rs +++ b/crates/perry-runtime/src/gc/tests/arguments_objects.rs @@ -100,6 +100,23 @@ fn arguments_bulk_construction_handles_empty_and_uncached_arities() { let accessor = get_accessor_descriptor(args as usize, "callee").unwrap(); assert_eq!(accessor.get, accessor.set); assert_ne!(accessor.get, 0); + let descriptors = &crate::state::state().descriptors; + assert!( + !descriptors + .property_descriptors + .borrow() + .keys() + .any(|(owner, _)| *owner == args as usize), + "an arguments object must not have address-keyed attributes" + ); + assert!( + !descriptors + .accessor_descriptors + .borrow() + .keys() + .any(|(owner, _)| *owner == args as usize), + "the restricted callee must live in its own slot" + ); } } @@ -121,6 +138,30 @@ fn arguments_shared_keys_survive_moving_gc_without_a_live_arguments_owner() { assert_eq!(get(after, "length").bits(), 3.0f64.to_bits()); } +#[test] +fn restricted_callee_accessor_survives_moving_gc_without_descriptor_entries() { + let _guard = CopyingNurseryTestGuard::new(1); + register_scanners(); + let undefined = f64::from_bits(crate::value::TAG_UNDEFINED); + let args = arguments(&[1.0], undefined, true); + js_shadow_slot_set(0, ptr_bits(args as usize)); + gc_collect_minor(); + + let moved = (js_shadow_slot_get(0) & POINTER_MASK) as *mut ObjectHeader; + assert_ne!(moved, args, "the arguments object must actually evacuate"); + let attrs = get_property_attrs(moved as usize, "callee").unwrap(); + assert!(!attrs.writable() && !attrs.enumerable() && !attrs.configurable()); + let accessor = get_accessor_descriptor(moved as usize, "callee").unwrap(); + assert_eq!(accessor.get, accessor.set); + assert_ne!(accessor.get, 0); + assert!(!crate::state::state() + .descriptors + .accessor_descriptors + .borrow() + .keys() + .any(|(owner, _)| *owner == moved as usize)); +} + #[test] fn arguments_values_callee_and_mapping_survive_moving_gc() { let _guard = CopyingNurseryTestGuard::new(1); diff --git a/crates/perry-runtime/src/object/arguments.rs b/crates/perry-runtime/src/object/arguments.rs index fe1f2efc7e..352c30886a 100644 --- a/crates/perry-runtime/src/object/arguments.rs +++ b/crates/perry-runtime/src/object/arguments.rs @@ -395,20 +395,22 @@ fn arguments_object_alloc( if restricted_callee { let thrower = thrower_closure_value(); obj.with_mut_ptr::(|obj| { - set_property_attrs( - obj as usize, - "length".to_string(), - PropertyAttrs::new(true, false, true), - ); - super::descriptor_state::install_fresh_accessor_property( - obj as usize, - "callee".to_string(), - AccessorDescriptor { - get: thrower.to_bits(), - set: thrower.to_bits(), - }, - PropertyAttrs::new(false, false, false), - ); + // Both attributes were born in the canonical key layout. The + // restricted callee's getter and setter live in its own value + // slot, as they do for ordinary accessor properties. Installing + // descriptors here would re-edit that layout on every call and + // leave entries in the address-keyed descriptor tables. + unsafe { + super::accessor_pair::store_own_accessor( + obj as usize, + "callee", + Some(super::accessor_pair::pair_from(&AccessorDescriptor { + get: thrower.to_bits(), + set: thrower.to_bits(), + })), + ); + } + super::descriptor_state::note_accessor_born_with_keys(obj as usize); }); } else { obj.with_mut_ptr(|obj| { diff --git a/crates/perry-runtime/src/object/descriptor_state.rs b/crates/perry-runtime/src/object/descriptor_state.rs index d6baa110a1..da5231c3ff 100644 --- a/crates/perry-runtime/src/object/descriptor_state.rs +++ b/crates/perry-runtime/src/object/descriptor_state.rs @@ -641,6 +641,13 @@ pub(crate) fn note_attrs_born_with_keys(obj: usize) { GLOBAL_DESCRIPTORS_IN_USE.store(true, Ordering::Relaxed); } +/// An accessor born in the object's attributed key layout also needs the +/// read-path accessor gate, even though no descriptor install runs. +pub(crate) fn note_accessor_born_with_keys(obj: usize) { + note_attrs_born_with_keys(obj); + state().descriptors.accessors_in_use.set(true); +} + /// Look up the property descriptor for (obj, key). Returns None if no entry exists, /// in which case the JS default `{ writable: true, enumerable: true, configurable: true }` applies. pub(crate) fn get_property_attrs(obj: usize, key: &str) -> Option { diff --git a/scripts/ci_e2e_scope.py b/scripts/ci_e2e_scope.py index 95671838cd..325ca5fda4 100755 --- a/scripts/ci_e2e_scope.py +++ b/scripts/ci_e2e_scope.py @@ -156,7 +156,6 @@ "temp_root_operand_temporaries", "typed_array_rmw_8692", "typed_array_update_lowering", - "typed_shape_declared_at_allocation", "typed_shape_descriptor", "typed_shape_descriptors", "system_boolean_result",