Bug 258302

Summary: WebAssembly SIMD results in incorrect float values and non working bitwise operations
Product: WebKit Reporter: John Hankinson <john>
Component: WebAssemblyAssignee: Justin Michaud <justin_michaud>
Status: RESOLVED FIXED    
Severity: Major CC: justin_michaud, karlcow, mark.lam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: BrowserCompat, InRadar
Version: Safari 16   
Hardware: iPhone / iPad   
OS: iOS 16   
Attachments:
Description Flags
Bad disassembly
none
Disable sandbox to load allow list none

John Hankinson
Reported 2023-06-20 09:18:19 PDT
I've not yet been able to narrow down the exact instructions that cause the issues, but something is very broken. I've noticed incorrect float values after some arithmetic operations, and my suspicion is that something with package integer bitwise ops and comparisons is not working as expected. The same code works on Chrome/Android/PC/Edge and Safari on Mac. (IE. everything else). The following site demonstrates the issues: https://www.terraspace.co.uk For which the output is identical everywhere except iOS 16.x (since wasm simd supported landed). Blocks of 4x pixels are not masking and updating as they should be.
Attachments
Bad disassembly (98.52 KB, text/plain)
2023-07-05 10:51 PDT, Justin Michaud
no flags
Disable sandbox to load allow list (2.56 KB, patch)
2023-07-05 10:51 PDT, Justin Michaud
no flags
Radar WebKit Bug Importer
Comment 2 2023-06-20 10:46:55 PDT
Justin Michaud
Comment 3 2023-06-20 12:06:33 PDT
I wonder if this is already fixed by https://bugs.webkit.org/show_bug.cgi?id=257842
John Hankinson
Comment 4 2023-06-20 13:06:48 PDT
The description of the fix certainly sounds very similar to the issue encountered. Will this be landing in 16.6 (possibly in the beta)?
Justin Michaud
Comment 5 2023-06-20 14:48:08 PDT
(In reply to John Hankinson from comment #4) > The description of the fix certainly sounds very similar to the issue > encountered. Will this be landing in 16.6 (possibly in the beta)? Apple does not comment on the content of future releases. I checked with a nightly though, and it seems that the bug still reproduces. Thanks for the links to other issues, this is super helpful!
Justin Michaud
Comment 6 2023-06-20 18:43:15 PDT
Is there any chance I can get source for a smaller test case that reproduces this, or source for your site itself? It would be super helpful for bisecting the root cause. On my end, I have confirmed that in an nightly build, turning off our highest tier fixes this bug: ``` 265333@main % __XPC_JSC_useOMGJIT=0 ./run-webkit-archive ``` setting defaultB3OptLevel=0 or disabling wasm call inlining does not fix the bug. Next I would like to bisect by function index.
John Hankinson
Comment 7 2023-06-21 08:23:23 PDT
I've emailed you the WAT functions involved, the source is quite long, but I think we can narrow it down to a single function as they all perform similar operations. Interestingly, I have been informed, although cannot verify it myself, that this issue is not present on an M1 mac, which I would have expected to generate the same aarch64 output from codegen as the mobile soc?
John Hankinson
Comment 8 2023-06-23 02:01:40 PDT
Let me know if there is anything I can help with re. the WAT functions or testing.
John Hankinson
Comment 9 2023-06-29 09:08:48 PDT
Are there any updates on progress with this yet?
Justin Michaud
Comment 10 2023-07-05 10:50:42 PDT
```__XPC_JSC_validateOptions=1 __XPC_JSC_dumpOMGDisassembly=1 __XPC_JSC_omgAllowlist=/Users/justin_michaud/Downloads/265333@main/allow.txt __XPC_JSC_useWebAssemblyOSR=0 __XPC_JSC_useConcurrentJIT=0 __XPC_JSC_maximumWasmDepthForInlining=0 __XPC_JSC_maximumWasmCalleeSizeForInlining=0 DYLD_FRAMEWORK_PATH=/Volumes/WebKit/ReleaseVersion/OpenSource/WebKitBuild/Release __XPC_DYLD_FAMEWORK_PATH=/Volumes/WebKit/ReleaseVersion/OpenSource/WebKitBuild/Release /Volumes/WebKit/ReleaseVersion/OpenSource/WebKitBuild/Release/Safari.app/Contents/MacOS/Safari -ExtensionsEnabled NO``` with sandbox disabled (see patch) and allow list: ``` 12 ``` reproduces the issue, producing the attached assembly
Justin Michaud
Comment 11 2023-07-05 10:51:02 PDT
Created attachment 466936 [details] Bad disassembly
Justin Michaud
Comment 12 2023-07-05 10:51:18 PDT
Created attachment 466937 [details] Disable sandbox to load allow list
Justin Michaud
Comment 13 2023-10-04 16:34:59 PDT
The culprit: (call $probe_begin (i32.const 22)) (call $probe (i64x2.extract_lane 0 (local.get $l25))) (call $probe (i64x2.extract_lane 1 (local.get $l25))) (call $probe (i64x2.extract_lane 0 (local.get $l63))) (call $probe (i64x2.extract_lane 1 (local.get $l63))) (call $probe (i64x2.extract_lane 0 (local.get $l42))) (call $probe (i64x2.extract_lane 1 (local.get $l42))) (call $probe32 (local.get $p0)) (call $probe (i64x2.extract_lane 0 (v128.load offset=16 (local.get $p0)))) (call $probe (i64x2.extract_lane 1 (v128.load offset=16 (local.get $p0)))) (i32.eqz (v128.any_true (local.get $l42))) (local.set $debug32) (call $probe32 (local.get $debug32)) (v128.not (local.get $l42)) <---------------------------------------Here is the first difference (local.set $debug128) (call $probe (i64x2.extract_lane 0 (local.get $debug128))) (call $probe (i64x2.extract_lane 1 (local.get $debug128))) (call $probe_end (i32.const 22))
Justin Michaud
Comment 14 2023-10-08 11:51:24 PDT
EWS
Comment 15 2023-10-09 09:34:00 PDT
Committed 269080@main (f9a3a2147af0): <https://commits.webkit.org/269080@main> Reviewed commits have been landed. Closing PR #18827 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.