AOT: miscompilations and crashes found since #967, builds that never ended, faster loops by Jarred-Sumner · Pull Request #971 · oven-sh/WebKit

Jarred-Sumner · 2026-11-01T02:04:50Z

Fixes for bun build --compile --aot and bun test --aot found since #967. The wrong answers and the crashes come first. 320 files, +8,875 −918; all but 21 of them are under Source/JavaScriptCore/aot.

Wrong answers of compiled programs

A list of numbers with one undefined in it, iterated by two loops. The first adds
the numbers up, the second counts the undefineds: it counted 0 instead of 1.
In a generator, a value read before a yield* and used after it was wiped when a
parameter that held an object was set to null or undefined in the same loop: 12
instead of 22. A second program never ended.
A RegExp made from text out of a list was compiled from the text the build had
predicted, even when the list was read under two names in one step and then changed.
main.mjs imports m.mjs, which imports x.cjs, which calls require() on m.mjs
under a name made at run time: main.mjs ran after x.cjs had ended, not before the
require() returned.
An import() made while other import() calls were waiting, and whose graph shares a
module with one of them, was held back behind all of them.
bun = require("bun"); copy = {}; copy[key] = bun[key]; copy.plugin(options): a member
copied off require("bun") under a computed key is followed to its call. The build now
sees that copy.plugin may be Bun.plugin.

Crashes of compiled programs

A segfault in eleven lines: a loop reads and stores a static field that holds an
object, after the field was deleted while a Proxy was on the class's prototype chain
and was defined again through the trap.
A segfault at address 0 after an out-of-memory error that the program catches: a rope
of 2^30 two-byte characters that cannot be flattened, ahead of a short loop in a try
with catch and finally.
Two segfaults in a few lines each. TypeError.prototype.list = [1, 2], then
catch (e) { put(e.constructor.prototype, { length: 3 }) } with
put(where, what) { where.list = what }, then TypeError.prototype.list.length read in
a function: the read took the list for what the build had seen stored by name. The same
with Bun.S3Client.prototype.file.list = [1, 2]; Bun.s3.file.list = { length: 3 }.
Where no list was involved, the read answered undefined. Both stores are now seen by
a read that names the object.

Builds that failed or never ended

Thirteen lines did not build in two minutes: two nested loops of eight iterations, a
swap in a try, an xor in its finally. B3's Value::returnsBool() recursed into both
operands without remembering what it had asked, so its cost was exponential.
One function spent 214 of 246 seconds in type propagation: its queue was last in,
first out.
The compiler aborted on for (;;) { if (source.take === never) return ..; const got = source.take(..); .. }. A loop that reads the same property of an unchanging object on
every iteration gets a version that asks ahead of the loop, and a type test on that read
was turned into a cast of the copy. Where both sides of the test's branch leave the
loop, the branch still hands on what the test proved, so its condition has to stay a
test.
Fourteen lines stopped the build with InvalidGuard: a read of a typed array at the
top of an iteration relied on the bounds check at the end of the previous one, and the
loop's counter was then replaced by a constant.
Quadratic time in large functions: three places in Air's register and stack
allocators, two in the ahead-of-time compiler's own passes (one is flow analysis), and the
lookup of a regular expression's symbols (arm64: twice as many alternatives cost 9.5
times as much, now 1.74 times).

`bun test --aot`, following oven-sh/bun#44833

An import that reaches a mock whose factory threw gets the error that the loader
kept. require() of a mocked module returns what the factory returned. A later mock
of a CommonJS file doesn't change what an earlier require() returned.
import { test, vi } from "vitest"; test("t", () => { vi.useFakeTimers(); }) ended in
abort(). The code that throws "was not compiled into this executable" looked the
function up in the table of built-ins, and a function that the table leaves out has no
record there. The error is thrown now; making the call work follows.
The engine's own tests no longer depend on how many built-ins Bun has.

Speed

New B3 pass, promoteLoopMemory, in the tree but switched on by nobody yet. a[k] += v[j]
and o.total += x reload on every iteration what the previous one stored; the pass reads
the word once ahead of the loop and carries it in a Phi, and the stores stay where they
are. The first commit here had the ahead-of-time compiler's policy turn it on, and the
last one takes that back: with holder.n = (holder.n + 1) | 0; if (j === 1) other.n = 50; if (holder instanceof Other) holder.bump(); in a loop and holder and other one
object, the store of 50 was lost. It never reached main. The pass comes back with its
fix in the next engine PR.
map.has(k) followed by map.get(k) searches once. Keys that are objects are searched
inline in loops.
A generator function made for each call: 2,585 to 175 instructions. tslib's
__awaiter: 3,876 to 1,441 per iteration.
Helpers that Babel and TypeScript emit: Reflect.construct behind tests, a built-in
bound and called in one function, descriptors read and defined again, for..in copies.
Remainders and quotients of whole numbers stay integers. A loop polls with one
faulting load on arm64, macOS x64 and Windows arm64 too.

Known costs, fixed in the next engine PR

The fix for the compiler abort keeps every type test in a loop's fast version:
everyday-emitters/five-arguments and three-arguments (emit() in a loop) cost five
times their instructions, 4.8 to 25 per iteration. Only the condition of a branch that
stays in the loop has to stay a test. The narrower form is written and tested.
A generator function made for each call gets its prototype object only when somebody
asks. The first gen instanceof X, Object.getPrototypeOf(gen), console.log(gen) or
read of the function's own .prototype converts that function's live generators by one
walk over the whole heap, once per generator function: about 6 ms with 27 MB of live
heap and 30 to 45 ms with 105 MB (heapUsed, on a busy machine). Compiled for..of,
spread, destructuring, .next() and JSON.stringify(gen) don't reach it. The fix gives
generators a subspace of their own, so the walk follows their number.
In a debug build (NDEBUG not defined, so GC validation is on), bun test --aot asserts
where a write to a locked built-in is refused with a host function on the stack, e.g.
Reflect.set(Buffer, "isEncoding", f). The check that decides whose file wrote it takes
a stack trace without an owner cell, and the write barrier of a frame asserts that it has
one. Release and ASAN builds raise the read-only error as intended. The same check walks
the stack up to three times per refused write. The fix walks once and builds no frames.

Not AOT-specific

Twenty-one files outside Source/JavaScriptCore/aot. The new pass is off: no policy turns it on yet. The changes in B3Value.cpp and Air's allocators are not behind a switch: the JIT runs them too. The one in B3LowerToAir.cpp is behind selection.extendsInstructionSelection, which is off by default.

A new B3 pass, off by default, and nothing turns it on yet.

b3/B3PromoteLoopMemory.{h,cpp}, Sources.txt: the pass. It keeps a word that a loop reads and writes at one address in a value across iterations.
b3/B3Generate.cpp: the pass is one more entry in the pipeline's table. It runs only where the procedure has loops and its policy says loops.promotesMemory.
b3/B3Policy.{h,cpp}: that switch, false by default, and a second one that only a test of the pass sets. 43 switches instead of 42.
b3/B3CompilationReport.h: four more counters, 19 instead of 15.
b3/testb3_bun_promote_loop_memory.cpp (new), b3/testb3_bun.{h,cpp}, b3/testb3_bun_policy.cpp, b3/testb3_bun_random_program.h, shell/CMakeLists.txt: its tests. testPipelinePromotesLoopMemoryOnlyWhereAsked holds that the default pipeline does not run it.

Compile time. The output is the same: the machine code of 1,155,808 functions out of 342 programs, compared before and after the register allocator's changes, is identical.

b3/B3Value.cpp, b3/testb3_bun_analyses.cpp: Value::returnsBool() asked both operands of BitAnd, BitOr, BitXor and Select again and again. It now remembers what it has asked. Same answers; the test builds the chain that took exponential time.
b3/air/AirAllocateRegistersByGreedy.cpp, b3/air/AirLimits.h: in a function with hundreds of calls, choosing a register to split a value around calls no longer walks every call in the value's live range once per register. From 256 clobber points on, for a value that no clobbering instruction names, a register is skipped when prefix sums of the least each call can cost already exceed the best split found, and a walk stops after 16 sites if the register is held by something that can't be evicted. The calls that clobber a whole set of registers are listed once per set and found by binary search.
b3/air/AirAllocateStackByGraphColoring.cpp, WTF/wtf/InterferenceGraph.h: two quadratic loops in the stack allocators of functions with thousands of slots. Where ranges of a slot are held against a cell, each side now skips what lies ahead of the other's next range. At a move between two slots the live set is added in one step, by a new overload addAllBut(u, alsoNot, others). Every slot gets the cell it got before.

One instruction on arm64, behind a switch that only the ahead-of-time compiler's policy sets.

b3/B3LowerToAir.cpp: an Upsilon that hands the constant 0 to a Phi that is kept zero-extended is Move32 %xzr, %tmp instead of Move %xzr, %tmp. It is the same machine instruction, but Air now sees a 32-bit definition, so the Move32 that widened a loop's counter goes away: for (let i = 0; i < count; i++) sum += i * 2 + 1 takes 5 instructions per iteration instead of 6. The set of such Phis is filled only where the procedure's policy says selection.extendsInstructionSelection (false in B3Policy.h), so the code the JIT tiers emit does not change.

Under ENABLE(AOT).

runtime/Structure.cpp: four lines in validateFlags(), which exists only in builds with assertions. A Structure whose class sets OverridesGetPrototype itself is no longer held to what its method table says, so the assertion does not fire for the generator functions that get their prototype object only when somebody asks for it.

Testing

The engine's own suites on this text: 5,771 IR tests, 0 failing, and the native tests. testb3 whole (219,069 checks) on the text in which the new pass landed.
Eleven servers (fastify, hono, elysia and others) built with --aot and compared with plain Bun: 11 of 11.
The fixes come with tests: .ir files under aot/tests here, and programs under test/aot in the Bun pull request that follows from the same snapshot.
62 programs that go with the wrong answers, the crashes and the failed builds above, each compiled and compared with plain Bun, on two release builds: with this engine 62 pass; with the engine as it is on main 52 fail.
The three small commits on top of the first: a release build of the head links, and it compiles for Linux, macOS and Windows. The nine programs that go with the two segfaults: 9 pass; on the engine one commit earlier 9 fail, two of them with SIGSEGV. The vitest program: exit 1 with the error; one commit earlier, exit 134.
The last commit, which switches the new pass off, changes the code that is generated for loops, and the runs above were made before it. On a release build of the head: six programs with the loop shown under Speed (the call is a function, a method or a setter; the other class is unrelated or a subclass; with types and without), each built alone and compared with plain Bun: 6 pass; one commit earlier 6 fail. All 8,702 programs stored under test/aot/parallel in the Bun pull request, each compiled and compared with plain Bun: 8,688 pass, 12 skip themselves, and 2 import bun:internal-for-testing, which this build refuses. The IR tests under aot/tests on the head: 5,772 pass, 0 fail. The head links, and it compiles for Linux, macOS and Windows.
A release link with Bun's text of the same snapshot, whose last step checks that the binary has no static initializers beyond the expected ones. It compiles for Linux, macOS and Windows. A build with AddressSanitizer and assertions compiles too.
The 89 unified sources under aot/, compiled with -fsyntax-only -mms-bitfields: every static_assert on the size of a struct holds under the Microsoft bit-field layout too. 0 errors.

