Bug 197855

Summary: Bound liveness of SetArgumentMaybe nodes when maximal flush insertion phase is enabled
Product: WebKit Reporter: Tadeu Zagallo <tzagallo>
Component: JavaScriptCoreAssignee: Saam Barati <sbarati>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, sbarati, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
maybe patch
none
Archive of layout-test-results from ews210 for win-future
none
patch msaboff: review+

Description Tadeu Zagallo 2019-05-13 15:36:53 PDT
...
Comment 1 Tadeu Zagallo 2019-05-13 15:47:49 PDT
Created attachment 369794 [details]
Patch
Comment 2 Tadeu Zagallo 2019-05-13 15:48:10 PDT
<rdar://problem/50236506>
Comment 3 Saam Barati 2019-05-13 16:35:31 PDT
I thought about this a bit, and I'd be sad for this to go. It's helped us find a lot of bugs. Posting an alternative patch here that fixes the crash in radar
Comment 4 Saam Barati 2019-05-13 16:35:50 PDT
Created attachment 369802 [details]
maybe patch
Comment 5 EWS Watchlist 2019-05-14 00:09:36 PDT
Comment on attachment 369794 [details]
Patch

Attachment 369794 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12185088

New failing tests:
fast/shadow-dom/svg-use-href-change-in-shadow-tree.html
Comment 6 EWS Watchlist 2019-05-14 00:09:40 PDT
Created attachment 369823 [details]
Archive of layout-test-results from ews210 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews210  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 7 Tadeu Zagallo 2019-05-14 02:03:17 PDT
(In reply to Saam Barati from comment #3)
> I thought about this a bit, and I'd be sad for this to go. It's helped us
> find a lot of bugs. Posting an alternative patch here that fixes the crash
> in radar

I think it's a bit unfortunate having to special case it to make it work, but it's a pretty simple patch, so sounds good to me. If ever run into this again we can reconsider. Do you want to go ahead and take the bug?
Comment 8 Saam Barati 2019-05-14 15:18:47 PDT
Created attachment 369898 [details]
patch
Comment 9 Saam Barati 2019-05-14 18:32:37 PDT
Here is another test I'll check in that crashes with:  --useMaximalFlushInsertionPhase=1 --jitPolicyScale=0 --useConcurrentJIT=0

```
function f0() {
}

function bar() {
  f0(...arguments);
}

const a = new Uint8Array(1);

function foo() {
  bar(0, 0);
  a.find(()=>{});
}


for (let i=0; i<3; i++) {
  foo();
}
```
Comment 10 Michael Saboff 2019-05-15 13:23:17 PDT
Comment on attachment 369898 [details]
patch

r=me
Comment 11 Saam Barati 2019-05-15 13:30:32 PDT
landed in:
https://trac.webkit.org/changeset/245341/webkit