[WebAssembly] Fix EHPadStack calculation in fixCallUnwindMismatches - #194184
Merged
Conversation
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.
Member
|
@llvm/pr-subscribers-backend-webassembly Author: Heejin Ahn (aheejin) ChangesAfter #187484, This is mean to replace #192968 and adds its test case to
Closes #192968. Full diff: https://github.com/llvm/llvm-project/pull/194184.diff 4 Files Affected:
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
index 540388cb17b68..baccfe70e66c6 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
@@ -1503,7 +1503,7 @@ void WebAssemblyCFGStackify::addNestedTryTable(MachineInstr *RangeBegin,
// Add a 'end_try_table' instruction in the EndTryTable BB created above.
MachineInstr *EndTryTable = BuildMI(EndTryTableBB, RangeEnd->getDebugLoc(),
TII.get(WebAssembly::END_TRY_TABLE));
- registerTryScope(TryTable, EndTryTable, nullptr);
+ registerTryScope(TryTable, EndTryTable, TrampolineBB);
}
// In the standard (exnref) EH, we fix unwind mismatches by adding a new
@@ -1838,8 +1838,52 @@ bool WebAssemblyCFGStackify::fixCallUnwindMismatches(MachineFunction &MF) {
EHPadStack.pop_back();
else if (MI.getOpcode() == WebAssembly::DELEGATE)
EHPadStack.push_back(MI.getOperand(0).getMBB());
- else if (WebAssembly::isCatch(MI.getOpcode()))
+ else if (WebAssembly::WasmUseLegacyEH &&
+ WebAssembly::isCatch(MI.getOpcode()))
EHPadStack.push_back(MI.getParent());
+ else if (!WebAssembly::WasmUseLegacyEH &&
+ MI.getOpcode() == WebAssembly::END_TRY_TABLE)
+ // In case of the legacy EH, 'catch' instruction is always an EH pad for
+ // the 'try' body that precedes it. But in the standard EH, because
+ // fixCatchUnwindMismatches runs before this, a new try_table's
+ // trampoline BB will be separated from try_table ~ end_try_table body:
+ //
+ // bb0:
+ // try_table (catch_all_ref %far_away_trampoline)
+ // ...
+ // end_try_table
+ // ...
+ // far_away_trampoline:
+ // catch_all_ref
+ // throw_ref
+ //
+ // And there can be multiple try_tables that target a single trampoline:
+ //
+ // bb0:
+ // try_table (catch_all_ref %far_away_trampolinle_bb)
+ // ...
+ // end_try_table
+ // ...
+ // bb1:
+ // try_table (catch_all_ref %far_away_trampolinle_bb)
+ // ...
+ // end_try_table
+ // ...
+ // far_away_trampoline:
+ // catch_all_ref
+ // throw_ref
+ //
+ // So we can't call WebAssembly::isCatch to add its parent EH pad to
+ // EHPadStack. Now we add to EHPadStack at end_try_table marker, by
+ // getting its matching try_table's destination. This works when the
+ // destination EH pad is either a normal EH pad or a trampoline created
+ // in fixCatchUnwindMismatches.
+ //
+ // Note that we don't need to distinguish this case in
+ // fixCatchUnwindMismatches because it runs before
+ // fixCallUnwindMismatches and there is no new try_tables and
+ // trampolines when it runs.
+ EHPadStack.push_back(TryToEHPad[EndToBegin[&MI]]);
// In this loop we only gather calls that have an EH pad to unwind. So
// there will be at most 1 such call (= invoke) in a BB, so after we've
@@ -1946,8 +1990,12 @@ bool WebAssemblyCFGStackify::fixCallUnwindMismatches(MachineFunction &MF) {
EHPadStack.pop_back();
else if (MI.getOpcode() == WebAssembly::DELEGATE)
EHPadStack.push_back(MI.getOperand(0).getMBB());
- else if (WebAssembly::isCatch(MI.getOpcode()))
+ else if (WebAssembly::WasmUseLegacyEH &&
+ WebAssembly::isCatch(MI.getOpcode()))
EHPadStack.push_back(MI.getParent());
+ else if (!WebAssembly::WasmUseLegacyEH &&
+ MI.getOpcode() == WebAssembly::END_TRY_TABLE)
+ EHPadStack.push_back(TryToEHPad[EndToBegin[&MI]]);
}
if (RangeEnd)
diff --git a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh-legacy.ll b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh-legacy.ll
index bb247d4f07db7..a35129effcdee 100644
--- a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh-legacy.ll
+++ b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh-legacy.ll
@@ -1822,6 +1822,61 @@ unreachable:
unreachable
}
+; A regression test for the case where fixCatchUnwindMismatches generates a
+; single trampoline BB targeted by multiple try_tables. This should not crash.
+define i32 @shared_trampoline(ptr %arg, i1 %arg1) personality ptr @__gxx_wasm_personality_v0 {
+bb:
+ br i1 %arg1, label %bb2, label %bb10
+
+bb2: ; preds = %bb
+ invoke void @f1(ptr null, ptr null)
+ to label %bb3 unwind label %bb8
+
+bb3: ; preds = %bb2
+ %i = invoke ptr @f2(ptr null, ptr null)
+ to label %bb4 unwind label %bb6
+
+bb4: ; preds = %bb3
+ %i5 = invoke ptr @f3(ptr null, ptr null)
+ to label %common.ret unwind label %bb8
+
+common.ret: ; preds = %bb21, %bb18, %bb13, %bb6, %bb4
+ %common.ret.op = phi i32 [ 0, %bb18 ], [ 0, %bb6 ], [ 0, %bb13 ], [ 0, %bb21 ], [ 0, %bb4 ]
+ ret i32 %common.ret.op
+
+bb6: ; preds = %bb3
+ %i7 = cleanuppad within none []
+ br label %common.ret
+
+bb8: ; preds = %bb4, %bb2
+ %i9 = cleanuppad within none []
+ cleanupret from %i9 unwind to caller
+
+bb10: ; preds = %bb
+ invoke void @f4(ptr null)
+ to label %bb21 unwind label %bb11
+
+bb11: ; preds = %bb10
+ %i12 = catchswitch within none [label %bb13] unwind to caller
+
+bb13: ; preds = %bb11
+ %i14 = catchpad within %i12 [ptr null]
+ %i15 = tail call ptr @llvm.wasm.get.exception(token %i14)
+ br label %common.ret
+
+bb16: ; preds = %bb21
+ %i17 = catchswitch within none [label %bb18] unwind to caller
+
+bb18: ; preds = %bb16
+ %i19 = catchpad within %i17 [ptr null]
+ %i20 = tail call ptr @llvm.wasm.get.exception(token %i19)
+ br label %common.ret
+
+bb21: ; preds = %bb10
+ %i22 = invoke i32 @f5(ptr null)
+ to label %common.ret unwind label %bb16
+}
+
declare void @resume_unwind(i32) #1
declare void @do_catch(ptr, i32) #0
declare void @start_cleanup()
@@ -1829,8 +1884,8 @@ declare void @end_cleanup()
declare void @panic_in_cleanup() #2
; Check if the unwind destination mismatch stats are correct
-; NOSORT: 26 wasm-cfg-stackify - Number of call unwind mismatches found
-; NOSORT: 7 wasm-cfg-stackify - Number of catch unwind mismatches found
+; NOSORT: 27 wasm-cfg-stackify - Number of call unwind mismatches found
+; NOSORT: 8 wasm-cfg-stackify - Number of catch unwind mismatches found
declare void @foo()
declare void @bar()
@@ -1838,6 +1893,11 @@ declare i32 @baz()
declare i32 @qux(i32)
declare void @quux(i32)
declare void @fun(i32)
+declare void @f1(ptr, ptr)
+declare ptr @f2(ptr, ptr)
+declare ptr @f3(ptr, ptr)
+declare void @f4(ptr)
+declare i32 @f5(ptr)
; Function Attrs: nounwind
declare void @nothrow(i32) #0
; Function Attrs: nounwind
diff --git a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
index 2cfc3e5d0edb2..e872ceec4ea41 100644
--- a/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
+++ b/llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll
@@ -1506,9 +1506,64 @@ unreachable: ; preds = %rethrow, %entry
unreachable
}
+; A regression test for the case where fixCatchUnwindMismatches generates a
+; single trampoline BB targeted by multiple try_tables. This should not crash.
+define i32 @shared_trampoline(ptr %arg, i1 %arg1) personality ptr @__gxx_wasm_personality_v0 {
+bb:
+ br i1 %arg1, label %bb2, label %bb10
+
+bb2: ; preds = %bb
+ invoke void @f1(ptr null, ptr null)
+ to label %bb3 unwind label %bb8
+
+bb3: ; preds = %bb2
+ %i = invoke ptr @f2(ptr null, ptr null)
+ to label %bb4 unwind label %bb6
+
+bb4: ; preds = %bb3
+ %i5 = invoke ptr @f3(ptr null, ptr null)
+ to label %common.ret unwind label %bb8
+
+common.ret: ; preds = %bb21, %bb18, %bb13, %bb6, %bb4
+ %common.ret.op = phi i32 [ 0, %bb18 ], [ 0, %bb6 ], [ 0, %bb13 ], [ 0, %bb21 ], [ 0, %bb4 ]
+ ret i32 %common.ret.op
+
+bb6: ; preds = %bb3
+ %i7 = cleanuppad within none []
+ br label %common.ret
+
+bb8: ; preds = %bb4, %bb2
+ %i9 = cleanuppad within none []
+ cleanupret from %i9 unwind to caller
+
+bb10: ; preds = %bb
+ invoke void @f4(ptr null)
+ to label %bb21 unwind label %bb11
+
+bb11: ; preds = %bb10
+ %i12 = catchswitch within none [label %bb13] unwind to caller
+
+bb13: ; preds = %bb11
+ %i14 = catchpad within %i12 [ptr null]
+ %i15 = tail call ptr @llvm.wasm.get.exception(token %i14)
+ br label %common.ret
+
+bb16: ; preds = %bb21
+ %i17 = catchswitch within none [label %bb18] unwind to caller
+
+bb18: ; preds = %bb16
+ %i19 = catchpad within %i17 [ptr null]
+ %i20 = tail call ptr @llvm.wasm.get.exception(token %i19)
+ br label %common.ret
+
+bb21: ; preds = %bb10
+ %i22 = invoke i32 @f5(ptr null)
+ to label %common.ret unwind label %bb16
+}
+
; Check if the unwind destination mismatch stats are correct
; NOSORT: 25 wasm-cfg-stackify - Number of call unwind mismatches found
-; NOSORT: 4 wasm-cfg-stackify - Number of catch unwind mismatches found
+; NOSORT: 5 wasm-cfg-stackify - Number of catch unwind mismatches found
declare void @foo()
declare void @bar()
@@ -1516,6 +1571,11 @@ declare i32 @baz()
declare i32 @qux(i32)
declare void @quux(i32)
declare void @fun(i32)
+declare void @f1(ptr, ptr)
+declare ptr @f2(ptr, ptr)
+declare ptr @f3(ptr, ptr)
+declare void @f4(ptr)
+declare i32 @f5(ptr)
; Function Attrs: nounwind
declare void @nothrow(i32) #0
; Function Attrs: nounwind
diff --git a/llvm/test/CodeGen/WebAssembly/exception.ll b/llvm/test/CodeGen/WebAssembly/exception.ll
index 19c3d1b158951..95156948e2ccb 100644
--- a/llvm/test/CodeGen/WebAssembly/exception.ll
+++ b/llvm/test/CodeGen/WebAssembly/exception.ll
@@ -469,9 +469,9 @@ unreachable: ; preds = %rethrow
; }
;
; ~Temp() generates cleanupret, which is lowered to a 'throw_ref' later. That
-; throw_ref's argument should correctly target the top-level cleanuppad
-; (catch_all_ref). This is a regression test for the bug where we did not
-; compute throw_ref's argument correctly.
+; hrow_ref's argument should correctly rethrow the exception caught by the
+; top-level cleanuppad (catch_all_ref). This is a regression test for the bug
+; where we did not compute throw_ref's argument correctly.
; CHECK-LABEL: inlined_cleanupret:
; CHECK: block exnref
@@ -486,41 +486,34 @@ unreachable: ; preds = %rethrow
; CHECK: block
; catch_all 0 dispatches to %terminate.i (the 'call _ZSt9terminatev' instruction).
; CHECK: try_table (catch_all 0)
-; 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: block
+; CHECK: block i32
+; CHECK: try_table (catch __cpp_exception 0)
+; CHECK: call __cxa_throw
; CHECK: end_try_table
-; 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: end_try_table
-; CHECK: end_try_table
-; CHECK: end_block
+; CHECK: end_block
+; CHECK: call __cxa_end_catch
+; CHECK: block i32
; CHECK: try_table (catch_all_ref 5)
-; CHECK: call __cxa_end_catch
+; CHECK: try_table (catch __cpp_exception 1)
+; Note that the throw_ref below rethrows the exception caught by the top-level
+; catch_all_ref (local 2)
+; CHECK: local.get 2
+; CHECK: throw_ref
+; CHECK: end_try_table
; CHECK: end_try_table
-; CHECK: return
; CHECK: end_block
-; CHECK: throw_ref
-; CHECK: end_try_table
-; CHECK: end_block
-; CHECK: call _ZSt9terminatev
+; 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: end_block
+; CHECK: call _ZSt9terminatev
; CHECK: end_block
-; CHECK: throw_ref
+; CHECK: end_block
+; CHECK: throw_ref
define void @inlined_cleanupret() personality ptr @__gxx_wasm_personality_v0 {
entry:
%exception = tail call ptr @__cxa_allocate_exception(i32 4)
|
Member
Author
|
cc @soilSpoon |
aheejin
commented
Apr 25, 2026
| MachineInstr *EndTryTable = BuildMI(EndTryTableBB, RangeEnd->getDebugLoc(), | ||
| TII.get(WebAssembly::END_TRY_TABLE)); | ||
| registerTryScope(TryTable, EndTryTable, nullptr); | ||
| registerTryScope(TryTable, EndTryTable, TrampolineBB); |
Member
Author
There was a problem hiding this comment.
This was somehow missing but no code was using this until this PR. Without this, we don't have the link for end_try_table->try_table->trampoline reigstered in
llvm-project/llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
Lines 257 to 264 in fc99b67
|
Confirmed — #194184 fully fixes the standardized-EH backend crash on the VTK 9.6.1 build that motivated #192968. Test setup
Result
Same backtrace as the reduced test in #192968 on every failure. With the patch all 11 TUs compile to valid wasm objects and the full build is clean. Failing TUs (all standardized-EH, all the same crash):
(My #192968 description said "three" — that was the count from the build I had open at the time; with default VTK modules and Happy to drop #192968 in favor of this PR — it's clearly the right fix. About #131561: I commented there initially because I thought it was the same bug, but on closer look it turned out to be a different issue, so I deleted the comments. No separate fix needed from my side. |
dschuff
approved these changes
Apr 27, 2026
| else if (WebAssembly::WasmUseLegacyEH && | ||
| WebAssembly::isCatch(MI.getOpcode())) | ||
| EHPadStack.push_back(MI.getParent()); | ||
| else if (!WebAssembly::WasmUseLegacyEH && |
Member
There was a problem hiding this comment.
Is the check for !WasmUseLegacyEH necessary? Can the opcode ever be END_TRY_TABLE otherwise?
Member
Author
There was a problem hiding this comment.
No it's not. Will remove it.
Co-authored-by: Derek Schuff <dschuff@chromium.org>
Member
Author
|
/cherry-pick d8c1f87 |
github-project-automation
Bot
moved this from Needs Triage
to Done
in LLVM Release Status
Member
|
Failed to cherry-pick: d8c1f87 https://github.com/llvm/llvm-project/actions/runs/25074822202 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
Member
Author
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.
dyung
pushed a commit
to aheejin/llvm-project
that referenced
this pull request
May 1, 2026
…ndMismatches (llvm#194184) Backport d8c1f87 Requested by: @aheejin
daunabomba
pushed a commit
to daunabomba/llvm-project
that referenced
this pull request
May 7, 2026
…ndMismatches (llvm#194184) Backport d8c1f87 Requested by: @aheejin
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.
After #187484,
fixCallUnwindMismatchesnow runs afterfixCatchUnwindMismatches. ButfixCallUnwindMismatcheswas not handling newly generatedtry_tables and trampoline BBs infixCatchUnwindMismatchescorrectly. See the comments for details.This is mean to replace #192968 and adds its test case to
cfg-stackify-eh.llandcfg-stackify-eh-legacy.ll. (The bug only happens in the standard (exnref) EH socfg-stackify-eh.ll, but I added the same test tocfg-stackify-eh-legacy.llas well for more coverage.)exception.ll'sinlined_cleanupret's results have changed because we were generating unnecessarytry_tables, 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 forinlined_cleanupret, because "throw_ref targets the top level cleanuppad" is confusing. (This comment was probably copy-pasted from "delegate targets ...".)Closes #192968.