[PATCH] [WinEH] Update C++ exception state numbering code

Andy Kaylor andrew.kaylor at intel.com
Wed May 6 16:22:18 PDT 2015


The main problem this change set is meant to address is that the EH states of catch handlers weren't being numbered correctly.

Remember the assertion that was removed earlier (CatchHigh > TryHigh).  It turns out that is important.


REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/AsmPrinter/Win64Exception.cpp:338
@@ -327,1 +337,3 @@
+      // FIXME: Should this be using FuncInfo.HandlerBaseState?
+      if (SawPotentiallyThrowing && LastEHState != -1) {
         FuncInfo.IPToStateList.push_back(std::make_pair(LastLabel, -1));
----------------
It isn't clear to me what this is doing.  I strongly suspect that the hard-coded -1 here is incorrect.

I added code which always adds an ip2state entry for the beginning of a function.  It seems that this entry is meant to handle the case where this state is needed in the parent function.  If this case is never hit, having the -1 state for the parent function may not be strictly necessary.  I added it to mimic the behavior of MSVC.

I suspect that this code will misbehave if we are in a catch handler.

================
Comment at: lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp:433
@@ -432,3 +453,2 @@
     createUnwindMapEntry(currentEHNumber(), nullptr);
-    ++NextState;
   }
----------------
I removed this line of code because we weren't actually using a new state value here.  The state number returned by currentEHNumber was previously claimed when the action at the top of the handler stack was pushed and the NextState value is incremented there.  Incrementing it here results in an EH state value that doesn't appear anywhere in our tables.

Honestly, I'm not entirely clear as to why an unwind entry is being created here.

Near as I can tell we can only get here when a cleanup handler has been popped from the handler stack, as might happen at the first call site after a non-try scope ended in the middle of a try block.  But wouldn't we have already created an unwind entry for that cleanup handler when it was pushed on the handler stack?

================
Comment at: lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp:481
@@ -450,1 +480,3 @@
+      FuncInfo.HandlerBaseState[F] = NextState;
+      ++NextState;
       calculateStateNumbers(*F);
----------------
Here I'm claiming a new EH state but not creating an unwind entry.  That may be the cause of the problem I saw with the MaxState value in the xdata table being larger than the size of the unwind map.

Do I need to create an unwind entry here?

http://reviews.llvm.org/D9512

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list