tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 2e87317ac55e46b74e2ec7095c655fa09dc3cae2
parent 5a1f649191a902e103679b4edec73a7d7f5d4999
Author: Bryan Thrall <bthrall@mozilla.com>
Date:   Wed, 12 Nov 2025 22:02:51 +0000

Bug 1989115 - Avoid OOM stack trace at certain times r=jandem

When FrameIter can't walk the stack properly, we can use AutoUnsafeStackTrace.
This will avoid crashes if we OOM and try to capture a stack trace in those
situations.

See also bug 1999042.

Differential Revision: https://phabricator.services.mozilla.com/D272095

Diffstat:
Mjs/src/jit/BaselineBailouts.cpp | 2++
Mjs/src/jit/BaselineDebugModeOSR.cpp | 3+++
Mjs/src/vm/JSContext.cpp | 34++++++++++++++++++++++++++--------
Mjs/src/vm/JSContext.h | 16+++++++++++++++-
Mjs/src/wasm/WasmBuiltins.cpp | 3+++
5 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/js/src/jit/BaselineBailouts.cpp b/js/src/jit/BaselineBailouts.cpp @@ -1564,6 +1564,8 @@ bool jit::BailoutIonToBaseline(JSContext* cx, JitActivation* activation, MaybeReadFallback recoverBailout(cx, activation, &iter, MaybeReadFallback::Fallback_DoNothing); + // FrameIter can't walk the stack trace while we are bailing out + AutoUnsafeStackTrace aust(cx); // Ensure that all value locations are readable from the SnapshotIterator. // Get the RInstructionResults from the JitActivation if the frame got // recovered ahead of the bailout. diff --git a/js/src/jit/BaselineDebugModeOSR.cpp b/js/src/jit/BaselineDebugModeOSR.cpp @@ -416,6 +416,9 @@ bool js::jit::RecompileBaselineScriptForDebugMode( observing ? "DEBUGGING" : "NORMAL EXECUTION"); AutoKeepJitScripts keepJitScripts(cx); + // FrameIter can't walk the stack if OOM happens because script doesn't have a + // BaselineScript while we recompile it. + AutoUnsafeStackTrace aust(cx); BaselineScript* oldBaselineScript = script->jitScript()->clearBaselineScript(cx->gcContext(), script); diff --git a/js/src/vm/JSContext.cpp b/js/src/vm/JSContext.cpp @@ -269,6 +269,12 @@ static void MaybeReportOutOfMemoryForDifferentialTesting() { } } +bool JSContext::safeToCaptureStackTrace() const { + // If we're in an unsafe ABI context, we don't need to capture a stack trace + // because the function will explicitly recover from OOM. + return !inUnsafeCallWithABI && !unsafeToCaptureStackTrace; +} + /* * Since memory has been exhausted, avoid the normal error-handling path which * allocates an error object, report and callstack. Instead simply throw the @@ -282,11 +288,7 @@ void JSContext::onOutOfMemory() { gc::AutoSuppressGC suppressGC(this); // Capture stack trace before doing anything else that might use memory. - // If we're in an unsafe ABI context, we don't need to capture a stack trace - // because the function will explicitly recover from OOM. - if (!inUnsafeCallWithABI) { - captureOOMStackTrace(); - } + maybeCaptureOOMStackTrace(); /* Report the oom. */ if (JS::OutOfMemoryCallback oomCallback = runtime()->oomCallback) { @@ -1227,6 +1229,7 @@ JSContext::JSContext(JSRuntime* runtime, const JS::ContextOptions& options) activation_(this, nullptr), profilingActivation_(nullptr), noExecuteDebuggerTop(this, nullptr), + unsafeToCaptureStackTrace(this, false), inUnsafeCallWithABI(this, false), hasAutoUnsafeCallWithABI(this, false), #ifdef DEBUG @@ -1359,7 +1362,7 @@ const char* JSContext::getOOMStackTrace() const { bool JSContext::hasOOMStackTrace() const { return oomStackTraceBufferValid_; } -void JSContext::captureOOMStackTrace() { +void JSContext::maybeCaptureOOMStackTrace() { // Clear any existing stack trace oomStackTraceBufferValid_ = false; @@ -1369,9 +1372,13 @@ void JSContext::captureOOMStackTrace() { // Write directly to pre-allocated buffer to avoid any memory allocation FixedBufferPrinter fbp(oomStackTraceBuffer_, OOMStackTraceBufferSize); - js::DumpBacktrace(this, fbp); - MOZ_ASSERT(strlen(oomStackTraceBuffer_) < OOMStackTraceBufferSize); + if (safeToCaptureStackTrace()) { + js::DumpBacktrace(this, fbp); + } else { + fbp.put("Unsafe to capture stack trace"); + } + MOZ_ASSERT(strlen(oomStackTraceBuffer_) < OOMStackTraceBufferSize); oomStackTraceBufferValid_ = true; } @@ -1750,6 +1757,17 @@ void JSContext::suspendExecutionTracing() { #endif +AutoUnsafeStackTrace::AutoUnsafeStackTrace(JSContext* cx) + : cx_(cx), nested_(cx_->unsafeToCaptureStackTrace) { + cx_->unsafeToCaptureStackTrace = true; +} + +AutoUnsafeStackTrace::~AutoUnsafeStackTrace() { + if (!nested_) { + cx_->unsafeToCaptureStackTrace = false; + } +} + AutoUnsafeCallWithABI::AutoUnsafeCallWithABI(UnsafeABIStrictness strictness) : cx_(TlsContext.get()), nested_(cx_ ? cx_->hasAutoUnsafeCallWithABI : false), diff --git a/js/src/vm/JSContext.h b/js/src/vm/JSContext.h @@ -280,6 +280,8 @@ struct JS_PUBLIC_API JSContext : public JS::RootingContext, return thing->compartment() == compartment(); } + bool safeToCaptureStackTrace() const; + void onOutOfMemory(); void* onOutOfMemory(js::AllocFunction allocFunc, arena_id_t arena, size_t nbytes, void* reallocPtr = nullptr) { @@ -499,6 +501,7 @@ struct JS_PUBLIC_API JSContext : public JS::RootingContext, */ js::ContextData<js::EnterDebuggeeNoExecute*> noExecuteDebuggerTop; + js::ContextData<bool> unsafeToCaptureStackTrace; js::ContextData<uint32_t> inUnsafeCallWithABI; js::ContextData<bool> hasAutoUnsafeCallWithABI; @@ -704,7 +707,7 @@ struct JS_PUBLIC_API JSContext : public JS::RootingContext, void unsetOOMStackTrace(); const char* getOOMStackTrace() const; bool hasOOMStackTrace() const; - void captureOOMStackTrace(); + void maybeCaptureOOMStackTrace(); js::ContextData<int32_t> reportGranularity; /* see vm/Probes.h */ @@ -1210,6 +1213,17 @@ class MOZ_RAII AutoNoteExclusiveDebuggerOnEval { } }; +// Should be used in functions that manipulate the stack so FrameIter is unable +// to iterate over it. +class MOZ_RAII AutoUnsafeStackTrace { + JSContext* cx_; + bool nested_; + + public: + explicit AutoUnsafeStackTrace(JSContext* cx); + ~AutoUnsafeStackTrace(); +}; + enum UnsafeABIStrictness { NoExceptions, AllowPendingExceptions }; // Should be used in functions called directly from JIT code (with diff --git a/js/src/wasm/WasmBuiltins.cpp b/js/src/wasm/WasmBuiltins.cpp @@ -1146,6 +1146,9 @@ static int32_t CoerceInPlace_JitEntry(int funcIndex, Instance* instance, // Allocate a BigInt without GC, corresponds to the similar VMFunction. static BigInt* AllocateBigIntTenuredNoGC() { JSContext* cx = TlsContext.get(); // Cold code (the caller is elaborate) + // WasmFrameIter doesn't know how to walk the stack from here (see bug + // 1999042), so we can't capture a stack trace if we OOM + AutoUnsafeStackTrace aust(cx); BigInt* bi = cx->newCell<BigInt, NoGC>(gc::Heap::Tenured); if (!bi) {