Bug 144883 - DFG ASSERTION FAILED: Bad liveness analysis result: live at root is not empty with eager compilation
Summary: DFG ASSERTION FAILED: Bad liveness analysis result: live at root is not empty...
Status: RESOLVED DUPLICATE of bug 143078
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Basile Clement
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-11 15:15 PDT by Basile Clement
Modified: 2015-05-11 21:58 PDT (History)
4 users (show)

See Also:


Attachments
DFG_CRASH trace (65.22 KB, text/plain)
2015-05-11 15:15 PDT, Basile Clement
no flags Details
Small example (323 bytes, application/x-javascript)
2015-05-11 16:44 PDT, Basile Clement
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Basile Clement 2015-05-11 15:15:23 PDT
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.
Comment 1 Basile Clement 2015-05-11 15:17:54 PDT
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.
Comment 2 Basile Clement 2015-05-11 16:44:14 PDT
Created attachment 252908 [details]
Small example
Comment 3 Basile Clement 2015-05-11 17:27:56 PDT
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.
Comment 4 Basile Clement 2015-05-11 18:32:49 PDT
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).
Comment 5 Filip Pizlo 2015-05-11 21:37:20 PDT
(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.
Comment 6 Filip Pizlo 2015-05-11 21:58:40 PDT

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