From f5d2f3a7cc0160f854e33a934accc6dea1ece5f1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 20:41:29 +0000 Subject: [PATCH] DFG: flush the handler being left before switching catch liveness to the handler being entered LiveCatchVariablePreservationPhase walks each block and, whenever a node's origin is covered by a different exception handler than the previous node, inserts Flushes for the locals live at the head of the handler it is leaving. The handler lookup lambda also refilled liveAtCatchHead with the liveness of the handler it found, so on a transition straight from one handler into another the flushes were computed from the handler being entered instead of the one being left. Transitions into no handler at all were unaffected, which is why ordinary try/catch (whose jump over the catch block sits inside the try range) did not hit this. The disposal code emitted for `using` ends the try range of its synthesized catch right after the dispose call, so the block containing that call ends with a synthesized Jump whose origin already belongs to the enclosing handler. The "body threw" flag, which only the synthesized catch reads, was therefore never flushed or given a Phi and was unavailable at the dispose call's exception exit: FTL compiles of such code fail OSR availability validation, and optimized code restores the flag as undefined when a dispose method throws after the body threw, so the body's error is dropped instead of being reported through a SuppressedError. Keep the lookup free of side effects, flush for the old handler first, and only then compute the liveness of the new one. --- ...e-call-live-catch-locals-ftl-validation.js | 16 ++++++ ...g-dispose-throw-after-body-throw-in-jit.js | 53 +++++++++++++++++++ .../DFGLiveCatchVariablePreservationPhase.cpp | 32 +++++++---- 3 files changed, 91 insertions(+), 10 deletions(-) create mode 100644 JSTests/stress/for-using-dispose-call-live-catch-locals-ftl-validation.js create mode 100644 JSTests/stress/using-dispose-throw-after-body-throw-in-jit.js diff --git a/JSTests/stress/for-using-dispose-call-live-catch-locals-ftl-validation.js b/JSTests/stress/for-using-dispose-call-live-catch-locals-ftl-validation.js new file mode 100644 index 000000000000..bafb1cd08c88 --- /dev/null +++ b/JSTests/stress/for-using-dispose-call-live-catch-locals-ftl-validation.js @@ -0,0 +1,16 @@ +//@ requireOptions("--useExplicitResourceManagement=1", "--useConcurrentJIT=0", "--validateGraph=1") + +// Found by fuzzing. The FTL compile of this eval code used to fail OSR +// availability validation ("Live bytecode local not available") on the local +// that the synthesized catch handler around the dispose call reads: the try +// range of that handler ends at a block boundary inside the enclosing for-of +// handler, and LiveCatchVariablePreservationPhase flushed the outer handler's +// locals instead of the inner handler's when it crossed from one to the other. + +(0, eval)(` + const resource = { [Symbol.dispose]() { } }; + for (using r of [resource]) { + try { r(); } catch (e) { } + } + for (let i = 0; i < 2000000; ++i) { } +`); diff --git a/JSTests/stress/using-dispose-throw-after-body-throw-in-jit.js b/JSTests/stress/using-dispose-throw-after-body-throw-in-jit.js new file mode 100644 index 000000000000..88b729c15619 --- /dev/null +++ b/JSTests/stress/using-dispose-throw-after-body-throw-in-jit.js @@ -0,0 +1,53 @@ +//@ requireOptions("--useExplicitResourceManagement=1", "--useConcurrentJIT=0") + +// The disposal code emitted for a `using` block keeps "did the body throw" in a +// local that is only read by the synthesized catch handler wrapped around the +// dispose call. That handler has never run by the time the function is +// optimized, so nothing in the compiled code reads the local and it is only +// kept alive for the exception exit of the dispose call. The try range of the +// synthesized catch ends right at that call, inside the range of the enclosing +// handler, so LiveCatchVariablePreservationPhase used to switch to the outer +// handler's liveness before flushing for the inner one and dropped the local. +// The exit then restored it as undefined and the body's error was lost instead +// of being reported through a SuppressedError. + +function shouldBe(actual, expected) { + if (actual !== expected) + throw new Error(`Expected ${expected} but got ${actual}`); +} + +const state = { throwOnDispose: false }; + +// Distinct executables so the dispose call site becomes megamorphic and stays a +// real call instead of being inlined (inlining splits the block and hides the bug). +const resources = []; +for (let i = 0; i < 16; ++i) { + resources.push({ + state, + [Symbol.dispose]: new Function(`if (this.state.throwOnDispose) throw new Error("dispose ${i}");`), + }); +} + +function run(resource, bodyShouldThrow) { + try { + { + using r = resource; + if (bodyShouldThrow) + throw new Error("body"); + } + } catch (e) { + return e; + } + return null; +} + +for (let i = 0; i < 20000; ++i) { + const error = run(resources[i % resources.length], i & 1); + shouldBe(error === null, !(i & 1)); +} + +state.throwOnDispose = true; +const error = run(resources[0], 1); +shouldBe(error instanceof SuppressedError, true); +shouldBe(error.error.message, "dispose 0"); +shouldBe(error.suppressed.message, "body"); diff --git a/Source/JavaScriptCore/dfg/DFGLiveCatchVariablePreservationPhase.cpp b/Source/JavaScriptCore/dfg/DFGLiveCatchVariablePreservationPhase.cpp index afff25864867..dd82fe5554e7 100644 --- a/Source/JavaScriptCore/dfg/DFGLiveCatchVariablePreservationPhase.cpp +++ b/Source/JavaScriptCore/dfg/DFGLiveCatchVariablePreservationPhase.cpp @@ -120,6 +120,7 @@ class LiveCatchVariablePreservationPhase { HandlerInfo* cachedHandlerResult; CodeOrigin cachedCodeOrigin; + CodeOrigin cachedCatchOrigin; auto catchHandler = [&] (CodeOrigin origin) -> HandlerInfo* { ASSERT(origin); if (origin == cachedCodeOrigin) @@ -133,13 +134,7 @@ class LiveCatchVariablePreservationPhase { InlineCallFrame* inlineCallFrame = origin.inlineCallFrame(); CodeBlock* codeBlock = m_graph.baselineCodeBlockFor(inlineCallFrame); if (HandlerInfo* handler = codeBlock->handlerForBytecodeIndex(bytecodeIndexToCheck)) { - liveAtCatchHead.fill(false); - - BytecodeIndex catchBytecodeIndex = BytecodeIndex(handler->target); - m_graph.forAllLocalsAndTmpsLiveInBytecode(CodeOrigin(catchBytecodeIndex, inlineCallFrame), [&] (Operand operand) { - liveAtCatchHead.operand(operand) = true; - }); - + cachedCatchOrigin = CodeOrigin(BytecodeIndex(handler->target), inlineCallFrame); cachedHandlerResult = handler; break; } @@ -156,6 +151,14 @@ class LiveCatchVariablePreservationPhase { return cachedHandlerResult; }; + // Liveness at the head of the handler most recently returned by catchHandler(). + auto computeLiveAtCatchHead = [&] { + liveAtCatchHead.fill(false); + m_graph.forAllLocalsAndTmpsLiveInBytecode(cachedCatchOrigin, [&] (Operand operand) { + liveAtCatchHead.operand(operand) = true; + }); + }; + Operands currentBlockAccessData(OperandsLike, block->variablesAtTail, nullptr); auto flushEverything = [&] (NodeOrigin origin, unsigned index) { @@ -188,9 +191,18 @@ class LiveCatchVariablePreservationPhase { { HandlerInfo* newHandler = catchHandler(node->origin.semantic); - if (newHandler != currentExceptionHandler && currentExceptionHandler) - flushEverything(node->origin, nodeIndex); - currentExceptionHandler = newHandler; + if (newHandler != currentExceptionHandler) { + // liveAtCatchHead still describes the handler we are leaving. Flush for it before + // switching over to the liveness of the handler we are entering, otherwise a + // transition straight from one handler into another (e.g. leaving a try range that + // is nested inside another one) flushes the outer handler's locals instead of the + // inner handler's. + if (currentExceptionHandler) + flushEverything(node->origin, nodeIndex); + currentExceptionHandler = newHandler; + if (newHandler) + computeLiveAtCatchHead(); + } } if (currentExceptionHandler && (node->op() == SetLocal || node->op() == SetArgumentDefinitely || node->op() == SetArgumentMaybe)) {