…ended, faster loops Wrong answers of compiled programs: - A list of numbers with one `undefined` in it, iterated by two loops. The first adds the numbers up, the second counts the `undefined`s: it counted 0 instead of 1. - In a generator, a value read before a `yield*` and used after it was wiped when a parameter that held an object was set to `null` or `undefined` in the same loop: 12 instead of 22. A second program never ended. - A `RegExp` made from text out of a list was compiled from the text the build had predicted, even when the list was read under two names in one step and then changed. - `main.mjs` imports `m.mjs`, which imports `x.cjs`, which calls `require()` on `m.mjs` under a name made at run time: `main.mjs` ran after `x.cjs` had ended, not before the `require()` returned. - An `import()` made while other `import()` calls were waiting, and whose graph shares a module with one of them, was held back behind all of them. Crashes of compiled programs: - A segfault in eleven lines: a loop reads and stores a static field that holds an object, after the field was deleted while a Proxy was on the class's prototype chain and was defined again through the trap. - A segfault at address 0 after an out-of-memory error that the program catches: a rope of 2^30 two-byte characters that cannot be flattened, ahead of a short loop in a `try` with `catch` and `finally`. Builds that failed or never ended: - Thirteen lines did not build in two minutes: two nested loops of eight iterations, a swap in a `try`, an xor in its `finally`. B3's `Value::returnsBool()` recursed into both operands without remembering what it had asked, so its cost was exponential. - One function spent 214 of 246 seconds in type propagation: its queue was last in, first out. - The compiler aborted on `for (;;) { if (source.take === never) return ..; const got = source.take(..); .. }`. A loop that reads the same property of an unchanging object on every iteration gets a version that asks ahead of the loop, and a type test on that read was turned into a cast of the copy. Where both sides of the test's branch leave the loop, the branch still hands on what the test proved, so its condition has to stay a test. - Fourteen lines stopped the build with `InvalidGuard`: a read of a typed array at the top of an iteration relied on the bounds check at the end of the previous one, and the loop's counter was then replaced by a constant. - Quadratic time in large functions: three places in Air's register and stack allocators, two in the ahead-of-time compiler's own passes (one is flow analysis), and the lookup of a regular expression's symbols (arm64: twice as many alternatives cost 9.5 times as much, now 1.74 times). `bun test --aot`, following oven-sh/bun#44833: - An `import` that reaches a mock whose factory threw gets the error that the loader kept. `require()` of a mocked module returns what the factory returned. A later mock of a CommonJS file doesn't change what an earlier `require()` returned. - The engine's own tests no longer depend on how many built-ins Bun has. Speed: - New B3 pass, promoteLoopMemory: `a[k] += v[j]` and `o.total += x` reloaded on every iteration what the previous one had stored. The word is read once ahead of the loop and carried in a Phi. The stores stay where they are. Off by default; only the ahead-of-time compiler's policy turns it on. - `map.has(k)` followed by `map.get(k)` searches once. Keys that are objects are searched inline in loops. - A generator function made for each call: 2,585 to 175 instructions. tslib's `__awaiter`: 3,876 to 1,441 per iteration. - Helpers that Babel and TypeScript emit: `Reflect.construct` behind tests, a built-in bound and called in one function, descriptors read and defined again, `for..in` copies. - Remainders and quotients of whole numbers stay integers. A loop polls with one faulting load on arm64, macOS x64 and Windows arm64 too.

…orted on `import { test, vi } from "vitest"; test("t", () => { vi.useFakeTimers(); })` under `bun test --aot` ended in `abort()`. The code that throws "was not compiled into this executable" looked the function up in the table of built-ins to ask whether it is an accessor's getter, and a function that the table leaves out has no record there. It asks `contains()` first now, so the error is thrown.

…lowed to its call `bun = require("bun"); copy = {}; copy[key] = bun[key]; copy.plugin(options)`: the build now sees that `copy.plugin` may be `Bun.plugin`. The test, `plugin-copy-late-mark.ir`, comes with Bun's half.

