release/22.x: [WebAssembly] Fix: fixCallUnwindMismatches after fixCatchUnwindMismatches (#187484) - #190996
Merged
Merged
Conversation
Member
Author
|
@aheejin What do you think about merging this PR to the release branch? |
aheejin
approved these changes
Apr 8, 2026
github-project-automation
Bot
moved this from Needs Triage
to Needs Merge
in LLVM Release Status
Member
Author
|
@llvm/pr-subscribers-backend-webassembly Author: llvmbot ChangesBackport f89b9a0 Requested by: @aheejin Full diff: https://github.com/llvm/llvm-project/pull/190996.diff 5 Files Affected:
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
index acb889f00a48c..44d54cadf289c 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
@@ -1836,6 +1836,8 @@ bool WebAssemblyCFGStackify::fixCallUnwindMismatches(MachineFunction &MF) {
for (auto &MI : reverse(MBB)) {
if (WebAssembly::isTry(MI.getOpcode()))
EHPadStack.pop_back();
+ else if (MI.getOpcode() == WebAssembly::DELEGATE)
+ EHPadStack.push_back(MI.getOperand(0).getMBB());
else if (WebAssembly::isCatch(MI.getOpcode()))
EHPadStack.push_back(MI.getParent());
@@ -1923,8 +1925,10 @@ bool WebAssemblyCFGStackify::fixCallUnwindMismatches(MachineFunction &MF) {
RecordCallerMismatchRange(EHPadStack.back());
// If EHPadStack is empty, that means it correctly unwinds to the caller
- // if it throws, so we're good. If MI does not throw, we're good too.
- else if (EHPadStack.empty() || !MayThrow) {
+ // if it throws, so we're good. A delegate targeting FakeCallerBB also
+ // correctly unwinds to the caller. If MI does not throw, we're good too.
+ else if (EHPadStack.empty() || EHPadStack.back() == FakeCallerBB ||
+ !MayThrow) {
}
// We found an instruction that unwinds to the caller but currently has an
@@ -1940,6 +1944,8 @@ bool WebAssemblyCFGStackify::fixCallUnwindMismatches(MachineFunction &MF) {
// Update EHPadStack.
if (WebAssembly::isTry(MI.getOpcode()))
EHPadStack.pop_back();
+ else if (MI.getOpcode() == WebAssembly::DELEGATE)
+ EHPadStack.push_back(MI.getOperand(0).getMBB());
else if (WebAssembly::isCatch(MI.getOpcode()))
EHPadStack.push_back(MI.getParent());
}
@@ -2169,7 +2175,8 @@ bool WebAssemblyCFGStackify::fixCatchUnwindMismatches(MachineFunction &MF) {
// The EHPad's next unwind destination is the caller, but we incorrectly
// unwind to another EH pad.
- else if (!EHPadStack.empty() && !EHInfo->hasUnwindDest(EHPad)) {
+ else if (!EHPadStack.empty() && EHPadStack.back() != FakeCallerBB &&
+ !EHInfo->hasUnwindDest(EHPad)) {
EHPadToUnwindDest[EHPad] = getFakeCallerBlock(MF);
LLVM_DEBUG(dbgs()
<< "- Catch unwind mismatch:\nEHPad = " << EHPad->getName()
@@ -2475,8 +2482,11 @@ void WebAssemblyCFGStackify::placeMarkers(MachineFunction &MF) {
// Add an 'unreachable' after 'end_try_table's.
addUnreachableAfterTryTables(MF, TII);
// Fix mismatches in unwind destinations induced by linearizing the code.
- fixCallUnwindMismatches(MF);
+ // Run fixCatchUnwindMismatches() first so that fixCallUnwindMismatches()
+ // will see and correct any new call/rethrow unwind mismatches introduced by
+ // fixCatchUnwindMismatches().
fixCatchUnwindMismatches(MF);
+ fixCallUnwindMismatches(MF);
// addUnreachableAfterTryTables and fixUnwindMismatches create new BBs, so
// we need to recalculate ScopeTops.
recalculateScopeTops(MF);
diff --git a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh-legacy.ll b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh-legacy.ll
index f0a1d9805c806..3a18a4351b76e 100644
--- a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh-legacy.ll
+++ b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh-legacy.ll
@@ -659,11 +659,9 @@ try.cont: ; preds = %catch.start0
; --- try-delegate ends (call unwind mismatch)
; NOSORT: catch
; NOSORT: call {{.*}} __cxa_begin_catch
-; --- try-delegate starts (call unwind mismatch)
-; NOSORT: try
-; NOSORT: call __cxa_end_catch
-; NOSORT: delegate 3 # label/catch{{[0-9]+}}: to caller
-; --- try-delegate ends (call unwind mismatch)
+; __cxa_end_catch doesn't need its own try-delegate because the enclosing
+; catch-mismatch delegate already routes to caller.
+; NOSORT: call __cxa_end_catch
; NOSORT: end_try
; NOSORT: delegate 1 # label/catch{{[0-9]+}}: to caller
; --- try-delegate ends (catch unwind mismatch)
@@ -1735,9 +1733,104 @@ unreachable: ; preds = %rethrow, %entry
unreachable
}
+; Regression test for issue #187302: fixCallUnwindMismatches() was run first and
+; found that the rethrow had the correct unwind target so did not wrap it in a
+; try/delegate. Then fixCatchUnwindMismatches() added a try/delegate for
+;
+; invoke void @do_catch unwind label %terminate_catchswitch
+;
+; because otherwise it was going to unwind to outer_catchswitch rather than
+; terminate_catchswitch. But this made the rethrow incorrectly transfer control
+; to terminate_catchswitch rather than outer_catchswitch.
+; Fixed by running fixCatchUnwindMismatches() before fixCallUnwindMismatches().
+;
+; This pattern occurs in Rust's catch_unwind inside a destructor.
+
+; CHECK-LABEL: catch_unwind_in_cleanup:
+; CHECK: catch_all
+; CHECK: try
+; CHECK: try
+; CHECK: call resume_unwind
+; CHECK: catch {{.*}} __cpp_exception
+; CHECK: call do_catch
+; end_cleanup and rethrow are inside the inner catch's scope, but each gets
+; its own try-delegate to route exceptions to the correct destination rather
+; than going through the catch-mismatch delegate to the terminate handler.
+; CHECK: try
+; CHECK: call end_cleanup
+; CHECK: delegate 7
+; (caller)
+; CHECK: try
+; CHECK: rethrow 7
+; (Rethrow exception caught by outermost catch_all)
+; CHECK: delegate 2
+; (outer_catchswitch)
+; CHECK: end_try
+; CHECK: delegate 3
+; (terminate)
+; CHECK: catch {{.*}} __cpp_exception
+; CHECK: call do_catch
+; CHECK: return
+define void @catch_unwind_in_cleanup() personality ptr @__gxx_wasm_personality_v0 {
+start:
+ invoke void @resume_unwind(i32 1)
+ to label %unreachable unwind label %outer_cleanuppad
+
+outer_cleanuppad:
+ %outer_pad = cleanuppad within none []
+ call void @start_cleanup() [ "funclet"(token %outer_pad) ]
+ invoke void @resume_unwind(i32 2) [ "funclet"(token %outer_pad) ]
+ to label %unreachable unwind label %inner_catchswitch
+
+inner_catchswitch:
+ %inner_cs = catchswitch within %outer_pad [label %inner_catchpad] unwind label %terminate_catchswitch
+
+inner_catchpad:
+ %inner_cp = catchpad within %inner_cs [ptr null]
+ %exn = call ptr @llvm.wasm.get.exception(token %inner_cp)
+ %sel = call i32 @llvm.wasm.get.ehselector(token %inner_cp)
+ invoke void @do_catch(ptr %exn, i32 2) [ "funclet"(token %inner_cp) ]
+ to label %inner_catchret unwind label %terminate_catchswitch
+
+terminate_catchswitch:
+ %term_cs = catchswitch within %outer_pad [label %terminate_catchpad] unwind label %outer_catchswitch
+
+terminate_catchpad:
+ %term_pad = catchpad within %term_cs [ptr null]
+ call void @panic_in_cleanup() [ "funclet"(token %term_pad) ]
+ unreachable
+
+inner_catchret:
+ catchret from %inner_cp to label %outer_cleanupret
+
+outer_cleanupret:
+ call void @end_cleanup() [ "funclet"(token %outer_pad) ]
+ cleanupret from %outer_pad unwind label %outer_catchswitch
+
+outer_catchswitch:
+ %outer_cs = catchswitch within none [label %outer_catchpad] unwind to caller
+outer_catchpad:
+ %outer_cp = catchpad within %outer_cs [ptr null]
+ %exn2 = call ptr @llvm.wasm.get.exception(token %outer_cp)
+ %sel2 = call i32 @llvm.wasm.get.ehselector(token %outer_cp)
+ call void @do_catch(ptr %exn2, i32 1) [ "funclet"(token %outer_cp) ]
+ catchret from %outer_cp to label %done
+
+done:
+ ret void
+unreachable:
+ unreachable
+}
+
+declare void @resume_unwind(i32) #1
+declare void @do_catch(ptr, i32) #0
+declare void @start_cleanup()
+declare void @end_cleanup()
+declare void @panic_in_cleanup() #2
+
; Check if the unwind destination mismatch stats are correct
-; NOSORT: 24 wasm-cfg-stackify - Number of call unwind mismatches found
-; NOSORT: 5 wasm-cfg-stackify - Number of catch unwind mismatches found
+; NOSORT: 25 wasm-cfg-stackify - Number of call unwind mismatches found
+; NOSORT: 7 wasm-cfg-stackify - Number of catch unwind mismatches found
declare void @foo()
declare void @bar()
diff --git a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
index 98de9a267b95a..93937fd96d887 100644
--- a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
+++ b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
@@ -1507,7 +1507,7 @@ unreachable: ; preds = %rethrow, %entry
}
; Check if the unwind destination mismatch stats are correct
-; NOSORT: 23 wasm-cfg-stackify - Number of call unwind mismatches found
+; NOSORT: 24 wasm-cfg-stackify - Number of call unwind mismatches found
; NOSORT: 4 wasm-cfg-stackify - Number of catch unwind mismatches found
declare void @foo()
diff --git a/llvm/test/CodeGen/WebAssembly/exception-legacy.ll b/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
index b96a664166f42..573f69df8b859 100644
--- a/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
+++ b/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
@@ -429,16 +429,14 @@ unreachable: ; preds = %rethrow
; CHECK: try
; CHECK: try
; CHECK: call __cxa_throw
-; CHECK: catch
+; CHECK: catch
; CHECK: call __cxa_end_catch
; CHECK: try
; CHECK: try
; Note that this rethrow targets the top-level catch_all
; CHECK: rethrow 4
; CHECK: catch
-; CHECK: try
-; CHECK: call __cxa_end_catch
-; CHECK: delegate 5
+; CHECK: call __cxa_end_catch
; CHECK: return
; CHECK: end_try
; CHECK: delegate 3
diff --git a/llvm/test/CodeGen/WebAssembly/exception.ll b/llvm/test/CodeGen/WebAssembly/exception.ll
index 873386b3dcc6d..19c3d1b158951 100644
--- a/llvm/test/CodeGen/WebAssembly/exception.ll
+++ b/llvm/test/CodeGen/WebAssembly/exception.ll
@@ -484,34 +484,43 @@ unreachable: ; preds = %rethrow
; try_table (catch_all_ref 0)'s caught exception is stored in local 2
; CHECK: local.set 2
; CHECK: block
+; catch_all 0 dispatches to %terminate.i (the 'call _ZSt9terminatev' instruction).
; CHECK: try_table (catch_all 0)
-; CHECK: block
-; CHECK: block i32
-; CHECK: try_table (catch __cpp_exception 0)
-; CHECK: call __cxa_throw
+; CHECK: block exnref
+; CHECK: block
+; CHECK: block i32
+; CHECK: try_table (catch __cpp_exception 0)
+; CHECK: call __cxa_throw
+; CHECK: end_try_table
+; CHECK: end_block
+;
+; invoke __cxa_end_catch... unwind label %terminate.i
+; should unwind to %terminate.i. catch_all_ref 1 points to the
+; try_table (catch_all 0) which in turn dispatches to %terminate.i
+; CHECK: try_table (catch_all_ref 1)
+; CHECK: call __cxa_end_catch
; CHECK: end_try_table
-; CHECK: end_block
-; CHECK: call __cxa_end_catch
-; CHECK: block i32
-; CHECK: try_table (catch_all_ref 5)
-; CHECK: try_table (catch __cpp_exception 1)
+; CHECK: block i32
+; CHECK: try_table (catch_all_ref 6)
+; CHECK: try_table (catch __cpp_exception 1)
; Note that the throw_ref below targets the top-level catch_all_ref (local 2)
-; CHECK: local.get 2
-; CHECK: throw_ref
+; CHECK: local.get 2
+; CHECK: throw_ref
+; CHECK: end_try_table
; CHECK: end_try_table
+; CHECK: end_block
+; CHECK: try_table (catch_all_ref 5)
+; CHECK: call __cxa_end_catch
; CHECK: end_try_table
+; CHECK: return
; CHECK: end_block
-; CHECK: try_table (catch_all_ref 4)
-; CHECK: call __cxa_end_catch
-; CHECK: end_try_table
-; CHECK: return
-; CHECK: end_block
-; CHECK: end_try_table
+; CHECK: throw_ref
+; CHECK: end_try_table
+; CHECK: end_block
+; CHECK: call _ZSt9terminatev
; CHECK: end_block
-; CHECK: call _ZSt9terminatev
; CHECK: end_block
-; CHECK: end_block
-; CHECK: throw_ref
+; CHECK: throw_ref
define void @inlined_cleanupret() personality ptr @__gxx_wasm_personality_v0 {
entry:
%exception = tail call ptr @__cxa_allocate_exception(i32 4)
|
…ches (llvm#187484) `fixCallUnwindMismatches()` adds an extra try block around call sites with incorrect unwind targets. `fixCatchUnwindMismatches()` handles catch blocks that have incorrect next unwind destinations. Previously we ran `fixCallUnwindMismatches()` first and then ran `fixCatchUnwindMismatches()`. The problem is that `fixCatchUnwindMismatches()` wraps entire try blocks which can change the unwind destination of the calls inside. If the calls had an incorrect unwind target to begin with, they will be wrapped already and so the outer wrapping won't alter their unwind target. However, if they start out with a correct unwind target, they won't get wrapped and then that can be messed up by `fixCatchUnwindMismatches()`. The fix is to run `fixCatchUnwindMismatches()` first. `fixCallUnwindMismatches()` never messes up the result of `fixCatchUnwindMismatches()` so this is the correct order. Resolves llvm#187302 (cherry picked from commit f89b9a0)
github-project-automation
Bot
moved this from Needs Merge
to Done
in LLVM Release Status
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport f89b9a0
Requested by: @aheejin