In FTLLowerDFGToB3.cpp::compileCreateRest, always use a contiguous array as the index...
authorrmorisset@apple.com <rmorisset@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Apr 2018 23:32:58 +0000 (23:32 +0000)
committerrmorisset@apple.com <rmorisset@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Apr 2018 23:32:58 +0000 (23:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184773
<rdar://problem/37773612>

Reviewed by Filip Pizlo.

JSTests:

This bug requires a race between the thread doing FTL compilation and the main thread, but it triggers in 100% of cases (before the fix) on my machine
so I decided to add it to the stress tests nonetheless.

* stress/create-rest-while-having-a-bad-time.js: Added.
(f):
(g):
(h):

Source/JavaScriptCore:

We were calling restParameterStructure(), which returns arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous).
arrayStructureForIndexingTypeDuringAllocation uses m_arrayStructureForIndexingShapeDuringAllocation, which is set to SlowPutArrayStorage when we are 'having a bad time'.
This is problematic, because the structure is then passed to allocateUninitializedContiguousJSArray, which ASSERTs that the indexing type is contiguous (or int32).
We solve the problem by using originalArrayStructureForIndexingType which always returns a structure with the right indexing type (contiguous), even if we are having a bad time.
This is safe, as we are under isWatchingHavingABadTimeWatchpoint, so if we have a bad time, the code we generate will never be installed.

* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileCreateRest):

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

JSTests/ChangeLog
JSTests/stress/create-rest-while-having-a-bad-time.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/runtime/JSGlobalObject.h

index 61dbcbf..461f6d6 100644 (file)
@@ -1,3 +1,19 @@
+2018-04-25  Robin Morisset  <rmorisset@apple.com>
+
+        In FTLLowerDFGToB3.cpp::compileCreateRest, always use a contiguous array as the indexing type when under isWatchingHavingABadTimeWatchpoint
+        https://bugs.webkit.org/show_bug.cgi?id=184773
+        <rdar://problem/37773612>
+
+        Reviewed by Filip Pizlo.
+
+        This bug requires a race between the thread doing FTL compilation and the main thread, but it triggers in 100% of cases (before the fix) on my machine
+        so I decided to add it to the stress tests nonetheless.
+
+        * stress/create-rest-while-having-a-bad-time.js: Added.
+        (f):
+        (g):
+        (h):
+
 2018-04-25  Keith Miller  <keith_miller@apple.com>
 
         Add missing scope release to functionProtoFuncToString
diff --git a/JSTests/stress/create-rest-while-having-a-bad-time.js b/JSTests/stress/create-rest-while-having-a-bad-time.js
new file mode 100644 (file)
index 0000000..e599681
--- /dev/null
@@ -0,0 +1,16 @@
+"use strict";
+function f(...v) {
+  return g(v);
+}
+function g() {
+  return h();
+}
+function h() {
+}
+
+for (let i = 0; i < 10000; ++i) {
+  f(0);
+  f(0, 0);
+}
+
+Object.defineProperty(Array.prototype, "42", {});
index 4f080a7..65a52c1 100644 (file)
@@ -1,3 +1,20 @@
+2018-04-25  Robin Morisset  <rmorisset@apple.com>
+
+        In FTLLowerDFGToB3.cpp::compileCreateRest, always use a contiguous array as the indexing type when under isWatchingHavingABadTimeWatchpoint
+        https://bugs.webkit.org/show_bug.cgi?id=184773
+        <rdar://problem/37773612>
+
+        Reviewed by Filip Pizlo.
+
+        We were calling restParameterStructure(), which returns arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous).
+        arrayStructureForIndexingTypeDuringAllocation uses m_arrayStructureForIndexingShapeDuringAllocation, which is set to SlowPutArrayStorage when we are 'having a bad time'.
+        This is problematic, because the structure is then passed to allocateUninitializedContiguousJSArray, which ASSERTs that the indexing type is contiguous (or int32).
+        We solve the problem by using originalArrayStructureForIndexingType which always returns a structure with the right indexing type (contiguous), even if we are having a bad time.
+        This is safe, as we are under isWatchingHavingABadTimeWatchpoint, so if we have a bad time, the code we generate will never be installed.
+
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileCreateRest):
+
 2018-04-25  Mark Lam  <mark.lam@apple.com>
 
         Push the definition of PtrTag down to the WTF layer.
index 157480b..cc3ee98 100644 (file)
@@ -5281,7 +5281,7 @@ private:
             LValue arrayLength = lowInt32(m_node->child1());
             LBasicBlock loopStart = m_out.newBlock();
             JSGlobalObject* globalObject = m_graph.globalObjectFor(m_node->origin.semantic);
-            RegisteredStructure structure = m_graph.registerStructure(globalObject->restParameterStructure());
+            RegisteredStructure structure = m_graph.registerStructure(globalObject->originalRestParameterStructure());
             ArrayValues arrayValues = allocateUninitializedContiguousJSArray(arrayLength, structure);
             LValue array = arrayValues.array;
             LValue butterfly = arrayValues.butterfly;
index db3f83f..7ad0f74 100644 (file)
@@ -676,6 +676,7 @@ public:
     Structure* callableProxyObjectStructure() const { return m_callableProxyObjectStructure.get(); }
     Structure* proxyRevokeStructure() const { return m_proxyRevokeStructure.get(); }
     Structure* restParameterStructure() const { return arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous); }
+    Structure* originalRestParameterStructure() const { return originalArrayStructureForIndexingType(ArrayWithContiguous); }
 #if ENABLE(WEBASSEMBLY)
     Structure* webAssemblyModuleRecordStructure() const { return m_webAssemblyModuleRecordStructure.get(); }
     Structure* webAssemblyFunctionStructure() const { return m_webAssemblyFunctionStructure.get(); }