Bug 144883

Summary:

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

Product:

WebKit

Reporter:

Basile Clement <basile_clement>

Component:

JavaScriptCore

Assignee:

Basile Clement <basile_clement>

Status:

RESOLVED DUPLICATE

Severity:

Normal

CC:

fpizlo, ggaren, mark.lam, msaboff

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
DFG_CRASH trace	none
Small example	none

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 ***