From 811579616ab0a4224e9bb5e9c9a3d6a41943c450 Mon Sep 17 00:00:00 2001 From: Ted Campbell Date: Mon, 13 Sep 2021 19:21:26 +0000 Subject: [PATCH] Bug 1730426 - Start explicitly throwing uncatchable exceptions. r=jandem Add Interrupt status to JS::ExceptionStatus and set it when intentionally throwing an uncatchable exception. This is only a guideline for now, so avoid excessive asserts for now. Differential Revision: https://phabricator.services.mozilla.com/D125363 --- js/public/Exception.h | 16 +++++++++++++++- js/src/builtin/TestingFunctions.cpp | 2 +- js/src/debugger/Debugger.cpp | 5 +++-- js/src/jit/JitFrames.cpp | 2 +- js/src/jsapi.cpp | 4 ++++ js/src/shell/js.cpp | 3 +++ js/src/vm/Interpreter.cpp | 2 +- js/src/vm/JSContext.cpp | 1 + js/src/vm/JSContext.h | 12 ++++++++++++ js/src/vm/Runtime.cpp | 2 ++ js/xpconnect/src/XPCShellImpl.cpp | 1 + 11 files changed, 44 insertions(+), 6 deletions(-) diff --git a/js/public/Exception.h b/js/public/Exception.h index a5b2773d9a39..7e401e6a3255 100644 --- a/js/public/Exception.h +++ b/js/public/Exception.h @@ -39,6 +39,12 @@ extern JS_PUBLIC_API void JS_SetPendingException( JSContext* cx, JS::HandleValue v, JS::ExceptionStackBehavior behavior = JS::ExceptionStackBehavior::Capture); +// Indicate that we are intentionally raising an interrupt/uncatchable +// exception. Returning false/nullptr without calling this will still be treated +// as an interrupt, but in the future the implicit behaviour may no longer be +// allowed. +extern JS_PUBLIC_API void JS_SetPendingInterrupt(JSContext* cx); + extern JS_PUBLIC_API void JS_ClearPendingException(JSContext* cx); /** @@ -56,7 +62,9 @@ namespace JS { // When propagating an exception up the call stack, we store the underlying // reason on the JSContext as one of the following enum values. // -// TODO: Track uncatchable exceptions explicitly. +// NOTE: It is not yet a hard requirement for uncatchable exceptions to set +// status to Interrupt so rely on the last return value. If Interrupt is +// not set then the status will remain as None. enum class ExceptionStatus { // No expection status. None, @@ -66,6 +74,12 @@ enum class ExceptionStatus { // non-error completion. ForcedReturn, + // An uncatchable exception that functions as an interrupt. This is used for + // watchdog mechanisms (like the slow-script notification) or the debugger + // API. It cannot be caught be JS catch blocks, but the debugger or event + // loops may still capture it. + Interrupt, + // Throwing a (catchable) exception. Certain well-known exceptions are // explicitly tracked for convenience. Throwing, diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp index 4a5624e1cd36..03ae3eb4f01c 100644 --- a/js/src/builtin/TestingFunctions.cpp +++ b/js/src/builtin/TestingFunctions.cpp @@ -3680,7 +3680,7 @@ static bool Terminate(JSContext* cx, unsigned arg, Value* vp) { fprintf(stderr, "terminate called\n"); } - JS_ClearPendingException(cx); + cx->setInterrupting(); return false; } diff --git a/js/src/debugger/Debugger.cpp b/js/src/debugger/Debugger.cpp index 6e558c7f387a..f3e949f04dc1 100644 --- a/js/src/debugger/Debugger.cpp +++ b/js/src/debugger/Debugger.cpp @@ -300,7 +300,7 @@ static void PropagateForcedReturn(JSContext* cx, AbstractFramePtr frame, return false; case ResumeMode::Terminate: - cx->clearPendingException(); + cx->setInterrupting(); return false; case ResumeMode::Return: @@ -1018,7 +1018,7 @@ NativeResumeMode DebugAPI::slowPathOnNativeCall(JSContext* cx, return NativeResumeMode::Abort; case ResumeMode::Terminate: - cx->clearPendingException(); + cx->setInterrupting(); return NativeResumeMode::Abort; case ResumeMode::Return: @@ -1951,6 +1951,7 @@ Completion Completion::fromJSResult(JSContext* cx, bool ok, const Value& rv) { } if (!cx->isExceptionPending()) { + cx->clearInterrupt(); return Completion(Terminate()); } diff --git a/js/src/jit/JitFrames.cpp b/js/src/jit/JitFrames.cpp index accfb6cc2664..294ebfbf4bd2 100644 --- a/js/src/jit/JitFrames.cpp +++ b/js/src/jit/JitFrames.cpp @@ -524,7 +524,7 @@ again: } // We may be propagating a forced return from a debugger hook function. - if (MOZ_UNLIKELY(cx->isPropagatingForcedReturn())) { + if (cx->isPropagatingForcedReturn()) { cx->clearPropagatingForcedReturn(); frameOk = true; } diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index c2366dc1fe9f..38a6c454edf8 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -3685,6 +3685,10 @@ JS_PUBLIC_API void JS_SetPendingException(JSContext* cx, HandleValue value, } } +JS_PUBLIC_API void JS_SetPendingInterrupt(JSContext* cx) { + cx->setInterrupting(); +} + JS_PUBLIC_API void JS_ClearPendingException(JSContext* cx) { AssertHeapIsIdle(); cx->clearPendingException(); diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index aa65986a4885..f1ddfd2bab5d 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -2617,6 +2617,7 @@ static bool Evaluate(JSContext* cx, unsigned argc, Value* vp) { ? JS_ExecuteScript(cx, script, args.rval()) : JS_ExecuteScript(cx, envChain, script, args.rval()))) { if (catchTermination && !JS_IsExceptionPending(cx)) { + cx->clearInterrupt(); JSAutoRealm ar1(cx, callerGlobal); JSString* str = JS_NewStringCopyZ(cx, "terminated"); if (!str) { @@ -3147,6 +3148,8 @@ static bool Quit(JSContext* cx, unsigned argc, Value* vp) { js::StopDrainingJobQueue(cx); sc->exitCode = code; sc->quitting = true; + + cx->setInterrupting(); return false; } diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 4e9b2bd5e07e..bdc444ffb17f 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -1220,7 +1220,7 @@ again: UnwindIteratorsForUncatchableException(cx, regs); // We may be propagating a forced return from a debugger hook function. - if (MOZ_UNLIKELY(cx->isPropagatingForcedReturn())) { + if (cx->isPropagatingForcedReturn()) { cx->clearPropagatingForcedReturn(); ok = true; } diff --git a/js/src/vm/JSContext.cpp b/js/src/vm/JSContext.cpp index 665edc632f1e..161eeb48465b 100644 --- a/js/src/vm/JSContext.cpp +++ b/js/src/vm/JSContext.cpp @@ -837,6 +837,7 @@ void InternalJobQueue::runJobs(JSContext* cx) { if (!JS::Call(cx, UndefinedHandleValue, job, args, &rval)) { // Nothing we can do about uncatchable exceptions. if (!cx->isExceptionPending()) { + cx->clearInterrupt(); continue; } RootedValue exn(cx); diff --git a/js/src/vm/JSContext.h b/js/src/vm/JSContext.h index 190af7f73a34..99a7023abe7d 100644 --- a/js/src/vm/JSContext.h +++ b/js/src/vm/JSContext.h @@ -799,6 +799,18 @@ struct JS_PUBLIC_API JSContext : public JS::RootingContext, return JS::IsCatchableExceptionStatus(status); } + void setInterrupting() { + MOZ_ASSERT(status == JS::ExceptionStatus::None); + status = JS::ExceptionStatus::Interrupt; + } + void clearInterrupt() { + // NOTE: Calling `setInterrupting` is still optional, but certainly there + // should not be any catchable exception pending. + MOZ_ASSERT(status == JS::ExceptionStatus::None || + status == JS::ExceptionStatus::Interrupt); + status = JS::ExceptionStatus::None; + } + [[nodiscard]] bool getPendingException(JS::MutableHandleValue rval); js::SavedFrame* getPendingExceptionStack(); diff --git a/js/src/vm/Runtime.cpp b/js/src/vm/Runtime.cpp index e2091966a02a..96912e0e16bc 100644 --- a/js/src/vm/Runtime.cpp +++ b/js/src/vm/Runtime.cpp @@ -482,6 +482,8 @@ static bool HandleInterrupt(JSContext* cx, bool invokeCallback) { chars = u"(stack not available)"; } WarnNumberUC(cx, JSMSG_TERMINATED, chars); + + cx->setInterrupting(); return false; } diff --git a/js/xpconnect/src/XPCShellImpl.cpp b/js/xpconnect/src/XPCShellImpl.cpp index f6be050e1a1b..0b1c928e9c70 100644 --- a/js/xpconnect/src/XPCShellImpl.cpp +++ b/js/xpconnect/src/XPCShellImpl.cpp @@ -423,6 +423,7 @@ static bool Quit(JSContext* cx, unsigned argc, Value* vp) { gQuitting = true; // exit(0); + JS_SetPendingInterrupt(cx); return false; }