Title: [242444] releases/WebKitGTK/webkit-2.24
Revision
242444
Author
carlo...@webkit.org
Date
2019-03-05 04:40:47 -0800 (Tue, 05 Mar 2019)

Log Message

Merge r241968 - DFGBytecodeParser should not declare that a node won't clobberExit if DFGFixupPhase can later declare it does clobberExit
https://bugs.webkit.org/show_bug.cgi?id=194953
<rdar://problem/47595253>

Reviewed by Saam Barati.

JSTests:

I could not make this work without the infinite loop, so I am using a watchdog to be able to use it as a regression test.

* stress/has-indexed-property-with-worsening-array-mode.js: Added.

Source/_javascript_Core:

For each node that
(a) may or may not clobberExit depending on their arrayMode
(b) and get their arrayMode from profiling information in DFGBytecodeParser
(c) and can have their arrayMode refined by DFGFixupPhase,
We must make sure to be conservative in the DFGBytecodeParser and treat it as if it unconditionnally clobbered the exit.
Otherwise we will hit a validation failure after fixup if the next node was marked ExitValid and exits to the same semantic origin.

The list of nodes that fit (a) is:
- StringCharAt
- HasIndexProperty
- GetByVal
- PutByValDirect
- PutByVal
- PutByValAlias
- GetIndexedPropertyStorage

Out of these, the following also fit (b) and (c):
- HasIndexedProperty
- GetByVal
- PutByValDirect
- PutByVal

GetByVal already had "m_exitOK = false; // GetByVal must be treated as if it clobbers exit state, since FixupPhase may make it generic."
So we just have to fix the other three the same way.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
(JSC::DFG::ByteCodeParser::handlePutByVal):

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.24/JSTests/ChangeLog (242443 => 242444)


--- releases/WebKitGTK/webkit-2.24/JSTests/ChangeLog	2019-03-05 12:40:42 UTC (rev 242443)
+++ releases/WebKitGTK/webkit-2.24/JSTests/ChangeLog	2019-03-05 12:40:47 UTC (rev 242444)
@@ -1,3 +1,15 @@
+2019-02-22  Robin Morisset  <rmoris...@apple.com>
+
+        DFGBytecodeParser should not declare that a node won't clobberExit if DFGFixupPhase can later declare it does clobberExit
+        https://bugs.webkit.org/show_bug.cgi?id=194953
+        <rdar://problem/47595253>
+
+        Reviewed by Saam Barati.
+
+        I could not make this work without the infinite loop, so I am using a watchdog to be able to use it as a regression test.
+
+        * stress/has-indexed-property-with-worsening-array-mode.js: Added.
+
 2019-02-18  Dominik Infuehr  <dinfu...@igalia.com>
 
         [ARM] Fix crash with sampling profiler

Added: releases/WebKitGTK/webkit-2.24/JSTests/stress/has-indexed-property-with-worsening-array-mode.js (0 => 242444)


--- releases/WebKitGTK/webkit-2.24/JSTests/stress/has-indexed-property-with-worsening-array-mode.js	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.24/JSTests/stress/has-indexed-property-with-worsening-array-mode.js	2019-03-05 12:40:47 UTC (rev 242444)
@@ -0,0 +1,6 @@
+//@ requireOptions("--watchdog=1000", "--watchdog-exception-ok", "--useMaximalFlushInsertionPhase=1")
+// This test only seems to reproduce the issue when it runs in an infinite loop. So we use the watchdog to time it out.
+for (let x in [0]) {
+  break
+}
+while(1);

Modified: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog (242443 => 242444)


--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog	2019-03-05 12:40:42 UTC (rev 242443)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog	2019-03-05 12:40:47 UTC (rev 242444)
@@ -1,5 +1,42 @@
 2019-02-22  Robin Morisset  <rmoris...@apple.com>
 
+        DFGBytecodeParser should not declare that a node won't clobberExit if DFGFixupPhase can later declare it does clobberExit
+        https://bugs.webkit.org/show_bug.cgi?id=194953
+        <rdar://problem/47595253>
+
+        Reviewed by Saam Barati.
+
+        For each node that
+        (a) may or may not clobberExit depending on their arrayMode
+        (b) and get their arrayMode from profiling information in DFGBytecodeParser
+        (c) and can have their arrayMode refined by DFGFixupPhase,
+        We must make sure to be conservative in the DFGBytecodeParser and treat it as if it unconditionnally clobbered the exit.
+        Otherwise we will hit a validation failure after fixup if the next node was marked ExitValid and exits to the same semantic origin.
+
+        The list of nodes that fit (a) is:
+        - StringCharAt
+        - HasIndexProperty
+        - GetByVal
+        - PutByValDirect
+        - PutByVal
+        - PutByValAlias
+        - GetIndexedPropertyStorage
+
+        Out of these, the following also fit (b) and (c):
+        - HasIndexedProperty
+        - GetByVal
+        - PutByValDirect
+        - PutByVal
+
+        GetByVal already had "m_exitOK = false; // GetByVal must be treated as if it clobbers exit state, since FixupPhase may make it generic."
+        So we just have to fix the other three the same way.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        (JSC::DFG::ByteCodeParser::handlePutByVal):
+
+2019-02-22  Robin Morisset  <rmoris...@apple.com>
+
         B3ReduceStrength: missing peephole optimizations for binary operations
         https://bugs.webkit.org/show_bug.cgi?id=194252
 

Modified: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (242443 => 242444)


--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2019-03-05 12:40:42 UTC (rev 242443)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2019-03-05 12:40:47 UTC (rev 242444)
@@ -6834,6 +6834,7 @@
             addVarArgChild(property);
             addVarArgChild(nullptr);
             Node* hasIterableProperty = addToGraph(Node::VarArg, HasIndexedProperty, OpInfo(arrayMode.asWord()), OpInfo(static_cast<uint32_t>(PropertySlot::InternalMethodType::GetOwnProperty)));
+            m_exitOK = false; // HasIndexedProperty must be treated as if it clobbers exit state, since FixupPhase may make it generic.
             set(bytecode.m_dst, hasIterableProperty);
             NEXT_OPCODE(op_has_indexed_property);
         }
@@ -7212,6 +7213,7 @@
         addVarArgChild(0); // Leave room for property storage.
         addVarArgChild(0); // Leave room for length.
         addToGraph(Node::VarArg, isDirect ? PutByValDirect : PutByVal, OpInfo(arrayMode.asWord()), OpInfo(0));
+        m_exitOK = false; // PutByVal and PutByValDirect must be treated as if they clobber exit state, since FixupPhase may make them generic.
     }
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to