From 07e34b96964a3b0a66258d1d8e314f9c20d1120a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 00:39:28 +0000 Subject: [PATCH] Bump WebKit (oven-sh/WebKit#475 preview): a property lookup stops at a lazy property whose builder threw A read of an unreified static-table property whose PropertyCallback builder throws is reported as a miss by setUpStaticFunctionSlot. The prototype walk loops in JSObject::getPropertySlot and JSObject::getNonIndexPropertySlot went on to the prototype with the exception pending (and getPropertySlot used the structure from before the builder ran, which asserts when the builder transitioned the object), and the megamorphic get_by_id, get_by_val, in_by_id and in_by_val slow paths recorded the miss for the object's structure, so every later megamorphic access of that property returned undefined (false for `in`) without running the builder again. reifyAllStaticProperties ran builders back to back without checking between them, which made spreading Bun abort under the exception check validator of debug builds. The WebKit change checks for the exception after the own-property step on static-table objects and after each builder in reifyAllStaticProperties. The tests read Bun.sql with the sql builder made to throw. One reads it through Bun and one through a function that inherits from Bun, both behind a Proxy prototype and with the validator enabled. One makes the builder reify another property of Bun first. One reads from megamorphic get_by_id, get_by_val, in_by_id and in_by_val sites, where the second access has to throw again. The process.env pre-read in the test above them worked around the structure assertion on Windows and is removed, and the file leaves test/no-validate-exceptions.txt now that spreading Bun is clean under the validator. --- scripts/build/deps/webkit.ts | 7 +- test/js/bun/util/BunObject.test.ts | 145 +++++++++++++++++++++++++++-- test/no-validate-exceptions.txt | 1 - 3 files changed, 144 insertions(+), 9 deletions(-) diff --git a/scripts/build/deps/webkit.ts b/scripts/build/deps/webkit.ts index 0edda8cc6797..b2105736b7a1 100644 --- a/scripts/build/deps/webkit.ts +++ b/scripts/build/deps/webkit.ts @@ -3,7 +3,12 @@ * for local mode. Override via `--webkit-version=` to test a branch. * From https://github.com/oven-sh/WebKit releases. */ -export const WEBKIT_VERSION = "ceb9f90fb774fdb1ebf1275ae1aaf136ec66c754"; +// Preview of oven-sh/WebKit#475 (on top of ceb9f90f, the previous pin here): a property +// lookup stops at a static-table lazy property whose builder threw, instead of walking on +// to the prototype and, on the megamorphic slow paths, recording the property as missing, +// and reifyAllStaticProperties checks each builder's exception scope. Swap in the merged +// sha once that PR lands. +export const WEBKIT_VERSION = "autobuild-preview-pr-475-94c5a2d5"; /** * WebKit (JavaScriptCore) — the JS engine. diff --git a/test/js/bun/util/BunObject.test.ts b/test/js/bun/util/BunObject.test.ts index 246c2d9f628e..bb7aa3f23552 100644 --- a/test/js/bun/util/BunObject.test.ts +++ b/test/js/bun/util/BunObject.test.ts @@ -39,17 +39,14 @@ test("a lazy property whose builtin fails to load throws from the read", async ( // The shell builtin ($) and the sql module body (sql, SQL, postgres) call Symbol(), so // breaking it makes each builder throw. The read must throw that error (debug builds used to // report the still-pending exception from inside the sql builders and abort) and the slot - // must stay unreified so a later read runs the builder again. - // - // process.env is read first because the shell builtin reads it before calling Symbol(), and - // building it on Windows reifies another property of the Bun object; doing that in the middle - // of the throwing read trips a separate structure assertion in debug builds. + // must stay unreified so a later read runs the builder again. On Windows the shell builtin also + // builds process.env before it calls Symbol(), which reifies Bun.inspect, so there the throwing + // read of $ transitions the Bun object mid-lookup as well: the case the next test sets up by hand. await using proc = Bun.spawn({ cmd: [ bunExe(), "-e", - `process.env; - globalThis.Symbol = NaN; + `globalThis.Symbol = NaN; const results = {}; for (const name of ["$", "sql", "SQL", "postgres"]) { results[name] = []; @@ -75,3 +72,137 @@ test("a lazy property whose builtin fails to load throws from the read", async ( exitCode: 0, }); }); + +test.concurrent("a lazy property builder that reifies another property of Bun and then throws", async () => { + // The sql module body reads Error.prototype (class ... extends Error). The proxy makes that read + // reify Bun.semver, which transitions the Bun object while the Bun.sql lookup is still running, + // and then throw. Debug builds used to abort in the lookup's prototype step, which still held + // the structure from before the transition (ASSERT in Structure::storedPrototype); the lookup now + // ends as soon as the builder has thrown. "phase" 1 proves the trap ran: if the sql module stops + // reading Error.prototype while it is evaluated, this test needs a new trigger. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const RealError = Error; + let phase = 0; + globalThis.Error = new Proxy(function () {}, { + get(target, key, receiver) { + if (key === "prototype" && phase === 0) { + phase = 1; + Bun.semver; + throw "boom"; + } + return Reflect.get(target, key, receiver); + }, + }); + let thrown; + try { Bun.sql; thrown = "no throw"; } catch (e) { thrown = e; } + globalThis.Error = RealError; + console.log(JSON.stringify([thrown, phase, typeof Bun.sql]));`, + ], + env: { ...bunEnv, BUN_JSC_validateExceptionChecks: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: JSON.stringify(["boom", 1, "function"]), + stderr: "", + exitCode: 0, + }); +}); + +// A read of an unreified lazy property whose builder throws has to end the lookup at the Bun +// object. It used to go on to Bun's prototype with the exception still pending. The Proxy put +// behind Bun here has its own getOwnPropertySlot, and running it with a pending exception is what +// BUN_JSC_validateExceptionChecks=1 aborts on in debug builds. Both prototype walk loops are +// covered: a plain receiver uses JSObject::getPropertySlot, and a receiver that overrides +// getOwnPropertySlot (a function) sends the rest of the walk through +// JSObject::getNonIndexPropertySlot. The second read, which lets the builder succeed, used to +// abort the same way through the function: that loop did not check after running a builder at all. +test.concurrent.each([ + ["Bun itself", "Bun"], + ["a function that inherits from Bun", "Object.setPrototypeOf(function () {}, Bun)"], +])("a lazy property builder that throws ends the lookup when the receiver is %s", async (_, receiver) => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `Object.setPrototypeOf(Bun, new Proxy(Object.prototype, {})); + const receiver = ${receiver}; + const RealSymbol = Symbol; + globalThis.Symbol = NaN; + let thrown; + try { receiver.sql; thrown = "no throw"; } catch (e) { thrown = e.constructor.name; } + globalThis.Symbol = RealSymbol; + console.log(JSON.stringify([thrown, typeof receiver.sql]));`, + ], + env: { ...bunEnv, BUN_JSC_validateExceptionChecks: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: JSON.stringify(["TypeError", "function"]), + stderr: "", + exitCode: 0, + }); +}); + +test.concurrent("a lazy property builder that throws is not recorded as a missing property", async () => { + // Each access site is trained on many object shapes first, so that it is megamorphic by the + // time it sees Bun and the access takes the megamorphic slow path. That path used to walk past + // the builder that threw and record "not present" for Bun's structure. A builder that throws + // stores nothing, so Bun kept that structure and every later access from a megamorphic site got + // undefined (false for `in`) instead of running the builder again. So the second access of each + // site has to throw like the first one. Symbol stays broken throughout because the three sql + // properties share one module, which would stop throwing once any of them loaded it. Each site + // has its own property because the record is per property name. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const byValKey = "postgres"; + const inByValKey = "$"; + const sites = { + get_by_id: o => typeof o.sql, + get_by_val: o => typeof o[byValKey], + in_by_id: o => "SQL" in o, + in_by_val: o => inByValKey in o, + }; + const shapes = []; + for (let i = 0; i < 32; i++) { + const o = { sql: 0, postgres: 0, SQL: 0, $: 0 }; + o["shape" + i] = i; + shapes.push(o); + } + for (let i = 0; i < 200; i++) { + for (const o of shapes) for (const name in sites) sites[name](o); + } + const access = site => { try { return site(Bun); } catch (e) { return e.constructor.name; } }; + const RealSymbol = Symbol; + globalThis.Symbol = NaN; + const results = {}; + for (const name in sites) results[name] = [access(sites[name]), access(sites[name])]; + globalThis.Symbol = RealSymbol; + results.afterwards = typeof Bun.sql; + console.log(JSON.stringify(results));`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ + get_by_id: ["TypeError", "TypeError"], + get_by_val: ["TypeError", "TypeError"], + in_by_id: ["TypeError", "TypeError"], + in_by_val: ["TypeError", "TypeError"], + afterwards: "function", + }), + stderr: "", + exitCode: 0, + }); +}); diff --git a/test/no-validate-exceptions.txt b/test/no-validate-exceptions.txt index b9970e3a3a00..31077a83d1a4 100644 --- a/test/no-validate-exceptions.txt +++ b/test/no-validate-exceptions.txt @@ -4,7 +4,6 @@ test/bake/dev/production.test.ts test/integration/vite-build/vite-build.test.ts test/js/bun/test/parallel/test-integration-rspack.ts -test/js/bun/util/BunObject.test.ts test/js/bun/util/fuzzy-wuzzy.test.ts test/js/node/module/node-module-module.test.js test/js/node/test/parallel/test-vm-module-referrer-realm.mjs