Bug 182982

Summary: ArgumentsEliminationPhase has a branch on GetByOffset that should be an assert
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, rmorisset, ticaiolima, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
ysuzuki: review+
patch for landing none

Saam Barati
Reported 2018-02-20 12:22:59 PST
The code is this: ``` case GetByOffset: { Node* candidate = node->child2().node(); if (!isEliminatedAllocation(candidate)) break; if (node->child2()->op() != PhantomClonedArguments) break; ... ``` But, if it's an elimination candidate, the only child node GetByOffset handles is ClonedArguments. This should totally be an assert. We're clearly doing something wrong if we don't transform this node when its child is an eliminated allocation.
Attachments
patch (2.36 KB, patch)
2018-02-22 18:24 PST, Saam Barati
ysuzuki: review+
patch for landing (2.36 KB, patch)
2018-02-22 18:44 PST, Saam Barati
no flags
Saam Barati
Comment 1 2018-02-22 18:24:29 PST
Yusuke Suzuki
Comment 2 2018-02-22 18:37:32 PST
Comment on attachment 334491 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=334491&action=review r=me > Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:711 > + ASSERT(node->child2()->op() == PhantomClonedArguments); Let's use `ASSERT(candidate->op() == PhantomClonedArguments)`.
Saam Barati
Comment 3 2018-02-22 18:44:09 PST
Created attachment 334495 [details] patch for landing
WebKit Commit Bot
Comment 4 2018-02-23 10:36:01 PST
Comment on attachment 334495 [details] patch for landing Clearing flags on attachment: 334495 Committed r228952: <https://trac.webkit.org/changeset/228952>
WebKit Commit Bot
Comment 5 2018-02-23 10:36:02 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 6 2018-02-23 10:37:26 PST
Note You need to log in before you can comment on or make changes to this bug.