Bug 174321 – Allocation sinking phase should consider a CheckStructure that would fail as an escape

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Saam Barati]

Otherwise, we might try to materialize an allocation, and think the set of structures it might have is the empty set. This is clearly wrong.

[Saam Barati]

<rdar://problem/32604963>

[Saam Barati]

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.

[Saam Barati]

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);
}
```

[Saam Barati]

Created attachment 315033 [details]
patch

[Keith Miller]

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

[Saam Barati]

Created attachment 315040 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 315040 [details]
patch for landing

Clearing flags on attachment: 315040

Committed r219317: <http://trac.webkit.org/changeset/219317>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.