⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 271265 in webkit


Ignore:
Timestamp:
Jan 7, 2021, 3:17:44 PM (6 years ago)
Author:
ysuzuki@apple.com
Message:

[JSC] New expression and value function call should reserve function register if arguments include assignments
​https://bugs.webkit.org/show_bug.cgi?id=220429
<rdar://problem/70598359>

Reviewed by Alexey Shvayka.

JSTests:

  • stress/comma-value-func-call-resolve.js: Added.

(shouldBe):
(fn.x):
(fn):

  • stress/construct-overwritten-variable.js:

(shouldThrow):
(new.x.x): Deleted.

  • stress/construct-spread-overwritten-variable-2.js:

(shouldThrow):
(new.x.x): Deleted.

  • stress/construct-spread-overwritten-variable.js:

(shouldThrow):
(new.x.x): Deleted.

  • stress/destructuring-func-call-resolve.js: Added.

(shouldBe):
(fn.x):
(fn):
(fn2.x):
(fn2):

  • stress/resolve-func-call-resolve.js: Added.

(shouldBe):
(fn.x):
(fn):

  • stress/tagged-template-call-resolve.js: Added.

(shouldBe):
(fn.x):
(fn):

  • test262/expectations.yaml:

Source/JavaScriptCore:

If the following code is executed, we need to reserve |x| before evaluating arguments since arguments can override
local |x| variable before calling it.

new x(x = 1)

We found there are two places we are not doing this.

  1. new expression
  2. function value call (it is checking isLocation(), but we can still use local variables for function if we use comma expression)

We introduced hasAssignment flag to ArgumentsNode, and reserve a function in a new temporary register if arguments include assignments.
We also need to increment assignmentCount in destructuring assignment.

  • bytecompiler/NodesCodegen.cpp:

(JSC::NewExprNode::emitBytecode):
(JSC::FunctionCallValueNode::emitBytecode):

  • parser/ASTBuilder.h:

(JSC::ASTBuilder::createArguments):

  • parser/NodeConstructors.h:

(JSC::ArgumentsNode::ArgumentsNode):

  • parser/Nodes.h:
  • parser/Parser.cpp:

(JSC::Parser<LexerType>::parseDestructuringPattern):
(JSC::Parser<LexerType>::parseArguments):

  • parser/SyntaxChecker.h:

(JSC::SyntaxChecker::createArguments):

