[Windows EH] Fix the order of Nested try-catches in $tryMap$ table

[tentzen]

Summary

This bug is exposed by Test7 of ehthrow.cxx in MSVC EH suite where
a rethrow occurs in a try-catch inside a catch (i.e., a nested Catch
handlers). See the test code in
https://github.com/microsoft/compiler-tests/blob/master/eh/ehthrow.cxx#L346

When an object is rethrown in a Catch handler, the copy-ctor of this
object must be executed after the destructions of live objects, but
BEFORE the dtors of live objects in parent handlers.

Today Windows runtime (__CxxFrameHandler3 & 4) expects nested Catch handers
are stored in pre-order (outer first, inner next) in $tryMap$ table, so
that given a State, its Catch's beginning State can be properly
retrieved. The Catch beginning state (which is also the ending State) is
the State where rethrown object's copy-ctor must take place.

LLVM currently stores nested catch handlers in post-ordering because
it's the natural way to compute the highest State in Catch.
The fix is to simply store TryCatch handler in pre-order, but update
Catch's highest State after child Catches are all processed.

[tentzen]

fix a typo in test case

[tentzen]

Hi, Reid,
the existing test case is fixed. I think there is no need to add one more unit test for this patch.
thanks,

[majnemer]

llvm/lib/CodeGen/WinEHPrepare.cpp

269

CatchHIgh -> CatchHigh

[tentzen]

fixed a comment.

[tentzen]

thank you David, good eyes!
Is there any more concern?

[majnemer]

LGTM

[tentzen]

Per discussion in https://reviews.llvm.org/D79760 , x86 test case there failed.

It turned out that Windows 32-bit runtime expects post-ordering when looking for catch-handler, but sharing the code with 64-bit runtime when looking up EH State during DTor cleaning-up.
As a result, MSVC/X86 today fails on Test7 in ehthrow.cxx.

Incorrect Ctor/Dtor ordering is less significant than Catch handling. for now LLVM must be compatible with MSVC.
This change also apply to the other two targets on Windows; Arm32 & Arm64.

llvm/lib/CodeGen/WinEHPrepare.cpp

for (const BasicBlock *PredBlock : predecessors(BB))
       if ((PredBlock = getEHPadFromPredecessor(PredBlock,
                                                CatchSwitch->getParentPad())))
         calculateCXXStateNumbers(FuncInfo, PredBlock->getFirstNonPHI(),
                                  TryLow);
     int CatchLow = addUnwindMapEntry(FuncInfo, ParentState, nullptr);
 
     // catchpads are separate funclets in C++ EH due to the way rethrow works.
     int TryHigh = CatchLow - 1;
+
+    // MSVC FrameHandler3/4 on x64&Arm64 expect Catch Handlers in $tryMap$
+    //  stored in pre-order (outer first, inner next), not post-order
+    //  Add to map here.  Fix the CatchHigh after children are processed
+    const Module *Mod = BB->getParent()->getParent();
+    bool IsPreOrder = Triple(Mod->getTargetTriple()).isArch64Bit();
+    if (IsPreOrder)
+      addTryBlockMapEntry(FuncInfo, TryLow, TryHigh, CatchLow, Handlers);
+    unsigned TBMEIdx = FuncInfo.TryBlockMap.size() - 1;
+
     for (const auto *CatchPad : Handlers) {
       FuncInfo.FuncletBaseStateMap[CatchPad] = CatchLow;
       for (const User *U : CatchPad->users()) {
         const auto *UserI = cast<Instruction>(U);
         if (auto *InnerCatchSwitch = dyn_cast<CatchSwitchInst>(UserI)) {
           BasicBlock *UnwindDest = InnerCatchSwitch->getUnwindDest();
           if (!UnwindDest || UnwindDest == CatchSwitch->getUnwindDest())
             calculateCXXStateNumbers(FuncInfo, UserI, CatchLow);
         }
         if (auto *InnerCleanupPad = dyn_cast<CleanupPadInst>(UserI)) {
           BasicBlock *UnwindDest = getCleanupRetUnwindDest(InnerCleanupPad);
           // If a nested cleanup pad reports a null unwind destination and the
           // enclosing catch pad doesn't it must be post-dominated by an
           // unreachable instruction.
           if (!UnwindDest || UnwindDest == CatchSwitch->getUnwindDest())
             calculateCXXStateNumbers(FuncInfo, UserI, CatchLow);
         }
       }
     }
     int CatchHigh = FuncInfo.getLastStateNumber();
