Summary: | Allocation sinking phase should consider a CheckStructure that would fail as an escape | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Saam Barati <saam> | ||||||||
Component: | JavaScriptCore | Assignee: | Saam Barati <saam> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | benjamin, commit-queue, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, ticaiolima, webkit-bug-importer, ysuzuki | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Saam Barati
2017-07-10 14:27:15 PDT
I can't write a program that exhibits this behavior. I know it's possible since I'm looking at a bunch of crashlogs that hit some assertions I added. I'll open a FIXME to try and generate test cases for this. I won't hold up the fix though. I was able to simulate the crash by manually editing the IR in a test I wrote which inserts a CheckStructure at a particular program point right before what would otherwise be an escape. When the escape tries to generate an object materialization, the object it's trying to materialize has the empty set of structures. This leads the phase to crash. Instead, I'm going to treat anything that would filter an allocations structure set to the empty set as an escape, since we've effectively proven that we would have exited at that point. This will lead us to generate a materialization right before such a CheckStructure (or MultiGetByOffset), and then when exiting that CheckStructure, we'll exit. Created attachment 315032 [details]
test IR editing
Uploading my IR editing code in case anybody wants to repeat my experiment. I ran it on this program:
```
function escape() { }
noInline(escape);
noDFG(escape);
function foo(c, x) {
let o1 = {};
let o2 = {x, o1};
o1.o2 = o2;
if (c) {
escape(o2);
}
}
noInline(foo);
for (let i = 0; i < 100000; ++i) {
foo(!!(i%2), i);
}
```
Created attachment 315033 [details]
patch
Comment on attachment 315033 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=315033&action=review > Source/JavaScriptCore/ChangeLog:10 > + objects in a cylce with each other, it would assume that each materialized typo: cylce => cycle > Source/JavaScriptCore/ChangeLog:14 > + The abstract interpretation part of the phase will model whats in the heap. typo: whats => what's Created attachment 315040 [details]
patch for landing
Comment on attachment 315040 [details] patch for landing Clearing flags on attachment: 315040 Committed r219317: <http://trac.webkit.org/changeset/219317> All reviewed patches have been landed. Closing bug. |