Bug 144883 – DFG ASSERTION FAILED: Bad liveness analysis result: live at root is not empty with eager compilation

• Component: JavaScriptCore
• Status: RESOLVED DUPLICATE

[Basile Clement]

Created attachment 252895 [details]
DFG_CRASH trace

Easily reproducible on http://dealbook.nytimes.com/2014/11/21/weekend-reading-is-the-new-york-fed-a-cop-or-a-fire-warden/?_r=0 with eager compilation enabled.

[Basile Clement]

In the attached DFG IR dump, Block #8 is having a PutHint on @121, but can be reached by #0-#1-#2-#6-#8 while @121 (a PhantomDirectArguments) is in block #3.

[Basile Clement]

Created attachment 252908 [details]
Small example

[Basile Clement]

This seems to be linked to interactions between PhantomDirectArguments and NewFunction sinking.

When a PhantomDirectArgument node is present for a closure, the OSR availability calculator is picking up its promoted ArgumentsCalleePLoc from the stack (line 189 in DFGOSRAvailabilityAnalysisPhase.cpp).
Then, if we end up sinking the callee, the object allocation sinking phase will see that location associated with the initial NewFunction node, and will generate a PutHint to update it (line 425 in DFGObjectAllocationSinkingPhase.cpp) on paths where the node is materialized and we have an availability for the ArgumentsCalleePLoc.
This is not correct because there may be other paths to this node where the PhantomDirectArgument is not present.

I unfortunately do not understand the role and supposed behavior of PhantomDirectArguments to be able to suggest a fix.

[Basile Clement]

After thinking about it more, I think fixing https://bugs.webkit.org/show_bug.cgi?id=143078 should fix this as well, as it would prevent OSR availability analysis from tricking us into creating PutHints over dead values (which I would assume to be the core of the issue here).

[Filip Pizlo]

(In reply to comment #4)
> After thinking about it more, I think fixing
> https://bugs.webkit.org/show_bug.cgi?id=143078 should fix this as well, as
> it would prevent OSR availability analysis from tricking us into creating
> PutHints over dead values (which I would assume to be the core of the issue
> here).

I agree.  The bug is really that we're not doing dominator pruning.  We continue to track availability even in blocks where the node doesn't dominate.  In this example, we have for example a heap availabiltity "ArgumentPLoc(@86, 0)=>ConflictingFlush/@53" at head of Block #5, but @86 doesn't dominate #5.

Doing liveness pruning will solve the problem comprehensively, because a node cannot be live in a block that it doesn't dominate.

[Filip Pizlo]

*** This bug has been marked as a duplicate of bug 143078 ***