Attachment 338767 Details for Bug 184773 – Patch for landing

[patch] Patch for landing

bug-184773-20180425210009.patch (text/plain), 5.06 KB, created by Robin Morisset on 2018-04-25 12:00:11 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Robin Morisset

Created: 2018-04-25 12:00:11 PDT

Size: 5.06 KB

patch

obsolete

>Subversion Revision: 231001
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index c95a4dadd730333809af2fb38a9cb3351e226e57..d1a176cb8cb671a3cfd7d09561da51a1af90eb2d 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -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  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         [GTK] Miscellaneous build cleanups
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index 157480ba691b46dce0dc241a9338b1e636ae7cc9..cc3ee9842db38d236d7111d768acd60557351c0c 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -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;
>diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.h b/Source/JavaScriptCore/runtime/JSGlobalObject.h
>index db3f83f50110fbac2dc0bfb068f88b522b077e13..7ad0f74ba26d3689d294a8afdff48ade0540b4dd 100644
>--- a/Source/JavaScriptCore/runtime/JSGlobalObject.h
>+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.h
>@@ -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(); }
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 6629c69ad2e12746f713312ea8e89d3f6a76bda8..7200c79435923183f539778de460da1daf7c7ec5 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -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-24  Keith Miller  <keith_miller@apple.com>
> 
>         fromCharCode is missing some exception checks
>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
>index 0000000000000000000000000000000000000000..e599681becc1abd08c4859e957464f6dfb5fd6bf
>--- /dev/null
>+++ b/JSTests/stress/create-rest-while-having-a-bad-time.js
>@@ -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", {});

Actions: View | Formatted Diff | Diff

Attachments on bug 184773: 338320 | 338767