Firefox · SpiderMonkey
CVE-2026-100793
Logic Error in SpiderMonkey
Overview
High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifjs/src/jit/LICM.cpp |
modified |
Files Changed
js/src/jit/LICM.cppjs/src/jit/MIRGraph.cppjs/src/jit/MIRGraph.hjs/src/jit/RangeAnalysis.cppjs/src/jit/WarpBuilder.cpp
Patch
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…
References
On This Page