Title: [278578] trunk
Revision
278578
Author
[email protected]
Date
2021-06-07 16:26:53 -0700 (Mon, 07 Jun 2021)

Log Message

Short circuit read modify write nodes emit byte code that uses the wrong locals
https://bugs.webkit.org/show_bug.cgi?id=226576
<rdar://problem/78810362>

Reviewed by Yusuke Suzuki.

JSTests:

* stress/short-circuit-read-modify-should-use-the-write-virtual-registers.js: Added.
(eval):

Source/_javascript_Core:

It's never a good idea to use the wrong local :-)

This patch also adds support for dumping predecessors of basic blocks
in the bytecode dump.

* bytecode/BytecodeDumper.cpp:
(JSC::CodeBlockBytecodeDumper<Block>::dumpGraph):
* bytecompiler/NodesCodegen.cpp:
(JSC::ShortCircuitReadModifyResolveNode::emitBytecode):
(JSC::ShortCircuitReadModifyDotNode::emitBytecode):
(JSC::ShortCircuitReadModifyBracketNode::emitBytecode):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (278577 => 278578)


--- trunk/JSTests/ChangeLog	2021-06-07 23:07:02 UTC (rev 278577)
+++ trunk/JSTests/ChangeLog	2021-06-07 23:26:53 UTC (rev 278578)
@@ -1,3 +1,14 @@
+2021-06-07  Saam Barati  <[email protected]>
+
+        Short circuit read modify write nodes emit byte code that uses the wrong locals
+        https://bugs.webkit.org/show_bug.cgi?id=226576
+        <rdar://problem/78810362>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/short-circuit-read-modify-should-use-the-write-virtual-registers.js: Added.
+        (eval):
+
 2021-06-07  Robin Morisset  <[email protected]>
 
         Optimize compareStrictEq when neither side is a double and at least one is neither a string nor a BigInt

Added: trunk/JSTests/stress/short-circuit-read-modify-should-use-the-write-virtual-registers.js (0 => 278578)


--- trunk/JSTests/stress/short-circuit-read-modify-should-use-the-write-virtual-registers.js	                        (rev 0)
+++ trunk/JSTests/stress/short-circuit-read-modify-should-use-the-write-virtual-registers.js	2021-06-07 23:26:53 UTC (rev 278578)
@@ -0,0 +1,33 @@
+eval(`
+    for (var i=0; i < 1000; i++) {
+        const x = 1;
+        let y = 42;
+        try {
+            y ||= x *= some;
+        }
+        catch { }
+        finally { }
+    }
+`);
+
+eval(`
+    for (var i=0; i < 1000; i++) {
+        const x = 1;
+        try {
+            foo.x ||= x *= some;
+        }
+        catch { }
+        finally { }
+    }
+`);
+
+eval(`
+    for(var i = 0; i < 1000; i++) {
+        const x = 1;
+        try {
+            foo[0] ||= x *= some;
+        }
+        catch { }
+        finally { }
+    }
+`);

Modified: trunk/Source/_javascript_Core/ChangeLog (278577 => 278578)


--- trunk/Source/_javascript_Core/ChangeLog	2021-06-07 23:07:02 UTC (rev 278577)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-06-07 23:26:53 UTC (rev 278578)
@@ -1,3 +1,23 @@
+2021-06-07  Saam Barati  <[email protected]>
+
+        Short circuit read modify write nodes emit byte code that uses the wrong locals
+        https://bugs.webkit.org/show_bug.cgi?id=226576
+        <rdar://problem/78810362>
+
+        Reviewed by Yusuke Suzuki.
+
+        It's never a good idea to use the wrong local :-)
+        
+        This patch also adds support for dumping predecessors of basic blocks
+        in the bytecode dump.
+
+        * bytecode/BytecodeDumper.cpp:
+        (JSC::CodeBlockBytecodeDumper<Block>::dumpGraph):
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::ShortCircuitReadModifyResolveNode::emitBytecode):
+        (JSC::ShortCircuitReadModifyDotNode::emitBytecode):
+        (JSC::ShortCircuitReadModifyBracketNode::emitBytecode):
+
 2021-06-07  Mark Lam  <[email protected]>
 
         Put the Baseline JIT prologue and op_loop_hint code in JIT thunks.

Modified: trunk/Source/_javascript_Core/bytecode/BytecodeDumper.cpp (278577 => 278578)


--- trunk/Source/_javascript_Core/bytecode/BytecodeDumper.cpp	2021-06-07 23:07:02 UTC (rev 278577)
+++ trunk/Source/_javascript_Core/bytecode/BytecodeDumper.cpp	2021-06-07 23:26:53 UTC (rev 278578)
@@ -274,6 +274,17 @@
 
     out.printf("\n");
 
