Bug 144678 - FunctionCallBracketNode should store the base value to the temporary when subscript has assignment
Summary: FunctionCallBracketNode should store the base value to the temporary when sub...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-06 02:38 PDT by Yusuke Suzuki
Modified: 2015-05-07 15:37 PDT (History)
7 users (show)

See Also:


Attachments
Patch (8.82 KB, patch)
2015-05-06 04:16 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (9.63 KB, patch)
2015-05-06 09:37 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 Yusuke Suzuki 2015-05-06 02:38:10 PDT
Currently, FunctionCallBracketNode directly use the RegisterID returned by emitNode.
But if the base part is the local register and the subscript part has assignment to it, the base result is accidentally rewritten.

function t() { var ok = {null: function () { } }; ok[ok = null](); }
t();  // Should not throw error.

Seeing the code, we need to use emitNodeForLeftHandSide.
Comment 1 Yusuke Suzuki 2015-05-06 04:16:52 PDT
Created attachment 252467 [details]
Patch
Comment 2 Yusuke Suzuki 2015-05-06 09:37:11 PDT
Created attachment 252481 [details]
Patch
Comment 3 Yusuke Suzuki 2015-05-06 09:37:23 PDT
Added more tests.
Comment 4 Yusuke Suzuki 2015-05-06 09:41:41 PDT
Comment on attachment 252481 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=252481&action=review

Added comments to the patch for ease of review.

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:709
> +        base = emitSuperBaseForCallee(generator);

Since super is not variable (so rewriting super cannot be done) and its return value is always temporary register, in this case, we need not to take care.

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:712
> +            base = generator.emitNode(m_base);

When the subscript is string, evaluating it has no side effect.
Comment 5 Geoffrey Garen 2015-05-07 11:04:49 PDT
Comment on attachment 252481 [details]
Patch

r=me
Comment 6 WebKit Commit Bot 2015-05-07 15:37:19 PDT
Comment on attachment 252481 [details]
Patch

Clearing flags on attachment: 252481

Committed r183955: <http://trac.webkit.org/changeset/183955>
Comment 7 WebKit Commit Bot 2015-05-07 15:37:25 PDT
All reviewed patches have been landed.  Closing bug.