Bug 222315 – Enable WASM on Windows

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Don Olmstead]

WASM works without FTL. Initial issue is around *nix specific calls to mprotect in the code but sure there's more issues than that.

[Radar WebKit Bug Importer]

<rdar://problem/74929318>

[Fujii Hironori]

Playwright: https://github.com/microsoft/playwright/issues/2876

[Ian Grunert]

In WebAssembly.asm there's some uses of register ws1 (which maps to t6) which will need to be changed (e.g. inside wasmPrologue()).

[Ian Grunert]

Created attachment 467508 [details]
Patch

[Ian Grunert]

Added a patch that builds on top of https://bugs.webkit.org/show_bug.cgi?id=260869 - with these changes WebKit compiles with WEBASSEMBLY enabled on Windows on my machine.

Minibrowser is currently segfaulting on WebAssembly.asm:392 when running a simple wasm example, e.g. https://mdn.github.io/webassembly-examples/js-api-examples/index.html

I'm a little stuck as to why it's segfaulting there. It looks like WTFConfig is getting linked correctly, unless there's additional trickery needed to import from a DLL exported variable that I'm missing.

Based on the build failures it looks like some of my x86 offlineasm changes need work as well.

[Ross Kirsling]

Comment on attachment 467508 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=467508&action=review

> Source/JavaScriptCore/offlineasm/x86.rb:144
> +        "$#{c}"

The build failures are because you need an explicit return here.

> Source/JavaScriptCore/wasm/WasmStreamingParser.cpp:-293
> -#else
> -    (void) dumpWasmSource;

This does have an effect on non-Win builds, so you'll also need to add `&& ASSERT_ENABLED` to the definition of dumpWasmSource.

[Darin Adler]

Comment on attachment 467508 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=467508&action=review

>> Source/JavaScriptCore/wasm/WasmStreamingParser.cpp:-293
>> -    (void) dumpWasmSource;
> 
> This does have an effect on non-Win builds, so you'll also need to add `&& ASSERT_ENABLED` to the definition of dumpWasmSource.

Please post a new patch that fixes this issue.

[Darin Adler]

Comment on attachment 467508 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=467508&action=review

>> Source/JavaScriptCore/offlineasm/x86.rb:144
>> +        "$#{c}"
> 
> The build failures are because you need an explicit return here.

I should have said "that fixes both of these issues".

[Ian Grunert]

Created attachment 467517 [details]
Patch

[Ian Grunert]

Added a patch with those two fixes. The larger problem remains; on Windows we hit a segfault in wasmPrologue on WebAssembly.asm:392 when running a simple wasm example.

I did find we were referencing the wrong register for PB on Windows in `Source/JavaScriptCore/wasm/WasmCallee.cpp`, though changing that didn't fix the segfault.

[Ian Grunert]

I can take a look at those JSC test failures next week. Though it does look like it's a subset of the overall wasm tests so it's very unlikely to be the same failure mode I'm seeing on Windows.

[Ian Grunert]

Created attachment 467540 [details]
Patch

[Darin Adler]

Comment on attachment 467540 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=467540&action=review

> Source/JavaScriptCore/wasm/WasmSlowPaths.cpp:764
> +        WASM_RETURN_TWO(bitwise_cast<void*>(static_cast<uintptr_t>(1L)), bitwise_cast<void*>(static_cast<uintptr_t>(Wasm::ExceptionType::OutOfBoundsTableAccess)));

This can just be 1 instead of 1L now that we are casting it to an integer type.

[Darin Adler]

Comment on attachment 467540 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=467540&action=review

>> Source/JavaScriptCore/wasm/WasmSlowPaths.cpp:764
>> +        WASM_RETURN_TWO(bitwise_cast<void*>(static_cast<uintptr_t>(1L)), bitwise_cast<void*>(static_cast<uintptr_t>(Wasm::ExceptionType::OutOfBoundsTableAccess)));
> 
> This can just be 1 instead of 1L now that we are casting it to an integer type.

This can just be 1 instead of 1L now that we are casting it to an integer type.

[Ross Kirsling]

Is it a correct understanding that the current patch still has the segfault on Windows?

[Ian Grunert]

Yep the segfault remains on Windows, the change to fix those test failures was in AT&T assembly output only (in the const0x function).

I'm tempted to try Intel assembly output on the Mac. That'd help validate the changes made to offlineasm for Intel output. Though I don't see anything obviously wrong in the assembly output, most of these instructions didn't change:

```
  _wasm_function_prologue::
    push rbp                             ; LowLevelInterpreter.asm:850
    mov rbp, rsp                         ; LowLevelInterpreter.asm:856
    sub rsp, 32                          ; WebAssembly.asm:256
    mov qword ptr [-8 + rbp], r13        ; WebAssembly.asm:260
    mov qword ptr [-16 + rbp], rbx       ; WebAssembly.asm:261
    mov r12, qword ptr [40 + rbx]        ; WebAssembly.asm:331
    mov r13, qword ptr [48 + rbx]        ; WebAssembly.asm:332
    mov qword ptr [16 + rbp], rbx        ; WebAssembly.asm:381
    mov rax, qword ptr [24 + rbp]        ; WebAssembly.asm:382
    and rax, -4                          ; WebAssembly.asm:384
    lea r11, __imp_g_wtfConfig           ; WebAssembly.asm:386
    mov r11, qword ptr [0 + r11]         ; WebAssembly.asm:387
    add rax, r11                         ; WebAssembly.asm:388
    mov qword ptr [-24 + rbp], rax       ; WebAssembly.asm:389
    mov r11d, dword ptr [48 + rax]       ; WebAssembly.asm:392 <-- segfault
```

[Ian Grunert]

Pull request: https://github.com/WebKit/WebKit/pull/18184

[EWS]

Committed 270429@main (937fd6bd0489): <https://commits.webkit.org/270429@main>

Reviewed commits have been landed. Closing PR #18184 and removing active labels.