[DFG][FTL] Spread onto PhantomNewArrayBuffer assumes JSFixedArray, but JSImmutableBut...
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Jun 2018 11:18:05 +0000 (11:18 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 16 Jun 2018 11:18:05 +0000 (11:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186460

Reviewed by Saam Barati.

JSTests:

* stress/spread-escapes-but-new-array-buffer-does-not-double.js: Added.
(assert):
(getProperties):
(theFunc):
(let.obj.valueOf):

Source/JavaScriptCore:

Spread(PhantomNewArrayBuffer) returns JSImmutableButterfly. But it is wrong.
We should return JSFixedArray for Spread. This patch adds a code generating
a JSFixedArray from JSImmutableButterfly.

Merging JSFixedArray into JSImmutableButterfly is possible future extension.

* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileSpread):
* runtime/JSFixedArray.h:

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

JSTests/ChangeLog
JSTests/stress/spread-escapes-but-new-array-buffer-does-not-double.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/runtime/JSFixedArray.h

index 1e33611..dd389db 100644 (file)
@@ -1,3 +1,16 @@
+2018-06-15  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG][FTL] Spread onto PhantomNewArrayBuffer assumes JSFixedArray, but JSImmutableButterfly is returned
+        https://bugs.webkit.org/show_bug.cgi?id=186460
+
+        Reviewed by Saam Barati.
+
+        * stress/spread-escapes-but-new-array-buffer-does-not-double.js: Added.
+        (assert):
+        (getProperties):
+        (theFunc):
+        (let.obj.valueOf):
+
 2018-06-14  Leo Balter  <leonardo.balter@gmail.com>
 
         Test262-Runner: Update config list with some failing tests
diff --git a/JSTests/stress/spread-escapes-but-new-array-buffer-does-not-double.js b/JSTests/stress/spread-escapes-but-new-array-buffer-does-not-double.js
new file mode 100644 (file)
index 0000000..8662b8d
--- /dev/null
@@ -0,0 +1,35 @@
+function assert(b) {
+    if (!b)
+        throw new Error;
+}
+noInline(assert);
+
+function getProperties(obj) {
+    let properties = [];
+    for (let name of Object.getOwnPropertyNames(obj)) {
+        properties.push(name);
+    }
+    return properties;
+}
+
+function theFunc(obj, index) {
+    let args = [42.195, 20.2];
+    let functions = getProperties(obj);
+    let func = functions[index % functions.length];
+    obj[func](...args);
+}
+
+let obj = {
+    valueOf: function (x, y) {
+        assert(x === 42.195);
+        assert(y === 20.2);
+        try {
+        } catch (e) {}
+    }
+};
+
+for (let i = 0; i < 1e5; ++i) {
+    for (let _i = 0; _i < 100; _i++) {
+    }
+    theFunc(obj, 897989);
+}
index 2bff9d2..da5b38e 100644 (file)
@@ -1,3 +1,20 @@
+2018-06-15  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG][FTL] Spread onto PhantomNewArrayBuffer assumes JSFixedArray, but JSImmutableButterfly is returned
+        https://bugs.webkit.org/show_bug.cgi?id=186460
+
+        Reviewed by Saam Barati.
+
+        Spread(PhantomNewArrayBuffer) returns JSImmutableButterfly. But it is wrong.
+        We should return JSFixedArray for Spread. This patch adds a code generating
+        a JSFixedArray from JSImmutableButterfly.
+
+        Merging JSFixedArray into JSImmutableButterfly is possible future extension.
+
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileSpread):
+        * runtime/JSFixedArray.h:
+
 2018-06-15  Saam Barati  <sbarati@apple.com>
 
         Annotate shrinkFootprintWhenIdle with NS_AVAILABLE
