Attachment 342801 Details for Bug 186460 – Patch

[patch] Patch

bug-186460-20180615200054.patch (text/plain), 6.24 KB, created by Yusuke Suzuki on 2018-06-15 04:00:55 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-06-15 04:00:55 PDT

Size: 6.24 KB

patch

obsolete

>Subversion Revision: 232867
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 7b05c3139fae7ae99cceb951ac39607392537af1..37ea0267d36612cff491ce63a0622c1cc8fdd90e 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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-14  Michael Saboff  <msaboff@apple.com>
> 
>         REGRESSION(232741): Crash running ARES-6
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index de31efb5fc1067a7a327d816dd358c31add0fc81..bdd1b7eef31238dc4371172d3c78b737ba55295f 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -5702,7 +5702,34 @@ class LowerDFGToB3 {
>     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(Int64, 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(Int64, 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;
>         }
> 
>diff --git a/Source/JavaScriptCore/runtime/JSFixedArray.h b/Source/JavaScriptCore/runtime/JSFixedArray.h
>index 1586133051c068c36470fe8c9613bd0b2158b867..260fa379c63719eebfd9fef68c4ae5aa1c411eca 100644
>--- a/Source/JavaScriptCore/runtime/JSFixedArray.h
>+++ b/Source/JavaScriptCore/runtime/JSFixedArray.h
>@@ -149,6 +149,11 @@ class JSFixedArray final : public JSCell {
> 
>     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 @@ class JSFixedArray final : public JSCell {
>             buffer()[i].setStartingValue(JSValue());
>     }
> 
>-    static Checked<size_t, RecordOverflow> allocationSize(Checked<size_t, RecordOverflow> numItems)
>-    {
>-        return offsetOfData() + numItems * sizeof(WriteBarrier<Unknown>);
>-    }
>-
>     unsigned m_size;
> };
> 
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 1e3361169b6966070b13c2386d5ef3798a82589b..34073fc9d02f9eaa4f447a5a0dc7f9bbb9a13215 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        * 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
>index 0000000000000000000000000000000000000000..8662b8d64834566764eb92215c1c6cd3767e7e82
>--- /dev/null
>+++ b/JSTests/stress/spread-escapes-but-new-array-buffer-does-not-double.js
>@@ -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);
>+}

Flags:

saam: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 186460: 342801 | 342834