From 6f120a3bf25c362d7037ff3185b3bf142380565e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 27 Sep 2026 06:01:51 +0000 Subject: [PATCH 1/3] [JSC] JSGlobalObject::addSymbolTableEntry makes the global object a dictionary before it adds a name A program of the form `var x = ;` runs on the JSONP fast path of Interpreter::executeProgram. That path calls createGlobalVarBinding, which adds the name to the symbol table of the global object. The Structure of the global object stays the same, so inline caches and optimized code that hold "x is not an own property" against that Structure keep their answer: function get() { return globalThis.x; } noInline(get); for (let i = 0; i < 100000; ++i) get(); loadString("var x = 33;"); get(); // undefined, expected 33 A program that runs as bytecode does not have the bug, because ProgramExecutable::initializeGlobalProperties makes the global object a dictionary before it creates a binding. addSymbolTableEntry now converts a global object whose Structure is not a dictionary before it adds the name. Caches hold the absence of a property only against a Structure that is not a dictionary, and the conversion gives the object another Structure and fires the transition watchpoint of the old one. A declaration of a name that exists creates no binding and converts nothing. addStaticGlobals, the other function that adds names to that symbol table, asserts the same condition. Its callers run while the global object is created, after init() has made it a dictionary. * JSTests/stress/jsonp-var-declaration-invalidates-global-object-property-caches.js: Added. * Source/JavaScriptCore/runtime/JSGlobalObject.cpp: (JSC::JSGlobalObject::addSymbolTableEntry): (JSC::JSGlobalObject::addStaticGlobals): --- ...validates-global-object-property-caches.js | 115 ++++++++++++++++++ .../JavaScriptCore/runtime/JSGlobalObject.cpp | 9 ++ 2 files changed, 124 insertions(+) create mode 100644 JSTests/stress/jsonp-var-declaration-invalidates-global-object-property-caches.js diff --git a/JSTests/stress/jsonp-var-declaration-invalidates-global-object-property-caches.js b/JSTests/stress/jsonp-var-declaration-invalidates-global-object-property-caches.js new file mode 100644 index 0000000000000..1762400c9bd45 --- /dev/null +++ b/JSTests/stress/jsonp-var-declaration-invalidates-global-object-property-caches.js @@ -0,0 +1,115 @@ +// A program of the form `var x = ;` runs on the JSONP fast path of Interpreter::executeProgram, without +// bytecode. The variable it declares is a new own property of the global object, so what inline caches and +// optimized code recorded about the global object before the declaration must not be used after it. + +function shouldBe(actual, expected, message) { + if (actual !== expected) + throw new Error(message + ": expected " + expected + " but got " + actual); +} + +// The same declarations as a program that runs as bytecode give the same results. +const forms = [ + ["JSONP", ""], + ["bytecode", " void 0;"], +]; + +for (const [form, suffix] of forms) { + // One realm per case: after the first declaration the global object is a dictionary that was flattened + // before, and caches give up on it. A second case in the same realm would not reach a cached site. + function test(name, setup, declaration, expected) { + const realm = $262.createRealm(); + const access = realm.evalScript(setup + ` + noInline(access); + for (let i = 0; i < ${testLoopCount}; ++i) + access(i); + access; + `); + realm.evalScript(declaration + suffix); + shouldBe(String(access(7)), expected, form + ", " + name); + } + + // A name that no object has. + test("get_by_id miss", `function access() { return globalThis.name1; }`, `var name1 = 33;`, "33"); + test("get_by_id miss, this", `function access() { return this.name1; }`, `var name1 = 33;`, "33"); + test("get_by_val, constant key", `const key = "name1"; function access() { return globalThis[key]; }`, `var name1 = 33;`, "33"); + test("get_by_id_with_this", `var home = { __proto__: globalThis, method() { return super.name1; } }; function access() { return home.method(); }`, `var name1 = 33;`, "33"); + test("get_by_val_with_this", `function access() { return Reflect.get(globalThis, "name1", globalThis); }`, `var name1 = 33;`, "33"); + test("object value", `function access() { return globalThis.name1?.a; }`, `var name1 = {"a":1};`, "1"); + test("two declarations", `function access() { return globalThis.name2; }`, `var name1 = 1; var name2 = [2];`, "2"); + + // A name that the prototype chain of the global object has. + test("prototype data property", `function access() { return typeof globalThis.toString; }`, `var toString = 33;`, "number"); + test("prototype getter", `Object.defineProperty(Object.prototype, "name1", { get() { return "getter"; }, configurable: true }); + function access() { return globalThis.name1; }`, `var name1 = 33;`, "33"); + test("prototype method call", `function access() { try { return globalThis.valueOf() === globalThis; } catch (e) { return e.name; } }`, `var valueOf = 33;`, "TypeError"); + test("prototype setter", `var calls = 0; Object.defineProperty(Object.prototype, "name1", { set(v) { calls++; }, get() { return "getter"; }, configurable: true }); + function access(v) { let before = calls; globalThis.name1 = v; return calls - before + ":" + globalThis.name1; }`, `var name1 = 33;`, "0:7"); + + // An identifier resolves against the global object itself, not against the JSGlobalProxy. + test("identifier read", `function access() { return typeof valueOf; }`, `var valueOf = 33;`, "number"); + test("identifier write", `var calls = 0; Object.defineProperty(Object.prototype, "name1", { set(v) { calls++; }, get() { return "getter"; }, configurable: true }); + function access(v) { let before = calls; name1 = v; return calls - before; }`, `var name1 = 33;`, "0"); + test("identifier delete", `function access() { return delete name1; }`, `var name1 = 33;`, "false"); + + // The cache is created between two statements of the program that declares the variable. + test("cache created by an earlier statement", `function read() { return globalThis.name2; } noInline(read); + var holder = { set warm(v) { for (let i = 0; i < ${testLoopCount}; ++i) read(); } }; + function access() { return read(); }`, `var name1 = 1; holder.warm = 1; var name2 = 2;`, "2"); + + // The reader is on the stack, in optimized code, when the program runs. + { + const realm = $262.createRealm(); + const result = realm.evalScript(` + function read() { return globalThis.name1; } + function loop(count) { + let seen = []; + for (let i = 0; i < count; ++i) { + if (i === count - 2) + $262.evalScript("var name1 = 33;${suffix}"); + let value = read(); + if (i >= count - 3) + seen.push(String(value)); + } + return seen.join(); + } + noInline(loop); + loop(${testLoopCount}); + `); + shouldBe(result, "undefined,33,33", form + ", reader on the stack"); + } + + // The same name again: no second binding, the value is assigned, the descriptor stays. + { + const realm = $262.createRealm(); + const access = realm.evalScript(` + function access() { return globalThis.name1; } + noInline(access); + for (let i = 0; i < ${testLoopCount}; ++i) + access(); + access; + `); + realm.evalScript(`var name1 = 1;` + suffix); + shouldBe(access(), 1, form + ", first run"); + realm.evalScript(`var name1 = 2;` + suffix); + shouldBe(access(), 2, form + ", second run"); + const descriptor = Object.getOwnPropertyDescriptor(realm.global, "name1"); + shouldBe(JSON.stringify(descriptor), `{"value":2,"writable":true,"enumerable":true,"configurable":false}`, form + ", descriptor"); + } +} + +// On the JSONP fast path only a new name gives the global object another Structure. A `var` of a name +// that exists creates no binding, and the caches that are valid stay. +{ + const global = $vm.createGlobalObject(); + // The list has five entries for each Structure. The first of the last five is the ID of the current one. + const structureID = () => $vm.getStructureTransitionList(global).at(-5); + global.$vm.evaluateWithScopeExtension(`var name1 = 1;`); + $vm.flattenDictionaryObject(global); + const flattened = structureID(); + global.$vm.evaluateWithScopeExtension(`var name1 = 2;`); + shouldBe(structureID(), flattened, "same name: same Structure"); + shouldBe(global.name1, 2, "same name: value"); + global.$vm.evaluateWithScopeExtension(`var name2 = 3;`); + shouldBe(structureID() !== flattened, true, "new name: another Structure"); + shouldBe(global.name2, 3, "new name: value"); +} diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp index 7b01d88493811..b4edde562ee75 100644 --- a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp +++ b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp @@ -2555,6 +2555,12 @@ template void JSGlobalObject::createGlobalFunctionBindingisDictionary()) + convertToDictionary(vm()); + ConcurrentJSLocker locker(symbolTable()->m_lock); ASSERT(!symbolTable()->contains(locker, ident.impl())); @@ -3305,6 +3311,9 @@ IntlCollator* JSGlobalObject::cachedLocaleCompareCollator(JSString* locale) void JSGlobalObject::addStaticGlobals(std::span globals) { + // See addSymbolTableEntry(). This runs while the global object is created: init() has made it a dictionary. + ASSERT(structure()->isDictionary()); + ScopeOffset startOffset = addVariables(globals.size(), jsUndefined()); for (auto [i, global] : indexedRange(globals)) { From daea05ba3fbfedf4d6e579553bc2dea30c10f817 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 28 Sep 2026 20:58:08 +0000 Subject: [PATCH 2/3] [JSC] The test of the JSONP declaration pins the guard, and has a case for the LLInt - The reader of the case "cache created by an earlier statement" is cold before the program runs. The second declaration of the program has to convert the global object. - Two cases use a global object with no JSGlobalProxy. The LLInt cache of a load from the prototype chain is stale there without the fix, also with --useJIT=0. - The last block checks that a new name on a dictionary keeps the Structure, and that a new name converts again after the global object was flattened again. - A function declaration as a program. --- ...validates-global-object-property-caches.js | 85 +++++++++++++++++-- 1 file changed, 76 insertions(+), 9 deletions(-) diff --git a/JSTests/stress/jsonp-var-declaration-invalidates-global-object-property-caches.js b/JSTests/stress/jsonp-var-declaration-invalidates-global-object-property-caches.js index 1762400c9bd45..14f6f4d64bb26 100644 --- a/JSTests/stress/jsonp-var-declaration-invalidates-global-object-property-caches.js +++ b/JSTests/stress/jsonp-var-declaration-invalidates-global-object-property-caches.js @@ -51,10 +51,33 @@ for (const [form, suffix] of forms) { function access(v) { let before = calls; name1 = v; return calls - before; }`, `var name1 = 33;`, "0"); test("identifier delete", `function access() { return delete name1; }`, `var name1 = 33;`, "false"); - // The cache is created between two statements of the program that declares the variable. - test("cache created by an earlier statement", `function read() { return globalThis.name2; } noInline(read); - var holder = { set warm(v) { for (let i = 0; i < ${testLoopCount}; ++i) read(); } }; - function access() { return read(); }`, `var name1 = 1; holder.warm = 1; var name2 = 2;`, "2"); + // The cache is created between two statements of the program that declares the variable. Nothing calls + // read() before the program runs, so the first declaration finds the dictionary that init() left. + { + const realm = $262.createRealm(); + const read = realm.evalScript(` + function read() { return globalThis.name2; } + noInline(read); + var holder = { set warm(v) { for (let i = 0; i < ${testLoopCount}; ++i) read(); } }; + read; + `); + realm.evalScript(`var name1 = 1; holder.warm = 1; var name2 = 2;` + suffix); + shouldBe(read(), 2, form + ", cache created by an earlier statement"); + } + + // A global object with no JSGlobalProxy in front of it. The LLInt caches a load from the prototype chain + // of such an object, so the first case does not need the JIT. + function testWithoutProxy(name, count, body, declaration, expected) { + const global = $vm.createGlobalObject(); + const access = new Function("object", body + " // " + form); + noInline(access); + for (let i = 0; i < count; ++i) + access(global); + global.$vm.evaluateWithScopeExtension(declaration + suffix); + shouldBe(String(access(global)), expected, form + ", " + name); + } + testWithoutProxy("no proxy, prototype data property in the LLInt", 20, `return typeof object.toString;`, `var toString = 33;`, "number"); + testWithoutProxy("no proxy, get_by_id miss", testLoopCount, `return object.name1;`, `var name1 = 33;`, "33"); // The reader is on the stack, in optimized code, when the program runs. { @@ -97,19 +120,63 @@ for (const [form, suffix] of forms) { } } -// On the JSONP fast path only a new name gives the global object another Structure. A `var` of a name -// that exists creates no binding, and the caches that are valid stay. +// A function declaration never runs on the JSONP fast path. Its binding goes through the same function. +{ + const realm = $262.createRealm(); + const access = realm.evalScript(` + function access() { return typeof globalThis.name1; } + noInline(access); + for (let i = 0; i < ${testLoopCount}; ++i) + access(); + access; + `); + realm.evalScript(`function name1() { }`); + shouldBe(access(), "function", "function declaration"); +} + +// Only a new name on a global object that is not a dictionary gives the global object another Structure. +// A `var` of a name that exists creates no binding. A dictionary has no cache that the new name makes wrong. +// In both cases the caches that are valid stay. { const global = $vm.createGlobalObject(); // The list has five entries for each Structure. The first of the last five is the ID of the current one. const structureID = () => $vm.getStructureTransitionList(global).at(-5); - global.$vm.evaluateWithScopeExtension(`var name1 = 1;`); + const evaluate = code => global.$vm.evaluateWithScopeExtension(code); + function warmReader(name) { + const read = new Function("object", `return object.${name};`); + noInline(read); + for (let i = 0; i < testLoopCount; ++i) + read(global); + return read; + } + + const initial = structureID(); + evaluate(`var name1 = 1;`); + shouldBe(structureID(), initial, "new name, the dictionary of init(): same Structure"); + shouldBe(global.name1, 1, "new name, the dictionary of init(): value"); + $vm.flattenDictionaryObject(global); const flattened = structureID(); - global.$vm.evaluateWithScopeExtension(`var name1 = 2;`); + evaluate(`var name1 = 2;`); shouldBe(structureID(), flattened, "same name: same Structure"); shouldBe(global.name1, 2, "same name: value"); - global.$vm.evaluateWithScopeExtension(`var name2 = 3;`); + evaluate(`var name2 = 3;`); shouldBe(structureID() !== flattened, true, "new name: another Structure"); shouldBe(global.name2, 3, "new name: value"); + + const dictionary = structureID(); + evaluate(`var name3 = 4;`); + shouldBe(structureID(), dictionary, "new name, dictionary: same Structure"); + shouldBe(global.name3, 4, "new name, dictionary: value"); + evaluate(`var name4 = 5; void 0;`); + shouldBe(structureID(), dictionary, "new name as bytecode, dictionary: same Structure"); + shouldBe(global.name4, 5, "new name as bytecode, dictionary: value"); + + // The global object leaves the dictionary state again, and a site caches a miss for it again. + $vm.flattenDictionaryObject(global); + const readName5 = warmReader("name5"); + const flattenedAgain = structureID(); + evaluate(`var name5 = 6;`); + shouldBe(structureID() !== flattenedAgain, true, "new name, flattened again: another Structure"); + shouldBe(readName5(global), 6, "new name, flattened again: value at the warm site"); } From 67fe85979ccd33ccac058a02a634d0c5862dc865 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:04:39 +0000 Subject: [PATCH 3/3] [JSC] The test of the JSONP declaration passes in the bytecode-cache mode $vm.evaluateWithScopeExtension() has no bytecode cache, and the second run of the bytecode-cache mode asserts that each program comes from the cache. So the cases that use it give it only programs of the JSONP form, which have no bytecode. --- ...validates-global-object-property-caches.js | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/JSTests/stress/jsonp-var-declaration-invalidates-global-object-property-caches.js b/JSTests/stress/jsonp-var-declaration-invalidates-global-object-property-caches.js index 14f6f4d64bb26..c705354eb84aa 100644 --- a/JSTests/stress/jsonp-var-declaration-invalidates-global-object-property-caches.js +++ b/JSTests/stress/jsonp-var-declaration-invalidates-global-object-property-caches.js @@ -65,20 +65,6 @@ for (const [form, suffix] of forms) { shouldBe(read(), 2, form + ", cache created by an earlier statement"); } - // A global object with no JSGlobalProxy in front of it. The LLInt caches a load from the prototype chain - // of such an object, so the first case does not need the JIT. - function testWithoutProxy(name, count, body, declaration, expected) { - const global = $vm.createGlobalObject(); - const access = new Function("object", body + " // " + form); - noInline(access); - for (let i = 0; i < count; ++i) - access(global); - global.$vm.evaluateWithScopeExtension(declaration + suffix); - shouldBe(String(access(global)), expected, form + ", " + name); - } - testWithoutProxy("no proxy, prototype data property in the LLInt", 20, `return typeof object.toString;`, `var toString = 33;`, "number"); - testWithoutProxy("no proxy, get_by_id miss", testLoopCount, `return object.name1;`, `var name1 = 33;`, "33"); - // The reader is on the stack, in optimized code, when the program runs. { const realm = $262.createRealm(); @@ -120,6 +106,23 @@ for (const [form, suffix] of forms) { } } +// A global object with no JSGlobalProxy in front of it. The LLInt caches a load from the prototype chain of +// such an object, so the first case does not need the JIT. $vm.evaluateWithScopeExtension() has no bytecode +// cache: give it only programs of the JSONP form, which have no bytecode. +{ + function testWithoutProxy(name, count, body, declaration, expected) { + const global = $vm.createGlobalObject(); + const access = new Function("object", body); + noInline(access); + for (let i = 0; i < count; ++i) + access(global); + global.$vm.evaluateWithScopeExtension(declaration); + shouldBe(String(access(global)), expected, name); + } + testWithoutProxy("no proxy, prototype data property in the LLInt", 20, `return typeof object.toString;`, `var toString = 33;`, "number"); + testWithoutProxy("no proxy, get_by_id miss", testLoopCount, `return object.name1;`, `var name1 = 33;`, "33"); +} + // A function declaration never runs on the JSONP fast path. Its binding goes through the same function. { const realm = $262.createRealm(); @@ -168,15 +171,12 @@ for (const [form, suffix] of forms) { evaluate(`var name3 = 4;`); shouldBe(structureID(), dictionary, "new name, dictionary: same Structure"); shouldBe(global.name3, 4, "new name, dictionary: value"); - evaluate(`var name4 = 5; void 0;`); - shouldBe(structureID(), dictionary, "new name as bytecode, dictionary: same Structure"); - shouldBe(global.name4, 5, "new name as bytecode, dictionary: value"); // The global object leaves the dictionary state again, and a site caches a miss for it again. $vm.flattenDictionaryObject(global); - const readName5 = warmReader("name5"); + const readName4 = warmReader("name4"); const flattenedAgain = structureID(); - evaluate(`var name5 = 6;`); + evaluate(`var name4 = 5;`); shouldBe(structureID() !== flattenedAgain, true, "new name, flattened again: another Structure"); - shouldBe(readName5(global), 6, "new name, flattened again: value at the warm site"); + shouldBe(readName4(global), 5, "new name, flattened again: value at the warm site"); }