Bug 263965

Summary: Concurrency bug in WebAssembly LLInt compilation
Product: WebKit Reporter: Ian Grunert <ian.grunert>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: Hironori.Fujii, justin_michaud, keith_miller, mark.lam, max, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=267686
Attachments:
Description Flags
Stack trace
none
workaround patch
none
WIP patch
none
WIP patch justin_michaud: review+

Ian Grunert
Reported 2023-10-31 07:25:25 PDT
On Windows, I reliably hit a heap corruption error on the release build when running the async clock yew wasm example (https://examples.yew.rs/async_clock/). The heap corruption is discovered when resizing a vector for the microtasks queue. It looks like it's a concurrency bug in WebAssembly LLInt compilation, which triggers on my machine because it has 16 cores / 24 threads. It doesn't happen if I reduce the numberOfWasmCompilerThreads in OptionsList.h. Might be something in WebAssembly LLInt compilation using a shared Vector without a lock. I suspect this isn't a Windows specific issue and may impact all platforms.
Attachments
Stack trace (3.92 KB, text/plain)
2023-10-31 09:26 PDT, Ian Grunert 		no flags
Details
workaround patch (839 bytes, patch)
2023-12-13 17:37 PST, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
WIP patch (560 bytes, patch)
2023-12-13 20:54 PST, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
WIP patch (1.05 KB, patch)
2023-12-14 15:58 PST, Fujii Hironori 		justin_michaud: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ian Grunert
Comment 1 2023-10-31 09:26:47 PDT
Created attachment 468427 [details] Stack trace
Radar WebKit Bug Importer
Comment 2 2023-11-07 06:26:13 PST
<rdar://problem/118054777>
Ian Grunert
Comment 3 2023-12-12 18:29:20 PST
I spent some time looking at this today. On a release build, I was able to repro this even with a single wasm compiler thread. With the logging enabled for WasmEntryPlan and WasmWorklist, on a single thread it was able to complete wasm compilation - crashed afterwards with the same stack trace as before. Couldn't repro on Gnome Web running under WSL. No problems running JetStream2, I wonder if it's something to do with the number of functions involved (520).
Fujii Hironori
Comment 4 2023-12-13 17:37:50 PST
Created attachment 469029 [details] workaround patch
Fujii Hironori
Comment 5 2023-12-13 20:54:16 PST
Created attachment 469035 [details] WIP patch
Fujii Hironori
Comment 6 2023-12-14 15:58:16 PST
Created attachment 469050 [details] WIP patch
Justin Michaud
Comment 7 2023-12-14 16:01:32 PST
@Fujii Hironori Nice catch! r=me
Fujii Hironori
Comment 8 2023-12-14 17:06:55 PST
I'm not confident this is clang's bug. This might be a JSC bug. JSC may break callee saved registers.
Ian Grunert
Comment 9 2024-08-14 14:07:53 PDT
The example site is broken now, I raised a bug against yew https://github.com/yewstack/yew/issues/3694 If you disable subresource integrity checking, the example loads and no longer hits a heap corruption error.
Note You need to log in before you can comment on or make changes to this bug.