Location:
trunk
Files:
4 added
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r271242 r271265  
     12021-01-07  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] New expression and value function call should reserve function register if arguments include assignments
     4        https://bugs.webkit.org/show_bug.cgi?id=220429
     5        <rdar://problem/70598359>
     6
     7        Reviewed by Alexey Shvayka.
     8
     9        * stress/comma-value-func-call-resolve.js: Added.
     10        (shouldBe):
     11        (fn.x):
     12        (fn):
     13        * stress/construct-overwritten-variable.js:
     14        (shouldThrow):
     15        (new.x.x): Deleted.
     16        * stress/construct-spread-overwritten-variable-2.js:
     17        (shouldThrow):
     18        (new.x.x): Deleted.
     19        * stress/construct-spread-overwritten-variable.js:
     20        (shouldThrow):
     21        (new.x.x): Deleted.
     22        * stress/destructuring-func-call-resolve.js: Added.
     23        (shouldBe):
     24        (fn.x):
     25        (fn):
     26        (fn2.x):
     27        (fn2):
     28        * stress/resolve-func-call-resolve.js: Added.
     29        (shouldBe):
     30        (fn.x):
     31        (fn):
     32        * stress/tagged-template-call-resolve.js: Added.
     33        (shouldBe):
     34        (fn.x):
     35        (fn):
     36        * test262/expectations.yaml:
     37
    1382021-01-07  Yusuke Suzuki  <ysuzuki@apple.com>
    239

  • trunk/JSTests/stress/construct-overwritten-variable.js

    r217062 r271265  
    11//@ runDefault
    22
    3 (function(){
     3function shouldThrow(func, errorMessage) {
     4    var errorThrown = false;
     5    var error = null;
     6    try {
     7        func();
     8    } catch (e) {
     9        errorThrown = true;
     10        error = e;
     11    }
     12    if (!errorThrown)
     13        throw new Error('not thrown');
     14    if (String(error) !== errorMessage)
     15        throw new Error(`bad error: ${String(error)}`);
     16}
     17
     18shouldThrow(function(){
    419    var x = 42;
    520    new x(x = function(){ });
    6 })();
     21}, `TypeError: 42 is not a constructor (evaluating 'new x(x = function(){ })')`);

  • trunk/JSTests/stress/construct-spread-overwritten-variable-2.js

    r217062 r271265  
    11//@ runDefault
    22
    3 (function(){
     3function shouldThrow(func, errorMessage) {
     4    var errorThrown = false;
     5    var error = null;
     6    try {
     7        func();
     8    } catch (e) {
     9        errorThrown = true;
     10        error = e;
     11    }
     12    if (!errorThrown)
     13        throw new Error('not thrown');
     14    if (String(error) !== errorMessage)
     15        throw new Error(`bad error: ${String(error)}`);
     16}
     17
     18shouldThrow(function(){
    419    var x = 42;
    520    new x(...[x = function(){ }]);
    6 })();
     21}, `TypeError: 42 is not a constructor (evaluating 'new x(...[x = function(){ }])')`);

  • trunk/JSTests/stress/construct-spread-overwritten-variable.js

    r217062 r271265  
    11//@ runDefault
    22
    3 (function(){
     3function shouldThrow(func, errorMessage) {
     4    var errorThrown = false;
     5    var error = null;
     6    try {
     7        func();
     8    } catch (e) {
     9        errorThrown = true;
     10        error = e;
     11    }
     12    if (!errorThrown)
     13        throw new Error('not thrown');
     14    if (String(error) !== errorMessage)
     15        throw new Error(`bad error: ${String(error)}`);
     16}
     17
     18shouldThrow(function(){
    419    var x = 42;
    520    var a = [1, 2, 3];
    621    new x(x = function(){ }, ...a);
    7 })();
     22}, `TypeError: 42 is not a constructor (evaluating 'new x(x = function(){ }, ...a)')`);

  • trunk/JSTests/test262/expectations.yaml

    r271225 r271265  
    16311631test/language/expressions/logical-assignment/lgcl-or-assignment-operator-non-simple-lhs.js:
    16321632  default: 'Test262: This statement should not be evaluated.'
    1633 test/language/expressions/new/ctorExpr-fn-ref-before-args-eval-fn-wrapup.js:
    1634   default: "TypeError: 1 is not a constructor (evaluating 'new x(x = 1)')"
    1635   strict mode: "TypeError: 1 is not a constructor (evaluating 'new x(x = 1)')"
    1636 test/language/expressions/new/ctorExpr-isCtor-after-args-eval-fn-wrapup.js:
    1637   default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
    1638   strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
    16391633test/language/expressions/new/non-ctor-err-realm.js:
    16401634  default: 'Test262Error: production including Arguments Expected a TypeError but got a TypeError'

  • trunk/Source/JavaScriptCore/ChangeLog

    r271240 r271265  
     12021-01-07  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] New expression and value function call should reserve function register if arguments include assignments
     4        https://bugs.webkit.org/show_bug.cgi?id=220429
     5        <rdar://problem/70598359>
     6
     7        Reviewed by Alexey Shvayka.
     8
     9        If the following code is executed, we need to reserve |x| before evaluating arguments since arguments can override
     10        local |x| variable before calling it.
     11
     12            new x(x = 1)
     13
     14        We found there are two places we are not doing this.
     15
     16            1. new expression
     17            2. function value call (it is checking `isLocation()`, but we can still use local variables for function if we use comma expression)
     18
     19        We introduced hasAssignment flag to ArgumentsNode, and reserve a function in a new temporary register if arguments include assignments.
     20        We also need to increment assignmentCount in destructuring assignment.
     21
     22        * bytecompiler/NodesCodegen.cpp:
     23        (JSC::NewExprNode::emitBytecode):
     24        (JSC::FunctionCallValueNode::emitBytecode):
     25        * parser/ASTBuilder.h:
     26        (JSC::ASTBuilder::createArguments):
     27        * parser/NodeConstructors.h:
     28        (JSC::ArgumentsNode::ArgumentsNode):
     29        * parser/Nodes.h:
     30        * parser/Parser.cpp:
     31        (JSC::Parser<LexerType>::parseDestructuringPattern):
     32        (JSC::Parser<LexerType>::parseArguments):
     33        * parser/SyntaxChecker.h:
     34        (JSC::SyntaxChecker::createArguments):
     35
    1362021-01-07  Mark Lam  <mark.lam@apple.com>
    237

  • trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

    r271121 r271265  
    980980    else
    981981        expectedFunction = NoExpectedFunction;
    982     RefPtr<RegisterID> func = generator.emitNode(m_expr);
     982
     983    RefPtr<RegisterID> func = nullptr;
     984    if (m_args && m_args->hasAssignments())
     985        func = generator.newTemporary();
     986    func = generator.emitNode(func.get(), m_expr);
    983987    RefPtr<RegisterID> returnValue = generator.finalDestination(dst, func.get());
    984988    CallArguments callArguments(generator, m_args);
    … …  
    11011105    }
    11021106
    1103     RefPtr<RegisterID> func = generator.emitNode(m_expr);
     1107    RefPtr<RegisterID> func = nullptr;
     1108    if (m_args && m_args->hasAssignments())
     1109        func = generator.newTemporary();
     1110    func = generator.emitNode(func.get(), m_expr);
    11041111    RefPtr<RegisterID> returnValue = generator.finalDestination(dst, func.get());
    11051112    if (isOptionalChainBase())

  • trunk/Source/JavaScriptCore/parser/ASTBuilder.h

    r266264 r271265  
    478478
    479479    ArgumentsNode* createArguments() { return new (m_parserArena) ArgumentsNode(); }
    480     ArgumentsNode* createArguments(ArgumentListNode* args) { return new (m_parserArena) ArgumentsNode(args); }
     480    ArgumentsNode* createArguments(ArgumentListNode* args, bool hasAssignments) { return new (m_parserArena) ArgumentsNode(args, hasAssignments); }
    481481    ArgumentListNode* createArgumentsList(const JSTokenLocation& location, ExpressionNode* arg) { return new (m_parserArena) ArgumentListNode(location, arg); }
    482482    ArgumentListNode* createArgumentsList(const JSTokenLocation& location, ArgumentListNode* args, ExpressionNode* arg) { return new (m_parserArena) ArgumentListNode(location, args, arg); }

  • trunk/Source/JavaScriptCore/parser/NodeConstructors.h

    r266264 r271265  
    365365    }
    366366
    367     inline ArgumentsNode::ArgumentsNode(ArgumentListNode* listNode)
     367    inline ArgumentsNode::ArgumentsNode(ArgumentListNode* listNode, bool hasAssignments)
    368368        : m_listNode(listNode)
     369        , m_hasAssignments(hasAssignments)
    369370    {
    370371    }

  • trunk/Source/JavaScriptCore/parser/Nodes.h

    r271121 r271265  
    915915    public:
    916916        ArgumentsNode();
    917         ArgumentsNode(ArgumentListNode*);
     917        ArgumentsNode(ArgumentListNode*, bool hasAssignments);
     918
     919        bool hasAssignments() const { return m_hasAssignments; }
    918920
    919921        ArgumentListNode* m_listNode;
     922    private:
     923        bool m_hasAssignments { false };
    920924    };
    921925

  • trunk/Source/JavaScriptCore/parser/Parser.cpp

    r270923 r271265  
    11271127{
    11281128    failIfStackOverflow();
     1129    m_parserState.assignmentCount++;
    11291130    int nonLHSCount = m_parserState.nonLHSCount;
    11301131    TreeDestructuringPattern pattern;
    … …  
    48374838    auto argumentsDivot = m_token.m_endPosition;
    48384839
     4840    int initialAssignments = m_parserState.assignmentCount;
    48394841    ArgumentType argType = ArgumentType::Normal;
    48404842    TreeExpression firstArg = parseArgument(context, argType);
    … …  
    48684870    if (hasSpread) {
    48694871        TreeExpression spreadArray = context.createSpreadExpression(location, context.createArray(location, context.createElementList(argList)), argumentsStart, argumentsDivot, m_lastTokenEndPosition);
    4870         return context.createArguments(context.createArgumentsList(location, spreadArray));
    4871     }
    4872 
    4873     return context.createArguments(argList);
     4872        return context.createArguments(context.createArgumentsList(location, spreadArray), initialAssignments != m_parserState.assignmentCount);
     4873    }
     4874
     4875    return context.createArguments(argList, initialAssignments != m_parserState.assignmentCount);
    48744876}
    48754877

  • trunk/Source/JavaScriptCore/parser/SyntaxChecker.h

    r266324 r271265  
    201201    void setFunctionNameStart(int, int) { }
    202202    int createArguments() { return ArgumentsResult; }
    203     int createArguments(int) { return ArgumentsResult; }
     203    int createArguments(int, bool) { return ArgumentsResult; }
    204204    ExpressionType createSpreadExpression(const JSTokenLocation&, ExpressionType, int, int, int) { return SpreadExpr; }
    205205    ExpressionType createObjectSpreadExpression(const JSTokenLocation&, ExpressionType, int, int, int) { return ObjectSpreadExpr; }
Note: See TracChangeset for help on using the changeset viewer.