From f0f4ade13d55f20f7ff5593b52fdf2843e776809 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:33:19 +0000 Subject: [PATCH 1/2] [JSC] DFG/FTL: a read of a global variable throws when its property is gone A name that is a property of the global object is read with op_get_from_scope and the resolve type GlobalProperty. When no property has the name, the read is a ReferenceError if its mode is ThrowIfNotFound. That is the mode of every read except `typeof name`. LLInt, Baseline and code compiled ahead of time throw it from their slow paths. The DFG bytecode parser loads the property directly when it can prove where the property is. When it cannot, it emitted a GetByIdFlush of the global object. That is a plain get by id: it gives undefined when it finds no property, and its inline cache caches that answer. So once a function was compiled by the DFG, a read of a global whose property was deleted gave undefined, and a call of it gave "x is not a function". The parser takes this path when the lower tiers have no structure to check: an accessor, a property with a getter in C++, a property that the global object inherits, a global object that is an uncacheable dictionary. Since 318918@main (webkit.org/b/320802) it also takes it when the structure that the lower tiers cached no longer matches and the site exited once. That is the case of a plain value that was deleted. Add the inline cache access type GetByIdThrowIfNotFound. Its two slow path operations throw the ReferenceError when they find no property, and never get as far as caching it. tryCacheGetBy builds no miss case for it, and the access type is not in the list that allows a megamorphic cache, so every way the access can end without a property is one of these operations. A hit is what it is for GetById: the same cases, the same handlers, the same machine code at the site. GetByIdData carries the ResolveMode of the access, and has to be given one. The parser passes the mode of the bytecode for the read of a global, and the DFG and FTL back ends pick the access type from it. The base of such a read is a scope, so the fixup phase gives it a cell edge, and there is no operation for a base that is not a cell. The write is not changed. A strict mode write to such a name is a PutDynamicVar since #757, and global-property-strict-write-not-found.js is the test for it. Without this change that test fails only where it reads. Upstream main has the same GetByIdFlush. * JSTests/microbenchmarks/global-accessor-property-read.js: Added. * JSTests/microbenchmarks/global-inherited-property-read.js: Added. * JSTests/stress/global-property-read-not-found.js: Added. * JSTests/stress/global-property-strict-write-not-found.js: Added. * Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp: * Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp: * Source/JavaScriptCore/bytecode/PropertyInlineCache.h: * Source/JavaScriptCore/bytecode/Repatch.cpp: * Source/JavaScriptCore/bytecode/Repatch.h: * Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp: * Source/JavaScriptCore/dfg/DFGFixupPhase.cpp: * Source/JavaScriptCore/dfg/DFGGraph.cpp: * Source/JavaScriptCore/dfg/DFGNode.cpp: * Source/JavaScriptCore/dfg/DFGNode.h: * Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp: * Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp: * Source/JavaScriptCore/jit/ICStats.h: * Source/JavaScriptCore/jit/JITOperations.cpp: * Source/JavaScriptCore/jit/JITOperations.h: --- .../global-accessor-property-read.js | 17 ++ .../global-inherited-property-read.js | 17 ++ .../stress/global-property-read-not-found.js | 272 ++++++++++++++++++ .../global-property-strict-write-not-found.js | 215 ++++++++++++++ .../bytecode/InlineCacheCompiler.cpp | 4 + .../bytecode/PropertyInlineCache.cpp | 6 + .../bytecode/PropertyInlineCache.h | 4 + Source/JavaScriptCore/bytecode/Repatch.cpp | 11 + Source/JavaScriptCore/bytecode/Repatch.h | 1 + .../JavaScriptCore/dfg/DFGByteCodeParser.cpp | 18 +- Source/JavaScriptCore/dfg/DFGFixupPhase.cpp | 7 + Source/JavaScriptCore/dfg/DFGGraph.cpp | 2 + Source/JavaScriptCore/dfg/DFGNode.cpp | 2 +- Source/JavaScriptCore/dfg/DFGNode.h | 21 ++ .../dfg/DFGSpeculativeJIT64.cpp | 6 +- Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp | 6 +- Source/JavaScriptCore/jit/ICStats.h | 2 + Source/JavaScriptCore/jit/JITOperations.cpp | 64 +++++ Source/JavaScriptCore/jit/JITOperations.h | 3 + 19 files changed, 665 insertions(+), 13 deletions(-) create mode 100644 JSTests/microbenchmarks/global-accessor-property-read.js create mode 100644 JSTests/microbenchmarks/global-inherited-property-read.js create mode 100644 JSTests/stress/global-property-read-not-found.js create mode 100644 JSTests/stress/global-property-strict-write-not-found.js diff --git a/JSTests/microbenchmarks/global-accessor-property-read.js b/JSTests/microbenchmarks/global-accessor-property-read.js new file mode 100644 index 0000000000000..622fb7e387bfe --- /dev/null +++ b/JSTests/microbenchmarks/global-accessor-property-read.js @@ -0,0 +1,17 @@ +// A read, by name, of a global that is an accessor. The DFG and the FTL cannot prove where such a property is, +// so they read it through an inline cache. +Object.defineProperty(globalThis, "accessorGlobal", { get() { return 3; }, configurable: true }); + +function test(count) +{ + let sum = 0; + for (let i = 0; i < count; ++i) + sum += accessorGlobal; + return sum; +} +noInline(test); + +for (let i = 0; i < 500; ++i) { + if (test(1e4) !== 3e4) + throw new Error("bad result"); +} diff --git a/JSTests/microbenchmarks/global-inherited-property-read.js b/JSTests/microbenchmarks/global-inherited-property-read.js new file mode 100644 index 0000000000000..ab3beca5d4da5 --- /dev/null +++ b/JSTests/microbenchmarks/global-inherited-property-read.js @@ -0,0 +1,17 @@ +// A read, by name, of a property that the global object inherits. The DFG and the FTL cannot prove where such +// a property is, so they read it through an inline cache. +Object.prototype.inheritedGlobal = 3; + +function test(count) +{ + let sum = 0; + for (let i = 0; i < count; ++i) + sum += inheritedGlobal; + return sum; +} +noInline(test); + +for (let i = 0; i < 500; ++i) { + if (test(1e4) !== 3e4) + throw new Error("bad result"); +} diff --git a/JSTests/stress/global-property-read-not-found.js b/JSTests/stress/global-property-read-not-found.js new file mode 100644 index 0000000000000..da734b38bbe44 --- /dev/null +++ b/JSTests/stress/global-property-read-not-found.js @@ -0,0 +1,272 @@ +//@ defaultRun; run("no-cjit-force-ic-failure", "--useConcurrentJIT=false", "--forceICFailure=true") + +// A name that resolves to a property of the global object is a ReferenceError when that +// property is not there, in every tier. The DFG and the FTL did a plain get by id of the +// global object when they could not prove where the property is, and a plain get by id +// of a property that is not there gives undefined. +// global-property-strict-write-not-found.js is the same for a write. + +function shouldBe(actual, expected, what) { + if (actual !== expected) + throw new Error(what + ": expected " + String(expected) + " but got " + String(actual)); +} + +function shouldThrow(func, expected, what) { + let error = null; + let result; + try { + result = func(); + } catch (e) { + error = e; + } + if (!error) + throw new Error(what + ": did not throw, returned " + String(result)); + if (String(error) !== expected) + throw new Error(what + ": threw " + String(error)); +} + +function isNotDefined(name) { + return "ReferenceError: " + name + " is not defined"; +} + +const global = globalThis; +// A call that throws costs far more than one that returns, so the loops that throw are short. The longer one is +// for a function that is optimized while it throws. The shorter one is for a function that already is. +const throwCount = Math.ceil(testLoopCount / 20); +const fewThrows = Math.ceil(testLoopCount / 50); + +// A data property that is there when the function is linked, and gone before the function is optimized. +global.linkedData = 1; +function readLinkedData() { return linkedData; } +noInline(readLinkedData); +shouldBe(readLinkedData(), 1, "linkedData before"); +delete global.linkedData; +for (let i = 0; i < throwCount; ++i) + shouldThrow(readLinkedData, isNotDefined("linkedData"), "linkedData at " + i); + +// The same for a call, a construction and an update: the read comes first. +global.calledFunction = function () { return 11; }; +global.constructedFunction = function () { }; +global.counter = 0; +function callIt() { return calledFunction(); } +function constructIt() { return new constructedFunction; } +function increment() { return counter++; } +noInline(callIt); +noInline(constructIt); +noInline(increment); +shouldBe(callIt(), 11, "calledFunction before"); +shouldBe(constructIt() instanceof constructedFunction, true, "constructedFunction before"); +shouldBe(increment(), 0, "counter before"); +delete global.calledFunction; +delete global.constructedFunction; +delete global.counter; +for (let i = 0; i < throwCount; ++i) { + shouldThrow(callIt, isNotDefined("calledFunction"), "calledFunction at " + i); + shouldThrow(constructIt, isNotDefined("constructedFunction"), "constructedFunction at " + i); + shouldThrow(increment, isNotDefined("counter"), "counter at " + i); +} +shouldBe(Object.hasOwn(global, "counter"), false, "counter was not created"); + +// A data property with the optimized code already there when it goes away, and the property coming back. +global.warmData = 1; +function readWarmData() { return warmData; } +noInline(readWarmData); +for (let i = 0; i < testLoopCount; ++i) + shouldBe(readWarmData(), 1, "warmData before"); +for (let round = 2; round < 5; ++round) { + delete global.warmData; + for (let i = 0; i < fewThrows; ++i) + shouldThrow(readWarmData, isNotDefined("warmData"), "warmData, round " + round + " at " + i); + global.warmData = round; + for (let i = 0; i < throwCount; ++i) + shouldBe(readWarmData(), round, "warmData again, round " + round); +} + +// A name that is not there when the function is linked, appears, and goes away. +function readLateData() { return lateData; } +noInline(readLateData); +shouldThrow(readLateData, isNotDefined("lateData"), "lateData before"); +global.lateData = 5; +for (let i = 0; i < testLoopCount; ++i) + shouldBe(readLateData(), 5, "lateData"); +delete global.lateData; +for (let i = 0; i < fewThrows; ++i) + shouldThrow(readLateData, isNotDefined("lateData"), "lateData at " + i); + +// An accessor: the lower tiers cache no structure for it, so the optimized code never had one to check. +let getterCalls = 0; +Object.defineProperty(global, "accessorProperty", { get() { ++getterCalls; return 7; }, configurable: true }); +function readAccessor() { return accessorProperty; } +noInline(readAccessor); +for (let i = 0; i < testLoopCount; ++i) + shouldBe(readAccessor(), 7, "accessorProperty"); +shouldBe(getterCalls, testLoopCount, "getter calls"); +delete global.accessorProperty; +for (let i = 0; i < fewThrows; ++i) + shouldThrow(readAccessor, isNotDefined("accessorProperty"), "accessorProperty at " + i); +shouldBe(getterCalls, testLoopCount, "getter calls after the delete"); +// A later script declares the name as a variable. The variable is no property of the structure of the global +// object, so an answer cached for "not there" would stay. +$.evalScript("var accessorProperty = 9;"); +for (let i = 0; i < throwCount; ++i) + shouldBe(readAccessor(), 9, "accessorProperty as a variable"); + +// A property that is there with the value undefined is not an error, whether it is a value or an accessor. +global.undefinedData = undefined; +Object.defineProperty(global, "undefinedAccessor", { get() { return undefined; }, configurable: true }); +function readUndefinedData() { return undefinedData; } +function readUndefinedAccessor() { return undefinedAccessor; } +noInline(readUndefinedData); +noInline(readUndefinedAccessor); +for (let i = 0; i < testLoopCount; ++i) { + shouldBe(readUndefinedData(), undefined, "undefinedData"); + shouldBe(readUndefinedAccessor(), undefined, "undefinedAccessor"); +} +delete global.undefinedData; +delete global.undefinedAccessor; +for (let i = 0; i < fewThrows; ++i) { + shouldThrow(readUndefinedData, isNotDefined("undefinedData"), "undefinedData at " + i); + shouldThrow(readUndefinedAccessor, isNotDefined("undefinedAccessor"), "undefinedAccessor at " + i); +} + +// A getter that deletes its own property and returns undefined: that read found the property. +let selfDeletingIsArmed = false; +function defineSelfDeleting() { + Object.defineProperty(global, "selfDeleting", { + get() { + if (selfDeletingIsArmed) + delete global.selfDeleting; + return undefined; + }, + configurable: true, + }); +} +defineSelfDeleting(); +function readSelfDeleting() { return selfDeleting; } +noInline(readSelfDeleting); +for (let i = 0; i < testLoopCount; ++i) + shouldBe(readSelfDeleting(), undefined, "selfDeleting"); +selfDeletingIsArmed = true; +for (let i = 0; i < 10; ++i) { + shouldBe(readSelfDeleting(), undefined, "selfDeleting, the read that deletes it, round " + i); + shouldBe(Object.hasOwn(global, "selfDeleting"), false, "selfDeleting is gone, round " + i); + shouldThrow(readSelfDeleting, isNotDefined("selfDeleting"), "selfDeleting after it deleted itself, round " + i); + defineSelfDeleting(); +} +delete global.selfDeleting; + +// A property that the global object inherits. +Object.prototype.inheritedData = 3; +function readInherited() { return inheritedData; } +noInline(readInherited); +for (let i = 0; i < testLoopCount; ++i) + shouldBe(readInherited(), 3, "inheritedData"); +delete Object.prototype.inheritedData; +for (let i = 0; i < fewThrows; ++i) + shouldThrow(readInherited, isNotDefined("inheritedData"), "inheritedData at " + i); + +// A built-in function of the global object. +function readBuiltin() { return escape; } +noInline(readBuiltin); +const originalEscape = escape; +for (let i = 0; i < testLoopCount; ++i) + shouldBe(readBuiltin(), originalEscape, "escape"); +delete global.escape; +for (let i = 0; i < fewThrows; ++i) + shouldThrow(readBuiltin, isNotDefined("escape"), "escape at " + i); + +// The name "length", which the inline caches treat specially for arrays and strings. +Object.defineProperty(global, "length", { get() { return 4; }, configurable: true }); +function readLength() { return length; } +noInline(readLength); +for (let i = 0; i < testLoopCount; ++i) + shouldBe(readLength(), 4, "length"); +delete global.length; +for (let i = 0; i < fewThrows; ++i) + shouldThrow(readLength, isNotDefined("length"), "length at " + i); + +// The read inlined into its caller. +Object.defineProperty(global, "inlinedAccessor", { get() { return 13; }, configurable: true }); +function readInlinedAccessor() { return inlinedAccessor; } +function callsReadInlinedAccessor() { + try { + return readInlinedAccessor(); + } catch (e) { + return String(e); + } +} +noInline(callsReadInlinedAccessor); +for (let i = 0; i < testLoopCount; ++i) + shouldBe(callsReadInlinedAccessor(), 13, "inlinedAccessor"); +delete global.inlinedAccessor; +for (let i = 0; i < fewThrows; ++i) + shouldBe(callsReadInlinedAccessor(), isNotDefined("inlinedAccessor"), "inlinedAccessor at " + i); + +// The property comes and goes many times, as a value and as an accessor: more times than an inline cache has cases. +function readComesAndGoes() { return comesAndGoes; } +noInline(readComesAndGoes); +global.comesAndGoes = -1; +for (let i = 0; i < testLoopCount; ++i) + shouldBe(readComesAndGoes(), -1, "comesAndGoes before"); +for (let cycle = 0; cycle < 40; ++cycle) { + delete global.comesAndGoes; + for (let i = 0; i < 5; ++i) + shouldThrow(readComesAndGoes, isNotDefined("comesAndGoes"), "comesAndGoes, cycle " + cycle + " at " + i); + if (cycle % 2) + global.comesAndGoes = cycle; + else + Object.defineProperty(global, "comesAndGoes", { get() { return cycle; }, configurable: true }); + for (let i = 0; i < 50; ++i) + shouldBe(readComesAndGoes(), cycle, "comesAndGoes, cycle " + cycle); +} +delete global.comesAndGoes; + +// A lexical declaration of the name after the property is gone. +Object.defineProperty(global, "declaredLater", { get() { return 19; }, configurable: true }); +function readDeclaredLater() { return declaredLater; } +noInline(readDeclaredLater); +for (let i = 0; i < testLoopCount; ++i) + shouldBe(readDeclaredLater(), 19, "declaredLater"); +delete global.declaredLater; +for (let i = 0; i < 20; ++i) + shouldThrow(readDeclaredLater, isNotDefined("declaredLater"), "declaredLater at " + i); +$.evalScript("let declaredLater = 23;"); +for (let i = 0; i < testLoopCount; ++i) + shouldBe(readDeclaredLater(), 23, "declaredLater as a lexical binding"); + +// Properties whose getters are C++ functions, on the prototype chain of another global object, read by name in +// the realm of that global object. +const customHolder = $vm.createCustomTestGetterSetter(); +const staticValueHolder = $vm.createStaticCustomValue(); +const staticAccessorHolder = $vm.createStaticCustomAccessor(); +Object.setPrototypeOf(staticAccessorHolder, null); +Object.setPrototypeOf(staticValueHolder, staticAccessorHolder); +Object.setPrototypeOf(customHolder, staticValueHolder); +const otherGlobal = $vm.createGlobalObject(customHolder); +function testCustomGetter(name, holder, expected) { + const read = otherGlobal.Function("return " + name + ";"); + noInline(read); + for (let i = 0; i < testLoopCount; ++i) + shouldBe(read(), expected, name); + shouldBe(delete holder[name], true, "delete " + name); + for (let i = 0; i < fewThrows; ++i) + shouldThrow(read, isNotDefined(name), name + " at " + i); +} +// A custom value: its getter gets the holder. The second one returns undefined, which is not an error. +testCustomGetter("customValue", customHolder, customHolder); +testCustomGetter("customValue2", customHolder, undefined); +// A custom accessor: its getter gets the object that was read from. +testCustomGetter("customAccessor", customHolder, otherGlobal); +// Properties of a static table: the structure of the holder does not have them. +testCustomGetter("testStaticValue", staticValueHolder, undefined); +testCustomGetter("testStaticAccessor", staticAccessorHolder, undefined); + +// typeof does not throw. +Object.defineProperty(global, "typeofAccessor", { get() { return 1; }, configurable: true }); +function typeofIt() { return typeof typeofAccessor; } +noInline(typeofIt); +for (let i = 0; i < testLoopCount; ++i) + shouldBe(typeofIt(), "number", "typeof"); +delete global.typeofAccessor; +for (let i = 0; i < testLoopCount; ++i) + shouldBe(typeofIt(), "undefined", "typeof at " + i); diff --git a/JSTests/stress/global-property-strict-write-not-found.js b/JSTests/stress/global-property-strict-write-not-found.js new file mode 100644 index 0000000000000..2991f7630f0eb --- /dev/null +++ b/JSTests/stress/global-property-strict-write-not-found.js @@ -0,0 +1,215 @@ +//@ defaultRun; run("no-cjit-force-ic-failure", "--useConcurrentJIT=false", "--forceICFailure=true") + +// A write in strict mode code to a name that resolves to a property of the global object is +// a ReferenceError when that property is not there, in every tier, and does not add it. A +// plain put by id of the global object adds the property. +// global-property-read-not-found.js is the same for a read. + +function shouldBe(actual, expected, what) { + if (actual !== expected) + throw new Error(what + ": expected " + String(expected) + " but got " + String(actual)); +} + +function shouldThrow(func, expected, what) { + let error = null; + let result; + try { + result = func(); + } catch (e) { + error = e; + } + if (!error) + throw new Error(what + ": did not throw, returned " + String(result)); + if (String(error) !== expected) + throw new Error(what + ": threw " + String(error)); +} + +function isNotDefined(name) { + return "ReferenceError: " + name + " is not defined"; +} + +const global = globalThis; +// A call that throws costs far more than one that returns, so the loops that throw are short. The longer one is +// for a function that is optimized while it throws. The shorter one is for a function that already is. +const throwCount = Math.ceil(testLoopCount / 20); +const fewThrows = Math.ceil(testLoopCount / 50); + +// A data property that is there when the function is linked, and gone before the function is optimized. +global.strictData = 0; +function writeStrict(value) { "use strict"; strictData = value; } +noInline(writeStrict); +writeStrict(1); +shouldBe(global.strictData, 1, "strictData before"); +delete global.strictData; +for (let i = 0; i < throwCount; ++i) { + shouldThrow(() => writeStrict(i), isNotDefined("strictData"), "strictData at " + i); + shouldBe(Object.hasOwn(global, "strictData"), false, "strictData was not created at " + i); +} + +// The same with the optimized code already there when the property goes away, and the property coming back. +global.warmStrictData = 0; +function writeWarmStrict(value) { "use strict"; warmStrictData = value; } +noInline(writeWarmStrict); +for (let i = 0; i < testLoopCount; ++i) + writeWarmStrict(i); +shouldBe(global.warmStrictData, testLoopCount - 1, "warmStrictData before"); +for (let round = 0; round < 3; ++round) { + delete global.warmStrictData; + for (let i = 0; i < fewThrows; ++i) { + shouldThrow(() => writeWarmStrict(i), isNotDefined("warmStrictData"), "warmStrictData, round " + round + " at " + i); + shouldBe(Object.hasOwn(global, "warmStrictData"), false, "warmStrictData was not created, round " + round + " at " + i); + } + global.warmStrictData = -1; + for (let i = 0; i < throwCount; ++i) { + writeWarmStrict(i); + shouldBe(global.warmStrictData, i, "warmStrictData again, round " + round); + } +} + +// A setter: the lower tiers cache no structure for it, so the optimized code never had one to check. +let setterValue = 0; +Object.defineProperty(global, "strictAccessor", { get() { return setterValue; }, set(value) { setterValue = value; }, configurable: true }); +function writeStrictAccessor(value) { "use strict"; strictAccessor = value; } +noInline(writeStrictAccessor); +for (let i = 0; i < testLoopCount; ++i) + writeStrictAccessor(i); +shouldBe(setterValue, testLoopCount - 1, "strictAccessor before"); +delete global.strictAccessor; +for (let i = 0; i < fewThrows; ++i) { + shouldThrow(() => writeStrictAccessor(-1), isNotDefined("strictAccessor"), "strictAccessor at " + i); + shouldBe(Object.hasOwn(global, "strictAccessor"), false, "strictAccessor was not created at " + i); +} +shouldBe(setterValue, testLoopCount - 1, "strictAccessor after"); + +// A setter that the global object inherits. +let inheritedSetterValue = 0; +Object.defineProperty(Object.prototype, "inheritedSetter", { get() { return inheritedSetterValue; }, set(value) { inheritedSetterValue = value; }, configurable: true }); +function writeInheritedSetter(value) { "use strict"; inheritedSetter = value; } +noInline(writeInheritedSetter); +for (let i = 0; i < testLoopCount; ++i) + writeInheritedSetter(i); +shouldBe(inheritedSetterValue, testLoopCount - 1, "inheritedSetter before"); +shouldBe(Object.hasOwn(global, "inheritedSetter"), false, "inheritedSetter is not a property of the global object"); +delete Object.prototype.inheritedSetter; +for (let i = 0; i < fewThrows; ++i) { + shouldThrow(() => writeInheritedSetter(-1), isNotDefined("inheritedSetter"), "inheritedSetter at " + i); + shouldBe("inheritedSetter" in global, false, "inheritedSetter was not created at " + i); +} +shouldBe(inheritedSetterValue, testLoopCount - 1, "inheritedSetter after"); + +// A later script declares a variable with the name of an inherited setter. The variable is no property of the +// structure of the global object, so a write cached for that structure would still call the setter. +let shadowedSetterCalls = 0; +Object.defineProperty(Object.prototype, "shadowedSetter", { get() { return -1; }, set(value) { ++shadowedSetterCalls; }, configurable: true }); +function writeShadowedSetter(value) { "use strict"; shadowedSetter = value; } +noInline(writeShadowedSetter); +for (let i = 0; i < testLoopCount; ++i) + writeShadowedSetter(i); +shouldBe(shadowedSetterCalls, testLoopCount, "shadowedSetter calls before"); +$.evalScript("var shadowedSetter = 33;"); +for (let i = 0; i < throwCount; ++i) + writeShadowedSetter(i); +shouldBe(shadowedSetterCalls, testLoopCount, "shadowedSetter calls after the declaration"); +shouldBe(new Function("return shadowedSetter;")(), throwCount - 1, "shadowedSetter as a variable"); +delete Object.prototype.shadowedSetter; + +// An accessor with no setter: a TypeError while it is there, a ReferenceError once it is gone. +Object.defineProperty(global, "getterOnly", { get() { return 29; }, configurable: true }); +function writeGetterOnly(value, reallyWrite) { "use strict"; if (reallyWrite) getterOnly = value; } +noInline(writeGetterOnly); +for (let i = 0; i < testLoopCount; ++i) + writeGetterOnly(i, false); +for (let i = 0; i < 20; ++i) + shouldThrow(() => writeGetterOnly(i, true), "TypeError: Attempted to assign to readonly property.", "getterOnly at " + i); +delete global.getterOnly; +for (let i = 0; i < fewThrows; ++i) { + shouldThrow(() => writeGetterOnly(i, true), isNotDefined("getterOnly"), "getterOnly, gone, at " + i); + shouldBe(Object.hasOwn(global, "getterOnly"), false, "getterOnly was not created at " + i); +} + +// A writable property that the global object inherits: the write adds the property to the global object. +// Once both are gone, nothing is added. +Object.prototype.inheritedWritable = 31; +function writeInheritedWritable(value) { "use strict"; inheritedWritable = value; } +noInline(writeInheritedWritable); +for (let i = 0; i < testLoopCount; ++i) { + writeInheritedWritable(i); + shouldBe(global.inheritedWritable, i, "inheritedWritable"); + if (!(i % 1024)) { + shouldBe(delete global.inheritedWritable, true, "delete inheritedWritable"); + shouldBe(global.inheritedWritable, 31, "inheritedWritable is inherited again"); + } +} +delete global.inheritedWritable; +delete Object.prototype.inheritedWritable; +for (let i = 0; i < fewThrows; ++i) { + shouldThrow(() => writeInheritedWritable(i), isNotDefined("inheritedWritable"), "inheritedWritable, gone, at " + i); + shouldBe("inheritedWritable" in global, false, "inheritedWritable was not created at " + i); +} + +// The value that is written deletes the property: the name was found, but the write still finds nothing to write to. +global.deletedByItsValue = 0; +function writeDeletedByItsValue(reallyDelete) { "use strict"; deletedByItsValue = reallyDelete ? delete global.deletedByItsValue : false; } +noInline(writeDeletedByItsValue); +for (let i = 0; i < testLoopCount; ++i) + writeDeletedByItsValue(false); +for (let i = 0; i < 20; ++i) { + shouldThrow(() => writeDeletedByItsValue(true), isNotDefined("deletedByItsValue"), "deletedByItsValue at " + i); + shouldBe(Object.hasOwn(global, "deletedByItsValue"), false, "deletedByItsValue was not created at " + i); + global.deletedByItsValue = 0; +} +delete global.deletedByItsValue; + +// A write in sloppy mode code does create the property. +global.sloppyData = 0; +function writeSloppy(value) { sloppyData = value; } +noInline(writeSloppy); +for (let i = 0; i < testLoopCount; ++i) + writeSloppy(i); +for (let i = 0; i < fewThrows; ++i) { + delete global.sloppyData; + writeSloppy(i); + shouldBe(global.sloppyData, i, "sloppyData at " + i); +} +delete global.sloppyData; + +// After this many deletes in a row the global object is a dictionary that nothing is cached for, and the structures +// it had before are dead: a collection resets the inline caches that hold them. Names that are there are still read +// and written, and names that are not are still errors. +global.survivor = 37; +Object.defineProperty(global, "survivingAccessor", { get() { return 41; }, set(value) { setterValue = value; }, configurable: true }); +global.casualty = 43; +function readSurvivor() { return survivor + survivingAccessor; } +function writeSurvivor(value) { "use strict"; survivor = value; survivingAccessor = value; } +function readCasualty() { return casualty; } +function writeCasualty(value) { "use strict"; casualty = value; } +noInline(readSurvivor); +noInline(writeSurvivor); +noInline(readCasualty); +noInline(writeCasualty); +function checkTheOnesThatAreThere(what) { + shouldBe(readSurvivor(), 37 + 41, "survivor, " + what); + writeSurvivor(37); + shouldBe(setterValue, 37, "survivingAccessor was written, " + what); +} +for (let round = 0; round < 2; ++round) { + for (let i = 0; i < testLoopCount / 2; ++i) { + checkTheOnesThatAreThere("round " + round); + shouldBe(readCasualty(), 43, "casualty, round " + round); + writeCasualty(43); + } + if (!round) { + for (let i = 0; i < 200; ++i) + global["temporary" + i] = i; + for (let i = 0; i < 200; ++i) + delete global["temporary" + i]; + fullGC(); + } +} +delete global.casualty; +for (let i = 0; i < fewThrows; ++i) { + checkTheOnesThatAreThere("at " + i); + shouldThrow(readCasualty, isNotDefined("casualty"), "casualty, read at " + i); + shouldThrow(() => writeCasualty(i), isNotDefined("casualty"), "casualty, written at " + i); + shouldBe(Object.hasOwn(global, "casualty"), false, "casualty was not created at " + i); +} diff --git a/Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp b/Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp index 2bfa8d6ddcd51..f3e1ed9216cd8 100644 --- a/Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp +++ b/Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp @@ -1749,10 +1749,12 @@ MacroAssemblerCodeRef InlineCacheCompiler::generateSlowPathCode( switch (type) { case AccessType::GetById: case AccessType::GetByIdDirect: + case AccessType::GetByIdThrowIfNotFound: case AccessType::InById: case AccessType::GetPrivateNameById: { using ArgumentTypes = FunctionTraits::ArgumentTypes; static_assert(std::same_as::ArgumentTypes, ArgumentTypes>); + static_assert(std::same_as::ArgumentTypes, ArgumentTypes>); static_assert(std::same_as::ArgumentTypes, ArgumentTypes>); static_assert(std::same_as::ArgumentTypes, ArgumentTypes>); return vm.getCTIStub(getByIdSlowPathCodeGenerator); @@ -7247,6 +7249,7 @@ AccessGenerationResult InlineCacheCompiler::compileOneAccessCaseHandler(const Ve switch (m_propertyCache.accessType) { case AccessType::GetById: case AccessType::GetByIdDirect: + case AccessType::GetByIdThrowIfNotFound: case AccessType::GetPrivateNameById: { switch (accessCase.m_type) { case AccessCase::GetGetter: @@ -7278,6 +7281,7 @@ AccessGenerationResult InlineCacheCompiler::compileOneAccessCaseHandler(const Ve break; } case AccessCase::Miss: { + ASSERT(m_propertyCache.accessType != AccessType::GetByIdThrowIfNotFound); ASSERT(canBeViaGlobalProxy(accessCase.m_type)); if (!accessCase.viaGlobalProxy()) { collectConditions(accessCase, watchedConditions, checkingConditions); diff --git a/Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp b/Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp index 5bd3e275e95dd..0d9d5e802b5e6 100644 --- a/Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp +++ b/Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp @@ -295,6 +295,9 @@ NEVER_INLINE void PropertyInlineCache::reset(const ConcurrentJSLockerBase& locke case AccessType::GetByIdDirect: resetGetBy(codeBlock, *this, GetByKind::ByIdDirect); break; + case AccessType::GetByIdThrowIfNotFound: + resetGetBy(codeBlock, *this, GetByKind::ByIdThrowIfNotFound); + break; case AccessType::GetByVal: resetGetBy(codeBlock, *this, GetByKind::ByVal); break; @@ -602,6 +605,8 @@ static CodePtr LEGACY_NODELETE slowOperationFromUnlinkedPropert return operationGetByIdOptimize; case AccessType::GetByIdDirect: return operationGetByIdDirectOptimize; + case AccessType::GetByIdThrowIfNotFound: + return operationGetByIdThrowIfNotFoundOptimize; case AccessType::GetByIdWithThis: return operationGetByIdWithThisOptimize; case AccessType::GetByValWithThis: @@ -697,6 +702,7 @@ PropertyInlineCache::Registers PropertyInlineCache::registers() const registers.propertyCacheGPR = BaselineJITRegisters::InById::propertyCacheGPR; break; case AccessType::GetByIdDirect: + case AccessType::GetByIdThrowIfNotFound: case AccessType::GetById: case AccessType::GetPrivateNameById: registers.baseGPR = BaselineJITRegisters::GetById::baseGPR; diff --git a/Source/JavaScriptCore/bytecode/PropertyInlineCache.h b/Source/JavaScriptCore/bytecode/PropertyInlineCache.h index e99d525cf7773..e3689639ec778 100644 --- a/Source/JavaScriptCore/bytecode/PropertyInlineCache.h +++ b/Source/JavaScriptCore/bytecode/PropertyInlineCache.h @@ -55,6 +55,7 @@ class PolymorphicAccess; macro(GetById) \ macro(GetByIdWithThis) \ macro(GetByIdDirect) \ + macro(GetByIdThrowIfNotFound) \ macro(GetByVal) \ macro(GetByValWithThis) \ macro(PutByIdStrict) \ @@ -689,6 +690,8 @@ inline auto appropriateGetByIdOptimizeFunction(AccessType type) -> decltype(&ope return operationGetByIdOptimize; case AccessType::GetByIdDirect: return operationGetByIdDirectOptimize; + case AccessType::GetByIdThrowIfNotFound: + return operationGetByIdThrowIfNotFoundOptimize; case AccessType::GetPrivateNameById: return operationGetPrivateNameByIdOptimize; case AccessType::GetByIdWithThis: @@ -762,6 +765,7 @@ inline bool hasConstantIdentifier(AccessType accessType) case AccessType::DeleteByIdSloppy: case AccessType::InById: case AccessType::GetByIdDirect: + case AccessType::GetByIdThrowIfNotFound: case AccessType::GetById: case AccessType::GetPrivateNameById: case AccessType::GetByIdWithThis: diff --git a/Source/JavaScriptCore/bytecode/Repatch.cpp b/Source/JavaScriptCore/bytecode/Repatch.cpp index 023557205e477..6bdadeb311af4 100644 --- a/Source/JavaScriptCore/bytecode/Repatch.cpp +++ b/Source/JavaScriptCore/bytecode/Repatch.cpp @@ -450,6 +450,8 @@ inline CodePtr LEGACY_NODELETE appropriateGetByOptimizeFunction return operationGetByIdWithThisOptimize; case GetByKind::ByIdDirect: return operationGetByIdDirectOptimize; + case GetByKind::ByIdThrowIfNotFound: + return operationGetByIdThrowIfNotFoundOptimize; case GetByKind::ByVal: return operationGetByValOptimize; case GetByKind::ByValWithThis: @@ -471,6 +473,8 @@ inline CodePtr LEGACY_NODELETE appropriateGetByGaveUpFunction(G return operationGetByIdWithThisGaveUp; case GetByKind::ByIdDirect: return operationGetByIdDirectGaveUp; + case GetByKind::ByIdThrowIfNotFound: + return operationGetByIdThrowIfNotFoundGaveUp; case GetByKind::ByVal: return operationGetByValGaveUp; case GetByKind::ByValWithThis: @@ -493,6 +497,13 @@ static InlineCacheAction tryCacheGetBy(JSGlobalObject* globalObject, CodeBlock* if (forceICFailure(globalObject)) return GiveUpOnCache; + + // For GetByKind::ByIdThrowIfNotFound finding no property is an error, so there is no miss to cache. Its + // operations throw and do not come here. + if (kind == GetByKind::ByIdThrowIfNotFound && slot.isUnset()) [[unlikely]] { + ASSERT_NOT_REACHED(); + return GiveUpOnCache; + } // FIXME: Cache property access for immediates. if (!baseValue.isCell()) diff --git a/Source/JavaScriptCore/bytecode/Repatch.h b/Source/JavaScriptCore/bytecode/Repatch.h index a50670dc1a3dc..4d7932549887e 100644 --- a/Source/JavaScriptCore/bytecode/Repatch.h +++ b/Source/JavaScriptCore/bytecode/Repatch.h @@ -40,6 +40,7 @@ enum class GetByKind { ByVal, ByIdWithThis, ByIdDirect, + ByIdThrowIfNotFound, ByValWithThis, PrivateName, PrivateNameById, diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp index 48e25d0ee305b..d2a9ca37fae9d 100644 --- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp +++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp @@ -7487,7 +7487,7 @@ void ByteCodeParser::handleGetById( getById = getByStatus.makesCalls() ? GetByIdFlush : GetById; else getById = getByStatus.makesCalls() ? GetByIdDirectFlush : GetByIdDirect; - auto* data = m_graph.m_getByIdData.add(GetByIdData { identifier, getByStatus.preferredCacheType() }); + auto* data = m_graph.m_getByIdData.add(GetByIdData { identifier, getByStatus.preferredCacheType(), DoNotThrowIfNotFound }); if (getByStatus.isModuleNamespace()) { if (handleModuleNamespaceLoad(destination, prediction, base, getByStatus)) { @@ -7711,7 +7711,7 @@ void ByteCodeParser::handleGetPrivateNameById( ASSERT(!getByStatus.isCustomAccessor()); ASSERT(!getByStatus.makesCalls()); if (!getByStatus.isSimple() || !getByStatus.numVariants() || !Options::useAccessInlining()) { - auto* data = m_graph.m_getByIdData.add(GetByIdData { identifier, CacheType::GetByIdSelf }); + auto* data = m_graph.m_getByIdData.add(GetByIdData { identifier, CacheType::GetByIdSelf, DoNotThrowIfNotFound }); set(destination, addToGraph(GetPrivateNameById, OpInfo(data), OpInfo(prediction), base, nullptr)); return; } @@ -7720,7 +7720,7 @@ void ByteCodeParser::handleGetPrivateNameById( if (!m_graph.m_plan.isFTL() || !Options::usePolymorphicAccessInlining() || getByStatus.numVariants() > Options::maxPolymorphicAccessInliningListSize()) { - auto* data = m_graph.m_getByIdData.add(GetByIdData { identifier, CacheType::GetByIdSelf }); + auto* data = m_graph.m_getByIdData.add(GetByIdData { identifier, CacheType::GetByIdSelf, DoNotThrowIfNotFound }); set(destination, addToGraph(GetPrivateNameById, OpInfo(data), OpInfo(prediction), base, nullptr)); return; } @@ -7759,7 +7759,7 @@ void ByteCodeParser::handleGetPrivateNameById( Node* loadedValue = load(prediction, base, unwrapped, identifierNumber, variant); if (!loadedValue) { - auto* data = m_graph.m_getByIdData.add(GetByIdData { identifier, CacheType::GetByIdSelf }); + auto* data = m_graph.m_getByIdData.add(GetByIdData { identifier, CacheType::GetByIdSelf, DoNotThrowIfNotFound }); set(destination, addToGraph(GetPrivateNameById, OpInfo(data), OpInfo(prediction), base, nullptr)); return; } @@ -9800,7 +9800,7 @@ void ByteCodeParser::parseBlock(unsigned limit) GetByStatus getByStatus = GetByStatus::computeFor(m_inlineStackTop->m_profiledBlock, m_inlineStackTop->m_baselineMap, m_icContextStack, currentCodeOrigin()); - auto* data = m_graph.m_getByIdData.add(GetByIdData { CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(m_inlineStackTop->m_profiledBlock, uid), CacheType::GetByIdSelf }); + auto* data = m_graph.m_getByIdData.add(GetByIdData { CacheableIdentifier::createFromIdentifierOwnedByCodeBlock(m_inlineStackTop->m_profiledBlock, uid), CacheType::GetByIdSelf, DoNotThrowIfNotFound }); set(bytecode.m_dst, addToGraph(getByStatus.isMegamorphic() && canUseMegamorphicGetById(*m_vm, uid) ? GetByIdWithThisMegamorphic : GetByIdWithThis, OpInfo(data), OpInfo(prediction), base, thisValue)); NEXT_OPCODE(op_get_by_id_with_this); @@ -10831,7 +10831,9 @@ void ByteCodeParser::parseBlock(unsigned limit) if (status.state() != GetByStatus::Simple || status.numVariants() != 1 || status[0].structureSet().size() != 1) { - auto* data = m_graph.m_getByIdData.add(GetByIdData { identifier, CacheType::GetByIdSelf }); + // Nothing here proves that the property is there. With ThrowIfNotFound the inline cache has to throw + // when it finds none, as the lower tiers do. A plain get by id gives undefined. + auto* data = m_graph.m_getByIdData.add(GetByIdData { identifier, CacheType::GetByIdSelf, getPutInfo.resolveMode() }); set(bytecode.m_dst, addToGraph(GetByIdFlush, OpInfo(data), OpInfo(prediction), get(bytecode.m_scope))); break; } @@ -12934,7 +12936,7 @@ void ByteCodeParser::handleIteratorNext(const JSInstruction* currentInstruction, Node* iteratedObject = getIteratedObject(); Node* length = nullptr; if (arrayMode.type() == Array::Generic) { - auto* data = m_graph.m_getByIdData.add(GetByIdData { CacheableIdentifier::createFromImmortalIdentifier(m_vm->propertyNames->length.impl()), CacheType::GetByIdSelf }); + auto* data = m_graph.m_getByIdData.add(GetByIdData { CacheableIdentifier::createFromImmortalIdentifier(m_vm->propertyNames->length.impl()), CacheType::GetByIdSelf, DoNotThrowIfNotFound }); length = addToGraph(GetById, OpInfo(data), OpInfo(SpecInt32Only), iteratedObject); } else { Node* butterfly = addToGraph(GetButterfly, iteratedObject); @@ -13594,7 +13596,7 @@ void ByteCodeParser::handleAsyncIteratorOpen(const JSInstruction* currentInstruc // harmless deopt to op_async_iterator_next's generic call path. m_iterator is live (set at symbolCall, // flushed by progressToNextCheckpoint), so the checkpoint has what it needs. BytecodeIndex getNextIndex = m_currentIndex; - auto* data = m_graph.m_getByIdData.add(GetByIdData { CacheableIdentifier::createFromImmortalIdentifier(nextImpl), getByStatus.preferredCacheType() }); + auto* data = m_graph.m_getByIdData.add(GetByIdData { CacheableIdentifier::createFromImmortalIdentifier(nextImpl), getByStatus.preferredCacheType(), DoNotThrowIfNotFound }); NodeType getByIdOp = getByStatus.makesCalls() ? GetByIdFlush : GetById; Node* fetched = addToGraph(getByIdOp, OpInfo(data), OpInfo(predict()), get(bytecode.m_iterator)); emitExitOK(); diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp index 9cb6594fb99f7..5bed8a2bb67f4 100644 --- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp +++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp @@ -2305,6 +2305,13 @@ class FixupPhase : public Phase { case GetById: case GetByIdFlush: { + // The base of a ThrowIfNotFound read is a scope. No conversion below is for it, and as a cell it + // needs no operation for a base that is not one. + if (node->resolveMode() == ThrowIfNotFound) { + fixEdge(node->child1()); + break; + } + // FIXME: This should be done in the ByteCodeParser based on reading the // PolymorphicAccess, which will surely tell us that this is a AccessCase::ArrayLength. // https://bugs.webkit.org/show_bug.cgi?id=154990 diff --git a/Source/JavaScriptCore/dfg/DFGGraph.cpp b/Source/JavaScriptCore/dfg/DFGGraph.cpp index 7bc5920dcb011..68a81adb36eab 100644 --- a/Source/JavaScriptCore/dfg/DFGGraph.cpp +++ b/Source/JavaScriptCore/dfg/DFGGraph.cpp @@ -275,6 +275,8 @@ void Graph::dump(PrintStream& out, const char* prefixStr, Node* node, DumpContex out.print(comma, "cachable-id {"_s, node->cacheableIdentifier(), "}"_s); if (node->hasCacheType()) out.print(comma, node->cacheType()); + if (node->hasResolveMode() && node->resolveMode() == ThrowIfNotFound) + out.print(comma, "ThrowIfNotFound"_s); if (node->hasPromotedLocationDescriptor()) out.print(comma, node->promotedLocationDescriptor()); if (node->hasClassInfo()) diff --git a/Source/JavaScriptCore/dfg/DFGNode.cpp b/Source/JavaScriptCore/dfg/DFGNode.cpp index a07d36a3cdbf3..a8bb1c737a69b 100644 --- a/Source/JavaScriptCore/dfg/DFGNode.cpp +++ b/Source/JavaScriptCore/dfg/DFGNode.cpp @@ -528,7 +528,7 @@ void Node::convertToGetByIdMaybeMegamorphic(Graph& graph, CacheableIdentifier id children.child1() = Edge(base.node(), CellUse); children.child2() = Edge(); children.child3() = Edge(); - auto* data = graph.m_getByIdData.add(GetByIdData { identifier, CacheType::GetByIdSelf }); + auto* data = graph.m_getByIdData.add(GetByIdData { identifier, CacheType::GetByIdSelf, DoNotThrowIfNotFound }); m_opInfo = data; } diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h index 9eb0785340bdd..11038ac6322bc 100644 --- a/Source/JavaScriptCore/dfg/DFGNode.h +++ b/Source/JavaScriptCore/dfg/DFGNode.h @@ -311,8 +311,18 @@ struct CallCustomAccessorData { }; struct GetByIdData { + GetByIdData(CacheableIdentifier identifier, CacheType cacheType, ResolveMode resolveMode) + : m_identifier(identifier) + , m_cacheType(cacheType) + , m_resolveMode(resolveMode) + { + } + CacheableIdentifier m_identifier; CacheType m_cacheType; + // ThrowIfNotFound is the read of a variable through the object that holds it: a name with no property is a + // ReferenceError there, not undefined. Only GetById and GetByIdFlush can have it. + ResolveMode m_resolveMode; }; enum class BucketOwnerType : uint32_t { @@ -1359,6 +1369,17 @@ struct Node { return getByIdData().m_cacheType; } + bool hasResolveMode() const + { + return hasGetByIdData(); + } + + ResolveMode resolveMode() + { + ASSERT(hasResolveMode()); + return getByIdData().m_resolveMode; + } + unsigned identifierNumber() { ASSERT(hasIdentifier()); diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp index 2351d075b44fe..0bd2b61825aa5 100644 --- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp +++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp @@ -4790,7 +4790,7 @@ void SpeculativeJIT::compile(Node* node) case GetById: case GetByIdFlush: { - compileGetById(node, AccessType::GetById); + compileGetById(node, node->resolveMode() == ThrowIfNotFound ? AccessType::GetByIdThrowIfNotFound : AccessType::GetById); break; } @@ -7155,7 +7155,9 @@ void SpeculativeJIT::compileGetByValWithThis(Node* node) void SpeculativeJIT::compileGetById(Node* node, AccessType accessType) { - ASSERT(accessType == AccessType::GetById || accessType == AccessType::GetByIdDirect); + ASSERT(accessType == AccessType::GetById || accessType == AccessType::GetByIdDirect || accessType == AccessType::GetByIdThrowIfNotFound); + ASSERT((accessType == AccessType::GetByIdThrowIfNotFound) == (node->resolveMode() == ThrowIfNotFound)); + ASSERT(accessType != AccessType::GetByIdThrowIfNotFound || node->child1().useKind() == CellUse); CacheType cacheType = CacheType::GetByIdSelf; if (node->cacheableIdentifier() == vm().propertyNames->length) cacheType = CacheType::ArrayLength; diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp index 4af5a8adaffd7..4566cf8869946 100644 --- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp +++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp @@ -1000,7 +1000,7 @@ class LowerDFGToB3 { break; case GetById: case GetByIdFlush: - compileGetById(AccessType::GetById); + compileGetById(m_node->resolveMode() == ThrowIfNotFound ? AccessType::GetByIdThrowIfNotFound : AccessType::GetById); break; case GetByIdMegamorphic: compileGetByIdMegamorphic(); @@ -4351,7 +4351,9 @@ class LowerDFGToB3 { void compileGetById(AccessType type) { - ASSERT(type == AccessType::GetById || type == AccessType::GetByIdDirect); + ASSERT(type == AccessType::GetById || type == AccessType::GetByIdDirect || type == AccessType::GetByIdThrowIfNotFound); + ASSERT((type == AccessType::GetByIdThrowIfNotFound) == (m_node->resolveMode() == ThrowIfNotFound)); + ASSERT(type != AccessType::GetByIdThrowIfNotFound || m_node->child1().useKind() == CellUse); JSGlobalObject* globalObject = m_graph.globalObjectFor(m_origin.semantic); switch (m_node->child1().useKind()) { case CellUse: { diff --git a/Source/JavaScriptCore/jit/ICStats.h b/Source/JavaScriptCore/jit/ICStats.h index 77e2c62a17165..bc972777c76bd 100644 --- a/Source/JavaScriptCore/jit/ICStats.h +++ b/Source/JavaScriptCore/jit/ICStats.h @@ -55,6 +55,8 @@ static constexpr bool traceHandlerChains = false; macro(OperationGetByIdGeneric) \ macro(OperationGetByIdBuildList) \ macro(OperationGetByIdOptimize) \ + macro(OperationGetByIdThrowIfNotFoundGaveUp) \ + macro(OperationGetByIdThrowIfNotFoundOptimize) \ macro(OperationGetByValOptimize) \ macro(OperationGetByIdWithThisOptimize) \ macro(OperationGetByIdWithThisGaveUp) \ diff --git a/Source/JavaScriptCore/jit/JITOperations.cpp b/Source/JavaScriptCore/jit/JITOperations.cpp index bf3ad17bb2273..d083f5e559b3f 100644 --- a/Source/JavaScriptCore/jit/JITOperations.cpp +++ b/Source/JavaScriptCore/jit/JITOperations.cpp @@ -541,6 +541,70 @@ JSC_DEFINE_JIT_OPERATION(operationGetByIdOptimize, EncodedJSValue, (EncodedJSVal }))); } +// AccessType::GetByIdThrowIfNotFound is the read of a variable through the object that holds it, the way +// op_get_from_scope does it with ThrowIfNotFound: a name with no property is a ReferenceError, not undefined. +// Every way such an access can end without a property comes through here, and nothing is cached for it. +template +static ALWAYS_INLINE JSValue getByIdThrowIfNotFound(JSGlobalObject* globalObject, VM& vm, JSValue baseValue, CacheableIdentifier identifier, const WhenFoundFunctor& whenFound) +{ + auto scope = DECLARE_THROW_SCOPE(vm); + + PropertyName propertyName = identifier; + PropertySlot slot(baseValue, PropertySlot::InternalMethodType::Get); + bool found = baseValue.getPropertySlot(globalObject, propertyName, slot); + EXCEPTION_ASSERT(!scope.exception() || !found); + if (!found) [[unlikely]] { + RETURN_IF_EXCEPTION(scope, { }); + throwException(globalObject, scope, createUndefinedVariableError(globalObject, Identifier::fromUid(vm, propertyName.uid()))); + return { }; + } + + whenFound(slot); + RELEASE_AND_RETURN(scope, slot.getValue(globalObject, propertyName)); +} + +JSC_DEFINE_JIT_OPERATION(operationGetByIdThrowIfNotFoundGaveUp, EncodedJSValue, (EncodedJSValue base, PropertyInlineCache* propertyCache)) +{ + SuperSamplerScope superSamplerScope(false); + + JSGlobalObject* globalObject = propertyCache->globalObject(); + VM& vm = globalObject->vm(); + CallFrame* callFrame = DECLARE_CALL_FRAME(vm); + ICSlowPathCallFrameTracer tracer(vm, callFrame, propertyCache); + auto scope = DECLARE_THROW_SCOPE(vm); + + propertyCache->tookSlowPath = true; + + JSValue baseValue = JSValue::decode(base); + + OPERATION_RETURN(scope, JSValue::encode(getByIdThrowIfNotFound(globalObject, vm, baseValue, propertyCache->identifier(), [&] (PropertySlot& slot) { + LOG_IC((ICEvent::OperationGetByIdThrowIfNotFoundGaveUp, baseValue.classInfoOrNull(), baseValue == slot.slotBase())); + }))); +} + +JSC_DEFINE_JIT_OPERATION(operationGetByIdThrowIfNotFoundOptimize, EncodedJSValue, (EncodedJSValue base, PropertyInlineCache* propertyCache)) +{ + SuperSamplerScope superSamplerScope(false); + + JSGlobalObject* globalObject = propertyCache->globalObject(); + VM& vm = globalObject->vm(); + CallFrame* callFrame = DECLARE_CALL_FRAME(vm); + ICSlowPathCallFrameTracer tracer(vm, callFrame, propertyCache); + auto scope = DECLARE_THROW_SCOPE(vm); + + CacheableIdentifier identifier = propertyCache->identifier(); + + JSValue baseValue = JSValue::decode(base); + + OPERATION_RETURN(scope, JSValue::encode(getByIdThrowIfNotFound(globalObject, vm, baseValue, identifier, [&] (PropertySlot& slot) { + LOG_IC((ICEvent::OperationGetByIdThrowIfNotFoundOptimize, baseValue.classInfoOrNull(), baseValue == slot.slotBase())); + + CodeBlock* codeBlock = callFrame->codeBlock(); + if (propertyCache->considerRepatchingCacheBy(vm, codeBlock, baseValue.structureOrNull(), identifier)) + repatchGetBy(globalObject, codeBlock, baseValue, identifier, slot, *propertyCache, GetByKind::ByIdThrowIfNotFound, /* isNonStringPrimitiveKey */ false); + }))); +} + JSC_DEFINE_JIT_OPERATION(operationGetByIdWithThisGaveUp, EncodedJSValue, (EncodedJSValue base, EncodedJSValue thisEncoded, PropertyInlineCache* propertyCache)) { SuperSamplerScope superSamplerScope(false); diff --git a/Source/JavaScriptCore/jit/JITOperations.h b/Source/JavaScriptCore/jit/JITOperations.h index b8a74ee425396..e9e7d0ac4eb0e 100644 --- a/Source/JavaScriptCore/jit/JITOperations.h +++ b/Source/JavaScriptCore/jit/JITOperations.h @@ -193,6 +193,9 @@ JSC_DECLARE_JIT_OPERATION(operationGetByIdDirectOptimize, EncodedJSValue, (Encod JSC_DECLARE_JIT_OPERATION(operationGetByIdDirectGaveUp, EncodedJSValue, (EncodedJSValue, PropertyInlineCache*)); JSC_DECLARE_JIT_OPERATION(operationGetByIdDirectGeneric, EncodedJSValue, (JSGlobalObject*, EncodedJSValue, uintptr_t)); +JSC_DECLARE_JIT_OPERATION(operationGetByIdThrowIfNotFoundOptimize, EncodedJSValue, (EncodedJSValue, PropertyInlineCache*)); +JSC_DECLARE_JIT_OPERATION(operationGetByIdThrowIfNotFoundGaveUp, EncodedJSValue, (EncodedJSValue, PropertyInlineCache*)); + JSC_DECLARE_JIT_OPERATION(operationGetByIdWithThisOptimize, EncodedJSValue, (EncodedJSValue, EncodedJSValue, PropertyInlineCache*)); JSC_DECLARE_JIT_OPERATION(operationGetByIdWithThisMegamorphic, EncodedJSValue, (EncodedJSValue, EncodedJSValue, PropertyInlineCache*)); JSC_DECLARE_JIT_OPERATION(operationGetByIdWithThisGaveUp, EncodedJSValue, (EncodedJSValue, EncodedJSValue, PropertyInlineCache*)); From edd7475cce8dbc5be3fd7fca1dd9f91c29cbed36 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 26 Oct 2026 12:04:19 +0000 Subject: [PATCH 2/2] [JSC] A GetByIdThrowIfNotFound cache that gives up drops its cases The base of such an access is one global object. A global object is a dictionary from its start, so no transition gives it a structure that it had before. When the cache gives up, its cases are for structures that are gone. The one exception is the last case of a cache that gives up because it is full: that case is for the structure of the moment. The cases stayed in front of the slow path call. A site that saw more than 8 structures of the global object then cost 38 instructions per read more in the DFG, and 7 in the FTL, than the same site with GetById, whose megamorphic case does not take a global object and resets the site. repatchGetBy now resets the cache when it gives up for this kind, with repatchGetBySlowPathCall, as the megamorphic operations do. Such a site now costs what it costs with GetById: 563 instructions per read of a getter global in the DFG (565 with GetById), 529 in the FTL (531). * Source/JavaScriptCore/bytecode/Repatch.cpp: (JSC::repatchGetBy): --- Source/JavaScriptCore/bytecode/Repatch.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Source/JavaScriptCore/bytecode/Repatch.cpp b/Source/JavaScriptCore/bytecode/Repatch.cpp index 6bdadeb311af4..923c238f738d4 100644 --- a/Source/JavaScriptCore/bytecode/Repatch.cpp +++ b/Source/JavaScriptCore/bytecode/Repatch.cpp @@ -803,6 +803,14 @@ void repatchGetBy(JSGlobalObject* globalObject, CodeBlock* codeBlock, JSValue ba break; } case GiveUpOnCache: + // The base of a ByIdThrowIfNotFound access is one global object. That is a dictionary from its start, so it + // never has a structure again that it had before: the cases here are dead, or will be at its next + // transition. Without them the access goes straight to its call, as a ById access does after its + // megamorphic case. + if (kind == GetByKind::ByIdThrowIfNotFound) { + repatchGetBySlowPathCall(codeBlock, propertyCache, kind); + break; + } repatchSlowPathCall(codeBlock, propertyCache, appropriateGetByGaveUpFunction(kind)); break; case RetryCacheLater: