Title: [278902] branches/safari-611-branch
Revision
278902
Author
alanc...@apple.com
Date
2021-06-15 14:49:32 -0700 (Tue, 15 Jun 2021)

Log Message

Cherry-pick r278578. rdar://problem/79355258

    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):

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

Modified Paths

Added Paths

Diff

Modified: branches/safari-611-branch/JSTests/ChangeLog (278901 => 278902)


--- branches/safari-611-branch/JSTests/ChangeLog	2021-06-15 21:49:28 UTC (rev 278901)
+++ branches/safari-611-branch/JSTests/ChangeLog	2021-06-15 21:49:32 UTC (rev 278902)
@@ -1,3 +1,46 @@
+2021-06-15  Alan Coon  <alanc...@apple.com>
+
+        Cherry-pick r278578. rdar://problem/79355258
+
+    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):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@278578 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-06-07  Saam Barati  <sbar...@apple.com>
+
+            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-05-20  Alan Coon  <alanc...@apple.com>
 
         Cherry-pick r277613. rdar://problem/78264256

Added: branches/safari-611-branch/JSTests/stress/short-circuit-read-modify-should-use-the-write-virtual-registers.js (0 => 278902)


--- branches/safari-611-branch/JSTests/stress/short-circuit-read-modify-should-use-the-write-virtual-registers.js	                        (rev 0)
+++ branches/safari-611-branch/JSTests/stress/short-circuit-read-modify-should-use-the-write-virtual-registers.js	2021-06-15 21:49:32 UTC (rev 278902)
@@ -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: branches/safari-611-branch/Source/_javascript_Core/ChangeLog (278901 => 278902)


--- branches/safari-611-branch/Source/_javascript_Core/ChangeLog	2021-06-15 21:49:28 UTC (rev 278901)
+++ branches/safari-611-branch/Source/_javascript_Core/ChangeLog	2021-06-15 21:49:32 UTC (rev 278902)
@@ -1,3 +1,55 @@
+2021-06-15  Alan Coon  <alanc...@apple.com>
+
+        Cherry-pick r278578. rdar://problem/79355258
+
+    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):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@278578 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-06-07  Saam Barati  <sbar...@apple.com>
+
+            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-05-20  Alan Coon  <alanc...@apple.com>
 
         Cherry-pick r277613. rdar://problem/78264256

Modified: branches/safari-611-branch/Source/_javascript_Core/bytecode/BytecodeDumper.cpp (278901 => 278902)


--- branches/safari-611-branch/Source/_javascript_Core/bytecode/BytecodeDumper.cpp	2021-06-15 21:49:28 UTC (rev 278901)
+++ branches/safari-611-branch/Source/_javascript_Core/bytecode/BytecodeDumper.cpp	2021-06-15 21:49:32 UTC (rev 278902)
@@ -277,6 +277,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;
@@ -283,6 +294,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: branches/safari-611-branch/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (278901 => 278902)


--- branches/safari-611-branch/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2021-06-15 21:49:28 UTC (rev 278901)
+++ branches/safari-611-branch/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2021-06-15 21:49:32 UTC (rev 278902)
@@ -3210,7 +3210,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)
@@ -3227,7 +3227,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());
 
@@ -3240,7 +3240,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());
@@ -3250,7 +3250,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();
@@ -3383,24 +3385,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());
@@ -3479,24 +3481,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
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to