r281485 was not sufficient in where it called disablePeepholeOptimization

saambarati · saambarati · commit a511792c9c44 · 2021-08-26T22:43:32.000Z
https://bugs.webkit.org/show_bug.cgi?id=229582 <rdar://82346980> Reviewed by Yusuke Suzuki. JSTests: * stress/for-in-disable-bytecode-generator-peephole-optimizations-after-rewrite-2.js: Added. (foo): Source/JavaScriptCore: r281485 accidentally deleted a place where we called disablePeepholeOptimization that was necessary. Basically, after we seek where the current instruction in BytecodeGenerator is, we need to disablePeepholeOptimization. * bytecompiler/BytecodeGenerator.cpp: (JSC::ForInContext::finalize): Canonical link: https://commits.webkit.org/241018@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@281665 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
@@ -1,3 +1,14 @@
+2021-08-26  Saam Barati  <sbarati@apple.com>
+
+        r281485 was not sufficient in where it called disablePeepholeOptimization
+        https://bugs.webkit.org/show_bug.cgi?id=229582
+        <rdar://82346980>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/for-in-disable-bytecode-generator-peephole-optimizations-after-rewrite-2.js: Added.
+        (foo):
+
 2021-08-24  Yusuke Suzuki  <ysuzuki@apple.com>
 
         Update Intl tests to support ICU 68
diff --git a/JSTests/stress/for-in-disable-bytecode-generator-peephole-optimizations-after-rewrite-2.js b/JSTests/stress/for-in-disable-bytecode-generator-peephole-optimizations-after-rewrite-2.js
@@ -0,0 +1,12 @@
+function foo() {
+    for (var c in b) {
+        if (b.hasOwnProperty(c)) {
+            var e = b[c];
+            c = "str";
+        }
+    }
+}
+
+try {
+    foo();
+} catch { }
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,18 @@
+2021-08-26  Saam Barati  <sbarati@apple.com>
+
+        r281485 was not sufficient in where it called disablePeepholeOptimization
+        https://bugs.webkit.org/show_bug.cgi?id=229582
+        <rdar://82346980>
+
+        Reviewed by Yusuke Suzuki.
+
+        r281485 accidentally deleted a place where we called disablePeepholeOptimization
+        that was necessary. Basically, after we seek where the current instruction
+        in BytecodeGenerator is, we need to disablePeepholeOptimization.
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::ForInContext::finalize):
+
 2021-08-26  Per Arne Vollan  <pvollan@apple.com>
 
         [AppleWin] JSC build failure
diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
@@ -5386,16 +5386,16 @@ void ForInContext::finalize(BytecodeGenerator& generator, UnlinkedCodeBlockGener
         auto end = branchInstIndex + instruction->size();
 
         generator.m_writer.seek(branchInstIndex);
+        generator.disablePeepholeOptimization();
 
         OpJmp::emit(&generator, BoundLabel(static_cast<int>(newBranchTarget) - static_cast<int>(branchInstIndex)));
 
         while (generator.m_writer.position() < end)
             OpNop::emit<OpcodeSize::Narrow>(&generator);
     }
 
-    generator.disablePeepholeOptimization(); // We might've just changed the last bytecode that was emitted.
-
     generator.m_writer.seek(generator.m_writer.size());
+    generator.disablePeepholeOptimization(); // We might've just changed the last bytecode that was emitted.
 }
 
 void StaticPropertyAnalysis::record()

Original file line number	Diff line number	Diff line change
`@@ -5386,16 +5386,16 @@ void ForInContext::finalize(BytecodeGenerator& generator, UnlinkedCodeBlockGener`
`5386`	`5386`	`auto end = branchInstIndex + instruction->size();`
`5387`	`5387`
`5388`	`5388`	`generator.m_writer.seek(branchInstIndex);`
	`5389`	`+ generator.disablePeepholeOptimization();`
`5389`	`5390`
`5390`	`5391`	`OpJmp::emit(&generator, BoundLabel(static_cast<int>(newBranchTarget) - static_cast<int>(branchInstIndex)));`
`5391`	`5392`
`5392`	`5393`	`while (generator.m_writer.position() < end)`
`5393`	`5394`	`OpNop::emit<OpcodeSize::Narrow>(&generator);`
`5394`	`5395`	`}`
`5395`	`5396`
`5396`		`- generator.disablePeepholeOptimization(); // We might've just changed the last bytecode that was emitted.`
`5397`		`-`
`5398`	`5397`	`generator.m_writer.seek(generator.m_writer.size());`
	`5398`	`+ generator.disablePeepholeOptimization(); // We might've just changed the last bytecode that was emitted.`
`5399`	`5399`	`}`
`5400`	`5400`
`5401`	`5401`	`void StaticPropertyAnalysis::record()`