[WebAssembly] Fix: fixCallUnwindMismatches after fixCatchUnwindMismatches#187484
Merged
[WebAssembly] Fix: fixCallUnwindMismatches after fixCatchUnwindMismatches#187484
Conversation
Member
|
@llvm/pr-subscribers-backend-webassembly Author: Hood Chatham (hoodmane) Changes
The fix is to run Resolves #187302 Full diff: https://github.com/llvm/llvm-project/pull/187484.diff 3 Files Affected:
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
index 7fd2fb692549e..4e6f70af070b2 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());
@@ -1847,6 +1849,8 @@ bool WebAssemblyCFGStackify::fixCallUnwindMismatches(MachineFunction &MF) {
!WebAssembly::mayThrow(MI))
continue;
SeenThrowableInstInBB = true;
+ LLVM_DEBUG(dbgs() << "- fixCallUnwindMismatches considering: " << MI
+ << " in MBB = " << MBB.getName() << "\n");
// If the EH pad on the stack top is where this instruction should unwind
// next, we're good.
@@ -1923,8 +1927,11 @@ 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 the caller (i.e.,
+ // FakeCallerBB on the stack) 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,8 +1947,15 @@ bool WebAssemblyCFGStackify::fixCallUnwindMismatches(MachineFunction &MF) {
// Update EHPadStack.
if (WebAssembly::isTry(MI.getOpcode()))
EHPadStack.pop_back();
- else if (WebAssembly::isCatch(MI.getOpcode()))
- EHPadStack.push_back(MI.getParent());
+ else if (MI.getOpcode() == WebAssembly::DELEGATE)
+ EHPadStack.push_back(MI.getOperand(0).getMBB());
+ else if (WebAssembly::isCatch(MI.getOpcode())) {
+ if (MBB.isEHPad()) {
+ EHPadStack.push_back(&MBB);
+ } else {
+ EHPadStack.push_back(&MBB);
+ }
+ }
}
if (RangeEnd)
@@ -2474,8 +2488,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/exception-legacy.ll b/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
index b96a664166f42..5537ca0e8f1ca 100644
--- a/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
+++ b/llvm/test/CodeGen/WebAssembly/exception-legacy.ll
@@ -429,16 +429,16 @@ unreachable: ; preds = %rethrow
; CHECK: try
; CHECK: try
; CHECK: call __cxa_throw
-; CHECK: catch
+; CHECK: catch {{.*}} __cpp_exception
+; CHECK: call $drop=, __cxa_begin_catch
; CHECK: call __cxa_end_catch
; CHECK: try
; CHECK: try
-; Note that this rethrow targets the top-level catch_all
+; Rethrows the original exception caught by the outermost catch_all
; CHECK: rethrow 4
-; CHECK: catch
-; CHECK: try
-; CHECK: call __cxa_end_catch
-; CHECK: delegate 5
+; CHECK: catch {{.*}} __cpp_exception
+; CHECK: call $drop=, __cxa_begin_catch
+; CHECK: call __cxa_end_catch
; CHECK: return
; CHECK: end_try
; CHECK: delegate 3
@@ -569,6 +569,101 @@ declare void @__cxa_throw(ptr, ptr, ptr) #1
declare void @_ZSt9terminatev()
declare ptr @_ZN4TempD2Ev(ptr returned)
+; 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
+
attributes #0 = { nounwind }
attributes #1 = { noreturn }
attributes #2 = { noreturn nounwind }
diff --git a/llvm/test/CodeGen/WebAssembly/exception.ll b/llvm/test/CodeGen/WebAssembly/exception.ll
index 873386b3dcc6d..5cdabfca4cc29 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
; 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)
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
hoodmane
force-pushed
the
fix-cfg-stackify
branch
3 times, most recently
from
d3057b9 to
3091d3a
Compare
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
…indMismatches()` `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 can't alter their unwind target. However, if they have 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
hoodmane
force-pushed
the
fix-cfg-stackify
branch
from
3091d3a to
0b5715d
Compare
aheejin
reviewed
Mar 26, 2026
aheejin
reviewed
Mar 26, 2026
Member
|
Are you planning to land this? I think we should backport this to the current release. |
Co-authored-by: Heejin Ahn <aheejin@gmail.com>
Contributor
Author
|
Okay @aheejin I think I addressed your comments assuming I understood them correctly. |
aheejin
approved these changes
Apr 6, 2026
Contributor
Author
|
Thanks for the review @aheejin! Would appreciate it if you could merge this and backport for me. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/22203 Here is the relevant piece of the build log for the reference |
Member
|
/cherry-pick f89b9a0 |
Member
|
/pull-request #190996 |
Contributor
Author
|
Thanks again @aheejin! |
c-rhodes
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Apr 10, 2026
…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)
YonahGoldberg
pushed a commit
to YonahGoldberg/llvm-project
that referenced
this pull request
Apr 21, 2026
…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
This was referenced Apr 25, 2026
aheejin
added a commit
that referenced
this pull request
Apr 28, 2026
aheejin
added a commit
that referenced
this pull request
Apr 28, 2026
…194184) After #187484, `fixCallUnwindMismatches` now runs after `fixCatchUnwindMismatches`. But `fixCallUnwindMismatches` was not handling newly generated `try_table`s and trampoline BBs in `fixCatchUnwindMismatches` correctly. See the comments for details. This is mean to replace #192968 and adds its test case to `cfg-stackify-eh.ll` and `cfg-stackify-eh-legacy.ll`. (The bug only happens in the standard (exnref) EH so `cfg-stackify-eh.ll`, but I added the same test to `cfg-stackify-eh-legacy.ll` as well for more coverage.) `exception.ll`'s `inlined_cleanupret`'s results have changed because we were generating unnecessary `try_table`s, thinking there were call unwind mismatches when there weren't. (This wouldn't have resulted in incorrect execution; it just added more unnecessary instructions.) This also fixes some comments for `inlined_cleanupret`, because "throw_ref targets the top level cleanuppad" is confusing. (This comment was probably copy-pasted from "delegate targets ...".) Closes #192968.
kirthana14m
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Apr 30, 2026
…94188) This code was written under the assumption that `fixCallUnwindMismatches` runs before `fixCatchUnwindMismatches`, but since llvm#187484 it's not true anymore.
kirthana14m
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Apr 30, 2026
…lvm#194184) After llvm#187484, `fixCallUnwindMismatches` now runs after `fixCatchUnwindMismatches`. But `fixCallUnwindMismatches` was not handling newly generated `try_table`s and trampoline BBs in `fixCatchUnwindMismatches` correctly. See the comments for details. This is mean to replace llvm#192968 and adds its test case to `cfg-stackify-eh.ll` and `cfg-stackify-eh-legacy.ll`. (The bug only happens in the standard (exnref) EH so `cfg-stackify-eh.ll`, but I added the same test to `cfg-stackify-eh-legacy.ll` as well for more coverage.) `exception.ll`'s `inlined_cleanupret`'s results have changed because we were generating unnecessary `try_table`s, thinking there were call unwind mismatches when there weren't. (This wouldn't have resulted in incorrect execution; it just added more unnecessary instructions.) This also fixes some comments for `inlined_cleanupret`, because "throw_ref targets the top level cleanuppad" is confusing. (This comment was probably copy-pasted from "delegate targets ...".) Closes llvm#192968.
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.
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 ranfixCallUnwindMismatches()first and then ranfixCatchUnwindMismatches(). The problem is thatfixCatchUnwindMismatches()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 byfixCatchUnwindMismatches().The fix is to run
fixCatchUnwindMismatches()first.fixCallUnwindMismatches()never messes up the result offixCatchUnwindMismatches()so this is the correct order.Resolves #187302
cc @aheejin, @dschuff