Parser::parseVariableDeclarationList should null check the node before attempting...
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Feb 2016 20:09:07 +0000 (20:09 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Feb 2016 20:09:07 +0000 (20:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154244
rdar://problem/24290670

Reviewed by Michael Saboff.

* parser/ASTBuilder.h:
(JSC::ASTBuilder::appendToCommaExpr): Catch the bug sooner in debug.
* parser/Parser.cpp:
(JSC::Parser<LexerType>::parseVariableDeclarationList): Fix the bug.
* tests/stress/for-let-comma.js: Added. This used to crash in debug and release.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@196587 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/parser/ASTBuilder.h
Source/JavaScriptCore/parser/Parser.cpp
Source/JavaScriptCore/tests/stress/for-let-comma.js [new file with mode: 0644]

index 3edc7bf..9eb26b8 100644 (file)
@@ -1,3 +1,17 @@
+2016-02-15  Filip Pizlo  <fpizlo@apple.com>
+
+        Parser::parseVariableDeclarationList should null check the node before attempting to create a new CommaExpr
+        https://bugs.webkit.org/show_bug.cgi?id=154244
+        rdar://problem/24290670
+
+        Reviewed by Michael Saboff.
+
+        * parser/ASTBuilder.h:
+        (JSC::ASTBuilder::appendToCommaExpr): Catch the bug sooner in debug.
+        * parser/Parser.cpp:
+        (JSC::Parser<LexerType>::parseVariableDeclarationList): Fix the bug.
+        * tests/stress/for-let-comma.js: Added. This used to crash in debug and release.
+
 2016-02-15  Benjamin Poulain  <bpoulain@apple.com>
 
         [JSC] Improve the interface of Inst::shouldTryAliasingDef()
index 4c878e9..31b83b4 100644 (file)
@@ -729,6 +729,7 @@ public:
     CommaNode* appendToCommaExpr(const JSTokenLocation& location, ExpressionNode*, ExpressionNode* tail, ExpressionNode* next)
     {
         ASSERT(tail->isCommaNode());
+        ASSERT(next);
         CommaNode* newTail = new (m_parserArena) CommaNode(location, next);
         static_cast<CommaNode*>(tail)->setNext(newTail);
         return newTail;
index 6a9c976..87695c5 100644 (file)
@@ -729,14 +729,16 @@ template <class TreeBuilder> TreeExpression Parser<LexerType>::parseVariableDecl
                 lastInitializer = rhs;
             }
         }
-        
-        if (!head)
-            head = node;
-        else if (!tail) {
-            head = context.createCommaExpr(location, head);
-            tail = context.appendToCommaExpr(location, head, head, node);
-        } else
-            tail = context.appendToCommaExpr(location, head, tail, node);
+
+        if (node) {
+            if (!head)
+                head = node;
+            else if (!tail) {
+                head = context.createCommaExpr(location, head);
+                tail = context.appendToCommaExpr(location, head, head, node);
+            } else
+                tail = context.appendToCommaExpr(location, head, tail, node);
+        }
     } while (match(COMMA));
     if (lastIdent)
         lastPattern = context.createBindingLocation(lastIdentToken.m_location, *lastIdent, lastIdentToken.m_startPosition, lastIdentToken.m_endPosition, assignmentContext);
diff --git a/Source/JavaScriptCore/tests/stress/for-let-comma.js b/Source/JavaScriptCore/tests/stress/for-let-comma.js
new file mode 100644 (file)
index 0000000..73c2e7c
--- /dev/null
@@ -0,0 +1,16 @@
+function foo() {
+    var array = [];
+    for (let x = 0, []; x < 10; ++x) { array.push(x); }
+    return array;
+}
+
+var result = foo();
+if (result.length != 10)
+    throw "Error: bad length: " + result.length;
+for (var i = 0; i < 10; ++i) {
+    if (result[i] != i)
+        throw "Error: bad entry at i = " + i + ": " + result[i];
+}
+if (result.length != 10)
+    throw "Error: bad length: " + result.length;
+