Bug 667131 - crash: 'yield' unnoticed due to calling maybeNoteGenerator() too late (r=cdleary)

js/src/jsparse.cpp

@@ -6676,12 +6676,20 @@ class CompExprTransplanter {
 
 /*
  * A helper for lazily checking for the presence of illegal |yield| or |arguments|
- * tokens inside of generator expressions.
+ * tokens inside of generator expressions. This must be done lazily since we don't
+ * know whether we're in a generator expression until we see the "for" token after
+ * we've already parsed the body expression.
  *
- * Use when entering a parenthesized context. If the nested expression is followed
- * by a |for| token (indicating that the parenthesized expression is a generator
- * expression), use the checkValidBody() method to see if any illegal tokens were
- * found.
+ * Use in any context which may turn out to be inside a generator expression. This
+ * includes parenthesized expressions and argument lists, and it includes the tail
+ * of generator expressions.
+ * 
+ * The guard will keep track of any |yield| or |arguments| tokens that occur while
+ * parsing the body. As soon as the parser reaches the end of the body expression,
+ * call endBody() to reset the context's state, and then immediately call:
+ *
+ * - checkValidBody() if this did turn out to be in a generator expression
+ * - maybeNoteGenerator() if this did not turn out to be in a generator expression
  */
 class GenexpGuard {
     JSTreeContext   *tc;
@@ -6703,6 +6711,7 @@ class GenexpGuard {
 
     void endBody();
     bool checkValidBody(JSParseNode *pn);
+    bool maybeNoteGenerator();
 };
 
 void
@@ -6711,6 +6720,13 @@ GenexpGuard::endBody()
     tc->parenDepth--;
 }
 
+/*
+ * Check whether a |yield| or |arguments| token has been encountered in the
+ * body expression, and if so, report an error.
+ *
+ * Call this after endBody() when determining that the body *was* in a
+ * generator expression.
+ */
 bool
 GenexpGuard::checkValidBody(JSParseNode *pn)
 {
@@ -6733,6 +6749,27 @@ GenexpGuard::checkValidBody(JSParseNode *pn)
     return true;
 }
 
+/*
+ * Check whether a |yield| token has been encountered in the body expression,
+ * and if so, note that the current function is a generator function.
+ *
+ * Call this after endBody() when determining that the body *was not* in a
+ * generator expression.
+ */
+bool
+GenexpGuard::maybeNoteGenerator()
+{
+    if (tc->yieldCount > 0) {
+        tc->flags |= TCF_FUN_IS_GENERATOR;
+        if (!tc->inFunction()) {
+            tc->parser->reportErrorNumber(NULL, JSREPORT_ERROR, JSMSG_BAD_RETURN_OR_YIELD,
+                                          js_yield_str);
+            return false;
+        }
+    }
+    return true;
+}
+
 /*
  * Any definitions nested within the comprehension expression of a generator
  * expression must move "down" one static level, which of course increases the
@@ -7055,8 +7092,12 @@ Parser::comprehensionTail(JSParseNode *kid, uintN blockid, bool isGenexp,
 
         guard.endBody();
 
-        if (isGenexp && !guard.checkValidBody(pn2))
+        if (isGenexp) {
+            if (!guard.checkValidBody(pn2))
+                return NULL;
+        } else if (!guard.maybeNoteGenerator()) {
             return NULL;
+        }
 
         switch (tt) {
 #if JS_HAS_DESTRUCTURING
@@ -7107,9 +7148,6 @@ Parser::comprehensionTail(JSParseNode *kid, uintN blockid, bool isGenexp,
         pnp = &pn2->pn_kid2;
     }
 
-    if (!maybeNoteGenerator())
-        return NULL;
-
     pn2 = UnaryNode::create(tc);
     if (!pn2)
         return NULL;
@@ -7229,27 +7267,6 @@ static const char js_generator_str[] = "generator";
 #endif /* JS_HAS_GENERATOR_EXPRS */
 #endif /* JS_HAS_GENERATORS */
 
-/*
- * Check whether a |yield| token has been encountered since the last reset point
- * (the creation of the tree context or the current GenexpGuard), and if so,
- * note that the current function is a generator function.
- *
- * Call this after the current GenexpGuard has determined whether it was inside
- * of a generator expression.
- */
-bool
-Parser::maybeNoteGenerator()
-{
-    if (tc->yieldCount > 0) {
-        tc->flags |= TCF_FUN_IS_GENERATOR;
-        if (!tc->inFunction()) {
-            reportErrorNumber(NULL, JSREPORT_ERROR, JSMSG_BAD_RETURN_OR_YIELD, js_yield_str);
-            return false;
-        }
-    }
-    return true;
-}
-
 JSBool
 Parser::argumentList(JSParseNode *listNode)
 {
@@ -7263,10 +7280,8 @@ Parser::argumentList(JSParseNode *listNode)
         JSParseNode *argNode = assignExpr();
         if (!argNode)
             return JS_FALSE;
-        if (arg0) {
+        if (arg0)
             guard.endBody();
-            arg0 = false;
-        }
 
 #if JS_HAS_GENERATORS
         if (argNode->pn_type == TOK_YIELD &&
@@ -7289,14 +7304,16 @@ Parser::argumentList(JSParseNode *listNode)
                                   js_generator_str);
                 return JS_FALSE;
             }
-        }
+        } else
 #endif
