High firefox Logic Error 🔧 Commit mapped

Overview

High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Impacthigh
DescriptionIncorrect boundary conditions in the JavaScript Engine: JIT component
ComponentSpiderMonkey
Bug ClassLogic Error
Tracker2082047
Fix commitd1a020277b64 (firefox) +220/-18
CISA KEVNot listed
CreditedOpenAI Codex Security (kylebot)
Disclosed2026-10-27

Changed Functions

FunctionChangeNotes
ArgumentsObject
js/src/jit/JSJitFrameIter.h
modified
SavedFrame
js/src/jit/JSJitFrameIter.h
modified
initialized_
js/src/jit/JitFrames.cpp
modified
exceptionStack_
js/src/jit/JitFrames.cpp
modified
if
js/src/jit/JitFrames.cpp
modified
BEGIN_TEST
js/src/jsapi-tests/testJitRInstructionResults.cpp
modified
for
js/src/jsapi-tests/testJitRInstructionResults.cpp
modified
if
js/src/jsapi-tests/testJitRInstructionResults.cpp
modified
switch
js/src/jsapi-tests/testJitRInstructionResults.cpp
modified
if
js/src/jsapi.cpp
modified
with
js/src/jit-test/tests/ion/bug2082047-2.js
modified
for
js/src/jit-test/tests/ion/bug2082047-2.js
modified
next
js/src/jit-test/tests/ion/bug2082047-2.js
modified

Files Changed

  • js/src/jit-test/tests/ion/bug2082047-2.js
  • js/src/jit/JSJitFrameIter.h
  • js/src/jit/JitFrames.cpp
  • js/src/jsapi-tests/moz.build
  • js/src/jsapi-tests/testJitRInstructionResults.cpp
  • js/src/jsapi.cpp
  • js/src/vm/JSContext.cpp
  • js/src/vm/JSContext.h
diff --git a/js/src/jit/JSJitFrameIter.h b/js/src/jit/JSJitFrameIter.h
index 636218a4dc3..fd51c9034cb 100644
--- a/js/src/jit/JSJitFrameIter.h
+++ b/js/src/jit/JSJitFrameIter.h
@@ -12,6 +12,7 @@
 #include "jit/JitCode.h"
 #include "jit/MachineState.h"
 #include "jit/Snapshots.h"
+#include "js/Exception.h"
 #include "js/ProfilingFrameIterator.h"
 #include "vm/JSFunction.h"
 #include "vm/JSScript.h"
