[X86] Don't give catch objects a displacement of zero

[majnemer]

Summary

We need to make sure that catch objects don't get assigned a
displacement of zero by sorting other objects after them.

Otherwise, we will confuse the runtime into thinking there is no catch
object to initialize.

N.B. This doesn't work if there are no other non-catch objects! It was
believed that we could use the UnwindHelp object but it is a fixed
object, not a normal stack object. :/

[rnk]

We discussed a more general solution of reclassifying all catch objects as fixed stack objects and completely taking over the frame layout of those objects. This solves a separate problem with catch objects and stack realignment.

lib/CodeGen/PrologEpilogInserter.cpp

if (MFI->getStackProtectorIndex() >= 0) {
     AssignProtectedObjSet(SmallArrayObjs, ProtectedObjs, MFI, StackGrowsDown,
                           Offset, MaxAlign, Skew);
     AssignProtectedObjSet(AddrOfObjs, ProtectedObjs, MFI, StackGrowsDown,
                           Offset, MaxAlign, Skew);
   }
 
   SmallVector<int, 8> ObjectsToAllocate;
 
-  int EHRegNodeFrameIndex = INT_MAX;
-  if (const WinEHFuncInfo *FuncInfo = Fn.getWinEHFuncInfo())
-    EHRegNodeFrameIndex = FuncInfo->EHRegNodeFrameIndex;
-
   // Then prepare to assign frame offsets to stack objects that are not used to
   // spill callee saved registers.
   for (unsigned i = 0, e = MFI->getObjectIndexEnd(); i != e; ++i) {
     if (MFI->isObjectPreAllocated(i) &&
         MFI->getUseLocalStackAllocationBlock())
       continue;
     if (i >= MinCSFrameIndex && i <= MaxCSFrameIndex)
       continue;
     if (RS && RS->isScavengingFrameIndex((int)i))
       continue;
     if (MFI->isDeadObjectIndex(i))
       continue;
     if (MFI->getStackProtectorIndex() == (int)i)
       continue;
-    if (EHRegNodeFrameIndex == (int)i)
-      continue;
     if (ProtectedObjs.count(i))
       continue;
 
     // Add the objects that we need to allocate to our working set.
     ObjectsToAllocate.push_back(i);
   }
 
-  // Allocate the EH registration node first if one is present.
-  if (EHRegNodeFrameIndex != INT_MAX)
-    AdjustStackOffset(MFI, EHRegNodeFrameIndex, StackGrowsDown, Offset,
-                      MaxAlign, Skew);
-
   // Give the targets a chance to order the objects the way they like it.
-  if (Fn.getTarget().getOptLevel() != CodeGenOpt::None &&
-      Fn.getTarget().Options.StackSymbolOrdering)
-    TFI.orderFrameObjects(Fn, ObjectsToAllocate);
-  
   // Now walk the objects and actually assign base offsets to them.
   for (auto &Object : ObjectsToAllocate)
     AdjustStackOffset(MFI, Object, StackGrowsDown, Offset, MaxAlign, Skew);
 
   // Make sure the special register scavenging spill slot is closest to the
   // stack pointer.
   if (RS && !EarlyScavengingSlots) {
     SmallVector<int, 2> SFIs;

lib/Target/X86/X86FrameLowering.h

public:
   /// Sets up EBP and optionally ESI based on the incoming EBP value.  Only
   /// needed for 32-bit. Used in funclet prologues and at catchret destinations.
   MachineBasicBlock::iterator
   restoreWin32EHStackPointers(MachineBasicBlock &MBB,
                               MachineBasicBlock::iterator MBBI, DebugLoc DL,
                               bool RestoreSP = false) const;
 
 private:
+  void optimizeFrameObjects(const MachineFunction &MF,
+                            SmallVectorImpl<int> &ObjectsToAllocate) const;
   uint64_t calculateMaxStackAlign(const MachineFunction &MF) const;
 
   /// Emit target stack probe as a call to a helper function
   MachineInstr *emitStackProbeCall(MachineFunction &MF, MachineBasicBlock &MBB,
                                    MachineBasicBlock::iterator MBBI,
                                    DebugLoc DL, bool InProlog) const;
 
   /// Emit target stack probe as an inline sequence.

lib/Target/X86/X86FrameLowering.cpp

struct X86FrameSortingComparator {
   }
 };
 } // namespace
 
 // Order the symbols in the local stack.
 // We want to place the local stack objects in some sort of sensible order.
 // The heuristic we use is to try and pack them according to static number
 // of uses and size of object in order to minimize code size.