+        if (arg0 && !guard.maybeNoteGenerator())
+            return JS_FALSE;
+
+        arg0 = false;
+
         listNode->append(argNode);
     } while (tokenStream.matchToken(TOK_COMMA));
 
-    if (!maybeNoteGenerator())
-        return JS_FALSE;
-
     if (tokenStream.getToken() != TOK_RP) {
         reportErrorNumber(NULL, JSREPORT_ERROR, JSMSG_PAREN_AFTER_ARGS);
         return JS_FALSE;
@@ -8898,10 +8915,10 @@ Parser::parenExpr(JSBool *genexp)
             pn->pn_pos.end = tokenStream.currentToken().pos.end;
             *genexp = JS_TRUE;
         }
-    }
+    } else
 #endif /* JS_HAS_GENERATOR_EXPRS */
 
-    if (!maybeNoteGenerator())
+    if (!guard.maybeNoteGenerator())
         return NULL;
 
     return pn;

js/src/jsparse.h

@@ -1145,8 +1145,6 @@ struct Parser : private js::AutoGCRooter
     inline bool reportErrorNumber(JSParseNode *pn, uintN flags, uintN errorNumber, ...);
 
 private:
-    bool maybeNoteGenerator();
-
     /*
      * JS parsers, from lowest to highest precedence.
      *

js/src/tests/js1_8/genexps/jstests.list

@@ -11,3 +11,4 @@ script regress-384991.js
 script regress-634472.js
 script regress-665286.js
 script regress-666852.js
+script regress-667131.js

js/src/tests/js1_8/genexps/regress-667131.js

@@ -0,0 +1,90 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is JavaScript Engine testing utilities.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Foundation.
+ * Portions created by the Initial Developer are Copyright (C) 2007
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s): Dave Herman
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 667131;
+var summary = 'yield ignored if maybeNoteGenerator called too late';
+var actual = '';
+var expect = '';
+
+function testGenerator(f, desc) {
+    reportCompare(f.isGenerator(), true, desc + ": is generator");
+    reportCompare(typeof f(), "object", desc + ": calling doesn't crash");
+}
+
+function reported1() {
+    (function(){})([yield[]], (""))
+}
+
+function reported2() {
+    (function(){})(let(w = "") yield, (e))
+}
+
+function simplified1() {
+    print([yield], (0))
+}
+
+function simplified2() {
+    print(let(w) yield, (0))
+}
+
+function f1(a) { [x for (x in yield) for (y in (a))] }
+function f2(a) { [x for (x in yield) if (y in (a))] }
+function f3(a) { ([x for (x in yield) for (y in (a))]) }
+function f4(a) { ([x for (x in yield) if (y in (a))]) }
+
+function f5() { print(<a>{yield}</a>, (0)) }
+function f6() { print(<>{yield}</>, (0)) }
+function f7() { print({a:yield},(0)) }
+
+function f8() { ([yield], (0)) }
+function f9() { (let(w)yield, (0)) }
+
+testGenerator(reported1, "reported function with array literal");
+testGenerator(reported2, "reported function with let-expression");
+testGenerator(simplified1, "reported function with array literal, simplified");
+testGenerator(simplified2, "reported function with let-expression, simplified");
+testGenerator(f1, "top-level array comprehension with paren expr in for-block");
+testGenerator(f2, "top-level array comprehension with paren expr in if-block");
+testGenerator(f3, "parenthesized array comprehension with paren expr in for-block");
+testGenerator(f4, "parenthesized array comprehension with paren expr in if-block");
+testGenerator(f5, "xml literal");
+testGenerator(f6, "xml list literal");
+testGenerator(f7, "object literal");
+testGenerator(f8, "array literal in paren exp");
+testGenerator(f9, "let-expression in paren exp");