…totype`, and to a method found on `Bun.s3` `TypeError.prototype.list = [1, 2]`, then `catch (e) { put(e.constructor.prototype, { length: 3 }) }` with `put(where, what) { where.list = what }`, then `TypeError.prototype.list.length` read in a function: the read took the list for what the build had seen stored by name, and the program ended in a segfault. The same with `Bun.S3Client.prototype.file.list = [1, 2]; Bun.s3.file.list = { length: 3 }`. Where no list was involved, the read answered `undefined`. Both stores are now seen by a read that names the object.

claude

Findings marked 🟡 are optional suggestions and need no follow-up push.

claude · 2026-11-01T02:46:57Z

+bool isWrittenByFileOfProduct(VM& vm)
+{
+    // Compiled code calls a built-in function itself, or through an operation that enters the VM. A refusal is rare, so the walk that lists every kind of frame will do.
+    constexpr size_t framesLookedAt = 32;
+    Vector<StackFrame> frames;
+    vm.interpreter.getStackTrace(nullptr, frames, 0, framesLookedAt);


🔴 Debug builds of a test-run program now crash on a RELEASE_ASSERT, instead of raising the read-only error, when a locked built-in is written below a host frame. isWrittenByFileOfProduct at AOTErrorMessages.cpp:3417 calls getStackTrace with a null owner. Every StackFrame it appends with a callee runs vm.writeBarrier(nullptr, cell), which fails ASSERT_GC_OBJECT_LOOKS_VALID under ENABLE(GC_VALIDATION). Fix: capture the frames without an owner-barriered StackFrame (walk with CompiledFrameIterator or StackVisitor and read only the source of each compiled frame), or pass a real owner cell; the same walk also now runs for every refused locked write, including silent sloppy-mode ones, so it should stay off that hot path.

Why this was flagged

A program built with replaced builtins admitted (admitsReplacedBuiltins) writes to a locked built-in, e.g. Reflect.set(Array.prototype, 'x', 1) in a test, or a sloppy assignment inside a callback below a native function. JSObject.cpp:102-104 computes lockedPropertyPolicy, which calls lockedObjectErrorMessage and reflectThrowsOnLockedProperty; both now call isWrittenByFileOfProduct (AOTBuiltinCells.cpp:799, AOTBuiltinCells.cpp:825). That function calls vm.interpreter.getStackTrace(nullptr, frames, 0, 32) at AOTErrorMessages.cpp:3417. With compiled code present Interpreter.cpp:856 goes to AOT::tryGetStackTrace, which at AOTStackTrace.cpp:1553 does results.constructAndAppend(vm, owner, callee) for a host frame on top and at AOTStackTrace.cpp:1484 for a compiled frame with a recorded callee, and appendFramesBelow lists native frames the same way (StackFrame.cpp:42-44). StackFrame constructs WriteBarrier(vm, owner, callee), whose setEarlyValue (WriteBarrierInlines.h:52-56) calls vm.writeBarrier(owner, value) with owner == nullptr.

Verification: normal (debug builds only; release builds are unaffected) — triggered whenever a program compiled with AdmittedCode::ReplacedBuiltins writes/deletes a locked built-in while any frame with a callee is on the walked stack (e.g. below a host function such as Reflect.set, or a native that invoked the callback, or any host/bytecode frame below the compiled run).

claude · 2026-11-01T02:46:57Z

+void CoroutineTable::willMakePrototypeOfGeneratorFunction(VM& vm, JSObject* function)
+{
+    if (!m_structureOfGeneratorsWithoutPrototype)
+        return;
+    uint32_t place = placeOfGeneratorFunctionState(uncheckedDowncast<Closure>(function)->closureNumber());
+    uint8_t& state = m_instance.coroutines.statesOfGeneratorFunctions[place];
+    if (state != static_cast<uint8_t>(GeneratorFunctionState::MakesGeneratorsWithoutPrototype))
+        return;
+    state = static_cast<uint8_t>(GeneratorFunctionState::MakesGeneratorsWithPrototype);
+    DeferGC deferGC(vm);
+    Vector<Generator*> found;
+    {
+        HeapIterationScope iterationScope(vm.heap);
+        vm.heap.objectSpace().forEachLiveCell(iterationScope, [&](HeapCell* cell, HeapCell::Kind kind) {
+            if (!isJSCellKind(kind) || static_cast<JSCell*>(cell)->classInfo() != Generator::info())
+                return IterationStatus::Continue;
+            auto* generator = uncheckedDowncast<Generator>(static_cast<JSCell*>(cell));
+            // One without a frame was left behind where the stack ran out in front of the body that makes the frame. Nobody has it.
+            if (!generator->takesPrototypeFromFunction() || !generator->savedFrame() || generator->structure()->realm() != m_instance.globalObject)
+                return IterationStatus::Continue;
+            // Each image of the realm has a table of its own.
+            if (placeOfGeneratorFunctionState(generator->closureNumberOfFunction()) == place && generator->instanceOfFunction() == &m_instance)
+                found.append(generator);
+            return IterationStatus::Continue;
+        });
+    }
+    for (Generator* generator : found)
+        generator->takePrototypeFromFunction(vm);


🟡 (optional) Programs with a large heap stall for a whole-heap walk the first time a generator reaches a generic JSC lookup, which the base never does. Generators made by the SyncWithHeapFrame path carry a Structure with OverridesGetPrototype, so JSObject::getPropertySlot (JSObjectInlines.h:267) calls Generator::getPrototype for any miss on the generator itself; that runs ensurePrototypeProperty, and willMakePrototypeOfGeneratorFunction at AOTCoroutineTable.cpp:111-124 iterates every live cell with forEachLiveCell under HeapIterationScope, which also stops all allocators. Fix: convert the generators of a place without walking the heap, so Object.getPrototypeOf, instanceof, Array.from(gen) and console.log(gen) cost no more than on the base.

Why this was flagged

A sync generator function whose closure was never asked for prototype makes its generators through the SyncWithHeapFrame path (AOTJobs.cpp:887-889); each has the Structure made at AOTCoroutineTable.cpp:488-491 with OverridesGetPrototype. Uncompiled code then does any lookup that misses on the generator object: Array.from(gen()), Object.getPrototypeOf(gen), gen instanceof X, or JSON.stringify(gen). JSObject::getPropertySlot at JSObjectInlines.h:267 sees overridesGetPrototype and calls Generator::getPrototype (AOTCoroutineTable.cpp:502-510), which calls ensurePrototypeProperty (AOTClosure.cpp:392-398) -> willMakePrototypeOfGeneratorFunction. With the place in state MakesGeneratorsWithoutPrototype that function takes a HeapIterationScope and runs vm.heap.objectSpace().forEachLiveCell over every block and precise allocation (MarkedSpaceInlines.h:46-60). On a heap of hundreds of MB that is tens to hundreds of milliseconds on the JS thread, once per place. On the base branch the generator was created with its function's prototype at NewGenerator (AOTJobs.cpp:891) and the same lookups are ordinary structure-chain reads with no walk. No safeguard limits the walk: the state check at AOTCoroutineTable.cpp:107 only skips places that never made such a generator.

Verification: Source/JavaScriptCore/aot/runtime/AOTCoroutineTable.cpp:99-100 ("one walk for a place in the table at most. JSGlobalObject::haveABadTime() is the model"); the stated bound (at most one walk per 4096-way place) is accurate, but each walk is a full object-space iteration whose cost scales with the entire live heap, and the trigger is ordinary code rather than the rare pattern haveABadTime guards.

claude · 2026-11-01T02:46:57Z

+        auto callers = std::ranges::equal_range(m_directCalls, activation.function, { }, &DirectCall::callee);
+        m_callersAreTooManyForOneWalk |= !m_testsBeingAnswered && m_valuesFollowed < m_budget && callers.size() > m_budget - m_valuesFollowed;


🟡 (optional) Builds of programs with a helper that has thousands of direct callers and a RegExp call inside it can take many times longer, up to callers times the walk budget. m_callersAreTooManyForOneWalk at AOTRegExpRequests.cpp:2664 is set exactly when callers exceed the remaining budget, and line 241 then sends the site to exploreForEachCaller, which resets m_valuesFollowed per caller (line 393) so each of N callers spends a fresh 16384 budget. m_leftWhereBudgetRanOut (line 744) only short-cuts a value whose remaining budget is at or below a previous run-out, which a fresh per-caller budget never is. Fix: keep a total budget across the per-caller walks, or cap the number of callers walked separately, so the cost stays bounded by the single-walk budget times a small constant.

Why this was flagged

Condition: a function with more direct callers than the budget left (the test at AOTRegExpRequests.cpp:2664 is callers.size() > m_budget - m_valuesFollowed) contains a RegExp request whose text or flags come from a parameter. Before this change m_didNotMultiplyAlternatives alone gated the per-caller fallback, so a walk over all callers simply ran out and the site was given up at the single budget. Now line 241 and line 409 send it to exploreForEachCaller, which per caller restores the budget (line 393, pre-existing) and walks again, so the total is callers x 16384 values. The check on m_leftWhereBudgetRanOut at lines 744-749 fires only when leftAtFirst <= the remembered remaining budget; each per-caller walk starts with a larger remaining budget than the shared walk that ran out, so the shortcut does not apply and each caller walks in full. Consequence: build-time blow-up of the AOT compiler for that program, with no cap. Remedy: a total budget across callers or a bound on how many callers get a walk of their own.

Verification: Line 2664 sets m_callersAreTooManyForOneWalk precisely when the all-callers loop will run out. Lines 241 and 409 now gate the per-caller fallback on it; on the base only m_didNotMultiplyAlternatives did, so the base spent one budget and gave the site up. exploreForEachCaller lines 393-398 give every one of the N top-level callers a fresh 16384 budget.

claude · 2026-11-01T02:46:57Z

+    if (!found || !(found->variablesBeforeMock || found->valueBeforeMock || (withExports && withExports->exportsBeforeMock)))
+        return;


🟣 pre-existing, not blocking: Test code that mocks a program file before anything loads it and then restores the mock keeps getting the mock from require() and import for the rest of the process. endMockLaidOverFile at AOTModuleLinking.cpp:925-926 returns before didEndMock(file) and before clearing exportsOfMock when nothing was saved under the mock. answerRequire at AOTModuleEvaluation.cpp:3436 still sees isMocked(target) and answers answerOfRequireToMock. Fix: on every end of a mock, clear the m_mocks bit and exportsOfMock first, and only then return when there is nothing to restore; the same ordering applies to endMockOfLibraryModule's early returns.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

A test calls mock.module("./file", factory) before any import or require of ./file, later ends the mock, then requires ./file. The host calls didEndMock (AOTModules.cpp:5232), which routes a program file to endMockLaidOverFile (AOTModules.cpp:5242). At AOTModuleLinking.cpp:925 found->variablesBeforeMock, found->valueBeforeMock and exportsBeforeMock are all null because keepWhatMockIsLaidOver (AOTModuleLinking.cpp:905) only saves what a file that already ran held, so the function returns at line 926 without reaching didEndMock(file) at line 928 or clearing loaded.exportsOfMock at line 929. Modules::didEndMock (AOTModuleLinking.cpp:397) is the only thing that drops the m_mocks bit, so isMocked(file) stays true. The next require() enters answerRequire, where AOTModuleEvaluation.cpp:3436 tests isMocked(target) and answers the stale mock's exports via answerOfRequireToMock instead of running the file. The base had no didEndMock hook at all, so this is the new restore path failing for the population of mocks laid before first load.

Verification: The base has no didEndMock host entry at all (AOTModuleHost.h:247 is added by this diff), so on the base a module mock never ends on any path. For an ahead-of-load mock, variablesBeforeMock, exportsBeforeMock and valueBeforeMock stay null, so endMockLaidOverFile returns at AOTModuleLinking.cpp:925-926 before didEndMock(file) at :928, isMocked stays true, and answerRequire (AOTModuleEvaluation.cpp:3436) keeps answering the mock.

claude

Findings marked 🟡 are optional suggestions and need no follow-up push.

Still open from earlier reviews (4):

🔴 Source/JavaScriptCore/aot/runtime/AOTErrorMessages.cpp:3417 — Debug builds of a test-run program now crash on a RELEASE_ASSERT, instead of raising the read-only error, when a locked…
Also unresolved: 3 minor or pre-existing.

If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.

claude · 2026-11-01T03:01:56Z

+    m_out.appendTo(hasCounters);
+    TypedPointer counter = m_out.address(heap, checkedCells, CheckedFrameCells::offsetOfBirthsAt(m_id.index(), context.valueID().index()));
+    LValue births = m_out.load32(counter);
+    LValue isLeftOut = m_out.bitAnd(m_out.aboveOrEqual(births, CheckedFrameCells::numberOfBirthsThatAreAllChecked), m_out.notZero32(m_out.bitAnd(births, CheckedFrameCells::intervalOfCheckedBirths - 1)));
+    m_out.branch(isLeftOut, normalEdge(staysInFrame), rareEdge(mayBeChecked));
+
+    m_out.appendTo(staysInFrame);
+    m_out.store32(m_out.add(births, 1), counter);
+    ValueFromBlock inFrame = m_out.anchor(birth);


🟡 nit (optional): Validated runs no longer retire the previous checked cell of a frame slot at a birth that is left out, so a stale reference to it inside the same frame stays readable where the base made it fault. The inline path at AOTCellAllocationEmitter.cpp:316-318 hands back the frame slot after the 1024th birth without the work CheckedFrameCells::make does first. Fix: before taking the staysInFrame edge, retire the slot's live granule (what AOTCheckedFrameCells.cpp:227-236 does) and the granules of frames already left, or route left-out births through a cheaper operation that does both; the counter bump alone is not enough.

Why this was flagged

A build that validates references (generator().validatesOrChecksProofs() at AOTCellAllocationEmitter.cpp:286) runs a loop that allocates an object in the same frame slot each turn. On the base every birth went through CheckedFrameCells::make, whose first steps (AOTCheckedFrameCells.cpp:223-236) call didLeaveFramesBelow and read the granule index back from the slot to end the earlier granule, so the earlier guarded cell became inaccessible at the very next birth and a reference kept from the previous turn faulted on use. With this change a birth whose counter satisfies isLeftOut (AOTCellAllocationEmitter.cpp:313) takes the staysInFrame edge, bumps the counter and returns the slot itself; the program then overwrites the slot word with the new object's header, so the next make() at AOTCheckedFrameCells.cpp:227 no longer finds the index and the earlier granule stays Live until didLeaveFrameOf runs at frame exit. A reference that outlived its birth is therefore not detected within the frame. Only validation builds are affected.

Verification: nit — triggers in an image built with --aot-validate / CheckProofsAtRunTime when a frame-sunk allocation runs more than 1024 times in one frame. The inline path at AOTCellAllocationEmitter.cpp:313-318 hands back the frame slot without calling MakeCheckedFrameCell, so the previous granule, which CheckedFrameCells::make (AOTCheckedFrameCells.cpp:221-236) retired on the base, stays live and readable.

The pass and its tests stay in B3. The ahead-of-time compiler's policy no longer turns it on. What broke, in a loop: holder.n = (holder.n + 1) | 0; if (j === when) other.n = 50; if (holder instanceof Other) bump(holder); with `function bump(o: Other) { o.n = (o.n + o.k) | 0 }`. Where `holder` and `other` are one object, a `Plain`, the store of 50 was lost: the function answered 7:29 instead of 55:317. The copy of `bump` in the loop never runs for that object, but its access named a narrower heap for the word, and the pass let that one heap decide that nobody else writes it.

claude

Code review found no new issues

No new issues were found in this update; 5 findings from earlier reviews are still open above.

Still open from earlier reviews (5):

🔴 Source/JavaScriptCore/aot/runtime/AOTErrorMessages.cpp:3417 — Debug builds of a test-run program now crash on a RELEASE_ASSERT, instead of raising the read-only error, when a locked…
Also unresolved: 4 minor or pre-existing.

If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.

github-actions · 2026-11-01T04:02:09Z

Preview build of cc1ea90: autobuild-preview-pr-971-cc1ea906

#971 went in as a squash. main's tree is the tree of #971's last commit, which this branch is built on, so this merge changes no file.

Jarred-Sumner added 4 commits October 31, 2026 18:55

claude Bot reviewed Nov 1, 2026

View reviewed changes

This was referenced Nov 1, 2026

AOT: hangs, wrong answers, failed builds; bun test --aot follows #44833's mocks oven-sh/bun#47605

Open

AOT: crashes and miscompilations found since #971, Map/Set index slots, a bounds test on split() #972

Open

Jarred-Sumner merged commit 8a7c63a into main Nov 1, 2026
88 checks passed

Jarred-Sumner added a commit that referenced this pull request Nov 1, 2026

Merge branch 'main' into aot-follow-up-1031-part-2

994c496

#971 went in as a squash. main's tree is the tree of #971's last commit, which this branch is built on, so this merge changes no file.

Navigation Menu

Repository navigation

AOT: miscompilations and crashes found since #967, builds that never ended, faster loops - #971

AOT: miscompilations and crashes found since #967, builds that never ended, faster loops#971
Jarred-Sumner merged 5 commits into
mainfrom
aot-follow-up-1031

Jarred-Sumner commented Nov 1, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

claude Bot Nov 1, 2026

Uh oh!

claude Bot Nov 1, 2026

Uh oh!

claude Bot Nov 1, 2026

Uh oh!

claude Bot Nov 1, 2026

Uh oh!

claude Bot left a comment

Uh oh!

claude Bot Nov 1, 2026

Uh oh!

claude Bot left a comment

Uh oh!

github-actions Bot commented Nov 1, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		auto callers = std::ranges::equal_range(m_directCalls, activation.function, { }, &DirectCall::callee);
		m_callersAreTooManyForOneWalk \|= !m_testsBeingAnswered && m_valuesFollowed < m_budget && callers.size() > m_budget - m_valuesFollowed;

		if (!found \|\| !(found->variablesBeforeMock \|\| found->valueBeforeMock \|\| (withExports && withExports->exportsBeforeMock)))
		return;

Conversation

Jarred-Sumner commented Nov 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Wrong answers of compiled programs

Crashes of compiled programs

Builds that failed or never ended

bun test --aot, following oven-sh/bun#44833

Speed

Known costs, fixed in the next engine PR

Not AOT-specific

Testing

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude Bot Nov 1, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Nov 1, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Nov 1, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Nov 1, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude Bot Nov 1, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 1, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Jarred-Sumner commented Nov 1, 2026 •

edited

Loading

`bun test --aot`, following oven-sh/bun#44833