Bug 721322 - Functions containing |expr.arguments| should be marked (conservatively) as using arguments. r=jorendorff
authorJeff Walden <jwalden@mit.edu>
Thu, 26 Jan 2012 17:04:00 -0800
changeset 87016 a5337afc5350a711ebce44d9e1644f5109818576
parent 87015 192205ad7f9cebc99c4ef46482ab57d6230e8590
child 87017 02e01981f3629cd48dfce476489fce5d78feab6a
push id805
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 18:17:35 +0000
treeherdermozilla-aurora@6fb3bf232436 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs721322
milestone12.0a1
Bug 721322 - Functions containing |expr.arguments| should be marked (conservatively) as using arguments. r=jorendorff
js/src/frontend/BytecodeEmitter.h
js/src/frontend/ParseNode.h
js/src/frontend/Parser.cpp
js/src/frontend/Parser.h
js/src/jit-test/tests/arguments/e4x-descendants-with-arguments.js
js/src/tests/ecma_5/extensions/arguments-property-access-in-function.js
js/src/tests/ecma_5/extensions/jstests.list
js/src/tests/js1_8/genexps/arguments-property-access-in-generator.js
js/src/tests/js1_8/genexps/jstests.list
--- a/js/src/frontend/BytecodeEmitter.h
+++ b/js/src/frontend/BytecodeEmitter.h
@@ -429,28 +429,57 @@ struct TreeContext {                /* t
         flags |= TCF_FUN_MUTATES_PARAMETER;
     }
 
     bool mutatesParameter() const {
         JS_ASSERT(inFunction());
         return flags & TCF_FUN_MUTATES_PARAMETER;
     }
 
-    void noteArgumentsUse(ParseNode *pn) {
+    /*
+     * Accessing the implicit |arguments| local binding in a function must
+     * trigger appropriate code generation such that the access works.
+     */
+    void noteArgumentsNameUse(ParseNode *node) {
         JS_ASSERT(inFunction());
-        countArgumentsUse(pn);
+        JS_ASSERT(node->isKind(PNK_NAME));
+        JS_ASSERT(node->pn_atom == parser->context->runtime->atomState.argumentsAtom);
+        countArgumentsUse(node);
         flags |= TCF_FUN_USES_ARGUMENTS;
         if (funbox)
             funbox->node->pn_dflags |= PND_FUNARG;
     }
 
-    void countArgumentsUse(ParseNode *pn) {
-        JS_ASSERT(pn->pn_atom == parser->context->runtime->atomState.argumentsAtom);
+    /*
+     * Non-dynamic accesses to a property named "arguments" inside a function
+     * have to deoptimize the function in case those accesses are to the
+     * function's arguments.  (However, this is unnecessary in strict mode
+     * functions because of the f.arguments poison-pill.  O frabjous day!)
+     */
+    void noteArgumentsPropertyAccess(ParseNode *node) {
+        JS_ASSERT(inFunction());
+        JS_ASSERT(&node->asPropertyAccess().name() ==
+                  parser->context->runtime->atomState.argumentsAtom);
+        if (!inStrictMode()) {
+            flags |= TCF_FUN_USES_ARGUMENTS;
+            if (funbox)
+                funbox->node->pn_dflags |= PND_FUNARG;
+        }
+    }
+
+    /*
+     * Uses of |arguments| must be noted so that such uses can be forbidden if
+     * they occur inside generator expressions (which desugar to functions and
+     * yields, in which |arguments| would have an entirely different meaning).
+     */
+    void countArgumentsUse(ParseNode *node) {
+        JS_ASSERT(node->isKind(PNK_NAME));
+        JS_ASSERT(node->pn_atom == parser->context->runtime->atomState.argumentsAtom);
         argumentsCount++;
-        argumentsNode = pn;
+        argumentsNode = node;
     }
 
     bool needsEagerArguments() const {
         return inStrictMode() && ((usesArguments() && mutatesParameter()) || callsEval());
     }
 
     void noteHasExtensibleScope() {
         flags |= TCF_FUN_EXTENSIBLE_SCOPE;
--- a/js/src/frontend/ParseNode.h
+++ b/js/src/frontend/ParseNode.h
@@ -479,16 +479,17 @@ enum ParseNodeArity {
 
 struct Definition;
 
 class LoopControlStatement;
 class BreakStatement;
 class ContinueStatement;
 class XMLProcessingInstruction;
 class ConditionalExpression;
+class PropertyAccess;
 
 struct ParseNode {
   private:
     uint32_t            pn_type   : 16, /* PNK_* type */
                         pn_op     : 8,  /* see JSOp enum and jsopcode.tbl */
                         pn_arity  : 5,  /* see ParseNodeArity enum */
                         pn_parens : 1,  /* this expr was enclosed in parens */
                         pn_used   : 1,  /* name node is on a use-chain */
@@ -922,16 +923,17 @@ struct ParseNode {
 
     /* Casting operations. */
     inline BreakStatement &asBreakStatement();
     inline ContinueStatement &asContinueStatement();
 #if JS_HAS_XML_SUPPORT
     inline XMLProcessingInstruction &asXMLProcessingInstruction();
 #endif
     inline ConditionalExpression &asConditionalExpression();
+    inline PropertyAccess &asPropertyAccess();
 };
 
 struct NullaryNode : public ParseNode {
     static inline NullaryNode *create(ParseNodeKind kind, TreeContext *tc) {
         return (NullaryNode *)ParseNode::create(kind, PN_NULLARY, tc);
     }
 };
 
@@ -1225,16 +1227,24 @@ class PropertyAccess : public ParseNode 
         return *pn_u.name.expr;
     }
 
     PropertyName &name() const {
         return *pn_u.name.atom->asPropertyName();
     }
 };
 
+inline PropertyAccess &
+ParseNode::asPropertyAccess()
+{
+    JS_ASSERT(isKind(PNK_DOT));
+    JS_ASSERT(pn_arity == PN_NAME);
+    return *static_cast<PropertyAccess *>(this);
+}
+
 class PropertyByValue : public ParseNode {
   public:
     PropertyByValue(ParseNode *lhs, ParseNode *propExpr,
                     const TokenPtr &begin, const TokenPtr &end)
       : ParseNode(PNK_LB, JSOP_GETELEM, PN_BINARY, TokenPos::make(begin, end))
     {
         pn_u.binary.left = lhs;
         pn_u.binary.right = propExpr;
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -4388,17 +4388,17 @@ Parser::variables(ParseNodeKind kind, St
                        : JSOP_SETNAME);
 
             NoteLValue(context, pn2, tc, data.fresh ? PND_INITIALIZED : PND_ASSIGNED);
 
             /* The declarator's position must include the initializer. */
             pn2->pn_pos.end = init->pn_pos.end;
 
             if (tc->inFunction() && name == context->runtime->atomState.argumentsAtom) {
-                tc->noteArgumentsUse(pn2);
+                tc->noteArgumentsNameUse(pn2);
                 if (!blockObj)
                     tc->flags |= TCF_FUN_HEAVYWEIGHT;
             }
         }
     } while (tokenStream.matchToken(TOK_COMMA));
 
     pn->pn_pos.end = pn->last()->pn_pos.end;
     return pn;
@@ -5741,21 +5741,31 @@ Parser::memberExpr(JSBool allowCallSynta
                     nextMember = new_<XMLDoubleColonProperty>(lhs, propertyId,
                                                               lhs->pn_pos.begin,
                                                               tokenStream.currentToken().pos.end);
                     if (!nextMember)
                         return NULL;
                 } else
 #endif
                 {
-                    nextMember = new_<PropertyAccess>(lhs, tokenStream.currentToken().name(),
+                    PropertyName *field = tokenStream.currentToken().name();
+                    nextMember = new_<PropertyAccess>(lhs, field,
                                                       lhs->pn_pos.begin,
                                                       tokenStream.currentToken().pos.end);
                     if (!nextMember)
                         return NULL;
+
+                    /*
+                     * A property access of the form |<expr>.arguments| might
+                     * access this function's arguments, so we need to flag a
+                     * potential arguments use to ensure an arguments object
+                     * will be created.  See bug 721322.
+                     */
+                    if (tc->inFunction() && field == context->runtime->atomState.argumentsAtom)
+                        tc->noteArgumentsPropertyAccess(nextMember);
                 }
             }
 #if JS_HAS_XML_SUPPORT
             else if (!tc->inStrictMode()) {
                 TokenPtr begin = lhs->pn_pos.begin;
                 if (tt == TOK_LP) {
                     /* Filters are effectively 'with', so deoptimize names. */
                     tc->flags |= TCF_FUN_HEAVYWEIGHT;
@@ -6609,48 +6619,47 @@ Parser::propertyQualifiedIdentifier()
         return NULL;
 
     tokenStream.consumeKnownToken(TOK_DBLCOLON);
     return qualifiedSuffix(node);
 }
 #endif
 
 ParseNode *
-Parser::identifierName(bool afterDot)
+Parser::identifierName(bool afterDoubleDot)
 {
     JS_ASSERT(tokenStream.isCurrentTokenType(TOK_NAME));
 
     PropertyName *name = tokenStream.currentToken().name();
     ParseNode *node = NameNode::create(PNK_NAME, name, tc);
     if (!node)
         return NULL;
     JS_ASSERT(tokenStream.currentToken().t_op == JSOP_NAME);
     node->setOp(JSOP_NAME);
 
     if ((tc->flags & (TCF_IN_FUNCTION | TCF_FUN_PARAM_ARGUMENTS)) == TCF_IN_FUNCTION &&
-        name == context->runtime->atomState.argumentsAtom) {
-        /*
-         * Flag arguments usage so we can avoid unsafe optimizations such
-         * as formal parameter assignment analysis (because of the hated
-         * feature whereby arguments alias formals). We do this even for
-         * a reference of the form foo.arguments, which ancient code may
-         * still use instead of arguments (more hate).
-         */
-        tc->noteArgumentsUse(node);
-
+        name == context->runtime->atomState.argumentsAtom)
+    {
         /*
          * Bind early to JSOP_ARGUMENTS to relieve later code from having
          * to do this work (new rule for the emitter to count on).
          */
-        if (!afterDot && !(tc->flags & TCF_DECL_DESTRUCTURING)
-            && !tc->inStatement(STMT_WITH)) {
-            node->setOp(JSOP_ARGUMENTS);
-            node->pn_dflags |= PND_BOUND;
+        if (!afterDoubleDot) {
+            /*
+             * Note use of |arguments| to ensure we can properly create the
+             * |arguments| object for this function.
+             */
+            tc->noteArgumentsNameUse(node);
+
+            if (!(tc->flags & TCF_DECL_DESTRUCTURING) && !tc->inStatement(STMT_WITH)) {
+                node->setOp(JSOP_ARGUMENTS);
+                node->pn_dflags |= PND_BOUND;
+            }
         }
-    } else if ((!afterDot
+    } else if ((!afterDoubleDot
 #if JS_HAS_XML_SUPPORT
                 || (!tc->inStrictMode() && tokenStream.peekToken() == TOK_DBLCOLON)
 #endif
                ) && !(tc->flags & TCF_DECL_DESTRUCTURING))
     {
         /* In case this is a generator expression outside of any function. */
         if (!tc->inFunction() && name == context->runtime->atomState.argumentsAtom)
             tc->countArgumentsUse(node);
@@ -6703,17 +6712,17 @@ Parser::identifierName(bool afterDot)
 
         node->pn_dflags |= (dn->pn_dflags & PND_FUNARG);
         if (stmt && stmt->type == STMT_WITH)
             node->pn_dflags |= PND_DEOPTIMIZED;
     }
 
 #if JS_HAS_XML_SUPPORT
     if (!tc->inStrictMode() && tokenStream.matchToken(TOK_DBLCOLON)) {
-        if (afterDot) {
+        if (afterDoubleDot) {
             if (!checkForFunctionNode(name, node))
                 return NULL;
         }
         node = qualifiedSuffix(node);
         if (!node)
             return NULL;
     }
 #endif
@@ -6730,17 +6739,17 @@ Parser::starOrAtPropertyIdentifier(Token
         reportErrorNumber(NULL, JSREPORT_ERROR, JSMSG_SYNTAX_ERROR);
         return NULL;
     }
     return (tt == TOK_AT) ? attributeIdentifier() : qualifiedIdentifier();
 }
 #endif
 
 ParseNode *
-Parser::primaryExpr(TokenKind tt, JSBool afterDot)
+Parser::primaryExpr(TokenKind tt, bool afterDoubleDot)
 {
     JS_ASSERT(tokenStream.isCurrentTokenType(tt));
 
     ParseNode *pn, *pn2, *pn3;
     JSOp op;
 
     JS_CHECK_RECURSION(context, return NULL);
 
@@ -7163,17 +7172,17 @@ Parser::primaryExpr(TokenKind tt, JSBool
         pn = new_<XMLProcessingInstruction>(tok.xmlPITarget(), tok.xmlPIData(), tok.pos);
         if (!pn)
             return NULL;
         break;
       }
 #endif
 
       case TOK_NAME:
-        pn = identifierName(afterDot);
+        pn = identifierName(afterDoubleDot);
         break;
 
       case TOK_REGEXP:
       {
         pn = NullaryNode::create(PNK_REGEXP, tc);
         if (!pn)
             return NULL;
 
--- a/js/src/frontend/Parser.h
+++ b/js/src/frontend/Parser.h
@@ -229,17 +229,17 @@ struct Parser : private AutoGCRooter
     ParseNode *shiftExpr1i();
     ParseNode *shiftExpr1n();
     ParseNode *addExpr1i();
     ParseNode *addExpr1n();
     ParseNode *mulExpr1i();
     ParseNode *mulExpr1n();
     ParseNode *unaryExpr();
     ParseNode *memberExpr(JSBool allowCallSyntax);
-    ParseNode *primaryExpr(TokenKind tt, JSBool afterDot);
+    ParseNode *primaryExpr(TokenKind tt, bool afterDoubleDot);
     ParseNode *parenExpr(JSBool *genexp = NULL);
 
     /*
      * Additional JS parsers.
      */
     enum FunctionType { Getter, Setter, Normal };
     bool functionArguments(TreeContext &funtc, FunctionBox *funbox, ParseNode **list);
 
@@ -254,17 +254,17 @@ struct Parser : private AutoGCRooter
     JSBool argumentList(ParseNode *listNode);
     ParseNode *bracketedExpr();
     ParseNode *letBlock(LetContext letContext);
     ParseNode *returnOrYield(bool useAssignExpr);
     ParseNode *destructuringExpr(BindData *data, TokenKind tt);
 
     bool checkForFunctionNode(PropertyName *name, ParseNode *node);
 
-    ParseNode *identifierName(bool afterDot);
+    ParseNode *identifierName(bool afterDoubleDot);
 
 #if JS_HAS_XML_SUPPORT
     ParseNode *endBracketedExpr();
 
     ParseNode *propertySelector();
     ParseNode *qualifiedSuffix(ParseNode *pn);
     ParseNode *qualifiedIdentifier();
     ParseNode *attributeIdentifier();
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/arguments/e4x-descendants-with-arguments.js
@@ -0,0 +1,6 @@
+function f()
+{
+  var x = <><arguments/><arguments/></>;
+  x..arguments;
+}
+f();
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_5/extensions/arguments-property-access-in-function.js
@@ -0,0 +1,58 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ * Contributor:
+ *   Jeff Walden <jwalden+code@mit.edu>
+ */
+
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 721322;
+var summary =
+  'f.arguments must trigger an arguments object in non-strict mode functions';
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+var obj =
+  {
+    test: function()
+    {
+      var args = obj.test.arguments;
+      assertEq(args !== null, true);
+      assertEq(args[0], 5);
+      assertEq(args[1], undefined);
+      assertEq(args.length, 2);
+    }
+  };
+obj.test(5, undefined);
+
+var sobj =
+  {
+    test: function()
+    {
+     "use strict";
+
+      try
+      {
+        var args = sobj.test.arguments;
+        throw new Error("access to arguments property of strict mode " +
+                        "function didn't throw");
+      }
+      catch (e)
+      {
+        assertEq(e instanceof TypeError, true,
+                 "should have thrown TypeError, instead got: " + e);
+      }
+    }
+  };
+sobj.test(5, undefined);
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
--- a/js/src/tests/ecma_5/extensions/jstests.list
+++ b/js/src/tests/ecma_5/extensions/jstests.list
@@ -1,9 +1,10 @@
 url-prefix ../../jsreftest.html?test=ecma_5/extensions/
+script arguments-property-access-in-function.js
 script 8.12.5-01.js
 script 15.4.4.11.js
 script 15.9.4.2.js
 script Boolean-toSource.js
 script Number-toSource.js
 script Object-keys-and-object-ids.js
 script String-toSource.js
 script bug352085.js
new file mode 100644
--- /dev/null
+++ b/js/src/tests/js1_8/genexps/arguments-property-access-in-generator.js
@@ -0,0 +1,27 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ * Contributor:
+ *   Jeff Walden <jwalden+code@mit.edu>
+ */
+
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 721322;
+var summary = 'Allow f.arguments in generator expressions';
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+eval("(function() { return (f.arguments for (x in [1])); })()");
+eval("(function() { var f = { arguments: 12 }; return [f.arguments for (x in [1])]; })()");
+
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
--- a/js/src/tests/js1_8/genexps/jstests.list
+++ b/js/src/tests/js1_8/genexps/jstests.list
@@ -1,9 +1,10 @@
 url-prefix ../../jsreftest.html?test=js1_8/genexps/
+script arguments-property-access-in-generator.js
 script regress-347739.js
 script regress-349012-01.js
 script regress-349326.js
 script regress-349331.js
 script regress-380237-01.js
 script regress-380237-02.js
 script regress-380237-03.js
 skip script regress-380237-04.js # obsolete test, need to remove minor failures to reenable.