Bug 174900 - ASSERTION FAILED: candidate->op() == PhantomCreateRest || candidate->op() == PhantomDirectArguments || candidate->op() == PhantomClonedArguments || candidate->op() == PhantomSpread || candidate->op() == PhantomNewArrayWithSpread
Summary: ASSERTION FAILED: candidate->op() == PhantomCreateRest || candidate->op() == ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-07-27 09:55 PDT by Mark Lam
Modified: 2017-07-28 16:26 PDT (History)
7 users (show)

See Also:


Attachments
Patch (3.95 KB, patch)
2017-07-27 11:14 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>