Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/9797-array-hole-inherited-setters.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix numeric writes into array holes bypassing inherited setters and read-only properties on `Array.prototype` and `Object.prototype`. Existing own elements and arrays with unmodified prototype chains keep their fast path.
13 changes: 7 additions & 6 deletions crates/perry-runtime/src/array/indexing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1027,14 +1027,15 @@ pub(crate) unsafe fn try_strict_dense_number_store(
// proves there are no holes and keeps its bit-for-bit old hot path; every
// other admitted layout proves ownership with the slot Perry is about to
// overwrite.
// The process latch leads: an array can only have an inherited index when
// SOME array has been retargeted, so a program that never calls
// `Object.setPrototypeOf` on an array keeps this lane bit-for-bit as it was
// (one relaxed load of a static bool, and the slot is never read here).
// `new Array(n)` fills are holey and would otherwise all fall off the lane.
// #9787: the shared invalidation byte covers indexed properties on both
// default prototypes as well as retargeted arrays. Checking only whether
// an array was retargeted misses Array.prototype / Object.prototype
// descriptors and lets this lane create an own element over a setter.
// Ordinary `new Array(n)` fills still pay one relaxed load and never read
// the old slot while all three prototype conditions remain clear.
let may_have_holes = flags & crate::gc::GC_ARRAY_RAW_F64_LAYOUT == 0
|| flags & crate::gc::GC_ARRAY_RAW_F64_HOLES != 0;
if crate::object::prototype_chain::array_static_proto_recorded()
if PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED.load(Ordering::Relaxed) != 0
&& may_have_holes
&& ptr::read(slot) == crate::value::TAG_HOLE
{
Expand Down
26 changes: 19 additions & 7 deletions crates/perry-runtime/src/array/strict_store_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@ fn strict_dense_number_store_fast_lane_matches_the_general_path() {

// #9220: an in-bounds hole is not an own property. The number lane
// must decline it so the strict entry can consult an inherited index
// setter / non-writable data descriptor before creating an element
// but ONLY once some array has been retargeted. With the process latch
// clear (the overwhelmingly common case, including every `new Array(n)`
// fill) the lane keeps filling holes exactly as it did before #9220.
// setter / non-writable data descriptor before creating an element.
// #9787: a default-prototype descriptor invalidates this lane even
// without a retargeted array. With the summary byte clear, ordinary
// `new Array(n)` fills retain their fast path.
//
// Indices 4 and 5 are the SAME shape — two in-bounds holes on one
// array — so the latch is the only variable between the two arms.
Expand All @@ -142,15 +142,27 @@ fn strict_dense_number_store_fast_lane_matches_the_general_path() {
assert!(!array_has_own_index(out, 5));
let latch_was =
crate::object::prototype_chain::test_swap_array_static_proto_recorded(false);
let summary_was = super::test_swap_array_index_fast_path_invalidated(0);
struct RestorePrototypeFlags(bool, u8);
impl Drop for RestorePrototypeFlags {
fn drop(&mut self) {
crate::object::prototype_chain::test_swap_array_static_proto_recorded(self.0);
super::test_swap_array_index_fast_path_invalidated(self.1);
}
}
let _restore = RestorePrototypeFlags(latch_was, summary_was);
assert!(
lane(out, 4, 8.0),
"no recorded array prototype: the hole fill stays on the fast lane"
"unmodified prototype chains: the hole fill stays on the fast lane"
);
assert!(array_has_own_index(out, 4));
crate::object::prototype_chain::test_swap_array_static_proto_recorded(true);
super::test_swap_array_index_fast_path_invalidated(1);
assert!(!lane(out, 5, 8.0), "hole slot requires the [[Set]] walk");
assert!(!array_has_own_index(out, 5));
crate::object::prototype_chain::test_swap_array_static_proto_recorded(latch_was);
assert!(
lane(out, 4, 9.0),
"an existing own element still bypasses inherited descriptors"
);
}
}

Expand Down
24 changes: 24 additions & 0 deletions crates/perry/tests/issue_9249_array_prototype_define_property.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,30 @@ console.log(hits, nums.length, nums[7]);
);
}

#[test]
fn default_array_prototype_setter_intercepts_in_bounds_holes() {
compile_and_run(
include_str!("../../../test-files/test_gap_9787_array_hole_inherited_setter.ts"),
"true:31 false array-proto-three 1\n\
true:31,true:37 false array-proto-three 5\n\
own 2 true 41 4\n\
deleted true:31,true:37,true:43 false array-proto-three 4\n\
removed true 47 5\n",
"array_prototype_holes",
);
}

#[test]
fn default_object_prototype_descriptors_intercept_in_bounds_holes() {
compile_and_run(
include_str!("../../../test-files/test_gap_9787_object_prototype_hole_setter.ts"),
"1 true 53 false object-proto-eight 10\n\
TypeError false getter-only 10\n\
TypeError false locked 10\n",
"object_prototype_holes",
);
}

#[test]
fn define_properties_array_prototype_index_setter_intercepts_boolean_store() {
compile_and_run(
Expand Down
36 changes: 36 additions & 0 deletions test-files/test_gap_9787_array_hole_inherited_setter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// An in-bounds hole has no own property: it must consult inherited setters.
"use strict";

const calls: string[] = [];
Object.defineProperty(Array.prototype, "3", {
configurable: true,
get() { return "array-proto-three"; },
set(this: any, value: any) { calls.push(`${Array.isArray(this)}:${value}`); },
});

try {
const outOfBounds: any[] = [0];
outOfBounds[3] = 31;
console.log(calls.join(","), Object.hasOwn(outOfBounds, 3), outOfBounds[3], outOfBounds.length);

const inBoundsHole: any[] = new Array(5);
inBoundsHole[3] = 37;
console.log(calls.join(","), Object.hasOwn(inBoundsHole, 3), inBoundsHole[3], inBoundsHole.length);

// An own undefined value is not a hole and must bypass the inherited setter.
const own: any[] = [0, 1, 2, undefined];
own[3] = 41;
console.log("own", calls.length, Object.hasOwn(own, 3), own[3], own.length);

// Deletion creates the same obligation as new Array(n)'s initial holes.
delete own[3];
own[3] = 43;
console.log("deleted", calls.join(","), Object.hasOwn(own, 3), own[3], own.length);
} finally {
delete (Array.prototype as any)[3];
}

// The invalidation latch stays set after deletion, but no setter remains.
const plain: any[] = new Array(5);
plain[3] = 47;
console.log("removed", Object.hasOwn(plain, 3), plain[3], plain.length);
54 changes: 54 additions & 0 deletions test-files/test_gap_9787_object_prototype_hole_setter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Object.prototype alone must invalidate the numeric hole-store fast path.
"use strict";

let calls = 0;
let receiverIsArray = false;
let assigned = 0;
Object.defineProperty(Object.prototype, "8", {
configurable: true,
get() { return "object-proto-eight"; },
set(this: any, value: number) {
calls++;
receiverIsArray = Array.isArray(this);
assigned = value;
},
});

let setterResult = "";
try {
const holes: any[] = new Array(10);
holes[8] = 53;
setterResult = [calls, receiverIsArray, assigned, Object.hasOwn(holes, 8), holes[8], holes.length].join(" ");
} finally {
delete (Object.prototype as any)[8];
}
console.log(setterResult);

Object.defineProperty(Object.prototype, "8", {
configurable: true,
get() { return "getter-only"; },
});
let getterResult = "";
try {
const holes: any[] = new Array(10);
let error = "none";
try { holes[8] = 59; } catch (e) { error = (e as Error).constructor.name; }
getterResult = [error, Object.hasOwn(holes, 8), holes[8], holes.length].join(" ");
} finally {
delete (Object.prototype as any)[8];
}
console.log(getterResult);

Object.defineProperty(Object.prototype, "8", {
configurable: true, value: "locked", writable: false,
});
let lockedResult = "";
try {
const holes: any[] = new Array(10);
let error = "none";
try { holes[8] = 61; } catch (e) { error = (e as Error).constructor.name; }
lockedResult = [error, Object.hasOwn(holes, 8), holes[8], holes.length].join(" ");
} finally {
delete (Object.prototype as any)[8];
}
console.log(lockedResult);
Loading