index de31efb..d3dc9b9 100644 (file)
@@ -5702,7 +5702,34 @@ private:
     void compileSpread()
     {
         if (m_node->child1()->op() == PhantomNewArrayBuffer) {
-            setJSValue(frozenPointer(m_node->child1()->cellOperand()));
+            LBasicBlock slowAllocation = m_out.newBlock();
+            LBasicBlock continuation = m_out.newBlock();
+
+            auto* immutableButterfly = m_node->child1()->castOperand<JSImmutableButterfly*>();
+
+            LValue fastFixedArrayValue = allocateVariableSizedCell<JSFixedArray>(
+                m_out.constIntPtr(JSFixedArray::allocationSize(immutableButterfly->length()).unsafeGet()),
+                m_graph.m_vm.fixedArrayStructure.get(), slowAllocation);
+            m_out.store32(m_out.constInt32(immutableButterfly->length()), fastFixedArrayValue, m_heaps.JSFixedArray_size);
+            ValueFromBlock fastFixedArray = m_out.anchor(fastFixedArrayValue);
+            m_out.jump(continuation);
+
+            LBasicBlock lastNext = m_out.appendTo(slowAllocation, continuation);
+            ValueFromBlock slowFixedArray = m_out.anchor(vmCall(pointerType(), m_out.operation(operationCreateFixedArray), m_callFrame, m_out.constInt32(immutableButterfly->length())));
+            m_out.jump(continuation);
+
+            m_out.appendTo(continuation, lastNext);
+            LValue fixedArray = m_out.phi(pointerType(), fastFixedArray, slowFixedArray);
+            for (unsigned i = 0; i < immutableButterfly->length(); i++) {
+                // Because forwarded values are drained as JSValue, we should not generate value
+                // in Double form even if PhantomNewArrayBuffer's indexingType is ArrayWithDouble.
+                int64_t value = JSValue::encode(immutableButterfly->get(i));
+                m_out.store64(
+                    m_out.constInt64(value),
+                    m_out.baseIndex(m_heaps.JSFixedArray_buffer, fixedArray, m_out.constIntPtr(i), jsNumber(i)));
+            }
+            mutatorFence();
+            setJSValue(fixedArray);
             return;
         }
 
@@ -5733,11 +5760,11 @@ private:
             m_out.jump(loopHeader);
 
             m_out.appendTo(slowAllocation, loopHeader);
-            ValueFromBlock slowArray = m_out.anchor(vmCall(Int64, m_out.operation(operationCreateFixedArray), m_callFrame, length));
+            ValueFromBlock slowArray = m_out.anchor(vmCall(pointerType(), m_out.operation(operationCreateFixedArray), m_callFrame, length));
             m_out.jump(loopHeader);
 
             m_out.appendTo(loopHeader, loopBody);
-            LValue fixedArray = m_out.phi(Int64, fastArray, slowArray);
+            LValue fixedArray = m_out.phi(pointerType(), fastArray, slowArray);
             ValueFromBlock startIndex = m_out.anchor(m_out.constIntPtr(0));
             m_out.branch(m_out.isZero32(length), unsure(continuation), unsure(loopBody));
 
@@ -5835,14 +5862,14 @@ private:
             }
 
             m_out.appendTo(slowPath, continuation);
-            ValueFromBlock slowResult = m_out.anchor(vmCall(Int64, m_out.operation(operationSpreadFastArray), m_callFrame, argument));
+            ValueFromBlock slowResult = m_out.anchor(vmCall(pointerType(), m_out.operation(operationSpreadFastArray), m_callFrame, argument));
             m_out.jump(continuation);
 
             m_out.appendTo(continuation, lastNext);
-            result = m_out.phi(Int64, fastResult, slowResult);
+            result = m_out.phi(pointerType(), fastResult, slowResult);
             mutatorFence();
         } else
-            result = vmCall(Int64, m_out.operation(operationSpreadGeneric), m_callFrame, argument);
+            result = vmCall(pointerType(), m_out.operation(operationSpreadGeneric), m_callFrame, argument);
 
         setJSValue(result);
     }
index 1586133..260fa37 100644 (file)
@@ -149,6 +149,11 @@ public:
 
     static void dumpToStream(const JSCell*, PrintStream&);
 
+    static Checked<size_t, RecordOverflow> allocationSize(Checked<size_t, RecordOverflow> numItems)
+    {
+        return offsetOfData() + numItems * sizeof(WriteBarrier<Unknown>);
+    }
+
 private:
     JSFixedArray(VM& vm, Structure* structure, unsigned size)
         : Base(vm, structure)
@@ -158,11 +163,6 @@ private:
             buffer()[i].setStartingValue(JSValue());
     }
 
-    static Checked<size_t, RecordOverflow> allocationSize(Checked<size_t, RecordOverflow> numItems)
-    {
-        return offsetOfData() + numItems * sizeof(WriteBarrier<Unknown>);
-    }
-
     unsigned m_size;
 };