Bug 197855 - Bound liveness of SetArgumentMaybe nodes when maximal flush insertion phase is enabled
Summary: Bound liveness of SetArgumentMaybe nodes when maximal flush insertion phase i...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-05-13 15:36 PDT by Tadeu Zagallo
Modified: 2019-05-15 13:30 PDT (History)
6 users (show)

See Also:


Attachments
Patch (32.69 KB, patch)
2019-05-13 15:47 PDT, Tadeu Zagallo
no flags Details | Formatted Diff | Diff
maybe patch (1.66 KB, patch)
2019-05-13 16:35 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews210 for win-future (13.53 MB, application/zip)
2019-05-14 00:09 PDT, Build Bot
no flags Details
patch (4.39 KB, patch)
2019-05-14 15:18 PDT, Saam Barati
msaboff: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 Build Bot 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 Build Bot 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