mirror of
https://github.com/LadybirdBrowser/ladybird
synced 2026-04-25 17:25:08 +02:00
LibJS: Extend Array.prototype.shift() fast path to holey arrays
indexed_take_first() already memmoves elements down for both Packed and Holey storage, but the caller at ArrayPrototype::shift() only entered the fast path for Packed arrays. Holey arrays fell through to the spec-literal per-element loop (has_property / get / set / delete_property_or_throw), which is substantially slower. Add a separate Holey predicate with the additional safety checks the spec semantics require: default_prototype_chain_intact() (so HasProperty on a hole doesn't escape to a poisoned prototype) and extensible() (so set() on a hole slot doesn't create a new own property on a non-extensible object). The existing Packed predicate is left unchanged -- packed arrays don't need these checks because every index in [0, size) is already an own data property. Allows us to fail at Cloudflare Turnstile way much faster!
This commit is contained in:
committed by
Andreas Kling
parent
ad7177eccb
commit
bfbc3352b5
Notes:
github-actions[bot]
2026-04-23 19:48:23 +00:00
Author: https://github.com/kalenikaliaksandr Commit: https://github.com/LadybirdBrowser/ladybird/commit/bfbc3352b52 Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/9059
@@ -152,6 +152,26 @@ static bool can_use_packed_array_fast_path(Array const& array)
|
||||
return array.is_simple_packed_array() && array.default_prototype_chain_intact();
|
||||
}
|
||||
|
||||
static bool can_use_packed_shift_fast_path(Array const& array)
|
||||
{
|
||||
// Packed: every index in [0, size) is an own property, so memmove semantics match the spec even if the prototype
|
||||
// chain has indexed properties (HasProperty never escapes to the proto) and even if the array is non-extensible
|
||||
// (no new own properties are created).
|
||||
return array.is_simple_packed_array() && array.length_is_writable();
|
||||
}
|
||||
|
||||
static bool can_use_holey_shift_fast_path(Array const& array)
|
||||
{
|
||||
// Holey: the spec path uses HasProperty + Get on every index, so a poisoned prototype changes the outcome on holes.
|
||||
// A set() on a hole slot also creates a new own property, which a non-extensible array rejects with TypeError.
|
||||
return !array.is_proxy_target()
|
||||
&& !array.may_interfere_with_indexed_property_access()
|
||||
&& array.indexed_storage_kind() == IndexedStorageKind::Holey
|
||||
&& array.default_prototype_chain_intact()
|
||||
&& array.extensible()
|
||||
&& array.length_is_writable();
|
||||
}
|
||||
|
||||
static Array* fast_array_species_result(Object& object)
|
||||
{
|
||||
auto* array = as_if<Array>(object);
|
||||
@@ -1306,7 +1326,8 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayPrototype::shift)
|
||||
// - has intact prototype chain, which means we don't have to worry about getters/setters potentially defined for holes.
|
||||
// - has simple storage type, which means all values have default attributes (if some elements have configurable=false, we cannot use fast path, because delete operation will fail).
|
||||
// then we could take a fast path by directly taking first element from indexed storage.
|
||||
if (auto* array = as_if<Array>(*this_object); array && array->is_simple_packed_array() && array->length_is_writable()) {
|
||||
if (auto* array = as_if<Array>(*this_object);
|
||||
array && (can_use_packed_shift_fast_path(*array) || can_use_holey_shift_fast_path(*array))) {
|
||||
auto first = array->indexed_take_first().value;
|
||||
if (first.is_special_empty_value())
|
||||
return js_undefined();
|
||||
|
||||
@@ -47,3 +47,78 @@ test("throws if the array length is not writable", () => {
|
||||
expect(1 in a).toBeFalse();
|
||||
expect(a.length).toBe(2);
|
||||
});
|
||||
|
||||
describe("holey arrays", () => {
|
||||
test("shift on clean holey array with interior hole preserves hole", () => {
|
||||
const a = [0, , 2];
|
||||
const result = a.shift();
|
||||
expect(result).toBe(0);
|
||||
expect(a.length).toBe(2);
|
||||
expect(0 in a).toBeFalse();
|
||||
expect(a[0]).toBeUndefined();
|
||||
expect(1 in a).toBeTrue();
|
||||
expect(a[1]).toBe(2);
|
||||
});
|
||||
|
||||
test("shift on clean holey array with leading hole returns undefined", () => {
|
||||
const a = [, 1, 2];
|
||||
const result = a.shift();
|
||||
expect(result).toBeUndefined();
|
||||
expect(a.length).toBe(2);
|
||||
expect(a[0]).toBe(1);
|
||||
expect(a[1]).toBe(2);
|
||||
});
|
||||
|
||||
test("shift on clean holey array with trailing hole propagates it", () => {
|
||||
const a = [0, 1, ,];
|
||||
expect(a.length).toBe(3);
|
||||
const result = a.shift();
|
||||
expect(result).toBe(0);
|
||||
expect(a.length).toBe(2);
|
||||
expect(a[0]).toBe(1);
|
||||
expect(1 in a).toBeFalse();
|
||||
});
|
||||
|
||||
test("holey shift with prototype pollution follows spec", () => {
|
||||
Array.prototype[1] = "polluted";
|
||||
try {
|
||||
const a = [0, , 2];
|
||||
const result = a.shift();
|
||||
expect(result).toBe(0);
|
||||
// Spec: HasProperty(a, 1) is true via proto, Get returns "polluted",
|
||||
// which is set as an own property at index 0.
|
||||
expect(a[0]).toBe("polluted");
|
||||
expect(0 in a).toBeTrue();
|
||||
expect(a[1]).toBe(2);
|
||||
expect(a.length).toBe(2);
|
||||
} finally {
|
||||
delete Array.prototype[1];
|
||||
}
|
||||
});
|
||||
|
||||
test("holey shift on non-extensible array throws TypeError", () => {
|
||||
const a = [, 1];
|
||||
Object.preventExtensions(a);
|
||||
expect(() => a.shift()).toThrow(TypeError);
|
||||
});
|
||||
|
||||
test("packed shift with prototype pollution still fast-paths correctly", () => {
|
||||
Array.prototype[0] = "polluted";
|
||||
try {
|
||||
const a = [1, 2, 3];
|
||||
expect(a.shift()).toBe(1);
|
||||
expect(a).toEqual([2, 3]);
|
||||
} finally {
|
||||
delete Array.prototype[0];
|
||||
}
|
||||
});
|
||||
|
||||
test("packed shift on non-extensible array works (no new properties)", () => {
|
||||
const a = [1, 2, 3];
|
||||
Object.preventExtensions(a);
|
||||
expect(a.shift()).toBe(1);
|
||||
expect(a.length).toBe(2);
|
||||
expect(a[0]).toBe(2);
|
||||
expect(a[1]).toBe(3);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user