+    Vector<Vector<unsigned>> predecessors;
+    predecessors.resize(graph.size());
+    for (auto& block : graph) {
+        if (block.isEntryBlock() || block.isExitBlock())
+            continue;
+        for (auto successorIndex : block.successors()) {
+            if (!predecessors[successorIndex].contains(block.index()))
+                predecessors[successorIndex].append(block.index());
+        }
+    }
+
     for (BytecodeBasicBlock& block : graph) {
         if (block.isEntryBlock() || block.isExitBlock())
             continue;
@@ -280,6 +291,13 @@
 
         out.print("bb#", block.index(), "\n");
 
+        out.print("Predecessors: [");
+        for (unsigned predecessor : predecessors[block.index()]) {
+            if (!graph[predecessor].isEntryBlock())
+                out.print(" #", predecessor);
+        }
+        out.print(" ]\n");
+
         for (unsigned i = 0; i < block.totalLength(); ) {
             auto& currentInstruction = instructions.at(i + block.leaderOffset());
             dumper.dumpBytecode(currentInstruction, icStatusMap);

Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (278577 => 278578)


--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2021-06-07 23:07:02 UTC (rev 278577)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2021-06-07 23:26:53 UTC (rev 278578)
@@ -3484,7 +3484,7 @@
             Ref<Label> afterAssignment = generator.newLabel();
             emitShortCircuitAssignment(generator, result.get(), m_operator, afterAssignment.get());
 
-            result = generator.emitNode(result.get(), m_right); // Execute side effects first.
+            generator.emitNode(result.get(), m_right); // Execute side effects first.
             bool threwException = generator.emitReadOnlyExceptionIfNeeded(var);
 
             if (!threwException)
@@ -3501,7 +3501,7 @@
             Ref<Label> afterAssignment = generator.newLabel();
             emitShortCircuitAssignment(generator, result.get(), m_operator, afterAssignment.get());
 
-            result = generator.emitNode(result.get(), m_right);
+            generator.emitNode(result.get(), m_right);
             generator.move(local.get(), result.get());
             generator.emitProfileType(result.get(), var, divotStart(), divotEnd());
 
@@ -3514,7 +3514,7 @@
         Ref<Label> afterAssignment = generator.newLabel();
         emitShortCircuitAssignment(generator, result.get(), m_operator, afterAssignment.get());
 
-        result = generator.emitNode(result.get(), m_right);
+        generator.emitNode(result.get(), m_right);
         generator.emitProfileType(result.get(), var, divotStart(), divotEnd());
 
         generator.emitLabel(afterAssignment.get());
@@ -3524,7 +3524,9 @@
     generator.emitExpressionInfo(newDivot, divotStart(), newDivot);
     RefPtr<RegisterID> scope = generator.emitResolveScope(nullptr, var);
 
-    RefPtr<RegisterID> result = generator.emitGetFromScope(generator.tempDestination(dst), scope.get(), var, ThrowIfNotFound);
+    RefPtr<RegisterID> result = generator.tempDestination(dst);
+
+    generator.emitGetFromScope(result.get(), scope.get(), var, ThrowIfNotFound);
     generator.emitTDZCheckIfNecessary(var, result.get(), nullptr);
 
     Ref<Label> afterAssignment = generator.newLabel();
@@ -3657,24 +3659,24 @@
     RefPtr<RegisterID> base = generator.emitNodeForLeftHandSide(m_base, m_rightHasAssignments, m_right->isPure(generator));
     RefPtr<RegisterID> thisValue;
 
-    RefPtr<RegisterID> result;
+    RefPtr<RegisterID> result = generator.tempDestination(dst);
 
     generator.emitExpressionInfo(subexpressionDivot(), subexpressionStart(), subexpressionEnd());
     if (m_base->isSuperNode()) {
         thisValue = generator.ensureThis();
-        result = generator.emitGetById(generator.tempDestination(dst), base.get(), thisValue.get(), m_ident);
+        generator.emitGetById(result.get(), base.get(), thisValue.get(), m_ident);
     } else
-        result = generator.emitGetById(generator.tempDestination(dst), base.get(), m_ident);
+        generator.emitGetById(result.get(), base.get(), m_ident);
 
     Ref<Label> afterAssignment = generator.newLabel();
     emitShortCircuitAssignment(generator, result.get(), m_operator, afterAssignment.get());
 
-    result = generator.emitNode(result.get(), m_right);
+    generator.emitNode(result.get(), m_right);
     generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
     if (m_base->isSuperNode())
-        result = generator.emitPutById(base.get(), thisValue.get(), m_ident, result.get());
+        generator.emitPutById(base.get(), thisValue.get(), m_ident, result.get());
     else
-        result = generator.emitPutById(base.get(), m_ident, result.get());
+        generator.emitPutById(base.get(), m_ident, result.get());
     generator.emitProfileType(result.get(), divotStart(), divotEnd());
 
     generator.emitLabel(afterAssignment.get());
@@ -3753,24 +3755,24 @@
     RefPtr<RegisterID> property = generator.emitNodeForLeftHandSideForProperty(m_subscript, m_rightHasAssignments, m_right->isPure(generator));
     RefPtr<RegisterID> thisValue;
 
-    RefPtr<RegisterID> result;
+    RefPtr<RegisterID> result = generator.tempDestination(dst);
 
     generator.emitExpressionInfo(subexpressionDivot(), subexpressionStart(), subexpressionEnd());
     if (m_base->isSuperNode()) {
         thisValue = generator.ensureThis();
-        result = generator.emitGetByVal(generator.tempDestination(dst), base.get(), thisValue.get(), property.get());
+        generator.emitGetByVal(result.get(), base.get(), thisValue.get(), property.get());
     } else
-        result = generator.emitGetByVal(generator.tempDestination(dst), base.get(), property.get());
+        generator.emitGetByVal(result.get(), base.get(), property.get());
 
     Ref<Label> afterAssignment = generator.newLabel();
     emitShortCircuitAssignment(generator, result.get(), m_operator, afterAssignment.get());
 
-    result = generator.emitNode(result.get(), m_right);
+    generator.emitNode(result.get(), m_right);
     generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
     if (m_base->isSuperNode())
-        result = generator.emitPutByVal(base.get(), thisValue.get(), property.get(), result.get());
+        generator.emitPutByVal(base.get(), thisValue.get(), property.get(), result.get());
     else
-        result = generator.emitPutByVal(base.get(), property.get(), result.get());
+        generator.emitPutByVal(base.get(), property.get(), result.get());
     generator.emitProfileType(result.get(), divotStart(), divotEnd());
 
     generator.emitLabel(afterAssignment.get());
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to