[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

       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 expect Catch Handlers in $tryMap$ are
+    //  stored in pre-order (outer first, inner next), not post-order
+    //  Add to map here.  Fix the CatchHIgh after children are processed
+    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();
-    addTryBlockMapEntry(FuncInfo, TryLow, TryHigh, CatchHigh, Handlers);
+    // Now child Catches are processed, update CatchHigh
+    FuncInfo.TryBlockMap[TBMEIdx].CatchHigh = CatchHigh;
+
     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/X86/win-catchpad-nested-cxx.ll

 ; 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)
+; 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: $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