Repository navigation
AOT: 77 fixes: crashes and wrong answers in executables, mocks and import() in test runs - #982
Merged
Merged
Conversation
AOT::countMockCall() returns at once unless the path for counts is set. It changes no answer. Goes with Bun's half, which calls it.
…s function travels
lib.js: `export const read = () => typeof someGlobal`. A test: `run(vi.stubGlobal)` with
`export const run = f => f("someGlobal", "stubbed")` in another file; `[vi.stubGlobal][0]("someGlobal", "stubbed")`;
`const held = { stub: vi.stubGlobal }; held.stub(..)`. Plain: `string`; compiled: `undefined`, without an error. Also through a
static field, an instance field, a Map, a generator, Object.values, a spread, a closure of another file. Goes with one word in Bun's
table of built-ins.
… the check With --aot-check=verify-each-pass, the pass that splits a function behind its entry checks tells the check that the function's body was replaced. The check compared the old body with the function that tests the arguments, and a build with assertions stopped there.
…ves without values is looked at The check compared nothing where the body behind the pass had no typed value: meant for bodies that are not typed yet, it also held for a body without any value, so a pass that empties a function was seen by nobody. A red control is in Bun.
- Under `--aot-check=verify-each-pass`, the check of a pass that answers "unchanged" held the numbers of blocks against each other and not the numbers of values: a value in no block went unseen. With two planted faults; their witnesses are in Bun. - In updateIntegersInMemory() a user that is listed already is the last one listed, so the search through the list goes. - In isSettledByMock() two operands without effects change places, the cheaper one first. - The counter of refusals tells a refusal by longer parts of the run time's own sentences: a program may well say "compiled code" itself. - A native test compares the bits of two numbers of values. No generated code changes. Outside aot/: runtime/AbstractModuleRecord.cpp +1 -1, in a function that a run without compiled code never calls.
… passed Found in review of #978. A text whose turn has passed adds no levels. Its test, chained-imports.test.ts, is in Bun: it observes the order in which chained import() calls settle.
… line, and `--explain=missed-optimizations` names some Three rows: InlineHashLookups, TagLookupsWithConstantKeys, TagLookupsWithOtherKeys. The sites of the third are named: "missed: lookup-key-not-constant at <file>:<line>:<column>". Goes with Bun's half.
…t to the prototype that the second was made with
Two objects of one constructor function in one function, the function's `prototype` replaced between them and put back:
`const q = new Q(); Object.defineProperty(Q, "prototype", { value: second }); q[key] = 1; const r = new Q(); ..put back..;
q.describe()`: plain "first", compiled "second", without an error, at default flags. The step of ThreadJumps that sent an unlikely way
past the next test of the class whose object it made is taken out.
`const bytes = new Uint8Array(memory.buffer); memory.toResizableBuffer();` and then `bytes.length`, `bytes[0]`: `bun run` prints `0 undefined`; compiled `65536 9`. A loop over bytes made 65,536 turns for 4; a loop of six turns crashed. The row of the table of built-ins that toFixedLengthBuffer and toResizableBuffer share did not say that it writes the storage of typed arrays.
… storage The library sorts a short list where it lies, and takes storage of int32 for contiguous storage on purpose. The store named B3's heap of contiguous elements alone, which B3 holds disjoint from that of int32 elements, so a read `a[i]` under the other name could have been carried across it.
`const [a, b] = await Promise.all([import("./a.mjs"), import("./b.cjs")])`: in node and in `bun build --compile` a.mjs runs first;
compiled ahead of time, b.cjs did. Where b.cjs has `require("./a.mjs")` and a.mjs calls `import("./b.cjs")`: the keys of b are
`default,fromB`; compiled `default` alone, and b.fromB undefined. Two lists of known differences in Bun get shorter with it.
The validator's rule for a promise of dereferenceable bytes also takes Add(ArgumentReg, Shl(ZExt32(y), constant)) with y no constant. Found in review of #978. Shared with the JIT: b3/B3KnownProperties.h +8 -2. Under B3's default policy no promise is ever stored, so the rule is never asked in the JIT.
…mised about it threadJumps copies a tail for each way in and puts the Phi's input for that way in the Phi's place; the copy kept the promises that were made of the Phi. A pure value of the copy is loop invariant then, and was hoisted with its load above the test that chose the way. A graph written by hand: 70 at level 0, a crash at level 2. Shared with the JIT: b3/B3RegionCloner.cpp +18 -2, B3RegionCloner.h +2, B3ThreadJumps.cpp +1 -1. The cloner is used by threadJumps, rotateLoops, versionLoops and unrollLoops alone; each runs only under a field of the policy that is false by default and that the JIT does not set.
main.ts `console.log(eval(process.argv[2]))` built with --disallow-code-generation-from-strings: `bun check --aot` said that the executable keeps the feature "Interpreter", which it used to leave out. A matter of size, not of what the flag forbids. Behind "a trimmed executable keeps the run-time operations that its image lists". Goes with Bun's src/trim/features/engine.def.
What the observer reports, not what a mock does: a built-in module under a mock said `none`; now `in place` where the mock replaces it and `laid over` where the loader held it. A new state: `ended while another waits`. Goes with Bun's half.
…nt asks what is new Behind "the walk that feeds a built-in that walks its argument has a statement of its own". Each round the walk's statement asked the same reads of a set one member larger, so the solver let go of every node asked for before and took the new ones up: each part four times as dear at a doubling, on a build of 4,000 chained literals. No answer of flow analysis changes.
…on's body without saying so stops the build With --aot-check=verify-each-pass, a pass over the program that moves a function's body into another function without saying so stops the build, which names the pass and the function. Before, the check compared the old body with whatever took its place. With a sabotage and a pair of tests.
…the first
a.test.js: `mock.module("pkg", ..); const { seen } = await import("./src/uses.js")`. b.test.js: `import { seen } from "./src/uses.js";
seen()`. uses.js: `import { f } from "pkg"; export const seen = () => f()`. Plain: `real` in b.test.js; compiled, over layers:
`mocked`, without an error.
It came with "whoever holds a module that was linked to a mock keeps that mock" (#979): five lines of that commit, in
AOTModules.cpp, are taken back.
StringEmitter::implOfFlatString() loads the text of a string's cell with a load that is marked immutable and promised dereferenceable.
It took any pointer to a cell, and the emitter of Map and Set lookups handed it a sliced or joined string because the type held only
literal strings ("a sliced or joined string typed as a union of string literals, as a Map or Set key, crashed or missed the key"). It
now takes a FlatStringCell, which only three functions make, so that the mistake does not compile.
Where more layouts have the names asked of an object than there is room to test for, they are ranked, last by how often each is made: a sum of products over the whole program. Two layouts made equally often could come out one rounding error apart, and the last digit chose which of them is tested for: five lines in a library file moved twelve functions of unrelated packages. The key is cut short of its last digits.
…used, not compiled unchecked Six branches read a string with no test of the position where the result's representation has no room for the answer to a position outside it (NaN or undefined): charCodeAt, codePointAt, at and the reads of a range. Nothing makes such a type today, so they were dead code; the build now stops there instead.
`TypeError: Cannot spy on 'answer' because module exports can't be replaced in a compiled executable` and `mock(module, fn) cannot replace a module that was compiled ahead of time: "<path>"` are what a user meets where the build missed a spy or a mock. In `bun test --aot` there is no executable, and exports can be replaced; the first named no module, and neither said what to do. The engine words both now, by the kind of run. Goes with Bun's half.
…ee takes
An untyped file holds `box.run.call(AbortSignal, [controller.signal])`, `box` defined by nobody; a typed file of the same build has
`class Holder3 { run(given: number[]) {} }` and one call `run([5, 6, 7])`. The build stopped: "verification failed after ResolveCalls;
StoredTypeMismatch .. (AssumeType)", at default flags.
Compiled code makes the size of a table's index as `2 << shift` in 32 bits; the shift is 31 above 201,326,592 entries. The run time now stops where it grows a table past what the walk in line can address. No cost in compiled code. And one in SinkAllocations.
…this` ended the executable
`class Row { port = 80 } function peek(base) { const target = base ?? new Row(); Object.defineProperty(target, "value",
{ get() { this.seen = 1; return 1 } }); target.value; return typeof target.seen }
console.log(peek(), peek(Object.defineProperties({}, JSON.parse("{}"))))`: plain `number number`; the executable ended with exit code
134: "AddProperty of seen on an object of layout 0 (Row), which the compiler assumed never happens". Also where the getter deletes a
member, freezes `this`, or fills a memo by `??=`. The twin of "a store under a key that the build doesn't know enters a setter that
deletes a field or calls a method".
…p that is reported Behind a pass over the whole program the planted fault was used up at the first body where a drop was made, also where the check has no word for it, and no later body was tried: a red control could show nothing. Found in review. Only under `--aot-check=verify-each-pass` with a planted fault.
dep.js lies in a lower layer than user.js and has run. a.test.js: `vi.doMock("./dep.js", () => ({ flag: "one" }));
(await import("./user.js")).read()`: one. b.test.js, the next file, mocks nothing and imports user.js again: plain `real`; compiled
`one`, without an error. Within a file too: behind vi.doUnmock() or a second vi.doMock(), an import() of user.js anew went on reading
the first mock. `bun test` takes whoever was linked to a mock out of its loader when the mock ends, and a load that comes by makes it
anew; a user above a mocked file is now linked anew where `bun test` loads it anew.
…t nothing passes stopped a build The verifier does not hold a value against the declared type of the place it is stored into where the value comes from a check that nothing passes: the code behind such a check runs in the generic version alone. `held = value` behind `value instanceof Kind` in a function called with .call() stopped a build under --aot-check=verify-each-pass.
…error": UpcastNotSubset after PlaceTypeTests PlaceTypeTests copies a loop into a fast and a slow one and lays a test of the type on the way back into the fast one; what the test leaves of an array's type had lost the array's proofs, and an upcast behind it was not typed anew, so its source no longer lay inside the type it casts to. The verifier's rule is right; the IR was inconsistent. Bun's main has it too. Its test is in Bun.
- FailedTests sizes its set once, where there is a failed edge or switch at all. - The native test of the net for held references, in builds with the address sanitizer, took any crash and any failed check of its child for "stopped"; it now asks for "AddressSanitizer: heap-use-after-free" in the output.
…atement is woken for It came with "build time: one question for all receivers first". Under `--aot-check=flow-fixpoint` a build stopped with "FlowEndedTooEarly: the flow analysis was at rest, and 2 of its rules still add something when they run once more over the solved sets". The question was asked of the receivers that have not escaped by now, which leaves no watch: when a receiver escaped later, the statement was not woken. It is asked of all the known receivers now; who has not escaped is asked behind it, as before.
`const s = spyOn(Array.from, "call"); Array.from.call(Array, [1]); s.mock.calls.length`: plain 1, compiled 0, and the test passed; with a mockImplementation the real function answered. Compiled code takes call, apply and bind of a built-in function for what Function.prototype holds. A function of the engine or the host refuses them as properties of its own; for a function of the library that is written in JavaScript spyOn() never asked. It is refused now in the words of the others. In `bun test --aot` alone. Goes with Bun's half.
… the other order of the comparison
It completes "one value under two local names, tested by truth, lost its null", which cured `b === a` alone.
`function f(a, b = a) { if (!a) { if (a === b) seen.push(typeof (a ?? "d")); b ^= 1; } else a == b; return b?.[0]?.[0]; }` called with
-1, 1, "", "1", null: plain `string,string`, compiled `string,number`, without an error. Bun's main has it too. PlaceTypeTests laid a third
cast whose claim was taken while the proof still had the type made from the stand-in; like the other two it now claims the type of the
value that it replaces. Goes with Bun's half.
…lone
`import { getPlatformProxy } from "wrangler"` and one test under `bun test --aot` took 369 s to build, 305 s of it in the rules of the flow
analysis. For `require.cache[path].exports = other`, a store under a key that is no name went over every member of every built-in holder
among the receivers, only to ask whether one is the exports of a module object. It asks under the name "exports" alone now, and not at all
where the receivers may be an unknown object. 369 s become 271 s; the image's bytes are the same.
…a listener
With `process.on("SIGTRAP", ..)` listeners (the npm package signal-exit lays one) a stop in compiled code ran on (x64) or hung (arm64).
Under the image's assembler policy breakpoint() now emits an illegal instruction (ud2, udf), so the process dies by SIGILL with the crash
report; what fills the gaps between functions is hlt on x64. The JIT's policy and bytes are unchanged, which testmasm pins. Goes with Bun's
half.
A new statistic, ParametersLetGo, counted in ChooseCallingConventions: how many pairs of a function and a parameter the callers may let go of. An IR test asks for it. No generated code changes.
- `mock.module("node:os", () => made)` or `vi.doMock("node:path", ..)` with a counting getter on the factory's object, over layers: four
lines read "mock 2" for plain's "mock 1": one read of the getter too many, without an error. It came with "over layers, the mock of one
test file leaked into the next".
- q.cjs has `const k = require("./k.js")` and was loaded by the earlier test file; behind `vi.doMock("./k.js")`, `require("./q.cjs").top()`
read the real k for the mock's, without an error. It came with "four mends in the run time's modules"; three lines of that commit are
taken back. What they had cured stands open again: `import * as early from "./early.js"` at the top of a test file, then
`vi.doMock("./dep.js", ..)` and `await import("./late.js")`: early, which imported dep.js ahead of the mock, reads the mock's values; only
in one realm over layers, and only where an earlier test file had loaded dep.js.
- barrel.js has `export * from "./dep.js"`; an earlier test file takes the barrel's namespace object; a later one says
`vi.doMock("./dep.js", () => ({ flag: "one", plain: "one", added: "added" }))` and loads another user of the barrel. Plain:
`one,added,added+flag+plain`. Compiled: `one,,flag+plain`. As one program and over layers.
- The same line over the layers that a run takes by itself, where the end of an earlier mock had the barrel linked anew too early.
- node_modules/reads, an ES module: `export const seen = async () => (await import("dep")).value();`; the test has
`vi.mock("dep", () => ({ value: () => "mocked" }))`. Plain: `real`, because Vitest leaves packages as they are. Compiled: `mocked`.
- lib.ts has `export * as space from "./space"`; the module has run; `mock.module("./lib.ts", () => ({ space: { inner: "mock inner" } }))`.
Plain: `space.inner` reads `mock inner`. Compiled: `real inner`, for a statement importer, a namespace object, import(), a captured value
and a barrel.
- A module whose body threw stayed failed behind vi.resetModules(), behind vi.doUnmock() and in the next test file; plain loads it again.
config.js: `if (!process.env.KEY) throw ..`; with the key set behind a reset: plain `k`, compiled `threw KEY is not set`.
They share AOTModules.cpp. Goes with Bun's half.
Jarred-Sumner
changed the title
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked the files shared with the JIT: trapsWithIllegalInstruction defaults to false and is set only in AssemblerPolicy::forImage(), so the JIT's breakpoint() and LinkBuffer::allocate() padding are unchanged; the AbstractModuleRecord.cpp reorder swaps two effect-free predicates inside #if ENABLE(AOT), and hasResolutionFailure() is a const lookup also behind #if ENABLE(AOT).
Extended reasoning...
The 19 files outside aot/ (assembler policy, LinkBuffer, two MacroAssembler headers, B3 fixSSA/threadJumps/known-properties/region cloner, AbstractModuleRecord, JSModuleLoader) were read in full; the JIT-visible paths are gated by a policy flag that only the AOT image policy sets, and the runtime/ edits sit behind ENABLE(AOT). The remaining 142 files are AOT compiler and run-time changes covered by the inline findings and the earlier review.
Findings marked 🟡 are optional suggestions and need no follow-up push.
Still open from earlier reviews (2):
- 🔴
Source/JavaScriptCore/aot/runtime/AOTModules.h:1370—In a bun test --aot run where a delayed import() has queued the round job and a graph load then sets the queue aside, c… - Also unresolved: 1 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.
Comment on lines
+1272
to
+1283
| bool Modules::tryReplaceNamespaceHandedOn(ModuleID handedOn, JSValue value) | ||
| { | ||
| if (!m_moduleWithMockBeingLaid) | ||
| return false; | ||
| Vector<ModuleID>& replaced = ensureLoaded(*m_moduleWithMockBeingLaid).namespacesReplacedByMock; | ||
| if (!replaced.contains(handedOn)) | ||
| replaced.append(handedOn); | ||
| ensureLoaded(handedOn).handedOnInPlaceOfNamespace.set(*this, value); | ||
| m_namespacesReplacedByMock.set(handedOn.index()); | ||
| didReplaceNamespaceHandedOn(handedOn); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🔴 In bun test --aot, mocking one module that has export * as inner from "./inner" also changes inner of every other module that re-exports it, which plain Bun leaves as the real namespace. tryReplaceNamespaceHandedOn at AOTModules.cpp:1279 stores the mock's value on the target module's LoadedModule, and readExport at :1038 hands it out for any exporter whose entry targets that module. Fix: key the replacement by the exporting module as well as the target (store it under the mocked module and look it up from readExport(module, index) by that pair), and end it per mocked module, so an unmocked re-exporter keeps the real namespace and a second mock over another re-exporter neither overwrites nor, at its end, clears the first.
Why this was flagged
a.js and b.js each contain export * as inner from "./inner"; a test file lays a mock over a.js whose factory gives an inner key (any mock that reaches mockExport for that name), then reads b.inner through import * as b from "./b" or a compiled namespace read. tryReplaceExport at AOTModules.cpp:1251-1252 calls tryReplaceNamespaceHandedOn(targetModule, value), which at :1279-1280 sets ensureLoaded(inner).handedOnInPlaceOfNamespace and the bit for inner alone; nothing records which exporter was mocked except the list used for ending. handedOnInPlaceOfNamespace at :1264-1270 looks up only moduleExport.targetModule(), and readExport at :1038-1039 returns that value for module b as well; didReplaceNamespaceHandedOn at :1286-1297 drops the cached structure of every namespace object that hands inner on, so b's object re-reads it too. On the base branch tryReplaceExport returned false for a Namespace entry, so b.inner stayed the real namespace, as in plain Bun.
Verification: normal — in bun test --aot, when a module that has already run and has export * as inner from "./inner" is mocked with a factory that supplies the inner key, and another module also has export * as inner from "./inner".
| bool hasSeveralImages = m_instance.vm->aotState().hasSeveralImages; | ||
| if (!m_momentsOfLoads.isEmpty()) { | ||
| if (!m_momentsOfLoads.isEmpty() && images == ImagesSearched::All) { | ||
| uint32_t momentOfTestFile = m_instance.vm->aotState().momentOfTestFile; |
There was a problem hiding this comment.
🟡 nit (optional): maintainers building with warnings as errors get an unused-variable error here. The local momentOfTestFile at AOTModules.cpp:2293 is no longer read since the comparison at :2318 was replaced by wasLoadedInThisTestFile(importer). Fix: drop the declaration at :2293 (the function now reads the moment through wasLoadedInThisTestFile at :2270).
Why this was flagged
In Modules::letGoOfImportersOf at Source/JavaScriptCore/aot/runtime/AOTModules.cpp:2293 the line uint32_t momentOfTestFile = m_instance.vm->aotState().momentOfTestFile; was kept while its only reader, the comparison m_momentsOfLoads[place] > momentOfTestFile at :2318, was replaced by wasLoadedInThisTestFile(importer). No other line between :2292 and :2340 names momentOfTestFile. Clang's -Wall emits -Wunused-variable for it; Source/cmake/WebKitCompilerFlags.cmake:166 turns warnings into errors under DEVELOPER_MODE, so a developer build with that on fails to compile, while CI lanes (no DEVELOPER_MODE) only print a warning. The base branch read the variable, so had no warning.
Verification: nit — triggered on any build with DEVELOPER_MODE on (CMAKE_COMPILE_WARNING_AS_ERROR) or any -Werror build. Source/JavaScriptCore/aot/runtime/AOTModules.cpp:2293 still declares uint32_t momentOfTestFile and no line in that block reads it: the diff removes its only reader, replacing it with wasLoadedInThisTestFile(importer) at :2318. Under warnings-as-errors the AOT bundle fails to compile.
Under the checking modes alone (BUN_JSC_validateAOTRuntime, an image built with --aot-validate). Seven functions that run for every object that is still alive lay under a pragma that compiles a file's functions for size, which takes no inlining to speak of. They are compiled as other code is; what is printed before the process stops stays under the pragma. +384 bytes of text. Nothing changes at default flags.
…tion
`expect(1)()`: plain `expect(1) is not a function. (In 'expect(1)()', 'expect(1)' is an instance of Expect)`, compiled
`Expect is not a function`. So too `make(1)()`, `o.n.k()(2, 3)`, `make(make(1))()`, `make?.(1)()`, `make(...[1])()` and `make({ a: 1 })()`.
Goes with Bun's half.
`Object.defineProperties(Object.prototype, Object.getOwnPropertyDescriptors({ get extra() { return 41; } }))` and
`Object.assign(Array.prototype, { get at() { return f; } })` throw a TypeError at run time, as documented, but the build gave no TS100014.
With a data property the warning came. An accessor names its key as a property does now.
`BUN_JSC_dumpAOTIR=FlowFacts=<path>` makes a build write what the flow analysis hands to ApplyFlowResults, a line for each fact, each thing named by its place in the source, so that two builds can be compared. A tool for testing: nothing changes where the word is unset. Goes with Bun's half.
`const d = ["$"].join(""); typeof Bun[d].braces`: plain and `bun build --compile` say `function`, compiled ahead of time `undefined`,
without an error. `Object.getOwnPropertyNames(Bun[d])`: 7 names for 2. The same for Bun.sql and Bun.SQL. The flow analysis let a key that
nobody knows name no member of Bun, on purpose; the read then answered a bare stand-in function, and a read off the stand-in is silent.
Goes with Bun's half.
…ification Where a type differs from what its rule gives, the message ends ": there stands X, the rule gives Y". With TESTAOT_SHOWS_FAILING_BODIES=1 and --single-process testaot prints the function in front of the run that failed and behind it. Two hooks in the verifier, null in the product, asked only behind a verification that has already failed.
The launcher prints the whole command line of a child that crashed or hung. A test alone.
- In executables, and in test runs as one program: `const first = import("./a.mjs"); await null; import("./b.mjs")`, a and b importing each
other, a reading a name of b at once: compiled code threw a ReferenceError. The later call's graph was complete in fewer jobs, so the
cycle was entered at b; node, `bun run` and `bun build --compile` enter it at a. Of 24,696 graphs on which the three agree, 2,438 were
wrong and 252 are: all 1,067 errors and all 8 wrong values are cured, none is newly wrong; what is left are orders, wrong on Bun's main
too.
- In a test, `await import("./main.mjs")` whose body awaits `import("./top.mjs")`, top importing m2 (which imports b.cjs, which requires
a.cjs) and a.cjs: the executable and every reference print `b start, a`; the test run printed `a, b start`, without an error. It came with
"`bun test --aot` as one program settles import() calls as the executable of the same files does".
- A test file with a hoisted `vi.mock("node:fs", ..)` never finished loading beside a test file that names `vi.resetModules()`. It came with
the same commit; the first of "four mends in the run time's modules" had cured one form alone.
- main.cjs `require("./mid.mjs")`; mid.mjs `import { loaded } from "./reader.mjs"`; reader.mjs
`import defer * as deferred from "./deferred.mjs"; export const loaded = deferred.loaded`; deferred.mjs
`export const loaded = import("./reader.mjs").then(..)`: plain and `bun build --compile` print `loaded`; compiled ahead of time the promise
never settled while the program's CommonJS entry was still running. Bun's main has it too.
They share AOTModules.cpp. Goes with Bun's half.
Jarred-Sumner
changed the title
There was a problem hiding this comment.
Findings marked 🟡 are optional suggestions and need no follow-up push.
The last 3 reviews of this pull request each found new blocking issues, repeatedly in Source/JavaScriptCore/aot/runtime/AOTModules.cpp. Where they share a root cause, one fix may close them together.
Still open from earlier reviews (3):
- 🔴
Source/JavaScriptCore/aot/runtime/AOTModules.cpp:1283—In bun test --aot, mocking one module that has export * as inner from "./inner" also changes inner of every other modul… - Also unresolved: 2 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.
| std::optional<MomentMockWasLaid> waiting = m_momentsWaitingMocksWereLaid.takeOptional(fileOrModule); | ||
| m_momentsStandingMocksWereLaid.set(fileOrModule, waiting && !stoodInPlace ? *waiting : momentOfNow()); | ||
| // Only over the mock of a preload that the preload has also loaded: `bun test` patches what a test file was the first to load, and a second vi.mock() of a test file reaches whoever reads its first. | ||
| if (JSObject* earlier = leavesEarlierMockToItsImporters ? mockInPlace(fileOrModule) : nullptr; earlier && earlier != mock && earlier == ensureLoaded(fileOrModule).mockLaidInPreload.get() && m_loadedInPreload.get(placeOfMomentOfLoad(fileOrModule))) { |
There was a problem hiding this comment.
🔴 A layered bun test --aot run can crash when a test file's vi.mock() comes over a preload's mock of an ES module whose image has noted no load yet. The new condition in setMockInPlace at AOTModules.cpp:2013 calls placeOfMomentOfLoad(fileOrModule), which at :2248 indexes m_momentsOfLoads[head], while that image's m_momentsOfLoads is still empty (it is filled only by noteModulesLoadedWith at AOTModuleEvaluation.cpp:1907). Vector::at() then crashes on the out-of-range index. Fix: treat an empty m_momentsOfLoads as "not loaded in preload" and skip placeOfMomentOfLoad there, as :2280 already guards with !m_momentsOfLoads.isEmpty(). On the base branch this branch did not exist, so no crash.
Why this was flagged
Trigger: a layered test run where src files sit in a lower image and test files in an upper one. A preload calls vi.mock() on an ES module of the lower image without importing it by statement; Modules::setMockInPlace (AOTModules.cpp:1992) runs with momentOfNow() returning isInPreload true, so :2024 stores the mock in mockLaidInPreload. The lower image's m_momentsOfLoads stays empty: it is only filled by noteModulesLoadedWith (AOTModuleEvaluation.cpp:1906-1907), which an upper graph reaches only through statement imports; a test file that uses import() alone fills nothing below. The test file's hoisted vi.mock() then calls setMock(..., leavesEarlierMockToItsImporters=true), and :2013 evaluates earlier == mockLaidInPreload true and then m_loadedInPreload.get(placeOfMomentOfLoad(fileOrModule)). placeOfMomentOfLoad reads m_momentsOfLoads[head] at :2248 on an empty Vector; WTF::Vector::at() calls OverflowHandler::overflowed() and the process dies. The sibling guard at :2280 shows the author expects the table to be empty in some test runs; :2013 has no such guard. The base branch had no such comparison and laid the second mock without reading the table.
Verification: normal — triggered in a layered bun test --aot run when a preload's vi.mock() lays a mock over an ES module of a lower image whose image has not yet noted any load, and a test file then lays its own vi.mock() over the same module. The condition added at AOTModules.cpp:2013 calls placeOfMomentOfLoad, which at :2242-2248 indexes m_momentsOfLoads[head]; Vector::at() crashes. Line 2013 is the one new call lacking that guard.
Comment on lines
+198
to
+202
| if (printerOfTypesForVerifier && value.numberOfOperands()) { | ||
| const Value& definition = m_function.at(value.operand(0)); | ||
| m_errors.last().detail = makeString(m_errors.last().detail, "; its first operand is v"_s, value.operand(0).index(), " ("_s, nameOf(definition.opcode), "): "_s, printerOfTypesForVerifier(m_program, typeOf(value.operand(0))), | ||
| definition.numberOfOperands() ? makeString(", made of v"_s, definition.operand(0).index(), ": "_s, printerOfTypesForVerifier(m_program, typeOf(definition.operand(0)))) : String()); | ||
| } |
There was a problem hiding this comment.
🟡 nit (optional): testaot can crash instead of printing a TypeOutOfDate diagnostic when the failing value's first operand slot is empty. AOTVerifierTypes.cpp:199 calls m_function.at(value.operand(0)) and typeOf(value.operand(0)) after checking only numberOfOperands(), but optional operands (GetModuleVariable's fact?, NewObject's prototype?) are stored as an empty ValueID whose index() is UINT32_MAX, so the lookup reads far past the values table. Fix: skip the operand detail unless value.operand(0) is non-empty, and likewise guard definition.operand(0) at :201 before calling typeOf on it, so that every optional-operand opcode gets a message rather than a fault.
Why this was flagged
testaot sets Typed::printerOfTypesForVerifier unconditionally at Source/JavaScriptCore/aot/tests/testaot.cpp:388, so the new block at AOTVerifierTypes.cpp:198-202 runs for every TypeOutOfDate failure in that tool. Optional operands are empty ValueIDs: ID::index() at AOTID.h:72-76 returns UINT32_MAX for the empty value. GetModuleVariable (operations.def:39, engine fact?: Fact is operand 0) has a declared type that verifyOwnType re-derives, and most reads carry no fact. When its type is out of date, line 199 does m_function.at(ValueID()) which at AOTIRFunction.h:164-165 only ASSERTs in debug and in release indexes values.span().data()[UINT32_MAX]; line 201 then calls typeOf on the same empty id. On the base branch the same failure produced the describeDifference() message and the test reported the error; after the change the release testaot in the tested lanes faults in place of the report. The numberOfOperands() check does not protect this because it counts operand slots, not filled ones.
Verification: In testaot, a TypeOutOfDate on a value whose operand 0 is an absent optional operand reaches Source/JavaScriptCore/aot/ir/AOTVerifierTypes.cpp:198-202, which guards only on value.numberOfOperands(). An empty ValueID has index UINT32_MAX (AOTID.h:72-77); Function::at (AOTIRFunction.h:159-166) only ASSERTs the bound, so release testaot segfaults and the diagnostic is never printed.
Jarred-Sumner
added a commit
that referenced
this pull request
Nov 1, 2026
A wrong answer of an executable at default flags, without an error. It came with #982; Bun's main is right. const T = []; function f(a, b) { let c = b; if (!b) { if (c !== b) T.push(1); } b == c; b?.[0]; if (a == b) { b -= 1; c--; if (typeof c === "object" && c !== null && "id" in c) T.push(2); } T.push(b, c); } f(1, 0); f(-2, -2); f(1, ""); f(null, undefined); console.log(T.map(String).join(" ")); `bun run` prints `0 0 -3 -3 NaN NaN`; compiled code printed `0 0 -3 -1 NaN NaN`. While type tests are placed, a condition's first proof stands for all of its proofs, and a cast took its claim from such a stand-in: the type inside `if (!b)`, `undefined|0`. A claim is taken only from a value that stood before the run of the pass. Two IR tests, one with a planted fault. Goes with Bun's half.
Jarred-Sumner
added a commit
that referenced
this pull request
Nov 2, 2026
* AOT: the checks of slot types go by a plan for each layout
Under the checking modes alone. Holding every slot of every object that is still alive against its type was 20 to 23% of all user
instructions of the TypeScript-compiler benchmark under checked proofs: for each field anew, whether it is a brand or an accessor, where it
lies, its type and its range; and over half of the visits were to fields that the object doesn't hold. All of that is the same for every
object of a layout, so a plan is made at the first object of a layout. The plan says only whether a slot is outside its type; the old walk
then names it, so a report reads as before. Nothing changes at default flags.
* AOT: the file of what the flow analysis hands on says why
Behind "a build can write down what the flow analysis hands on": the line of a layout ends with one column more, why the analysis could not
rule more out. A tool for testing: nothing changes where the word is unset.
* AOT: a call of Math.random() is not removed or run twice
`setRandomSeed()` from `bun:jsc` makes the sequence of Math.random()
reproducible, so a program can count the calls. The compiler believed that
nobody could: the call was declared to write only the runtime's own state.
function roll(use) {
const r = Math.random();
return use ? r : -1;
}
Where `use` is false the call was removed, and every later number differed from
`bun run`. The same happened to a bare `Math.random();`, to an argument that the
callee does not read and to a property that nobody reads.
The call could also run twice. A typed function that fails a type test before
it has done anything visible starts again in its generic version:
function f(holder: Holder) {
const r = Math.random();
const inner = holder.inner;
return inner.x + inner.y + r;
}
With an `inner` that is not laid out as a Point, `r` was the second number.
Math.random() now also reads and writes external state, as Date.now() does.
Two tests had the old effects written into them and are restated.
* AOT: the refusal of a load by a dynamic path names the flag too
"Cannot load '<path>': this executable can't load a module from a dynamic path. Pass the file to bun compile as another entry point" now
goes on ", or build with --aot-allow-code-from-disk", as the other refusal of a load does. Words alone. Goes with Bun's half.
* AOT: native code that runs out of stack in an executable is reported
Native code that ran out of stack in an executable built with --aot ended the process with SIGSEGV, nothing on stderr, no crash report, and
a raw terminal left raw. Bun's crash handler registers an alternate stack; the engine then installed its own actions for SIGSEGV and SIGBUS
over it without SA_ONSTACK, and only the thread that started had an alternate stack. A thread that has none gets one, and the two actions
take it. Goes with Bun's half.
* AOT: seven mends in the run time's modules and mocks
- `mock.module("./m.js", () => { calls++; imports.push(import("./m.js")); throw ..; })`, then two `import()`s of m.js one behind the other.
Plain asks the factory each time (`failed 2`, `failed 3`); compiled as one program, the second was answered with the first's error and
nothing was called, without a sign.
- A mock that works, laid over the one that failed, then `import()` by a name that the build can't read: plain `mock`; compiled, rejected
with the first error.
- a.test: `mock.module("node:os", () => ({ EOL: "mock EOL" }))`, then a graph that ends in node:os and passes through barrel.mjs. b.test:
`mock.module("./barrel.mjs", () => ({ own: "mock own" }))`, then `import { own } from "./outer.mjs"`. Plain: `mock own`. Over two layers:
`SyntaxError: Export named 'own' not found in module outer.mjs`.
- main: `try { require("./file.cjs") } catch {}`; file.cjs:
`import("./file.cjs").then(.., error => console.log(error.message)); throw new RangeError("thrown by the file")`. `bun run` takes the file
out of require.cache at the throw, so the import() runs the file again and is rejected with what the second run throws. Compiled:
rejected with the first error, one run.
- A plugin of the run answers for `virtual:aliased` in onResolve; `await import("virtual:aliased")` twice behind an import statement of the
same name. `bun test` asks the plugin at every call; compiled, at the statement alone.
- a.test.js: `vi.doMock("./dep.cjs", () => ({ who: "one" })); const held = await import("./held.js")`; held.js:
`import { who } from "./dep.cjs"`; b.test.js: `import { read } from "./held.js"`. Plain and one program: `real`. Over layers: `undefined`,
though dep.cjs's body ran.
- main.cjs: `const a = require("./a.mjs")`; a.mjs: `import("./b.cjs"); export const fromA = ..`; b.cjs: `require("./a.mjs").fromA`. node and
`bun build --compile`: "a ends | main has a | b starts | b sees a". Compiled ahead of time, as `bun run`:
"a ends | b starts | b sees undefined | main has a". It goes as node and `--compile` now.
They share AOTModules.cpp. Goes with Bun's half.
* AOT: testaot can ask the laws of the algebra of types of every type of a program
`RUN: laws-of-types`: 26 laws of intersectWith, castTo, unionWith, without, isSubsetOf and overlaps over every pair, and for three laws every
triple, of the JavaScript types that a program holds: a line for each shape of pair that breaks one, and a sum for each broken law. With
TESTAOT_LISTS_BROKEN_LAWS_OF_TYPES=<file> the same behind every command of every test. A tool of the tests alone; no test pins its sums yet,
because they go by the table of built-ins that the engine is built with.
* AOT: a step of the build counts who reads a field that assumes more than a type says
aot/codegen/generator/reads-that-assume.ts counts, file by file, who names the heap of a string's value and who calls the four helpers for
flat strings, and holds the counts to aot/codegen/AOTReadsThatAssume.txt: 22 places, each with the reason why it is right. A place that the
list lacks stops the build before the first file compiles. `AOT_CHECK_READS_THAT_ASSUME()` in aot/AOT.cmake runs it here; Bun's build has
its own step. No generated code changes.
* AOT: the build warns of a store under a name that native code locks on a host object
`process.versions.node = "1.2.3"` and its like throw at run time, and the build gave no warning. The table of built-ins now says which names
native code locks, and the check for writes to locked built-ins reads it. It feeds a diagnostic alone: no answer of a program and no
generated code changes. Goes with Bun's half.
* AOT: the end of a mock gives back what the mock replaced
Behind the sixth of "seven mends in the run time's mocks". a.js and b.js both have `export * as inner from "./inner"` and both are mocked;
then `vi.doUnmock("./b.js")`. Plain gives back what b's mock replaced, `mock of a`; compiled code read `real`. A mock that replaces a
namespace that is handed on now keeps what stood there, and its end gives that back. A parameter that nothing read is gone. Goes with Bun's
half.
* AOT: a link that fails behind a computed name throws at the load, in plain's words
`for (const file of ["one.js", "broken.js", "two.js"]) await import("./" + file)`; broken.js: `import { nope } from "./dep.js"`. `bun run` and
`bun build --compile`: `SyntaxError: Export named 'nope' not found in module '..dep.js'.` Compiled ahead of time:
`ResolveMessage: Cannot find module '/$bunfs/root/broken.js' imported from '/$bunfs/root/broken.js'`. Where broken.js only imports the file
with the fault, the build was refused where `--compile` builds. Goes with Bun's half.
* AOT: the statistics tell the lines of packages, of declarations and of Bun apart
Four more statistics beside SourceBytes and SourceLines, for `bun build --compile --aot`'s new closing words, "compiled 57,464 lines in
18.64 s (3,082 lines/s)". Counters alone. Goes with Bun's half.
* AOT: the detail of a stale type names an operand only where there is one
It came with "testaot says both types, and prints the body around a failed verification". Behind a TypeOutOfDate the detail read the first
operand after asking only how many there are; an optional operand that is left out (NewObject's prototype) is an empty id, and testaot died
by SIGSEGV where it should have named the stale type. The block runs only where testaot has set the printer of types: no product build
enters it.
* AOT: a store that is listed at its allocation goes by the proof of the pass
A verifying build stopped with "verification failed after SplitUnions; .. (PropSet) in b0 may throw or run code, and is gone". The removal
is right: `v24 = NewObject ..; PropSet v24, v12 key="inner"; FinishObject v24` becomes `NewObject -, v12, ..`, only where the store's plan is
a plain store to a slot of an object that the function has just made. The check asked too much. Goes with Bun's half.
* AOT: a test: a number stored into a new extension has the owner's barrier
The barrier for a new extension is decided as if a cell were stored, whatever the value's type. A build that decided by the value's type
would have passed every other test. A pair of tests alone, as shipped and with the planted fault StoresNeedNoBarrier.
* AOT: require() asks what the imports under a type have made of a data file
`import typed from "./data.json" with { type: "json" }` and `require("./data.json")` in one program gave two objects in compiled code: a store
through one was not seen through the other, without an error. Under `bun run` require() hands out the namespace of what the import made,
whose default is that one object; `bun build --compile` has one object too. Goes with Bun's half.
* AOT: a test pins the laws of the algebra of types over the types that it names
Behind "testaot can ask the laws of the algebra of types of every type of a program": `RUN: laws-of-types` takes for its corpus the types
that the test names, so its sums no longer go by the table of built-ins that the engine is built with. tests/types/laws.ir: 21 types, 20 of
26 laws hold; the sums of the other six may only shrink. Tests alone.
* AOT: a delete by key looks at the key, and an assignment asks for the target's prototype
Two wrong answers of executables at default flags, without an error; Bun's main has both.
- `Reflect.deleteProperty(Object.freeze({ m: y }), "m")` answered true for false. Nobody read the field, so it was removed, and the delete
found nothing to refuse. A delete by key now looks under its key, as a test does.
- `Object.assign({ __proto__: { set a(v) { o.b = "S"; } } }, o).b` gave "x" for "S". The expansion reads every field of the source first,
and the setter that the target inherits ran too late. Such a call stays a call.
Goes with Bun's half.
* AOT: the file of what the flow analysis hands on has no blind spot
Behind "a build can write down what the flow analysis hands on": a check that simply stays, and a call whose callees nobody knows, had no
line, so two builds that differ in hundreds of them read alike. Every check and every call of a value in a function that is entered has a
line now. A tool for testing: nothing changes where the word is unset. Goes with Bun's half.
* AOT: Bun read under a key that nobody knows keeps what code of the library makes, not every member
It narrows "Bun read under a key that nobody knows keeps every member". Where that rule fired, all 115 members of Bun counted as used,
Bun.FFI and Bun.Transpiler among them, and `--trim` kept the run time whole: one large application grew from 428.9 to 475.8 MB. Only the 16
members that code of the library makes ever got the bare stand-in whose members read `undefined`; they alone are kept now. What native code
makes is there whoever reads it. `typeof Bun[d].braces` stays `function`. Goes with Bun's half.
* AOT: testaot asks for its process id the portable way
It came with "testaot can ask the laws of the algebra of types of every type of a program": `<unistd.h>` and getpid(), for the name of the
file that TESTAOT_LISTS_BROKEN_LAWS_OF_TYPES asks for. The Windows lanes have no such header: "fatal error: 'unistd.h' file not found".
getCurrentProcessID() of <wtf/ProcessID.h> instead. A test tool alone.
* AOT: a long text with accents is encoded by the library
`new TextEncoder().encode(text)` and `.encodeInto()` of a Latin-1 text with accents took 10 to 14 times as long in an executable as under
`bun run`: one scalar loop counted the text and a second wrote it a byte at a time. Over 16 characters simdutf does both now, as plain Bun's
encoder does; up to 16 characters the old loops stay. Goes with Bun's half.
* AOT: a call for a module of a graph under way settles behind the call for the graph
main: `import("./a.mjs").then(x); import("./b.mjs").then(y)`; a.mjs: `import "./c.mjs"`; b.mjs: `await import("./c.mjs")`. node, `bun run`
and `bun build --compile`: b begins, c runs, a runs, x, b ends, y. Compiled ahead of time: b begins, c, a, b ends, x, y, without an error.
The modules ran the same; the order in which the calls settle didn't. Goes with Bun's half.
* AOT: a refusal that the run time cannot decide decides no exact test
At the IR alone: no JavaScript was found that shows it. `v1 = TypeTest v0 testedType=builtin(ImportMeta)`; on its refusal
`v2 = AssumeType v0 provenType=number|builtin(ImportMeta); v3 = TypeTest v2 testedType=builtin(ImportMeta)`: the verifier admits it, and
SimplifyControlFlow decided v3 false and assumed a number. With an IR test.
* AOT: what only an image of the library alone reached is deleted
The engine's half of the build option `--compiledLibrary`, which Bun no longer has: two options of the compile call, the flag of an image
(its bit stays free; no other flag's bit moves), and what only they reached. It needs a Bun without the option.
* AOT: a planted fault that drops the card of a variable
A word for the tests, and an IR test that is red with the word and green as shipped. No product code. Goes with Bun's half: each is red
without the other.
* AOT: a test run lists the compiled files that ran from source, and why
In `bun test --aot` a file that the image holds ran from its text in the interpreter, without a word, behind `import("./a.js?" + n)`,
`vi.resetModules()`, `delete require.cache[..]`, and where a `Bun.ModuleGraph` imported it: such tests don't test the code that an
executable runs. The run time notes each such file with its reason, for one list. No answer of a program changes. Goes with Bun's half.
* AOT: the stubs align with the same padding as the rest of the code
On arm64 the padding that aligns the stubs was still a breakpoint (the assembler's align() emits brk), so with `process.on("SIGTRAP")`
listeners a wild jump into it hung. The stubs align with udf now, as the rest of the code does. x64's bytes are unchanged. Goes with Bun's
half, whose arm64 case is red without this.
* AOT: an image that has noted no load has no moments
By reading: no program was found that fails. Two functions of the run time's modules read the list of the moments of loads without asking
whether the image has noted a load, and three callers came without the guard that the others have. Both ask it themselves now: an image
without moments answers with the module's own place, and "not loaded in this test file".
* AOT: the recheck of guards reads only what its functions reach
Build time under the program verifier alone: no byte of any output depends on it. Where the first verification reports a guard that isn't
available, the summaries of effects were computed over the whole program from nothing, twice, to verify a few functions again. Now they are
computed over what the bodies of those functions name; every other function counts as unknown code.
* AOT: the iterator of a map is told by its cell
The table of built-ins gives %MapIterator% its cell type, so the algebra of types tells it from Map and Array (the meet is never, both
ways). Two tests, red without it: seven iterators pass, made by the shell, by compiled code, of a class below Map, with the prototype taken
away; fourteen others, the prototype object and an heir of it among them, don't. Goes with Bun's half.
* AOT: a guard that is not available names what may have ended it
Behind GuardNotAvailable the verifier's detail says which guard it is and, of the operations in front of the use that may end what it
proves, the nearest six. It needs no printer of types, so a build under `--aot-check=verify-each-pass` says it too. A diagnostic alone.
* AOT: a cast may stand between the header's parameter and the step of a for-in
`bun build --compile --aot main.js` ended with exit 134, 'panic: abort() called', 'Stopped while compiling a function (main.js:4) in
ExpandOwnKeys'. Bun's main has it.
`function f1(L) { for (const q in L) { if (L[+q] === 2) return 1; } return 0; }`, declared in a callback that is kept and called later, and
called in a try in a loop over `const L = [2]`. A test's body is such a callback. The pass took the position that the step takes for a
parameter of the loop's header, which no rule of the IR says: a cast may stand between them.
* AOT: the iterator of a set is told by its cell
As "the iterator of a map is told by its cell", for %SetIterator%. Its lines in the same two tests: seven iterators of sets pass, whoever
made them; the iterators of maps and sixteen others don't. Goes with Bun's half.
* AOT: an object held from ahead of the mock never reads the mock
`import * as early from "./early.js"` at the head of a test file, then `vi.doMock("./dep.js", ..)`: early.js read the mock's values of dep.js
where `bun test` reads the file's, without an error: in one realm, in a run that is built in layers, with dep.js loaded by an earlier test
file. Of the two places where a mock can answer a read from outside, only the first respected that the module is read past its mock. Goes
with Bun's half.
* AOT: one forward walk builds a body anew in place of the compaction's five helpers
aot/ir/AOTForwardWalk.{h,cpp}: a walk that reads a body once, in an order in which a block stands behind the blocks that dominate it, and
builds the body anew while it reads. compactFunction() calls it, and the five static helpers that it replaces go. The order of the blocks is
today's. No output changes.
* AOT: an import of a module that compiled code began settles when it has run
main.mjs: `require("./mid.mjs")`; mid.mjs: `import "./a.mjs"; require("./b.mjs")`; b.mjs: `import("./mid.mjs").then(settled, settled)`.
`bun run` and `bun build --compile`: settled. Compiled ahead of time the promise never settled: exit 0, and nothing said. The same with
`await import("./mid.mjs")` in the entry, with `await` in an async function of b, and with two calls. Goes with Bun's half.
* AOT: the jobs behind a delivered text run ahead of the next file's body
Under `bun test --aot`, `import("./a.js").then(() => setTimeout(() => { import("./b.js"); /* thirty awaits, then a line */ }, 0))` ran b's
body ahead of the thirty jobs. `bun test` runs the jobs first, because a file's text comes in a later turn. It came with "an import cycle is
entered where the first import() call enters it"; Bun's main is wrong there too. Goes with Bun's half, which calls a new entry of the run
time.
* AOT: a call or a new in the place of what is constructed is quoted as Bun prints it
`const p = Object.create(K.prototype); new p()`. Plain: "K is not a constructor (evaluating 'new (Object.create(K.prototype))')". Compiled:
"K is not a constructor": the quote was missing, or wrong: 'new make', 'new new K', 'new q' for `new p.q()`. 71 of 158 forms differed. The
text of an error alone. Goes with Bun's half.
* AOT: a closure that a callee hands back keeps what it captured
`function lender(way, n) { let a = n * 2; const r = way(() => a++); a += 10; return r; }`, where `way` is one of fifteen functions at that
site and the one that runs is `function returnsIt(f) { return f; }`. The closure called later: plain prints `12 13`; compiled code printed a
wrong number or crashed. Goes with Bun's half.
* AOT: the recheck of guards scans for borrowed parameters only what it reads
Behind "the recheck of guards reads only what its functions reach": build time under the program verifier alone. Each of the recheck's two
readings still made the table of borrowed parameters from nothing, by a scan of every body of the program. Now the scan takes the bodies
below the functions under recheck, and the second reading takes the first one's table. `--aot-check=verify-each-pass` and a validating
build gain the second too.
* AOT: an import of a file of an image below walks its mocks once
`vi.doMock("./k.js", async () => { .. await .. }); vi.doMock("./m.js", async () => { .. }); await import("./a.js")`, where a.js imports k and
m, in a test file behind another test file whose mock's factory has run. `bun test`: k's factory begins, k's ends, m's begins, m's ends.
`bun test --aot` over two or three layers: k begins, m begins, m ends, k ends, without an error. For a file of an image below, import()
walked the mocks twice, and nobody read the second walk's answer.
* AOT: a copy of a parameter that holds -2 is not taken for 0
A wrong answer of an executable at default flags, without an error. It came with #982; Bun's main is right.
const T = [];
function f(a, b) {
let c = b;
if (!b) {
if (c !== b) T.push(1);
}
b == c;
b?.[0];
if (a == b) {
b -= 1;
c--;
if (typeof c === "object" && c !== null && "id" in c) T.push(2);
}
T.push(b, c);
}
f(1, 0); f(-2, -2); f(1, ""); f(null, undefined);
console.log(T.map(String).join(" "));
`bun run` prints `0 0 -3 -3 NaN NaN`; compiled code printed `0 0 -3 -1 NaN NaN`. While type tests are placed, a condition's first proof
stands for all of its proofs, and a cast took its claim from such a stand-in: the type inside `if (!b)`, `undefined|0`. A claim is taken
only from a value that stood before the run of the pass. Two IR tests, one with a planted fault. Goes with Bun's half.
* AOT: what is asked of the objects of every escaped class is asked once
Build time, quadratic in a made-up shape and next to nothing on a real program. `class A0 {} class B0 extends A0 {} globalThis["a" + 0] = B0`,
n times: every class that is out of sight has the objects of every such class among its own, and one step of the flow analysis read every
member of every layout's set: n layouts by n objects. Now a layout keeps its own objects and two bits, and what is asked of every member is
asked of each shared set once. The answers are joins, so the order can't move them. Goes with Bun's half.
* AOT: a test that runs the frames pass alone gives the verifier the tables that the pass published
The test tool's road for one pass hands the verifier no results of the flow analysis, so under `check=VerifyEachPass` it reported
CalleeRetainsFramePointer for a callee that AllocateInFrames had just proven to keep nothing. Where the verifier's inputs hold no flow they
get the pass's own table. In a build the inputs always hold it: no output changes. With an IR test.
* AOT: two system calls at the start of an executable, and only its own handler is changed
Behind "native code that runs out of stack in an executable is reported", which cost every start four `rt_sigaction` calls and one
`sigaltstack`: a compiled hello world made more system calls than a plain executable. The main thread is no longer asked whether it has an
alternate stack, because the host gave it one before the engine started; and only SIGSEGV is looked at, but on Darwin, where running into a
guard page raises SIGBUS.
`SA_SIGINFO` says nothing about whose an action is: a library that sets its own SIGSEGV handler had `SA_ONSTACK` forced on it. Now only a
handler that lies in the executable's own text is changed. Goes with Bun's half.
* AOT: the mocks of one handed-on namespace stand in a stack
It replaces the way of "the end of a mock gives back what the mock replaced". m1.js and m2.js each have `export * as ns from "./h.js"`; a test
lays mock A over m1, then mock B over m2.
- `vi.doUnmock("./m1.js")` while B stands: compiled code read `real | real`; `bun test` reads `B | B`.
- Then `vi.doUnmock("./m2.js")`: the end gave back what B had replaced, A's value, though A had ended. Every later test file of the realm
read A, and nothing cleared it.
The mocks that replace one namespace stand in a stack that is kept with the module whose object is handed on. The latest answers; an end
takes its own entry out wherever it sits; when nothing is left the module's own object is handed on. There plain `bun test` leaves the first
mock's object in place: a difference that is meant. Goes with Bun's half.
* AOT: a declared iteration may step the iterator itself
It mends "the iterator of a map is told by its cell" and "the iterator of a set is told by its cell", under `--aot-validate` alone: no answer
was wrong at default flags. Every validating executable ended by SIGABRT at `Array.from(map.values())` and `map.values().toArray()`:
"AOT native call: .. internal state changed, the entry writes , not InternalFields". An entry that says that it iterates its operand writes
nothing by itself; the write is `next`'s, and native code steps the iterator.
* AOT: fix the macOS build
AOTForwardWalk.cpp uses entryBlock without including AOTCFG.h, which declares
it. AOTModules.cpp writes ArrayAllocationProfile inside namespace JSC::AOT,
where the name can also mean JSC::AOT::ArrayAllocationProfile, so the call to
constructArray() found no overload.
Both compiled only because of what stood earlier in their unified-source
bundles. Adding a file moved the bundles on macOS.
This was referenced Nov 2, 2026
[JSC] A job of the module loader runs under the async context of the import() that asked for it
#941
Open
robobun
added a commit
that referenced
this pull request
Nov 3, 2026
main has eight commits since 4d45755, the commit this branch started at: #967, #971, #972, #974, #978, #979, #982 and #983. Two files conflict, each in one place. Both sides changed a line that calls one of WTF's atomic functions: this branch gave it the MemoryOrder template argument (WebKit/WebKit@6ab0aebc81), main changed the code beside it. - aot/analysis/AOTCallGraph.cpp: main's new loop over body.expectedFunctionTypes, with atomicLoad<MemoryOrder::Relaxed>() and atomicStore<MemoryOrder::Relaxed>(). - aot/runtime/AOTInstance.cpp: main's `if constexpr (hasFaultingLoopPolls)` in setPollsEnabled(), behind atomicStore<MemoryOrder::Relaxed>(). No conflict, and the same change: aot/valueflow/AOTFlowApply.cpp has three calls in the old form that main added. They take the template argument. The other 21 files that both sides changed merge as they are.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #979: 77 commits, one for each fix. Each commit message has the JavaScript that broke, what plain Bun does and what compiled code did. The tests and stored programs for these come with Bun's half.
It mends two faults that #979 brought, both in
bun test --aot, both without an error, both with "whoever holds a module that was linked to a mock keeps that mock". Over layers the next test file read a mock that had ended with the first: a.test.js:mock.module("pkg", ..); const { seen } = await import("./src/uses.js"); b.test.js:import { seen } from "./src/uses.js"; seen(); uses.js:import { f } from "pkg"; export const seen = () => f(): plainreal, compiledmocked: five lines of that commit are taken back. And a package that had got the original read a preload'svi.mock()aftervi.doUnmock()had ended it in an earlier test file: plainreal, compiledpreload: one line more. No build of Bun pins #979.All but 19 files are under
Source/JavaScriptCore/aot. Those 19 are listed at the end.On the review's finding that a mock over one module that has
export * as inner from "./inner"also changesinnerof every other module that re-exports it: plainbun testdoes the same. a.js and b.js eachexport * as inner from "./inner.js";mock.module("./a.js", () => ({ inner: { v: "mock of a" } })):a.inner.vandb.inner.vboth readmock of ain released Bun, in this branch's plainbun testand in this branch'sbun test --aot;--aoton main readsrealfor both. One difference is left, inbun test --aotalone: with both mocked,vi.doUnmock("./b.js")gives back the real namespace where plain gives back a's mock; on main neither is replaced at all.Crashes and wrong answers in executables
prev2 = prev; prev = curin a loop gave prev2 the new prevtryin a loop over[[], {}]ended the executablethisended the executablememory.toResizableBuffer()kept its old lengthBuilds that were refused
--aot-validaterefused valid programs with "internal compiler error": UpcastNotSubset after PlaceTypeTests--aot-validaterefused a valid program: a function given by one caller, a Proxy over a function by anotherB3
bun test --aotbun test --aotas one program settles import() calls as the executable of the same files doesvi.stubGlobaldefines its name on the global object however its function travelsBuilds with assertions or the exception-scope validator
--aot-check=verify-each-pass, a body that is split tells the checkBuild time
module.exportsasks under that name aloneVerifying builds; default builds do not change
--aot-check=verify-each-pass, a function that a pass leaves without values is looked at--aot-check=verify-each-pass, a pass that moves a function's body without saying so stops the build--aot-check=verify-each-pass, a store behind a check that nothing passes stopped a build--aot-check=verify-each-pass, a handler that only threw on is said to goHardening of the compiler
The same program gives the same code
Size
Planted faults, counters, logs and tests
--explain=statisticscounts the Map and Set lookups emitted in line, and--explain=missed-optimizationsnames someReview of the previous PRs
Outside
aot/runtime/AbstractModuleRecord.cpp, +1 -1: inisSettledByMock(), inside#if ENABLE(AOT), the function of its own that a run without compiled code never calls: two operands without effects change places.runtime/JSModuleLoader.h, +1 -0: inside#if ENABLE(AOT)("behind vi.resetModules() a failed import fails again in the same words"):hasResolutionFailure(), an inline const question about a table that the loader has already. Only aot/runtime/AOTModules.cpp asks it.Shared with the JIT
assembler/AssemblerPolicy.h, +2 -0: "a stop in compiled code ends the process also where SIGTRAP has a listener": a new field,trapsWithIllegalInstruction, false by default; onlyforImage()sets it.assembler/LinkBuffer.cpp, +4 -0: for the same commit as AssemblerPolicy.h: wheretrapsWithIllegalInstructionis set, allocate() pads to the allocation granule with nops, in front of the loop that pads with breakpoint().assembler/MacroAssemblerARM64.h, +4 -0: for the same commit as AssemblerPolicy.h: breakpoint() emits udf wheretrapsWithIllegalInstructionis set. The JIT's policy has it off.assembler/MacroAssemblerX86_64.h, +4 -0: for the same commit as AssemblerPolicy.h: breakpoint() emits ud2 wheretrapsWithIllegalInstructionis set. The JIT's policy has it off.assembler/testmasm.cpp, +22 -0: a test alone: it pins the bytes of breakpoint() under both policies.b3/B3FixSSA.cpp, +2 -0: "an ahead-of-time build stops where fixSSA would invent a value": two lines where fixSSA puts a zero for a variable that no Set reaches, behindoptimization.checksVariablesAreSet: false by default, set only by aot/codegen/AOTB3Policy.cpp.b3/B3FoldPathConstants.cpp, +21 -21: for the same commit as B3FixSSA.cpp: names alone.b3/B3KnownProperties.h, +8 -2: "a decoded narrow cell may be promised dereferenceable": the validator's rule takes one form more. Under B3's default policy no promise is ever stored, so the rule is never asked in the JIT.b3/B3Policy.h, +1 -0: for the same commit as B3FixSSA.cpp:validation.takesIdentitiesInHeadersForPhis, false by default; only a test sets it.b3/B3RegionCloner.cpp, +18 -2: "a copy that threadJumps makes for one way in forgets what was promised about it". The cloner is used by threadJumps, rotateLoops, versionLoops and unrollLoops alone (B3Generate.cpp's table of phases); each runs only under a field of the policy that is false by default and that the JIT does not set.b3/B3RegionCloner.h, +2 -0: for the same commit as B3RegionCloner.cpp.b3/B3ThreadJumps.cpp, +1 -1: for the same commit as B3RegionCloner.cpp; the phase runs only wherepolicy().optimization.threadsJumpsis set.b3/B3VersionLoops.cpp, +1 -1: for the same commit as B3FixSSA.cpp: one line readsvalidation.takesIdentitiesInHeadersForPhis, which brings an old fault back for a test. The phase runs only wherepolicy().loops.versionsis set, which the JIT does not set.b3/testb3_bun_known_properties.cpp, +56 -0: a test alone.b3/testb3_bun_random.cpp, +3 -0: a test alone.b3/testb3_bun_validate.h, +15 -0: a test alone.b3/testb3_bun_version_loops.cpp, +14 -2: a test alone.What was checked
testaotcompiles by itself there, without the precompiled prefix that Bun's graph gives it, as this repository's CMake compiles it, but for two (saturatingSumin AOTResolveAccesses.cpp,Optionsin AOTValueFlowAnalysis.cpp), which did not on AOT: crashes and miscompilations found since #971, Map/Set index slots, a bounds test on split() #972 or AOT: follow-up to #972: crashes, wrong answers, bun test --aot, build time #974 either. All 89 unified sources compile with-mms-bitfields, so the size assertions hold under MSVC's layout of bit-fields; a control with one assertion tightened fails.