@@ -19,6 +20,7 @@
 namespace js {
 
 class ArgumentsObject;
+class SavedFrame;
 
 namespace jit {
 
@@ -309,6 +311,12 @@ class RInstructionResults {
   // avoid evaluating the recover instruction twice.
   bool initialized_;
 
+  // Set if computing the results failed. Later attempts to run the recover
+  // instructions will report the same failure.
+  mozilla::Maybe<JS::ExceptionStatus> failureStatus_;
+  HeapPtr<Value> exception_;
+  HeapPtr<SavedFrame*> exceptionStack_;
+
  public:
   explicit RInstructionResults(JitFrameLayout* fp);
   RInstructionResults(RInstructionResults&& src);
@@ -321,6 +329,10 @@ class RInstructionResults {
   bool isInitialized() const;
   size_t length() const;
 
+  bool hasFailure() const { return failureStatus_.isSome(); }
+  void saveFailure(JSContext* cx);
+  void restoreFailure(JSContext* cx) const;
+
   JitFrameLayout* frame() const;
 
   HeapPtr<Value>& operator[](size_t index);
diff --git a/js/src/jit/JitFrames.cpp b/js/src/jit/JitFrames.cpp
index b9d2f98937b..684b13c6e4e 100644
--- a/js/src/jit/JitFrames.cpp
+++ b/js/src/jit/JitFrames.cpp
@@ -29,6 +29,7 @@
 #include "vm/JSFunction.h"
 #include "vm/JSObject.h"
 #include "vm/JSScript.h"
+#include "vm/SavedFrame.h"
 #include "vm/Stack.h"  // js::ResumeFrameArgs
 #include "wasm/WasmBuiltins.h"
 #include "wasm/WasmInstance.h"
@@ -1680,8 +1681,14 @@ RInstructionResults::RInstructionResults(JitFrameLayout* fp)
 RInstructionResults::RInstructionResults(RInstructionResults&& src)
     : results_(std::move(src.results_)),
       fp_(src.fp_),
-      initialized_(src.initialized_) {
+      initialized_(src.initialized_),
+      failureStatus_(src.failureStatus_),
+      exception_(src.exception_),
+      exceptionStack_(src.exceptionStack_) {
   src.initialized_ = false;
+  src.failureStatus_.reset();
+  src.exception_ = UndefinedValue();
+  src.exceptionStack_ = nullptr;
 }
 
 RInstructionResults& RInstructionResults::operator=(RInstructionResults&& rhs) {
@@ -1718,6 +1725,21 @@ bool RInstructionResults::isInitialized() const { return initialized_; }
 
 size_t RInstructionResults::length() const { return results_->length(); }
 
+void RInstructionResults::saveFailure(JSContext* cx) {
+  MOZ_ASSERT(!hasFailure());
+  failureStatus_.emplace(cx->exceptionStatus());
+  if (JS::IsCatchableExceptionStatus(*failureStatus_)) {
+    exception_ = cx->getPendingExceptionUnwrapped();
+    exceptionStack_ = cx->getPendingExceptionStack();
+  }
+}
+
+void RInstructionResults::restoreFailure(JSContext* cx) const {
+  MOZ_ASSERT(hasFailure());
+  cx->restorePendingExceptionState(*failureStatus_, exception_,
+                                   exceptionStack_);
+}
+
 JitFrameLayout* RInstructionResults::frame() const {
   MOZ_ASSERT(fp_);
   return fp_;
@@ -1728,9 +1750,12 @@ HeapPtr<Value>& RInstructionResults::operator[](size_t index) {
 }
 
 void RInstructionResults::trace(JSTracer* trc) {
-  // Note: The vector necessary exists, otherwise this object would not have
-  // been stored on the activation from where the trace function is called.
-  TraceRange(trc, results_->length(), results_->begin(), "ion-recover-results");
+  if (results_) {
+    TraceRange(trc, results_->length(), results_->begin(),
+               "ion-recover-results");
+  }
+  TraceEdge(trc, &exception_, "ion-recover-exception");
+  TraceEdge(trc, &exceptionStack_, "ion-recover-exception-stack");
 }
 
 SnapshotIterator::SnapshotIterator(const JSJitFrameIter& iter,
@@ -2290,7 +2315,15 @@ bool SnapshotIterator::initInstructionResults(MaybeReadFallback& fallback) {
 
   JitFrameLayout* fp = fallback.frame->jsFrame();
   RInstructionResults* results = fallback.activation->maybeIonFrameRecovery(fp);
-  if (!results) {
+  if (results) {
+    // If a previous attempt failed, report the same failure. RObjectState and
+    // RArrayState may have already written to objects allocated by Ion, and we
+    // must not run them again.
+    if (results->hasFailure()) {
+      results->restoreFailure(cx);
+      return false;
+    }
+  } else {
     AutoRealm ar(cx, fallback.frame->script());
 
     // We are going to run recover instructions. To avoid problems where recover
@@ -2323,14 +2356,12 @@ bool SnapshotIterator::initInstructionResults(MaybeReadFallback& fallback) {
     MachineState machine = fallback.frame->machineState();
     SnapshotIterator s(*fallback.frame, &machine);
     if (!s.computeInstructionResults(cx, results)) {
-      // If the evaluation failed because of OOMs, then we discard the
-      // current set of result that we collected so far.
-      fallback.activation->removeIonFrameRecovery(fp);
+      results->saveFailure(cx);
       return false;
     }
   }
 
-  MOZ_ASSERT(results->isInitialized());
+  MOZ_RELEASE_ASSERT(results->isInitialized());
   MOZ_RELEASE_ASSERT(results->length() == recover_.numInstructions() - 1);
   instructionResults_ = results;
   return true;
diff --git a/js/src/jsapi-tests/moz.build b/js/src/jsapi-tests/moz.build
index 5d399527684..ab7e3c9d712 100644
--- a/js/src/jsapi-tests/moz.build
+++ b/js/src/jsapi-tests/moz.build
@@ -182,6 +182,7 @@ if not CONFIG["JS_CODEGEN_NONE"]:
         "testJitMoveEmitterCycles.cpp",
         "testJitRangeAnalysis.cpp",
         "testJitRegisterSet.cpp",
+        "testJitRInstructionResults.cpp",
         "testJitRValueAlloc.cpp",
         "testsJit.cpp",
     ]
diff --git a/js/src/jsapi-tests/testJitRInstructionResults.cpp b/js/src/jsapi-tests/testJitRInstructionResults.cpp
new file mode 100644
index 00000000000..ae027a3e0bf
--- /dev/null
+++ b/js/src/jsapi-tests/testJitRInstructionResults.cpp
@@ -0,0 +1,97 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "jit/JSJitFrameIter.h"
+#include "js/Exception.h"
+#include "js/PropertyAndElement.h"
+#include "jsapi-tests/tests.h"
+#include "vm/JSContext.h"
+#include "vm/SavedFrame.h"
+
+using js::jit::RInstructionResults;
+
+enum class FailureKind { Exception, OutOfMemory, Uncatchable };
+
+BEGIN_TEST(testJitRInstructionResults_failure) {
+  for (FailureKind kind : {FailureKind::Exception, FailureKind::OutOfMemory,
+                           FailureKind::Uncatchable}) {
+    for (bool initialize : {false, true}) {
+      CHECK(testFailure(kind, initialize));
+    }
+  }
+  return true;
+}
+
+bool testFailure(FailureKind kind, bool initialize) {
+  JS::Rooted<RInstructionResults> results(cx, RInstructionResults(nullptr));
+  if (initialize) {
+    CHECK(results.get().init(cx, 3));
+  }
+
+  JS::ExceptionStatus status;
+  {
+    switch (kind) {
+      case FailureKind::Exception: {
+        JS::RootedValue exception(cx);
+        EVAL("({ marker: 42 })", &exception);
+        JS::RootedValue error(cx);
+        EVAL("new Error('saved stack')", &error);
+        JS::RootedObject errorObject(cx, &error.toObject());
+        JS::RootedObject stack(cx, JS::ExceptionStackOrNull(errorObject));
+        CHECK(stack);
+        JS::Rooted<js::SavedFrame*> savedFrame(cx,
+                                               &stack->as<js::SavedFrame>());
+        cx->setPendingException(exception, savedFrame);
+        break;
+      }
+      case FailureKind::OutOfMemory:
+        js::ReportOutOfMemory(cx);
+        break;
+      case FailureKind::Uncatchable:
+        cx->reportUncatchableException();
+        break;
+    }
+    status = cx->exceptionStatus();
+    results.get().saveFailure(cx);
+    CHECK(cx->exceptionStatus() == status);
+    JS_ClearPendingException(cx);
+  }
+
+  // The saved failure is the only reference to the exception now.
+  JS::Rooted<RInstructionResults> moved(cx, RInstructionResults(nullptr));
+  moved.get() = std::move(results.get());
+  JS_GC(cx);
+
+  JS::RootedValue firstException(cx);
+  JS::RootedObject firstStack(cx);
+  for (size_t i = 0; i < 2; i++) {
+    CHECK(moved.get().hasFailure());
+    moved.get().restoreFailure(cx);
+    CHECK(cx->exceptionStatus() == status);
+    CHECK(JS_IsExceptionPending(cx) == (kind != FailureKind::Uncatchable));
+
+    if (kind == FailureKind::Exception) {
+      JS::ExceptionStack pending(cx);
+      CHECK(JS::GetPendingExceptionStack(cx, &pending));
+      CHECK(pending.exception().isObject());
+      CHECK(pending.stack());
+      JS::RootedObject exception(cx, &pending.exception().toObject());
+      JS::RootedValue marker(cx);
+      CHECK(JS_GetProperty(cx, exception, "marker", &marker));
+      CHECK_SAME(marker, JS::Int32Value(42));
+      if (i == 0) {
+        firstException = pending.exception();
+        firstStack = pending.stack();
+      } else {
+        CHECK_SAME(pending.exception(), firstException);
+        CHECK(pending.stack() == firstStack);
+      }
+    } else {
+      CHECK(!cx->getPendingExceptionStack());
+    }
+    JS_ClearPendingException(cx);
+  }
+  return true;
+}
+END_TEST(testJitRInstructionResults_failure)
diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp
index acb743cf731..04fb2a5e66e 100644
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4179,11 +4179,9 @@ void JS::AutoSaveExceptionState::drop() {
 }
 
 void JS::AutoSaveExceptionState::restore() {
-  context->status = status;
-  context->unwrappedException() = exceptionValue;
-  if (exceptionStack) {
-    context->unwrappedExceptionStack() = &exceptionStack->as<SavedFrame>();
-  }
+  SavedFrame* stack =
+      exceptionStack ? &exceptionStack->as<SavedFrame>() : nullptr;
+  context->restorePendingExceptionState(status, exceptionValue, stack);
   drop();
 }
 
diff --git a/js/src/vm/JSContext.cpp b/js/src/vm/JSContext.cpp
index c9b0b24d0f2..707d0901ede 100644
--- a/js/src/vm/JSContext.cpp
+++ b/js/src/vm/JSContext.cpp
@@ -1432,12 +1432,10 @@ SavedFrame* JSContext::getPendingExceptionStack() {
   return unwrappedExceptionStack();
 }
 
-#ifdef DEBUG
 const JS::Value& JSContext::getPendingExceptionUnwrapped() {
   MOZ_ASSERT(isExceptionPending());
   return unwrappedException();
 }
-#endif
 
 bool JSContext::isThrowingDebuggeeWouldRun() {
   return isExceptionPending() && unwrappedException().isObject() &&
diff --git a/js/src/vm/JSContext.h b/js/src/vm/JSContext.h
index 0ee80aa3d15..ada76cd543e 100644
--- a/js/src/vm/JSContext.h
+++ b/js/src/vm/JSContext.h
@@ -912,12 +912,10 @@ struct JS_PUBLIC_API JSContext : public JS::RootingContext,
    */
   js::SavedFrame* getPendingExceptionStack();
 
-#ifdef DEBUG
   /**
    * Return the pending exception (without wrapping).
    */
   const JS::Value& getPendingExceptionUnwrapped();
-#endif
 
   bool isThrowingDebuggeeWouldRun();
 
@@ -932,6 +930,17 @@ struct JS_PUBLIC_API JSContext : public JS::RootingContext,
     unwrappedExceptionStack() = nullptr;
   }
 
+  JS::ExceptionStatus exceptionStatus() const { return status; }
+
+  // Set the pending exception state to values saved earlier.
+  void restorePendingExceptionState(JS::ExceptionStatus newStatus,
+                                    const JS::Value& exception,
+                                    js::SavedFrame* stack) {
+    status = newStatus;
+    unwrappedException() = exception;
+    unwrappedExceptionStack() = stack;
+  }
+
   bool isThrowingOutOfMemory() const {
     return status == JS::ExceptionStatus::OutOfMemory;
   }
diff --git a/js/src/jit-test/tests/ion/bug2082047-2.js b/js/src/jit-test/tests/ion/bug2082047-2.js
new file mode 100644
index 00000000000..b85859711fb
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug2082047-2.js
@@ -0,0 +1,56 @@
+// |jit-test| --fast-warmup; --no-threads; allow-unhandlable-oom; skip-if: !hasFunction.oomAtAllocation
+
+with ({}) {}
+gczeal(0);
+
+// Avoid allocating exception stacks before reaching the bailout.
+for (var i = 0; i < 100; i++) {
+  try { throw 0; } catch (e) {}
+}
+
+var active = false;
+
+function receive() {
+  if (!active) return 7;
+  oomAtAllocation(4);
+  throw 12345;
+}
+
+// Reading helper's arguments needs the recovery results again. The failure is
+// remembered, so this read crashes with [unhandlable oom] instead of
+// re-running ArrayState.
+function close() {
+  if (active) helper.arguments;
+  return {};
+}
+
+var iterator = {
+  next() { return {value: undefined, done: false}; },
+  return: close,
+  [Symbol.iterator]() { return this; }
+};
+var other = newGlobal({newCompartment: true});
+other.parent = this;
+var cross = other.Function('return parent.receive()');
+
+// ArrayState can succeed before recovery of the plain object's allocation fails.
+// Reading the snapshot again must not initialize the array twice.
+var helper = Function('a', 'b',
+  'try { return cross() + a[0] + b.p; } catch (e) { return -1; }');
+var outer = Function(`
+  var b = {p: 3};
+  var a = [11, 22];
+  var eq = (a === a);
+  var [v = helper(a, b)] = iterator;
+  return v + (eq ? 1 : 0);
+`);
+for (var i = 0; i < 1000; i++) outer();
+
+active = true;
+try {
+  outer();
+} catch (e) {
+} finally {
+  resetOOMFailure();
+  active = false;
+}
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/js/src/jit-test/tests/ion/bug2082047-2.js b/js/src/jit-test/tests/ion/bug2082047-2.js
new file mode 100644
index 00000000000..b85859711fb
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug2082047-2.js
@@ -0,0 +1,56 @@
+// |jit-test| --fast-warmup; --no-threads; allow-unhandlable-oom; skip-if: !hasFunction.oomAtAllocation
+
+with ({}) {}
+gczeal(0);
+
+// Avoid allocating exception stacks before reaching the bailout.
+for (var i = 0; i < 100; i++) {
+  try { throw 0; } catch (e) {}
+}
+
+var active = false;
+
+function receive() {
+  if (!active) return 7;
+  oomAtAllocation(4);
+  throw 12345;
+}
+
+// Reading helper's arguments needs the recovery results again. The failure is
+// remembered, so this read crashes with [unhandlable oom] instead of
+// re-running ArrayState.
+function close() {
+  if (active) helper.arguments;
+  return {};
+}
+
+var iterator = {
+  next() { return {value: undefined, done: false}; },
+  return: close,
+  [Symbol.iterator]() { return this; }
+};
+var other = newGlobal({newCompartment: true});
+other.parent = this;
+var cross = other.Function('return parent.receive()');
+
+// ArrayState can succeed before recovery of the plain object's allocation fails.
+// Reading the snapshot again must not initialize the array twice.
+var helper = Function('a', 'b',
+  'try { return cross() + a[0] + b.p; } catch (e) { return -1; }');
+var outer = Function(`
+  var b = {p: 3};
+  var a = [11, 22];
+  var eq = (a === a);
+  var [v = helper(a, b)] = iterator;
+  return v + (eq ? 1 : 0);
+`);
+for (var i = 0; i < 1000; i++) outer();
+
+active = true;
+try {
+  outer();
+} catch (e) {
+} finally {
+  resetOOMFailure();
+  active = false;
+}
Loading diff…