[WebAssemblly] Fix EHPadStack update in fixCallUnwindMismatches

[aheejin]

Summary

Updating EHPadStack with respect to TRY and CATCH instructions
have to be done after checking all other conditions, not before. Because
we did this before checking other conditions, when we encounter TRY
and we want to record the current mismatching range, we already have
popped up the entry from EHPadStack, which we need to access to record
the range.

The baz call in the added test needs try-delegate because the previous
TRY marker placement for quux was placed before baz, because baz's
return value was stackified in RegStackify. If this wasn't stackified
this try-delegate is not strictly necessary, but at the moment it is not
easy to identify cases like this. I plan to transfer nounwind
attributes from the LLVM IR to prevent cases like this. The call in the
test does not have unwind attribute in order to test this bug, but in
many cases of this pattern the previous call has nounwind attribute.

llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp

LLVM_DEBUG(dbgs() << "- Call unwind mismatch: MBB = "
                       << "\nOriginal dest = caller  Current dest = "
                       << CurrentDest->getName() << "\n\n");
     RangeBegin = RangeEnd = nullptr; // Reset range pointers
   };
 
   for (auto &MBB : reverse(MF)) {
     bool SeenThrowableInstInBB = false;
     for (auto &MI : reverse(MBB)) {
-      if (MI.getOpcode() == WebAssembly::TRY)
-        EHPadStack.pop_back();
-      else if (WebAssembly::isCatch(MI.getOpcode()))
-        EHPadStack.push_back(MI.getParent());
       bool MayThrow = WebAssembly::mayThrow(MI);
 
       // If MBB has an EH pad successor and this is the last instruction that
       // may throw, this instruction unwinds to the EH pad and not to the
       // caller.
-      if (MBB.hasEHPadSuccessor() && MayThrow && !SeenThrowableInstInBB) {
+      if (MBB.hasEHPadSuccessor() && MayThrow && !SeenThrowableInstInBB)
         SeenThrowableInstInBB = true;
-        continue;
-      }
 
       // We wrap up the current range when we see a marker even if we haven't
       // finished a BB.
-      if (RangeEnd && WebAssembly::isMarker(MI.getOpcode())) {
+      else if (RangeEnd && WebAssembly::isMarker(MI.getOpcode()))
         RecordCallerMismatchRange(EHPadStack.back());
-        continue;
-      }
 
       // 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.
-      if (EHPadStack.empty() || !MayThrow)
-        continue;
+      else if (EHPadStack.empty() || !MayThrow) {
+      }
 
       // We found an instruction that unwinds to the caller but currently has an
       // incorrect unwind destination. Create a new range or increment the
       // currently existing range.
+      else {
         if (!RangeEnd)
           RangeBegin = RangeEnd = &MI;
         else
           RangeBegin = &MI;
       }
 
+      // Update EHPadStack.
+      if (MI.getOpcode() == WebAssembly::TRY)
+        EHPadStack.pop_back();
+      else if (WebAssembly::isCatch(MI.getOpcode()))
+        EHPadStack.push_back(MI.getParent());
+    }
+
     if (RangeEnd)
       RecordCallerMismatchRange(EHPadStack.back());
   }
 
   assert(EHPadStack.empty());
 
   // We don't have any unwind destination mismatches to resolve.
   if (UnwindDestToTryRanges.empty())

llvm/test/CodeGen/WebAssembly/cfg-stackify-eh.ll

for.inc:                                          ; preds = %for.body
   br label %for.cond
 
 ehcleanup:                                        ; preds = %if.then
   %0 = cleanuppad within none []
   %call3 = call %class.Object* @_ZN6ObjectD2Ev(%class.Object* %t) #4 [ "funclet"(token %0) ]
   cleanupret from %0 unwind to caller
 }
 
+; This crashed when updating EHPadStack within fixCallUniwindMismatch had a bug.
+; This should not crash and try-delegate has to be created around 'call @baz',
+; because the initial TRY placement for 'call @quux' was done before 'call @baz'
+; because 'call @baz''s return value is stackified.
+
+; CHECK-LABEL: test19
+; CHECK: try
+; CHECK:   try
+; CHECK:     call $[[RET:[0-9]+]]=, baz
+; CHECK:   delegate  1
+; CHECK:    call  quux, $[[RET]]
+; CHECK: catch_all
+; CHECK: end_try
+define void @test19() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
+entry:
+  %call = call i32 @baz()
+  invoke void @quux(i32 %call)
+          to label %invoke.cont unwind label %ehcleanup
+
+ehcleanup:                                        ; preds = %entry
+  %0 = cleanuppad within none []
+  cleanupret from %0 unwind to caller
+
+invoke.cont:                                      ; preds = %entry
+  unreachable
+}
+
 ; Check if the unwind destination mismatch stats are correct
-; NOSORT: 18 wasm-cfg-stackify    - Number of call unwind mismatches found
+; NOSORT: 19 wasm-cfg-stackify    - Number of call unwind mismatches found
 ; NOSORT:  3 wasm-cfg-stackify    - Number of catch unwind mismatches found
 
 declare void @foo()
 declare void @bar()
 declare i32 @baz()
 declare i32 @qux(i32)
 declare void @quux(i32)
 declare void @fun(i32)