[JSC] Wasm: offset the jitless marshalling frames in bytes, not by an array index by robobun · Pull Request #617 · oven-sh/WebKit

robobun · 2026-09-10T11:10:02Z

Problem

On a sanitizer build with the JIT off, the first wasm call to a JS import segfaults: AddressSanitizer: SEGV on unknown address, #0 operationWasmToJSExitMarshalArguments WasmOperations.cpp:320. The faulting instruction is the shadow check movabs $0x200000007fff7fff,%rax; cmpb $0x0,(%rax), which runs before the real slot load.
The cause is the access lambda in that function: &reinterpret_cast<V*>(arr)[i / sizeof(V)]. i is an int and sizeof(V) is a size_t, so the division is unsigned. WasmToJSCallableFunctionSlot is -0x8 (WasmCallingConvention.h:48), and the index becomes 0x1FFFFFFFFFFFFFFF. The pointer wraps back to arr - 8, so a normal build reads the right slot, but the access is out of bounds and clang folds its shadow address into a constant.
43 existing JSTests/wasm files die on that read in a --useJIT=0 run of an ASan build.

Fix

The four access lambdas take a ptrdiff_t byte offset and offset the pointer in bytes. There is no division, so a negative offset stays negative.
Every call site already passes a byte offset that is a multiple of sizeof(V), so each address is unchanged. Two of the lambdas also take negative offsets from calleeSPOffsetFromFP: the JS-to-wasm entry wrapper writes the callee's stack arguments below the frame pointer.
Verified on a JSCOnly RelWithDebInfo + ASan build (clang 21.1.8). JSTests/wasm/stress/jitless-call-js-import.js (new) segfaults before the change and passes after. A --useJIT=0 run of wasm/function-tests, wasm/js-api, wasm/noJIT, wasm/references, wasm/gc and wasm/function-references (237 files, the new test among them) fixes 43 and regresses none.

Background

Without the JIT there is no compiled entry thunk for a call across the wasm and JS boundary. The LLInt wrapper calls these C++ operations instead, and they copy the arguments and the results between the two frames by hand.
WasmToJSCallableFunctionSlot is the frame slot where the wasm-to-JS wrapper stores the callable function. It is at -0x8, below the frame pointer (InPlaceInterpreter.asm:1040).
AddressSanitizer checks a shadow byte at (address >> 3) + 0x7fff8000 before each load. 0x200000007fff7fff is that map applied to 0xFFFFFFFFFFFFFFF8, the out-of-bounds address before it wraps.

Notes

Repro, 42 bytes of wasm: (import "e" "f" (func)) (func (export "g") call 0).

const bytes = new Uint8Array([0,0x61,0x73,0x6d,1,0,0,0, 1,4,1,0x60,0,0, 2,7,1,1,0x65,1,0x66,0,0, 3,2,1,0, 7,5,1,1,0x67,0,1, 10,6,1,4,0,0x10,0,0x0b]);
const { g } = new WebAssembly.Instance(new WebAssembly.Module(bytes), { e: { f() {} } }).exports;
g();
print("ok");

jsc --useJIT=0 repro.js on the ASan build: SEGV before the change, ok after.

The fold is compiler dependent. clang 17 emits the correct shadow check for the old lambda. clang 21.1.8 folds it. Without -fsanitize=address, clang 21.1.8 compiles the old lambda to mov -0x8(%rdi),%rax, so a release build is correct. That is why only sanitizer builds see this.

The 43 newly passing files cover the import call in both directions: function-tests/many-arguments-to-function.js and many-args-tail-call-sp-restored.js for stack arguments, function-tests/i64-conversion.js and gc/wasm-js-marshal-i31-return.js for the value conversions, js-api/export-arity.js for the argument count slot. 16 other files exit non-zero in the same way before and after the change, because running them bare skips the options the harness passes (js-api/memory64-js-api.js and similar).

The new test runs each block wasmTestLoopCount times, which the shell sets to 100 with the JIT off. It takes 0.13 s on the ASan build.

Upstream WebKit/WebKit main has the same four lambdas, unchanged, so the bug is there too. Its access call sites are identical to ours.

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

