Spread expressions are not fair game for direct binding

https://bugs.webkit.org/show_bug.cgi?id=154042
rdar://problem/24291413

Reviewed by Saam Barati.

Prior to this change we crashed on this:

    var [x] = [...y];

Because NodesCodegen thinks that this is a direct binding.  It's not, because we cannot
directly generate bytecode for "...y".  This is a unique property of spread expressions, so
its sufficient to just bail out of direct binding if we see a spread expression. That's what
this patch does.

* bytecompiler/NodesCodegen.cpp:
(JSC::ArrayPatternNode::emitDirectBinding):
* tests/stress/spread-in-tail.js: Added.
(foo):
(catch):

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

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index edc2be7..a13e501 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,26 @@
+2016-02-09  Filip Pizlo  <fpizlo@apple.com>
+
+        Spread expressions are not fair game for direct binding
+        https://bugs.webkit.org/show_bug.cgi?id=154042
+        rdar://problem/24291413
+
+        Reviewed by Saam Barati.
+
+        Prior to this change we crashed on this:
+
+            var [x] = [...y];
+
+        Because NodesCodegen thinks that this is a direct binding.  It's not, because we cannot
+        directly generate bytecode for "...y".  This is a unique property of spread expressions, so
+        its sufficient to just bail out of direct binding if we see a spread expression. That's what
+        this patch does.
+
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::ArrayPatternNode::emitDirectBinding):
+        * tests/stress/spread-in-tail.js: Added.
+        (foo):
+        (catch):
+
 2016-02-09  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r196286.
@@ -38,6 +61,7 @@
 
         runtimeTypeForValue should protect against seeing TDZ value
         https://bugs.webkit.org/show_bug.cgi?id=154023
+        rdar://problem/24291413
 
         Reviewed by Michael Saboff.
 

diff --git a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
index 096444a..5df18a9 100644 (file)
--- a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
+++ b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
@@ -3431,17 +3431,22 @@ void ArrayPatternNode::bindValue(BytecodeGenerator& generator, RegisterID* rhs)
 RegisterID* ArrayPatternNode::emitDirectBinding(BytecodeGenerator& generator, RegisterID* dst, ExpressionNode* rhs)
 {
     if (!rhs->isSimpleArray())
-        return 0;
+        return nullptr;
+
+    ElementNode* elementNodes = static_cast<ArrayNode*>(rhs)->elements();
+    Vector<ExpressionNode*> elements;
+    for (; elementNodes; elementNodes = elementNodes->next()) {
+        ExpressionNode* value = elementNodes->value();
+        if (value->isSpreadExpression())
+            return nullptr;
+        elements.append(value);
+    }
 
     RefPtr<RegisterID> resultRegister;
     if (dst && dst != generator.ignoredResult())
         resultRegister = generator.emitNewArray(generator.newTemporary(), 0, 0);
-    ElementNode* elementNodes = static_cast<ArrayNode*>(rhs)->elements();
-    Vector<ExpressionNode*> elements;
-    for (; elementNodes; elementNodes = elementNodes->next())
-        elements.append(elementNodes->value());
     if (m_targetPatterns.size() != elements.size())
-        return 0;
+        return nullptr;
     Vector<RefPtr<RegisterID>> registers;
     registers.reserveCapacity(m_targetPatterns.size());
     for (size_t i = 0; i < m_targetPatterns.size(); i++) {

diff --git a/Source/JavaScriptCore/tests/stress/spread-in-tail.js b/Source/JavaScriptCore/tests/stress/spread-in-tail.js
new file mode 100644 (file)
index 0000000..a2a8cb8
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/spread-in-tail.js
@@ -0,0 +1,18 @@
+function foo(b, y, d) {
+    // Note that when we fixed this, this simpler thing would also crash:
+    //     var [x] = [...y];
+    
+    var [a, x, c] = [b, ...y, d];
+    return [a, x, c];
+}
+
+function test(args, expected) {
+    var actual = foo.apply(this, args);
+    if ("" + actual != "" + expected)
+        throw "Error: bad result: " + actual;
+}
+
+test([1, [], 2], [1, 2, void 0]);
+test([1, [2], 3], [1, 2, 3]);
+test([1, [2, 3], 4], [1, 2, 3]);
+