Bug 174900

Summary: ASSERTION FAILED: candidate->op() == PhantomCreateRest || candidate->op() == PhantomDirectArguments || candidate->op() == PhantomClonedArguments || candidate->op() == PhantomSpread || candidate->op() == PhantomNewArrayWithSpread
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, jfbastien, keith_miller, msaboff, saam, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Mark Lam 2017-07-27 09:55:27 PDT
Debug build JSC tests are failing:
** The following JSC stress test failures have been introduced:
	microbenchmarks/deltablue-varargs.js.ftl-eager
	microbenchmarks/deltablue-varargs.js.ftl-eager-no-cjit
	microbenchmarks/deltablue-varargs.js.ftl-eager-no-cjit-b3o1

The commit range that introduced this failure is: 219725:219732.
The likely culprit (given the assertion failure) is https://trac.webkit.org/r219727.
Comment 1 Yusuke Suzuki 2017-07-27 10:15:48 PDT
I'm looking it now.
Comment 2 Yusuke Suzuki 2017-07-27 10:17:44 PDT
OK, I've reproduced it with ftl-eager-no-cjit configuration. Cool, it's no CJIT :D
Comment 3 Yusuke Suzuki 2017-07-27 10:39:57 PDT
In this arguments elimination phase, due to high cost of AI, we intentionally do not run AI.
Instead, we use ForceOSRExit etc. (pseudo terminals) not to look into unreachable nodes.
The problem is that even transforming phase also checks this pseudo terminals.


BB1
1: ForceOSRExit
2: CreateDirectArguments

BB2
3: GetButterfly(@2)
4: ForceOSRExit

In the above case, @2 is not converted to PhantomDirectArguments.
But @3 is processed. And the assertion fires.
Comment 4 Yusuke Suzuki 2017-07-27 10:41:08 PDT
(In reply to Yusuke Suzuki from comment #3)
> In this arguments elimination phase, due to high cost of AI, we
> intentionally do not run AI.
> Instead, we use ForceOSRExit etc. (pseudo terminals) not to look into
> unreachable nodes.
> The problem is that even transforming phase also checks this pseudo
> terminals.
> 
> 
> BB1
> 1: ForceOSRExit
> 2: CreateDirectArguments
> 
> BB2
> 3: GetButterfly(@2)
> 4: ForceOSRExit
> 
> In the above case, @2 is not converted to PhantomDirectArguments.
> But @3 is processed. And the assertion fires.

I think the reasonable fix is not listing candidates after pseudo exits.
Comment 5 Saam Barati 2017-07-27 10:57:25 PDT
(In reply to Yusuke Suzuki from comment #4)
> (In reply to Yusuke Suzuki from comment #3)
> > In this arguments elimination phase, due to high cost of AI, we
> > intentionally do not run AI.
> > Instead, we use ForceOSRExit etc. (pseudo terminals) not to look into
> > unreachable nodes.
> > The problem is that even transforming phase also checks this pseudo
> > terminals.
> > 
> > 
> > BB1
> > 1: ForceOSRExit
> > 2: CreateDirectArguments
> > 
> > BB2
> > 3: GetButterfly(@2)
> > 4: ForceOSRExit
> > 
> > In the above case, @2 is not converted to PhantomDirectArguments.
> > But @3 is processed. And the assertion fires.
> 
> I think the reasonable fix is not listing candidates after pseudo exits.

Sounds reasonable to me.
Comment 6 Yusuke Suzuki 2017-07-27 11:14:47 PDT
Created attachment 316556 [details]
Patch
Comment 7 WebKit Commit Bot 2017-07-28 00:26:00 PDT
Comment on attachment 316556 [details]
Patch

Clearing flags on attachment: 316556

Committed r219997: <http://trac.webkit.org/changeset/219997>
Comment 8 WebKit Commit Bot 2017-07-28 00:26:02 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2017-07-28 16:26:24 PDT
<rdar://problem/33601432>