coderabbitai · 2026-09-10T11:27:24Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Essentials

Run ID: af1ab737-000d-46cd-99ca-4ee8776f27de

📥 Commits

Reviewing files that changed from the base of the PR and between dfd6964 and c41fd31.

📒 Files selected for processing (2)

JSTests/wasm/noJIT/call-js-import.js
Source/JavaScriptCore/wasm/WasmOperations.cpp

Included review availability: Your plan provides up to 5 included reviews per hour; 1 remains after this review.

Walkthrough

Changes

The change updates WebAssembly frame access to interpret locations as signed byte offsets. A no-JIT test validates JavaScript import calls, return values, and mixed-type stack argument marshalling.

WebAssembly marshalling

Layer / File(s)	Summary
Signed frame access `Source/JavaScriptCore/wasm/WasmOperations.cpp`	JS-to-Wasm and Wasm-to-JS argument and return helpers now use signed byte-offset pointer arithmetic.
No-JIT import validation `JSTests/wasm/noJIT/call-js-import.js`	The test covers repeated zero-argument calls and mixed-type calls with many arguments, return values, call counts, and received arguments.

Merge Risk: ⚪ Minimal · up to c41fd

The fix preserves wasm frame addresses while preventing invalid unsigned offset handling, and added no-JIT import coverage passes. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the main fix: changing jitless WebAssembly marshalling frame offsets from array-index addressing to byte offsets.
Description check	✅ Passed	The description is detailed and covers the bug, root cause, implementation, regression test, affected files, and verification results. It does not include the template's formal Bugzilla link or review…

Warning

Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use path_filters to narrow the review scope.

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-09-10T11:39:04Z

Preview Builds

Commit	Release	Date
`9b1532ce`	`autobuild-preview-pr-617-9b1532ce`	2026-09-10 12:19:11 UTC
`c41fd31a`	`autobuild-preview-pr-617-c41fd31a`	2026-09-10 11:39:03 UTC

… array index With the JIT off, a call between wasm and JS is marshalled by the operations in WasmOperations.cpp. Each one has an `access` lambda that returns a frame slot. The lambda took the offset as an `int` and turned it into an array index: `&reinterpret_cast<V*>(arr)[i / sizeof(V)]`. `sizeof(V)` is a `size_t`, so the division converts the offset to an unsigned value first. `WasmToJSCallableFunctionSlot` is -0x8, so the index becomes 0x1FFFFFFFFFFFFFFF. The pointer arithmetic wraps to the right address in a normal build, but it is out of bounds, and an instrumented build folds the AddressSanitizer shadow check for it into a constant address. The first wasm call to a JS import then reads that address and segfaults in operationWasmToJSExitMarshalArguments. The JS-to-wasm entry wrapper passes negative offsets too: it writes the callee's stack arguments below the frame pointer. Take the offset as a `ptrdiff_t` and offset the pointer in bytes. Every call site already passes a byte offset that is a multiple of `sizeof(V)`, so the address does not change. JSTests/wasm/stress/jitless-call-js-import.js covers a jitless call to a JS import, with and without stack arguments.

claude

Code review found no issues

No high-confidence issues detected in this change.

claude Bot reviewed Sep 10, 2026

View reviewed changes

Comment thread JSTests/wasm/noJIT/call-js-import.js Outdated

robobun force-pushed the robobun/17955cfd/wasm-jitless-frame-byte-offset branch from c41fd31 to 9b1532c Compare September 10, 2026 11:50

claude Bot reviewed Sep 10, 2026

View reviewed changes

Navigation Menu

Uh oh!

[JSC] Wasm: offset the jitless marshalling frames in bytes, not by an array index - #617

[JSC] Wasm: offset the jitless marshalling frames in bytes, not by an array index#617
robobun wants to merge 1 commit into
mainfrom
robobun/17955cfd/wasm-jitless-frame-byte-offset

robobun commented Sep 10, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 10, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

robobun commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Background

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

github-actions Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Preview Builds

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented Sep 10, 2026 •

edited

Loading

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading

github-actions Bot commented Sep 10, 2026 •

edited

Loading