X86: In debug build, emit CFI data in X86FrameLowering and X86CallFrameOptimization

[alexinbeijing]

Summary

Both X86FrameLowering and X86CallFrameOptimization emit PUSHes/POPs and ADDs/SUBs to the stack pointer. If each of these instructions is not accompanied by an appropriate CFI instruction, then when single-stepping through the resulting binaries in a debugger, backtraces will be unavailable at many points. Actually, it's worse than that; sometimes the debugger will show a wildly incorrect backtrace.

There are still places where more work is needed on CFI generation, but this seems to cover the most important ones. At least, I can single-step through a test program and get a correct backtrace at every point.

Note that some optimizations which merge consecutive instructions may need to become smarter to perform merging when a CFI instruction is in the middle.

I'm not happy with the duplication of the (rather verbose) code to check whether generation of Dwarf data was requested or not; it seems that this should just be done once per build, or once per compilation unit, and the results used everywhere else.

[alexinbeijing]

NOTE: I have found a couple of bugs here. I will post an updated revision soon.

[alexinbeijing]

A few tweaks and bug fixes here.

[alexinbeijing]

Note that this breaks a couple regression tests which depend on the exact name of emitted temp labels. (It emits more temp labels of its own.)

[asl]

Please update all these tests to pass, surely.

[rnk]

Thanks for looking into this, I agree this is important functionality. I also agree this code is fairly duplicated and needs cleanup.

I have a feeling that emitting asynch unwind tables by default is going to break some tools that consume our unwind tables. I believe ld64 processes our .eh_frame data to produce it's own compact CFI format, and it may choke on these kinds of cfa updates. We're going to need a way to turn off these fine-grained updates if something breaks. The simplest way to flag this would be to add a bool to TargetOptions and default it to true. If it causes someone problems, we can flip the default (or tweak it based on platform) instead of reverting the patch. Ultimately, we can wire this up to -fasynchronous-unwind-tables in clang, and (hopefully) keep that on by default.

This also needs much more comprehensive testing, new tests in addition to the ones already present.

lib/Target/X86/X86FrameLowering.cpp

226–231

I won't ask you to refactor this goo, I'll see if I can do it myself at some point.

258–261

Can you maybe wrap this all up in a static helper like BuildAdjustCFAMI? Anything would be good.

285–286

This doesn't appear to handle the sub case?

lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp

 
 void AsmPrinter::emitCFIInstruction(const MCCFIInstruction &Inst) const {
   switch (Inst.getOperation()) {
   default:
     llvm_unreachable("Unexpected instruction");
   case MCCFIInstruction::OpDefCfaOffset:
     OutStreamer->EmitCFIDefCfaOffset(Inst.getOffset());
     break;
+  case MCCFIInstruction::OpAdjustCfaOffset:
+    OutStreamer->EmitCFIAdjustCfaOffset(Inst.getOffset());
+    break;
   case MCCFIInstruction::OpDefCfa:
     OutStreamer->EmitCFIDefCfa(Inst.getRegister(), Inst.getOffset());
     break;
   case MCCFIInstruction::OpDefCfaRegister:
     OutStreamer->EmitCFIDefCfaRegister(Inst.getRegister());
     break;
   case MCCFIInstruction::OpOffset:
     OutStreamer->EmitCFIOffset(Inst.getRegister(), Inst.getOffset());

lib/Target/X86/X86CallFrameOptimization.cpp

 #include "X86.h"
 #include "X86InstrInfo.h"
 #include "X86Subtarget.h"
 #include "X86MachineFunctionInfo.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/IR/Function.h"
+#include "llvm/MC/MCAsmInfo.h"
+#include "llvm/MC/MCDwarf.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetInstrInfo.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "x86-cf-opt"
 
if (MOV->getOpcode() == X86::MOV32mi) {
         DefMov->eraseFromParent();
       } else {
         BuildMI(MBB, Context.Call, DL, TII->get(X86::PUSH32r))
             .addReg(Reg)
             .getInstr();
       }
     }
 
+    const Function *Fn = MF.getFunction();
+    MachineModuleInfo &MMI = MF.getMMI();
+    bool IsWinEH = MF.getTarget().getMCAsmInfo()->usesWindowsCFI();
+    bool NeedsDwarfCFI =
+        !IsWinEH && (MMI.hasDebugInfo() || Fn->needsUnwindTableEntry());
+    if (NeedsDwarfCFI) {
+      unsigned CFIIndex = MMI.addFrameInst(
+          MCCFIInstruction::createAdjustCfaOffset(nullptr, 4));
+      BuildMI(MBB, Context.Call, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
+          .addCFIIndex(CFIIndex);
+    }
+
     MBB.erase(MOV);
   }
 
   // The stack-pointer copy is no longer used in the call sequences.
   // There should not be any other users, but we can't commit to that, so:
   if (MRI->use_empty(Context.SPCopy->getOperand(0).getReg()))
     Context.SPCopy->eraseFromParent();
 

lib/Target/X86/X86FrameLowering.cpp

void emitSPUpdate(MachineBasicBlock &MBB, MachineBasicBlock::iterator &MBBI,
   else
     Opc = isSub
       ? getSUBriOpcode(Is64BitStackPtr, Offset)
       : getADDriOpcode(Is64BitStackPtr, Offset);
 
   uint64_t Chunk = (1LL << 31) - 1;
   DebugLoc DL = MBB.findDebugLoc(MBBI);
 
+  MachineFunction &MF = *MBB.getParent();
+  const Function *Fn = MF.getFunction();
+  MachineModuleInfo &MMI = MF.getMMI();
+  bool IsWinEH = MF.getTarget().getMCAsmInfo()->usesWindowsCFI();
+  bool NeedsDwarfCFI =
+      !IsWinEH && (MMI.hasDebugInfo() || Fn->needsUnwindTableEntry());
+
   while (Offset) {
     if (Offset > Chunk) {
       // Rather than emit a long series of instructions for large offsets,
       // load the offset into a register and do one sub/add
       unsigned Reg = 0;
 
       if (isSub && !isEAXLiveIn(*MBB.getParent()))
         Reg = (unsigned)(Is64BitTarget ? X86::RAX : X86::EAX);
       else
         Reg = findDeadCallerSavedReg(MBB, MBBI, TRI, Is64BitTarget);
 
       if (Reg) {
         Opc = Is64BitTarget ? X86::MOV64ri : X86::MOV32ri;
         BuildMI(MBB, MBBI, DL, TII.get(Opc), Reg)
           .addImm(Offset);
         Opc = isSub
           ? getSUBrrOpcode(Is64BitTarget)
           : getADDrrOpcode(Is64BitTarget);
         MachineInstr *MI = BuildMI(MBB, MBBI, DL, TII.get(Opc), StackPtr)
           .addReg(StackPtr)
           .addReg(Reg);
         MI->getOperand(3).setIsDead(); // The EFLAGS implicit def is dead.
         Offset = 0;
+
+        if (NeedsDwarfCFI) {
+          unsigned CFIIndex = MMI.addFrameInst(
+              MCCFIInstruction::createAdjustCfaOffset(nullptr, -Offset));
+          BuildMI(MBB, MBBI, DL, TII.get(TargetOpcode::CFI_INSTRUCTION))
+              .addCFIIndex(CFIIndex);
+        }
+
         continue;
       }
     }
 
     uint64_t ThisVal = std::min(Offset, Chunk);
     if (ThisVal == (Is64BitTarget ? 8 : 4)) {
       // Use push / pop instead.
       unsigned Reg = isSub
         ? (unsigned)(Is64BitTarget ? X86::RAX : X86::EAX)
         : findDeadCallerSavedReg(MBB, MBBI, TRI, Is64BitTarget);
       if (Reg) {
         Opc = isSub
           ? (Is64BitTarget ? X86::PUSH64r : X86::PUSH32r)
           : (Is64BitTarget ? X86::POP64r  : X86::POP32r);
         MachineInstr *MI = BuildMI(MBB, MBBI, DL, TII.get(Opc))
           .addReg(Reg, getDefRegState(!isSub) | getUndefRegState(isSub));
         if (isSub)
           MI->setFlag(MachineInstr::FrameSetup);
         Offset -= ThisVal;
+
+        if (NeedsDwarfCFI) {
+          unsigned CFIIndex = MMI.addFrameInst(
+              MCCFIInstruction::createAdjustCfaOffset(nullptr, -ThisVal));
+          BuildMI(MBB, MBBI, DL, TII.get(TargetOpcode::CFI_INSTRUCTION))
+              .addCFIIndex(CFIIndex)->setFlag(MachineInstr::FrameSetup);
+        }
+
         continue;
       }
     }
 
     MachineInstr *MI = nullptr;
 
     if (UseLEA) {
       MI =  addRegOffset(BuildMI(MBB, MBBI, DL, TII.get(Opc), StackPtr),
                           StackPtr, false, isSub ? -ThisVal : ThisVal);
     } else {
       MI = BuildMI(MBB, MBBI, DL, TII.get(Opc), StackPtr)
             .addReg(StackPtr)
             .addImm(ThisVal);
       MI->getOperand(3).setIsDead(); // The EFLAGS implicit def is dead.
+
+      if (NeedsDwarfCFI) {
+        unsigned CFIIndex = MMI.addFrameInst(
+            MCCFIInstruction::createAdjustCfaOffset(nullptr, isSub ? ThisVal : -ThisVal));
+        MachineInstr *CFIAdjust = BuildMI(MBB, MBBI, DL,
+          TII.get(TargetOpcode::CFI_INSTRUCTION)).addCFIIndex(CFIIndex);
+        if (isSub)
+          CFIAdjust->setFlag(MachineInstr::FrameSetup);
+      }
     }
 
     if (isSub)
       MI->setFlag(MachineInstr::FrameSetup);
 
     Offset -= ThisVal;
   }
 }
void X86FrameLowering::emitEpilogue(MachineFunction &MF,
   uint64_t SEHStackAllocAmt = NumBytes;
 
   // Skip the callee-saved pop instructions.
   while (MBBI != MBB.begin()) {
     MachineBasicBlock::iterator PI = std::prev(MBBI);
     unsigned Opc = PI->getOpcode();
 
     if (Opc != X86::POP32r && Opc != X86::POP64r && Opc != X86::DBG_VALUE &&
-        !PI->isTerminator())
+        Opc != X86::CFI_INSTRUCTION && !PI->isTerminator())
       break;
 
     --MBBI;
   }
   MachineBasicBlock::iterator FirstCSPop = MBBI;
 
   DL = MBBI->getDebugLoc();
 
if (X86::GR64RegClass.contains(Reg) ||
       continue;
 
     const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(Reg);
     TII.loadRegFromStackSlot(MBB, MI, Reg, CSI[i].getFrameIdx(), RC, TRI);
   }
 
   // POP GPRs.
   unsigned Opc = STI.is64Bit() ? X86::POP64r : X86::POP32r;
+  unsigned CFIAdjustment = STI.is64Bit() ? -8 : -4;
   for (unsigned i = 0, e = CSI.size(); i != e; ++i) {
     unsigned Reg = CSI[i].getReg();
     if (!X86::GR64RegClass.contains(Reg) &&
         !X86::GR32RegClass.contains(Reg))
       continue;
 
     BuildMI(MBB, MI, DL, TII.get(Opc), Reg);
+
+    const Function *Fn = MF.getFunction();
+    MachineModuleInfo &MMI = MF.getMMI();
+    bool IsWinEH = MF.getTarget().getMCAsmInfo()->usesWindowsCFI();
+    bool NeedsDwarfCFI =
+        !IsWinEH && (MMI.hasDebugInfo() || Fn->needsUnwindTableEntry());
+    if (NeedsDwarfCFI) {
+      unsigned CFIIndex = MF.getMMI().addFrameInst(
+          MCCFIInstruction::createAdjustCfaOffset(nullptr, CFIAdjustment));
+      BuildMI(MBB, MI, DL, TII.get(TargetOpcode::CFI_INSTRUCTION))
+          .addCFIIndex(CFIIndex);
+    }
   }
   return true;
 }
 
 void
 X86FrameLowering::processFunctionBeforeCalleeSavedScan(MachineFunction &MF,
                                                        RegScavenger *RS) const {
   MachineFrameInfo *MFI = MF.getFrameInfo();
eliminateCallFramePseudoInstr(MachineFunction &MF, MachineBasicBlock &MBB,
   unsigned Opcode = I->getOpcode();
   bool isDestroy = Opcode == TII.getCallFrameDestroyOpcode();
   bool IsLP64 = STI.isTarget64BitLP64();
   DebugLoc DL = I->getDebugLoc();
   uint64_t Amount = !reserveCallFrame ? I->getOperand(0).getImm() : 0;
   uint64_t InternalAmt = (isDestroy || Amount) ? I->getOperand(1).getImm() : 0;
   I = MBB.erase(I);
 
+  const Function *Fn = MF.getFunction();
+  MachineModuleInfo &MMI = MF.getMMI();
+  bool IsWinEH = MF.getTarget().getMCAsmInfo()->usesWindowsCFI();
+  bool NeedsDwarfCFI =
+      !IsWinEH && (MMI.hasDebugInfo() || Fn->needsUnwindTableEntry());
+  uint64_t CFIAdjustment = 0;
+
   if (!reserveCallFrame) {
     // If the stack pointer can be changed after prologue, turn the
     // adjcallstackup instruction into a 'sub ESP, <amt>' and the
     // adjcallstackdown instruction into 'add ESP, <amt>'
     if (Amount == 0)
       return;
 
     // We need to keep the stack aligned properly.  To do this, we round the
     // amount of space needed for the outgoing arguments up to the next
     // alignment boundary.
     unsigned StackAlign = getStackAlignment();
     Amount = RoundUpToAlignment(Amount, StackAlign);
 
     MachineInstr *New = nullptr;
 
     // Factor out the amount that gets handled inside the sequence
     // (Pushes of argument for frame setup, callee pops for frame destroy)
     Amount -= InternalAmt;
 
     if (Amount) {
       if (Opcode == TII.getCallFrameSetupOpcode()) {
         New = BuildMI(MF, DL, TII.get(getSUBriOpcode(IsLP64, Amount)), StackPtr)
           .addReg(StackPtr).addImm(Amount);
+        CFIAdjustment = Amount;
       } else {
         assert(Opcode == TII.getCallFrameDestroyOpcode());
 
         unsigned Opc = getADDriOpcode(IsLP64, Amount);
         New = BuildMI(MF, DL, TII.get(Opc), StackPtr)
           .addReg(StackPtr).addImm(Amount);
+        CFIAdjustment = -Amount;
       }
     }
 
     if (New) {
       // The EFLAGS implicit def is dead.
       New->getOperand(3).setIsDead();
 
       // Replace the pseudo instruction with a new instruction.
       MBB.insert(I, New);
+
+      if (NeedsDwarfCFI) {
+        unsigned CFIIndex = MMI.addFrameInst(
+            MCCFIInstruction::createAdjustCfaOffset(nullptr, CFIAdjustment));
+        BuildMI(MBB, I, DL, TII.get(TargetOpcode::CFI_INSTRUCTION))
+            .addCFIIndex(CFIIndex);
+      }
     }
 
     return;
   }
 
   if (Opcode == TII.getCallFrameDestroyOpcode() && InternalAmt) {
     // If we are performing frame pointer elimination and if the callee pops
     // something off the stack pointer, add it back.  We do this until we have