-void X86FrameLowering::orderFrameObjects(
+void X86FrameLowering::optimizeFrameObjects(
     const MachineFunction &MF, SmallVectorImpl<int> &ObjectsToAllocate) const {
   const MachineFrameInfo *MFI = MF.getFrameInfo();
 
-  // Don't waste time if there's nothing to do.
-  if (ObjectsToAllocate.empty())
-    return;
-
   // Create an array of all MFI objects. We won't need all of these
   // objects, but we're going to create a full array of them to make
   // it easier to index into when we're counting "uses" down below.
   // We want to be able to easily/cheaply access an object by simply
   // indexing into it, instead of having to search for it every time.
   std::vector<X86FrameSortingObject> SortingObjects(MFI->getObjectIndexEnd());
 
   // Walk the objects we care about and mark them as such in our working
for (auto &Obj : SortingObjects) {
     ObjectsToAllocate[i++] = Obj.ObjectIndex;
   }
 
   // Flip it if we're accessing off of the FP.
   if (!TRI->needsStackRealignment(MF) && hasFP(MF))
     std::reverse(ObjectsToAllocate.begin(), ObjectsToAllocate.end());
 }
 
+void X86FrameLowering::orderFrameObjects(
+    const MachineFunction &MF, SmallVectorImpl<int> &ObjectsToAllocate) const {
+  // Don't waste time if there's nothing to do.
+  if (ObjectsToAllocate.empty())
+    return;
+
+  if (MF.getTarget().getOptLevel() != CodeGenOpt::None &&
+      MF.getTarget().Options.StackSymbolOrdering)
+    optimizeFrameObjects(MF, ObjectsToAllocate);
+
+  // We might need to re-order the objects to handle WinEH constraints.
+  const WinEHFuncInfo *FuncInfo = MF.getWinEHFuncInfo();
+  if (!FuncInfo)
+    return;
+
+  // Gather up all the catch objects' frame indicies in the function.
+  SmallSet<int, 2> CatchObjects;
+  for (const WinEHTryBlockMapEntry &TBME : FuncInfo->TryBlockMap)
+    for (const WinEHHandlerType &HT : TBME.HandlerArray)
+      if (HT.CatchObj.FrameIndex != INT_MAX)
+        CatchObjects.insert(HT.CatchObj.FrameIndex);
+
+  // No catch objects? Nothing to re-order.
+  if (CatchObjects.empty())
+    return;
+
+  // Make sure the EH registration node comes before the catch objects.
+  int EHRegNodeFrameIndex = FuncInfo->EHRegNodeFrameIndex;
+  if (EHRegNodeFrameIndex != INT_MAX) {
+    // Find the earliest catch object.
+    auto EarliestCatchObject = ObjectsToAllocate.end();
+    for (int &Object : ObjectsToAllocate) {
+      if (CatchObjects.count(Object) > 0) {
+        EarliestCatchObject = &Object;
+        break;
+      }
+    }
+    // Find the EH registration node.
+    auto EHNodeIter = find(ObjectsToAllocate, EHRegNodeFrameIndex);
+    if (EHNodeIter > EarliestCatchObject) {
+      // Move the EH registration node after the before the first catch object.
+      ObjectsToAllocate.erase(EHNodeIter);
+      ObjectsToAllocate.insert(EarliestCatchObject, EHRegNodeFrameIndex);
+    }
+  }
+
+  // Avoid having a catch object at the top of the stack relative to RSP at the
+  // end of the prologue.
+  const MachineFrameInfo *MFI = MF.getFrameInfo();
+  if (STI.isTargetWin64() && MFI->hasVarSizedObjects()) {
+    // Find the last catch object and normal object.
+    auto LastCatchObject = ObjectsToAllocate.end();
+    auto LastNonCatchObject = ObjectsToAllocate.end();
+    for (int &Object : reverse(ObjectsToAllocate)) {
+      // Skip dynamic stack allocations.
+      if (MFI->isVariableSizedObjectIndex(Object))
+        continue;
+      if (CatchObjects.count(Object) != 0) {
+        if (LastCatchObject == ObjectsToAllocate.end())
+          LastCatchObject = &Object;
+      } else {
+        if (LastNonCatchObject == ObjectsToAllocate.end())
+          LastNonCatchObject = &Object;
+      }
+
+      if (LastCatchObject != ObjectsToAllocate.end() &&
+          LastNonCatchObject != ObjectsToAllocate.end())
+        break;
+    }
+
+    assert(LastCatchObject != ObjectsToAllocate.end() && "no catch object!");
+
+    // Swap the last catch object with the last normal object if the catch
+    // object came after it.
+    if (LastNonCatchObject != ObjectsToAllocate.end()) {
+      if (LastCatchObject > LastNonCatchObject)
+        std::swap(*LastCatchObject, *LastNonCatchObject);
+    } else {
+    }
+  }
+}
+
 
 unsigned X86FrameLowering::getWinEHParentFrameOffset(const MachineFunction &MF) const {
   // RDX, the parent frame pointer, is homed into 16(%rsp) in the prologue.
   unsigned Offset = 16;
   // RBP is immediately pushed.
   Offset += SlotSize;
   // All callee-saved registers are then pushed.
   Offset += MF.getInfo<X86MachineFunctionInfo>()->getCalleeSavedFrameSize();

test/CodeGen/X86/catchpad-dynamic-alloca.ll

+; RUN: llc < %s | FileCheck %s
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+declare void @rt_init()
+
+declare i32 @__CxxFrameHandler3(...)
+
+define void @test1(void ()* %fp, i64 %n) personality i32 (...)* @__CxxFrameHandler3 {
+entry:
+  %t.i = alloca i8*
+  %t.ii = alloca i8
+  %.alloca8 = alloca i8, i64 %n
+  store volatile i8 0, i8* %t.ii
+  store volatile i8 0, i8* %.alloca8
+  invoke void @rt_init()
+          to label %try.cont unwind label %catch.switch
+
+try.cont:
+  invoke void %fp()
+          to label %exit unwind label %catch.switch
+
+exit:
+  ret void
+
+catch.pad:
+  %cp = catchpad within %cs [i8* null, i32 0, i8** %t.i]
+  catchret from %cp to label %exit
+
+catch.switch:
+  %cs = catchswitch within none [label %catch.pad] unwind to caller
+}
+
+; CHECK-LABEL: $handlerMap$0$test1:
+; CHECK:      .long   0
+; CHECK-NEXT: .long   0
+; CHECK-NEXT: .long   8
+
+; FIXME: define void @test2(void ()* %fp, i64 %n) personality i32 (...)* @__CxxFrameHandler3 {
+; FIXME: entry:
+; FIXME:   %t.i = alloca i128
+; FIXME:   %.alloca8 = alloca i8, i64 %n
+; FIXME:   store volatile i8 0, i8* %.alloca8
+; FIXME:   invoke void @rt_init()
+; FIXME:           to label %try.cont unwind label %catch.switch
+; FIXME:
+; FIXME: try.cont:
+; FIXME:   invoke void %fp()
+; FIXME:           to label %exit unwind label %catch.switch
+; FIXME:
+; FIXME: exit:
+; FIXME:   ret void
+; FIXME:
+; FIXME: catch.pad:
+; FIXME:   %cp = catchpad within %cs [i8* null, i32 0, i128* %t.i]
+; FIXME:   catchret from %cp to label %exit
+; FIXME:
+; FIXME: catch.switch:
+; FIXME:   %cs = catchswitch within none [label %catch.pad] unwind to caller
+; FIXME: }

test/CodeGen/X86/cleanuppad-inalloca.ll

 ;   ESP must be restored via EBP due to "dynamic" alloca.
 ; CHECK: leal -{{[0-9]+}}(%ebp), %esp
 ; CHECK: popl %ebp
 ; CHECK: retl
 
 ; CHECK: "?dtor$2@?0?passes_two@4HA":
 ; CHECK: pushl %ebp
 ; CHECK: subl $8, %esp
-; CHECK: addl $12, %ebp
+; CHECK: addl $16, %ebp
 ; CHECK: {{movl|leal}} -{{[0-9]+}}(%ebp), %ecx
 ; CHECK: calll "??1A@@QAE@XZ"
 ; CHECK: addl $8, %esp
 ; CHECK: retl
 
 declare void @takes_two(<{ %struct.A, %struct.A }>* inalloca) #0
 
 declare x86_thiscallcc %struct.A* @"\01??0A@@QAE@XZ"(%struct.A* returned) #0
 
 declare i32 @__CxxFrameHandler3(...)
 
 declare x86_thiscallcc void @"\01??1A@@QAE@XZ"(%struct.A*) #0
 
 attributes #0 = { "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }