Bug 222315

<rdar://problem/74929318>

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

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

Created attachment 467508 [details] Patch

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.

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.

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.

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".

Created attachment 467517 [details] Patch

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.

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.

Created attachment 467540 [details] Patch

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.

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.

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

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 ```

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

Committed 270429@main (937fd6bd0489): <https://commits.webkit.org/270429@main> Reviewed commits have been landed. Closing PR #18184 and removing active labels.