WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
236037
Wasm crash on
https://copy.sh/v86/?profile=dsl
https://bugs.webkit.org/show_bug.cgi?id=236037
Summary
Wasm crash on https://copy.sh/v86/?profile=dsl
Yusuke Suzuki
Reported
2022-02-02 13:54:55 PST
And in the other examples in
https://github.com/copy/v86
Attachments
asm.txt
(16.18 MB, text/plain)
2022-02-06 20:42 PST
,
Saam Barati
no flags
Details
quick-and-dirty-fix
(2.82 KB, patch)
2022-02-06 21:05 PST
,
Saam Barati
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
patch
(5.32 KB, patch)
2022-02-07 15:57 PST
,
Saam Barati
mark.lam
: review+
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
patch for landing
(5.33 KB, patch)
2022-02-07 18:16 PST
,
Saam Barati
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1
2022-02-03 16:45:22 PST
rdar://88358719
Saam Barati
Comment 2
2022-02-04 10:06:40 PST
crash in
https://copy.sh/v86/?profile=dsl
Saam Barati
Comment 3
2022-02-04 11:38:54 PST
Turning off OMG makes the crash go away. Will try to configure more options to see what reproduces and what doesn't.
Saam Barati
Comment 4
2022-02-04 11:58:07 PST
it seems like an issue with OMG+fast memory. If I turn on OMG, and turn off fast memory, I don't crash.
Saam Barati
Comment 5
2022-02-04 15:42:31 PST
Reproduces with concurrent JIT off.
Saam Barati
Comment 6
2022-02-04 15:43:15 PST
Reproduces with OSR entry turned off
Saam Barati
Comment 7
2022-02-05 11:03:21 PST
seems like we need to be running with webAssemblyOMGOptimizationLevel=2 for this to reproduce. It also doesn't seem to be a bug in IRC, since running with O1 and forcing on IRC does not crash either
Yusuke Suzuki
Comment 8
2022-02-05 11:15:43 PST
(In reply to Saam Barati from
comment #7
)
> seems like we need to be running with webAssemblyOMGOptimizationLevel=2 for > this to reproduce. It also doesn't seem to be a bug in IRC, since running > with O1 and forcing on IRC does not crash either
Nice! In that case, typically I comment out several path in B3Generate.cpp to see which pass can trigger this problem :) 84 if (procedure.optLevel() >= 2) { 85 reduceDoubleToFloat(procedure); 86 reduceStrength(procedure); 87 // FIXME: Re-enable B3 hoistLoopInvariantValues 88 //
https://bugs.webkit.org/show_bug.cgi?id=212651
89 if (Options::useB3HoistLoopInvariantValues()) 90 hoistLoopInvariantValues(procedure); 91 if (eliminateCommonSubexpressions(procedure)) 92 eliminateCommonSubexpressions(procedure); 93 eliminateDeadCode(procedure); 94 inferSwitches(procedure); 95 reduceLoopStrength(procedure); 96 if (Options::useB3TailDup()) 97 duplicateTails(procedure); 98 fixSSA(procedure); 99 foldPathConstants(procedure); 100 // FIXME: Add more optimizations here. 101 //
https://bugs.webkit.org/show_bug.cgi?id=150507
Saam Barati
Comment 9
2022-02-06 20:33:38 PST
(In reply to Yusuke Suzuki from
comment #8
)
> (In reply to Saam Barati from
comment #7
) > > seems like we need to be running with webAssemblyOMGOptimizationLevel=2 for > > this to reproduce. It also doesn't seem to be a bug in IRC, since running > > with O1 and forcing on IRC does not crash either > > Nice! In that case, typically I comment out several path in B3Generate.cpp > to see which pass can trigger this problem :) > > 84 if (procedure.optLevel() >= 2) { > 85 reduceDoubleToFloat(procedure); > 86 reduceStrength(procedure); > 87 // FIXME: Re-enable B3 hoistLoopInvariantValues > 88 //
https://bugs.webkit.org/show_bug.cgi?id=212651
> 89 if (Options::useB3HoistLoopInvariantValues()) > 90 hoistLoopInvariantValues(procedure); > 91 if (eliminateCommonSubexpressions(procedure)) > 92 eliminateCommonSubexpressions(procedure); > 93 eliminateDeadCode(procedure); > 94 inferSwitches(procedure); > 95 reduceLoopStrength(procedure); > 96 if (Options::useB3TailDup()) > 97 duplicateTails(procedure); > 98 fixSSA(procedure); > 99 foldPathConstants(procedure); > 100 // FIXME: Add more optimizations here. > 101 //
https://bugs.webkit.org/show_bug.cgi?id=150507
Yeah, I've been doing this. No luck so far. Though commenting out some stuff seems to change how frequently the crash reproduces.
Saam Barati
Comment 10
2022-02-06 20:40:42 PST
Unrelated, but we emit some silly code we should fix: ``` 0x120b5282c: str x0, [sp, #17128] 0x120b52830: ldr x0, [sp, #17128] ```
Saam Barati
Comment 11
2022-02-06 20:42:50 PST
Created
attachment 451062
[details]
asm.txt Attaching asm of a function we're crashing in. Noted which instruction we're crashing in with "CRASH". We have this code sequence: 0x120b52850: ldr x0, [sp, #17096] 0x120b52854: add x0, x22, x0 0x120b52858: movz x16, #0xc640 0x120b5285c: add x16, sp, x16, uxtx 0x120b52860: ldur x1, [x16] 0x120b52864: ldr w20, [x0, x1] <--- CRASH The interesting thing here is I can't find where we store to [sp, #17096], which makes me wonder if it's uninitialized. But there's a chance we're storing it somehow that I haven't been able to figure out by statically looking at the asm.
Saam Barati
Comment 12
2022-02-06 20:46:41 PST
(In reply to Saam Barati from
comment #11
)
> Created
attachment 451062
[details]
> asm.txt > > Attaching asm of a function we're crashing in. Noted which instruction we're > crashing in with "CRASH". > > We have this code sequence: > > 0x120b52850: ldr x0, [sp, #17096] > 0x120b52854: add x0, x22, x0 > 0x120b52858: movz x16, #0xc640 > 0x120b5285c: add x16, sp, x16, uxtx > 0x120b52860: ldur x1, [x16] > 0x120b52864: ldr w20, [x0, x1] <--- CRASH > > > The interesting thing here is I can't find where we store to [sp, #17096], > which makes me wonder if it's uninitialized. But there's a chance we're > storing it somehow that I haven't been able to figure out by statically > looking at the asm.
Nevermind, was happening right in front of my face: 0x120b52840: movz x16, #0x42c8 0x120b52844: add x16, sp, x16, uxtx 0x120b52848: stur w0, [x16] 0x120b5284c: stur wzr, [x16] in: 0x120b52840: movz x16, #0x42c8 0x120b52844: add x16, sp, x16, uxtx 0x120b52848: stur w0, [x16] 0x120b5284c: stur wzr, [x16] 0x120b52850: ldr x0, [sp, #17096] 0x120b52854: add x0, x22, x0 0x120b52858: movz x16, #0xc640 0x120b5285c: add x16, sp, x16, uxtx 0x120b52860: ldur x1, [x16] 0x120b52864: ldr w20, [x0, x1] <--- CRASH But it seems like we're storing zero to it right after storing something else. So weird.
Saam Barati
Comment 13
2022-02-06 20:51:41 PST
Check out these first two moves of constants followed by stores: 0x120b5283c: movz x16, #0x42cc 0x120b52840: movz x16, #0x42c8 0x120b52844: add x16, sp, x16, uxtx 0x120b52848: stur w0, [x16] 0x120b5284c: stur wzr, [x16] 0x120b52850: ldr x0, [sp, #17096] 0x120b52854: add x0, x22, x0 0x120b52858: movz x16, #0xc640 0x120b5285c: add x16, sp, x16, uxtx 0x120b52860: ldur x1, [x16] 0x120b52864: ldr w20, [x0, x1] <--- CRASH Can't get over the fact that it seems like we shouldn't be moving into x16 back to back like that, and somehow the code should've been ordered differently so we store to both locations independently.
Saam Barati
Comment 14
2022-02-06 20:56:18 PST
I'm pretty suspicious of a bug inside AirLowerStackArgs.
Saam Barati
Comment 15
2022-02-06 21:02:11 PST
(In reply to Saam Barati from
comment #14
)
> I'm pretty suspicious of a bug inside AirLowerStackArgs.
I found a bug inside AirLowerStackArgs. But I don't know if it's our crash or not.
Saam Barati
Comment 16
2022-02-06 21:05:32 PST
Created
attachment 451064
[details]
quick-and-dirty-fix this is the bug I spotted. Will make a nicer patch once I test it.
Saam Barati
Comment 17
2022-02-06 21:30:17 PST
(In reply to Saam Barati from
comment #16
)
> Created
attachment 451064
[details]
> quick-and-dirty-fix > > this is the bug I spotted. Will make a nicer patch once I test it.
This is the right fix. It also fixes a bug where the site just hangs when loading. We used to crash or hang 100% of the time, and this fixes that issue. However, I did encounter one crash that I still need to investigate. The crash happened ~1/20 loads. So maybe it's a different issue. I'll upload a patch to fix this tomorrow. Ima try to think about how to write a test case, too. It might be tricky. I'll open a separate bug to investigate the other issue tomorrow.
Saam Barati
Comment 18
2022-02-07 15:57:44 PST
Created
attachment 451165
[details]
patch
Mark Lam
Comment 19
2022-02-07 17:11:59 PST
Comment on
attachment 451165
[details]
patch View in context:
https://bugs.webkit.org/attachment.cgi?id=451165&action=review
r=me
> Source/JavaScriptCore/ChangeLog:22 > + mov x16, k > + mov x16, k2 > + mov x1, [x16] > + mov zr, [x16]
Did you mean this instead? ``` mov k, x16 mov k2, x16 mov x1, [x16] mov zr, [x16] ``` I find this example confusing: the first 2 instructions imply the dest is the left operand, while the last 2 instructions imply that the dest is the right operand. Can you make it consistent?
> Source/JavaScriptCore/ChangeLog:28 > + mov x16, k > + mov x1, [x16] > + mov x16, k2 > + mov zr, [x16]
Ditto.
Saam Barati
Comment 20
2022-02-07 18:12:13 PST
Comment on
attachment 451165
[details]
patch View in context:
https://bugs.webkit.org/attachment.cgi?id=451165&action=review
>> Source/JavaScriptCore/ChangeLog:22 >> + mov zr, [x16] > > Did you mean this instead? > ``` > mov k, x16 > mov k2, x16 > mov x1, [x16] > mov zr, [x16] > ``` > I find this example confusing: the first 2 instructions imply the dest is the left operand, while the last 2 instructions imply that the dest is the right operand. Can you make it consistent?
We're moving constants k and k2 into x16. Let me switch to using arm syntax and it'll clear things up.
Saam Barati
Comment 21
2022-02-07 18:16:24 PST
Created
attachment 451189
[details]
patch for landing
EWS
Comment 22
2022-02-07 19:01:40 PST
Committed
r289354
(
246942@main
): <
https://commits.webkit.org/246942@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 451189
[details]
.
Saam Barati
Comment 23
2022-02-07 21:56:49 PST
<
rdar://88611690
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug