High firefox Logic Error 🔧 Commit mapped

Overview

High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Impacthigh
DescriptionJIT miscompilation in the JavaScript Engine component
ComponentSpiderMonkey
Bug ClassLogic Error
Tracker2073268
Fix commit7b2fe6b3526a (firefox) +47/-6
CISA KEVNot listed
CreditedAmy Burnett of OpenAI
Disclosed2026-09-29

Changed Functions

FunctionChangeNotes
if
js/src/jit/LICM.cpp
modified

Files Changed

  • js/src/jit/LICM.cpp
  • js/src/jit/MIRGraph.cpp
  • js/src/jit/MIRGraph.h
  • js/src/jit/RangeAnalysis.cpp
  • js/src/jit/WarpBuilder.cpp
diff --git a/js/src/jit/LICM.cpp b/js/src/jit/LICM.cpp
index 2077c1c5fc8..30f83e0d55c 100644
--- a/js/src/jit/LICM.cpp
+++ b/js/src/jit/LICM.cpp
@@ -305,7 +305,11 @@ bool jit::LICM(const MIRGenerator* mir, MIRGraph& graph) {
     //     addition to its normal entry is tricky.  In theory we could clone
     //     the instruction and insert phis.  In practice we don't bother.
     //
-    // (b) If the loop contains a large number of blocks, we play safe and
+    // (b) If the loop has a generator resume dispatch, hoisted instructions
+    //     would also be executed on the generator-resume path into the loop.
+    //     See bug 2073268.
+    //
+    // (c) If the loop contains a large number of blocks, we play safe and
     //     punt, in order to reduce the risk of creating excessive register
     //     pressure by hoisting lots of values out of the loop.  In a larger
     //     loop there's more likely to be duplication of invariant expressions
@@ -313,21 +317,26 @@ bool jit::LICM(const MIRGenerator* mir, MIRGraph& graph) {
     //     within the scope of the loop body, so there's less loss from not
     //     lifting them out of the loop entirely.
     //
-    // (c) If the loop contains a multiway switch with many successors, there
+    // (d) If the loop contains a multiway switch with many successors, there
     //     could be paths with low probabilities, from which LICMing will be a
     //     net loss, especially if a large number of values are hoisted out.
     //     See bug 1708381 for a spectacular example and bug 1712078 for
     //     further discussion.
     //
-    // It's preferable to perform test (c) only if (a) and (b) pass since (c)
-    // is more expensive to determine -- requiring a visit to all the MIR
-    // nodes -- than (a) or (b), which only involve visiting all blocks.
+    // It's preferable to perform test (d) only if (a), (b) and (c) pass since
+    // (d) is more expensive to determine -- requiring a visit to all the MIR
+    // nodes -- than the others, which only involve visiting all blocks.
 
     bool doVisit = true;
     if (canOsr) {
       JitSpew(JitSpew_LICM, "  Skipping loop with header block%u due to OSR",
               header->id());
       doVisit = false;
+    } else if (header->hasGeneratorResumeEntry()) {
+      JitSpew(JitSpew_LICM,
+              "  Skipping loop with header block%u due to generator resume",
+              header->id());
+      doVisit = false;
     } else if (numBlocks > LargestAllowedLoop) {
       JitSpew(JitSpew_LICM,
               "  Skipping loop with header block%u "
diff --git a/js/src/jit/MIRGraph.cpp b/js/src/jit/MIRGraph.cpp
index ffb63546d2f..4add4fba202 100644
--- a/js/src/jit/MIRGraph.cpp
+++ b/js/src/jit/MIRGraph.cpp
@@ -1631,6 +1631,24 @@ void jit::AssertBasicGraphCoherency(MIRGraph& graph, bool force) {
           CheckPredecessorImpliesSuccessor(*block, block->getPredecessor(i)));
     }
 
+    if (block->isLoopHeader() && block->hasGeneratorResumeEntry()) {
+      // For loops with a generator resume entry, we currently don't allow
+      // hoisting instructions into the preheader. See bug 2073268.
+      MBasicBlock* preheader = block->loopPredecessor();
+      for (MInstructionIterator ins(preheader->begin());
+           ins != preheader->end(); ins++) {
+        MOZ_ASSERT(ins->isGoto() || ins->isBox() || ins->isConstant() ||
+                   ins->isBeta() || ins->isAssertRange());
+      }
+      // Similarly, the loop header itself must only branch to the resume
+      // dispatch block. See WarpBuilder::startResumePath.
+      for (MInstructionIterator ins(block->begin()); ins != block->end();
+           ins++) {
+        MOZ_ASSERT(ins->isIsResumingGenerator() || ins->isTest() ||
+                   ins->isGoto() || ins->isConstant() || ins->isAssertRange());
+      }
+    }
+
     if (MResumePoint* resume = block->entryResumePoint()) {
       MOZ_ASSERT(!resume->instruction());
       MOZ_ASSERT(resume->block() == *block);
diff --git a/js/src/jit/MIRGraph.h b/js/src/jit/MIRGraph.h
index cda03d9a4ac..630e085d032 100644
--- a/js/src/jit/MIRGraph.h
+++ b/js/src/jit/MIRGraph.h
@@ -66,6 +66,9 @@ class MBasicBlock : public TempObject, public InlineListNode<MBasicBlock> {
   // This block will unconditionally bail out.
   bool alwaysBails_ = false;
 
+  // Set for loop header blocks with a generator resume dispatch block.
+  bool hasGeneratorResumeEntry_ = false;
+
   // Represents the execution frequency of this block, considered unknown by
   // default. Various passes can use this information for optimizations.
   Frequency frequency_ = Frequency::Unknown;
@@ -163,6 +166,12 @@ class MBasicBlock : public TempObject, public InlineListNode<MBasicBlock> {
   void setAlwaysBails() { alwaysBails_ = true; }
   bool alwaysBails() const { return alwaysBails_; }
 
+  void setHasGeneratorResumeEntry() {
+    MOZ_ASSERT(isPendingLoopHeader());
+    hasGeneratorResumeEntry_ = true;
+  }
+  bool hasGeneratorResumeEntry() const { return hasGeneratorResumeEntry_; }
+
   // Move the definition to the top of the stack.
   void pick(int32_t depth);
 
diff --git a/js/src/jit/RangeAnalysis.cpp b/js/src/jit/RangeAnalysis.cpp
index 6ff34ea874e..a0daf8e28ea 100644
--- a/js/src/jit/RangeAnalysis.cpp
+++ b/js/src/jit/RangeAnalysis.cpp
@@ -2020,7 +2020,10 @@ bool RangeAnalysis::analyzeLoop(const MBasicBlock* header) {
     analyzeLoopPhi(iterationBound, *iter);
   }
 
-  if (!mir->compilingWasm() && !mir->outerInfo().hadBoundsCheckBailout()) {
+  // Disallow hoisting for loops that have a generator resume dispatch. This
+  // matches jit::LICM.
+  if (!mir->compilingWasm() && !mir->outerInfo().hadBoundsCheckBailout() &&
+      !header->hasGeneratorResumeEntry()) {
     // Try to hoist any bounds checks from the loop using symbolic bounds.
 
     Vector<MBoundsCheck*, 0, JitAllocPolicy> hoistedChecks(alloc());
diff --git a/js/src/jit/WarpBuilder.cpp b/js/src/jit/WarpBuilder.cpp
index 9ef5c632acb..67bbf2674bb 100644
--- a/js/src/jit/WarpBuilder.cpp
+++ b/js/src/jit/WarpBuilder.cpp
@@ -1661,6 +1661,8 @@ bool WarpBuilder::build_LoopHead(BytecodeLocation loc) {
   // Create the generator resume index dispatch for the new loop header. After
   // this, |current| is the normal non-resume loop body.
   if (!entries.empty()) {
+    current->setHasGeneratorResumeEntry();
+
     MBasicBlock* body = nullptr;
     if (!startResumePath(current, loc, &body)) {
       return false;
Loading diff…