[WebAssembly] Fix: fixCallUnwindMismatches after fixCatchUnwindMismatches (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

Author: hoodmane

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()
@@ -2474,8 +2481,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);

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()

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()

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

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)