+    // Now child Catches are processed, update CatchHigh
+    if (IsPreOrder)
+      FuncInfo.TryBlockMap[TBMEIdx].CatchHigh = CatchHigh;
+    else // PostOrder
       addTryBlockMapEntry(FuncInfo, TryLow, TryHigh, CatchHigh, Handlers);
+
     LLVM_DEBUG(dbgs() << "TryLow[" << BB->getName() << "]: " << TryLow << '\n');
     LLVM_DEBUG(dbgs() << "TryHigh[" << BB->getName() << "]: " << TryHigh
                       << '\n');
     LLVM_DEBUG(dbgs() << "CatchHigh[" << BB->getName() << "]: " << CatchHigh
                       << '\n');
   } else {
     auto *CleanupPad = cast<CleanupPadInst>(FirstNonPHI);
 

llvm/test/CodeGen/AArch64/win-catchpad-nested-cxx.ll

+; RUN: llc -verify-machineinstrs -mtriple=aarch64-pc-windows-msvc < %s \
+; RUN:     | FileCheck --check-prefix=CHECK %s
+
+; Loosely based on IR for this C++ source code:
+;   void f(int p);
+;   void try_in_catch() {
+;     try {
+;       f(1);
+;     } catch (...) {
+;       try {
+;         f(2);
+;       } catch (...) {
+;         f(3);
+;       }
+;     }
+;   }
+
+declare void @f(i32 %p)
+declare i32 @__CxxFrameHandler3(...)
+
+define i32 @try_in_catch() personality i32 (...)* @__CxxFrameHandler3 {
+entry:
+  invoke void @f(i32 1)
+          to label %try.cont unwind label %catch.dispatch.1
+try.cont:
+  ret i32 0
+
+catch.dispatch.1:
+  %cs1 = catchswitch within none [label %handler1] unwind to caller
+handler1:
+  %h1 = catchpad within %cs1 [i8* null, i32 64, i8* null]
+  invoke void @f(i32 2) [ "funclet"(token %h1) ]
+          to label %catchret1 unwind label %catch.dispatch.2
+catchret1:
+  catchret from %h1 to label %try.cont
+
+catch.dispatch.2:
+  %cs2 = catchswitch within %h1 [label %handler2] unwind to caller
+handler2:
+  %h2 = catchpad within %cs2 [i8* null, i32 64, i8* null]
+  call void @f(i32 3)
+  catchret from %h2 to label %catchret1
+}
+
+; CHECK-LABEL: $cppxdata$try_in_catch:
+; CHECK-NEXT: .word   429065506
+; CHECK-NEXT: .word   4
+; CHECK-NEXT: .word   ($stateUnwindMap$try_in_catch)
+; CHECK-NEXT: .word   2
+; CHECK-NEXT: .word   ($tryMap$try_in_catch)
+; ip2state num + ptr
+; CHECK-NEXT: .word   7
+; CHECK-NEXT: .word   ($ip2state$try_in_catch)
+; unwindhelp offset
+; CHECK-NEXT: .word   -16
+; CHECK-NEXT: .word   0
+; EHFlags
+; CHECK-NEXT: .word   1
+
+; CHECK-LABEL: $tryMap$try_in_catch:
+; CHECK-NEXT: .word   0
+; CHECK-NEXT: .word   0
+; CHECK-NEXT: .word   3
+; CHECK-NEXT: .word   1
+; CHECK-NEXT: .word   ($handlerMap$0$try_in_catch)
+; CHECK-NEXT: .word   2
+; CHECK-NEXT: .word   2
+; CHECK-NEXT: .word   3
+; CHECK-NEXT: .word   1
+; CHECK-NEXT: .word   ($handlerMap$1$try_in_catch)
+
+; CHECK: $handlerMap$0$try_in_catch:
+; CHECK-NEXT:   .word   64
+; CHECK-NEXT:   .word   0
+; CHECK-NEXT:   .word   0
+; CHECK-NEXT:   .word   "?catch${{[0-9]+}}@?0?try_in_catch@4HA"
+; CHECK-NEXT:   .word   0
+
+; CHECK: $handlerMap$1$try_in_catch:
+; CHECK-NEXT:   .word   64
+; CHECK-NEXT:   .word   0
+; CHECK-NEXT:   .word   0
+; CHECK-NEXT:   .word   "?catch${{[0-9]+}}@?0?try_in_catch@4HA"
+; CHECK-NEXT:   .word   0
+
+; CHECK: $ip2state$try_in_catch:
+; CHECK-NEXT: .word   .Lfunc_begin0@IMGREL
+; CHECK-NEXT: .word   -1
+; CHECK-NEXT: .word   .Ltmp0@IMGREL
+; CHECK-NEXT: .word   0
+; CHECK-NEXT: .word   .Ltmp1@IMGREL
+; CHECK-NEXT: .word   -1
+; CHECK-NEXT: .word   "?catch$2@?0?try_in_catch@4HA"@IMGREL
+; CHECK-NEXT: .word   1
+; CHECK-NEXT: .word   .Ltmp2@IMGREL
+; CHECK-NEXT: .word   2
+; CHECK-NEXT: .word   .Ltmp3@IMGREL
+; CHECK-NEXT: .word   1
+; CHECK-NEXT: .word   "?catch$4@?0?try_in_catch@4HA"@IMGREL
+; CHECK-NEXT: .word   3

llvm/test/CodeGen/X86/win-catchpad-nested-cxx.ll

 ; X64-NEXT: .long   7
 ; X64-NEXT: .long   ($ip2state$try_in_catch)
 ; unwindhelp offset
 ; X64-NEXT: .long   40
 ; CHECK-NEXT: .long   0
 ; EHFlags
 ; CHECK-NEXT: .long   1
 
-; CHECK: $tryMap$try_in_catch:
-; CHECK-NEXT: .long   2
-; CHECK-NEXT: .long   2
-; CHECK-NEXT: .long   3
-; CHECK-NEXT: .long   1
-; CHECK-NEXT: .long   ($handlerMap$0$try_in_catch)
-; CHECK-NEXT: .long   0
-; CHECK-NEXT: .long   0
-; CHECK-NEXT: .long   3
-; CHECK-NEXT: .long   1
-; CHECK-NEXT: .long   ($handlerMap$1$try_in_catch)
+; X86-LABEL: $tryMap$try_in_catch:
+; X86-NEXT: .long   2
+; X86-NEXT: .long   2
+; X86-NEXT: .long   3
+; X86-NEXT: .long   1
+; X86-NEXT: .long   ($handlerMap$0$try_in_catch)
+; X86-NEXT: .long   0
+; X86-NEXT: .long   0
+; X86-NEXT: .long   3
+; X86-NEXT: .long   1
+; X86-NEXT: .long   ($handlerMap$1$try_in_catch)
+
+; X64-LABEL: $tryMap$try_in_catch:
+; X64-NEXT: .long   0
+; X64-NEXT: .long   0
+; X64-NEXT: .long   3
+; X64-NEXT: .long   1
+; X64-NEXT: .long   ($handlerMap$0$try_in_catch)
+; X64-NEXT: .long   2
+; X64-NEXT: .long   2
+; X64-NEXT: .long   3
+; X64-NEXT: .long   1
+; X64-NEXT: .long   ($handlerMap$1$try_in_catch)
 
 ; CHECK: $handlerMap$0$try_in_catch:
 ; CHECK-NEXT:   .long   64
 ; CHECK-NEXT:   .long   0
 ; CHECK-NEXT:   .long   0
 ; CHECK-NEXT:   .long   "?catch${{[0-9]+}}@?0?try_in_catch@4HA"
 ; X64-NEXT:   .long   56