[X86] Correct dwarf unwind information in function epilogue

CFI instructions that set appropriate cfa offset and cfa register are now
inserted in emitEpilogue() in X86FrameLowering.

Majority of the changes in this patch:

1. Ensure that CFI instructions do not affect code generation.
2. Enable maintaining correct information about cfa offset and cfa register
in a function when basic blocks are reordered, merged, split, duplicated.

These changes are target independent and described below.

Changed CFI instructions so that they:

1. are duplicable
2. are not counted as instructions when tail duplicating or tail merging
3. can be compared as equal

Add information to each MachineBasicBlock about cfa offset and cfa register
that are valid at its entry and exit (incoming and outgoing CFI info). Add
support for updating this information when basic blocks are merged, split,
duplicated, created. Add a verification pass (CFIInfoVerifier) that checks
that outgoing cfa offset and register of predecessor blocks match incoming
values of their successors.

Incoming and outgoing CFI information is used by a late pass
(CFIInstrInserter) that corrects CFA calculation rule for a basic block if
needed. That means that additional CFI instructions get inserted at basic
block beginning to correct the rule for calculating CFA. Having CFI
instructions in function epilogue can cause incorrect CFA calculation rule
for some basic blocks. This can happen if, due to basic block reordering,
or the existence of multiple epilogue blocks, some of the blocks have wrong
cfa offset and register values set by the epilogue block above them.

Patch by Violeta Vukobrat.

Differential Revision: https://reviews.llvm.org/D18046

llvm-svn: 306529

llvm/include/llvm/CodeGen/MachineBasicBlock.h

@@ -23,6 +23,7 @@
 #include "llvm/CodeGen/MachineInstrBundleIterator.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/MC/LaneBitmask.h"
+#include "llvm/MC/MCDwarf.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/Support/BranchProbability.h"
 #include <cassert>
@@ -757,6 +758,52 @@ class MachineBasicBlock
   /// unless you know what you're doing, because it doesn't update Pred's
   /// successors list. Use Pred->removeSuccessor instead.
   void removePredecessor(MachineBasicBlock *Pred);
+
+  // Value of cfa offset valid at basic block entry.
+  int IncomingCFAOffset = -1;
+  // Value of cfa offset valid at basic block exit.
+  int OutgoingCFAOffset = -1;
+  // Value of cfa register valid at basic block entry.
+  unsigned IncomingCFARegister = 0;
+  // Value of cfa register valid at basic block exit.
+  unsigned OutgoingCFARegister = 0;
+  // If a block contains a def_cfa_offset or def_cfa directive.
+  bool DefOffset = false;
+  // If a block contains a def_cfa_register or def_cfa directive.
+  bool DefRegister = false;
+
+ public:
+  int getIncomingCFAOffset() { return IncomingCFAOffset; }
+  void setIncomingCFAOffset(int Offset) { IncomingCFAOffset = Offset; }
+  int getOutgoingCFAOffset() { return OutgoingCFAOffset; }
+  void setOutgoingCFAOffset(int Offset) { OutgoingCFAOffset = Offset; }
+  unsigned getIncomingCFARegister() { return IncomingCFARegister; }
+  void setIncomingCFARegister(unsigned Register) {
+    IncomingCFARegister = Register;
+  }
+  unsigned getOutgoingCFARegister() { return OutgoingCFARegister; }
+  void setOutgoingCFARegister(unsigned Register) {
+    OutgoingCFARegister = Register;
+  }
+
+  bool hasDefOffset() { return DefOffset; }
+  bool hasDefRegister() { return DefRegister; }
+  void setDefOffset(bool SetsOffset) { DefOffset = SetsOffset; }
+  void setDefRegister(bool SetsRegister) { DefRegister = SetsRegister; }
+
+  // Update the outgoing cfa offset and register for this block based on the CFI
+  // instruction inserted at Pos.
+  void updateCFIInfo(MachineBasicBlock::iterator Pos);
+  // Update the cfa offset and register values for all successors of this block.
+  void updateCFIInfoSucc();
+  // Recalculate outgoing cfa offset and register. Use existing incoming offset
+  // and register values if UseExistingIncoming is set to true. If it is false,
+  // use new values passed as arguments.
+  void recalculateCFIInfo(bool UseExistingIncoming, int NewIncomingOffset = -1,
+                          unsigned NewIncomingRegister = 0);
+  // Update outgoing cfa offset and register of the block after it is merged
+  // with MBB.
+  void mergeCFIInfo(MachineBasicBlock *MBB);
 };
 
 raw_ostream& operator<<(raw_ostream &OS, const MachineBasicBlock &MBB);

llvm/include/llvm/CodeGen/MachineInstr.h

@@ -789,6 +789,7 @@ class MachineInstr
       && getOperand(1).isImm();
   }
 
+  bool isDirective() const { return isDebugValue() || isCFIInstruction(); }
   bool isPHI() const { return getOpcode() == TargetOpcode::PHI; }
   bool isKill() const { return getOpcode() == TargetOpcode::KILL; }
   bool isImplicitDef() const { return getOpcode()==TargetOpcode::IMPLICIT_DEF; }

llvm/include/llvm/CodeGen/Passes.h

@@ -420,6 +420,14 @@ namespace llvm {
   /// shuffles.
   FunctionPass *createExpandReductionsPass();
 
+  /// This pass verifies that outgoing cfa offset and register of predecessor
+  /// blocks match incoming cfa offset and register of their successors.
+  FunctionPass *createCFIInfoVerifier();
+
+  /// This pass inserts required CFI instruction at basic block beginning to
+  /// correct the CFA calculation rule for that block if necessary.
+  FunctionPass *createCFIInstrInserter();
+
 } // End llvm namespace
 
 #endif

llvm/include/llvm/InitializePasses.h

@@ -87,6 +87,8 @@ void initializeCFGOnlyViewerLegacyPassPass(PassRegistry&);
 void initializeCFGPrinterLegacyPassPass(PassRegistry&);
 void initializeCFGSimplifyPassPass(PassRegistry&);
 void initializeCFGViewerLegacyPassPass(PassRegistry&);
+void initializeCFIInfoVerifierPass(PassRegistry&);
+void initializeCFIInstrInserterPass(PassRegistry&);
 void initializeCFLAndersAAWrapperPassPass(PassRegistry&);
 void initializeCFLSteensAAWrapperPassPass(PassRegistry&);
 void initializeCallGraphDOTPrinterPass(PassRegistry&);

llvm/include/llvm/Target/Target.td

@@ -824,7 +824,7 @@ def CFI_INSTRUCTION : Instruction {
   let InOperandList = (ins i32imm:$id);
   let AsmString = "";
   let hasCtrlDep = 1;
-  let isNotDuplicable = 1;
+  let isNotDuplicable = 0;
 }
 def EH_LABEL : Instruction {
   let OutOperandList = (outs);

llvm/include/llvm/Target/TargetFrameLowering.h

@@ -339,6 +339,19 @@ class TargetFrameLowering {
           return false;
     return true;
   }
+
+  // Set initial incoming and outgoing cfa offset and register values for basic
+  // blocks. Initial values are the ones valid at the beginning of the function
+  // (before any stack operations). Incoming and outgoing cfa offset and
+  // register values are used to keep track of offset and register that are
+  // valid at basic block entry and exit. This information is used by a late
+  // pass that corrects the CFA calculation rule for a basic block if needed.
+  // Having CFI instructions in function epilogue can cause incorrect CFA
+  // calculation rule for some basic blocks. This can happen if, due to basic
+  // block reordering, or the existence of multiple epilogue blocks, some of the
+  // blocks have wrong cfa offset and register values set by the epilogue block
+  // above them.
+  virtual void initializeCFIInfo(MachineFunction & MF) const {}
 };
 
 } // End llvm namespace

llvm/lib/CodeGen/BranchFolding.cpp

@@ -304,9 +304,9 @@ static unsigned ComputeCommonTailLength(MachineBasicBlock *MBB1,
   while (I1 != MBB1->begin() && I2 != MBB2->begin()) {
     --I1; --I2;
     // Skip debugging pseudos; necessary to avoid changing the code.
-    while (I1->isDebugValue()) {
+    while (I1->isDirective()) {
       if (I1==MBB1->begin()) {
-        while (I2->isDebugValue()) {
+        while (I2->isDirective()) {
           if (I2==MBB2->begin())
             // I1==DBG at begin; I2==DBG at begin
             return TailLen;
@@ -319,7 +319,7 @@ static unsigned ComputeCommonTailLength(MachineBasicBlock *MBB1,
       --I1;
     }
     // I1==first (untested) non-DBG preceding known match
-    while (I2->isDebugValue()) {
+    while (I2->isDirective()) {
       if (I2==MBB2->begin()) {
         ++I1;
         // I1==non-DBG, or first of DBGs not at begin; I2==DBG at begin
@@ -362,6 +362,35 @@ static unsigned ComputeCommonTailLength(MachineBasicBlock *MBB1,
     }
     ++I1;
   }
+
+  // Ensure that I1 and I2 do not point to a CFI_INSTRUCTION. This can happen if
+  // I1 and I2 are non-identical when compared and then one or both of them ends
+  // up pointing to a CFI instruction after being incremented. For example:
+  /*
+    BB1:
+    ...
+    INSTRUCTION_A
+    ADD32ri8  <- last common instruction
+    ...
+    BB2:
+    ...
+    INSTRUCTION_B
+    CFI_INSTRUCTION
+    ADD32ri8  <- last common instruction
+    ...
+  */
+  // When INSTRUCTION_A and INSTRUCTION_B are compared as not equal, after
+  // incrementing the iterators, I1 will point to ADD, however I2 will point to
+  // the CFI instruction. Later on, this leads to BB2 being 'hacked off' at the
+  // wrong place (in ReplaceTailWithBranchTo()) which results in losing this CFI
+  // instruction.
+  while (I1 != MBB1->end() && I1->isCFIInstruction()) {
+    ++I1;
+  }
+
+  while (I2 != MBB2->end() && I2->isCFIInstruction()) {
+    ++I2;
+  }
   return TailLen;
 }
 
@@ -417,6 +446,14 @@ MachineBasicBlock *BranchFolder::SplitMBBAt(MachineBasicBlock &CurMBB,
     FuncletMembership[NewMBB] = n;
   }
 
+  // Recalculate CFI info for CurMBB. Use existing incoming cfa offset and
+  // register.
+  CurMBB.recalculateCFIInfo(true);
+  // Recalculate CFI info for NewMBB. Use CurMBB's outgoing cfa offset and
+  // register as NewMBB's incoming.
+  NewMBB->recalculateCFIInfo(false, CurMBB.getOutgoingCFAOffset(),
+                             CurMBB.getOutgoingCFARegister());
+
   return NewMBB;
 }
 
@@ -426,7 +463,7 @@ static unsigned EstimateRuntime(MachineBasicBlock::iterator I,
                                 MachineBasicBlock::iterator E) {
   unsigned Time = 0;
   for (; I != E; ++I) {
-    if (I->isDebugValue())
+    if (I->isDirective())
       continue;
     if (I->isCall())
       Time += 10;
@@ -780,7 +817,7 @@ void BranchFolder::MergeCommonTailDebugLocs(unsigned commonTailIndex) {
   }
 
   for (auto &MI : *MBB) {
-    if (MI.isDebugValue())
+    if (MI.isDirective())
       continue;
     DebugLoc DL = MI.getDebugLoc();
     for (unsigned int i = 0 ; i < NextCommonInsts.size() ; i++) {
@@ -790,7 +827,7 @@ void BranchFolder::MergeCommonTailDebugLocs(unsigned commonTailIndex) {
       auto &Pos = NextCommonInsts[i];
       assert(Pos != SameTails[i].getBlock()->end() &&
           "Reached BB end within common tail");
-      while (Pos->isDebugValue()) {
+      while (Pos->isDirective()) {
         ++Pos;
         assert(Pos != SameTails[i].getBlock()->end() &&
             "Reached BB end within common tail");
@@ -823,12 +860,12 @@ mergeOperations(MachineBasicBlock::iterator MBBIStartPos,
     assert(MBBI != MBBIE && "Reached BB end within common tail length!");
     (void)MBBIE;
 
-    if (MBBI->isDebugValue()) {
+    if (MBBI->isDirective()) {
       ++MBBI;
       continue;
     }
 
-    while ((MBBICommon != MBBIECommon) && MBBICommon->isDebugValue())
+    while ((MBBICommon != MBBIECommon) && MBBICommon->isDirective())
       ++MBBICommon;
 
     assert(MBBICommon != MBBIECommon &&
@@ -971,6 +1008,11 @@ bool BranchFolder::TryTailMergeBlocks(MachineBasicBlock *SuccBB,
       mergeOperations(SameTails[i].getTailStartPos(), *MBB);
       // Hack the end off BB i, making it jump to BB commonTailIndex instead.
       ReplaceTailWithBranchTo(SameTails[i].getTailStartPos(), MBB);
+
+      // Recalculate CFI info for BB. Use existing incoming cfa offset and
+      // register.
+      SameTails[i].getBlock()->recalculateCFIInfo(true);
+
       // BB i is no longer a predecessor of SuccBB; remove it from the worklist.
       MergePotentials.erase(SameTails[i].getMPIter());
     }
@@ -1381,6 +1423,10 @@ bool BranchFolder::OptimizeBlock(MachineBasicBlock *MBB) {
       assert(PrevBB.succ_empty());
       PrevBB.transferSuccessors(MBB);
       MadeChange = true;
+
+      // Update CFI info for PrevBB.
+      PrevBB.mergeCFIInfo(MBB);
+
       return MadeChange;
     }
 

llvm/lib/CodeGen/CFIInfoVerifier.cpp

@@ -0,0 +1,123 @@
+//===----------- CFIInfoVerifier.cpp - CFI Information Verifier -----------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This pass verifies incoming and outgoing CFI information of basic blocks. CFI
+// information is information about offset and register set by CFI directives,
+// valid at the start and end of a basic block. This pass checks that outgoing
+// information of predecessors matches incoming information of their successors.
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
+#include "llvm/CodeGen/Passes.h"
+#include "llvm/Target/TargetMachine.h"
+using namespace llvm;
+
+namespace {
+class CFIInfoVerifier : public MachineFunctionPass {
+ public:
+  static char ID;
+
+  CFIInfoVerifier() : MachineFunctionPass(ID) {
+    initializeCFIInfoVerifierPass(*PassRegistry::getPassRegistry());
+  }
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.setPreservesAll();
+    MachineFunctionPass::getAnalysisUsage(AU);
+  }
+
+  bool runOnMachineFunction(MachineFunction &MF) override {
+    bool NeedsDwarfCFI = (MF.getMMI().hasDebugInfo() ||
+                          MF.getFunction()->needsUnwindTableEntry()) &&
+                         (!MF.getTarget().getTargetTriple().isOSDarwin() &&
+                          !MF.getTarget().getTargetTriple().isOSWindows());
+    if (!NeedsDwarfCFI) return false;
+    verify(MF);
+    return false;
+  }
+
+ private:
+  // Go through each MBB in a function and check that outgoing offset and
+  // register of its predecessors match incoming offset and register of that
+  // MBB, as well as that incoming offset and register of its successors match
+  // outgoing offset and register of the MBB.
+  void verify(MachineFunction &MF);
+  void report(const char *msg, MachineBasicBlock &MBB);
+};
+}
+
+char CFIInfoVerifier::ID = 0;
+INITIALIZE_PASS(CFIInfoVerifier, "cfiinfoverifier",
+                "Verify that corresponding in/out CFI info matches", false,
+                false)
+FunctionPass *llvm::createCFIInfoVerifier() { return new CFIInfoVerifier(); }
+
+void CFIInfoVerifier::verify(MachineFunction &MF) {
+  for (auto &CurrMBB : MF) {
+    for (auto Pred : CurrMBB.predecessors()) {
+      // Check that outgoing offset values of predecessors match the incoming
+      // offset value of CurrMBB
+      if (Pred->getOutgoingCFAOffset() != CurrMBB.getIncomingCFAOffset()) {
+        report("The outgoing offset of a predecessor is inconsistent.",
+               CurrMBB);
+        errs() << "Predecessor BB#" << Pred->getNumber()
+               << " has outgoing offset (" << Pred->getOutgoingCFAOffset()
+               << "), while BB#" << CurrMBB.getNumber()
+               << " has incoming offset (" << CurrMBB.getIncomingCFAOffset()
+               << ").\n";
+      }
+      // Check that outgoing register values of predecessors match the incoming
+      // register value of CurrMBB
+      if (Pred->getOutgoingCFARegister() != CurrMBB.getIncomingCFARegister()) {
+        report("The outgoing register of a predecessor is inconsistent.",
+               CurrMBB);
+        errs() << "Predecessor BB#" << Pred->getNumber()
+               << " has outgoing register (" << Pred->getOutgoingCFARegister()
+               << "), while BB#" << CurrMBB.getNumber()
+               << " has incoming register (" << CurrMBB.getIncomingCFARegister()
+               << ").\n";
+      }
+    }
+
+    for (auto Succ : CurrMBB.successors()) {
+      // Check that incoming offset values of successors match the outgoing
+      // offset value of CurrMBB
+      if (Succ->getIncomingCFAOffset() != CurrMBB.getOutgoingCFAOffset()) {
+        report("The incoming offset of a successor is inconsistent.", CurrMBB);
+        errs() << "Successor BB#" << Succ->getNumber()
+               << " has incoming offset (" << Succ->getIncomingCFAOffset()
+               << "), while BB#" << CurrMBB.getNumber()
+               << " has outgoing offset (" << CurrMBB.getOutgoingCFAOffset()
+               << ").\n";
+      }
+      // Check that incoming register values of successors match the outgoing
+      // register value of CurrMBB
+      if (Succ->getIncomingCFARegister() != CurrMBB.getOutgoingCFARegister()) {
+        report("The incoming register of a successor is inconsistent.",
+               CurrMBB);
+        errs() << "Successor BB#" << Succ->getNumber()
+               << " has incoming register (" << Succ->getIncomingCFARegister()
+               << "), while BB#" << CurrMBB.getNumber()
+               << " has outgoing register (" << CurrMBB.getOutgoingCFARegister()
+               << ").\n";
+      }
+    }
+  }
+}
+
+void CFIInfoVerifier::report(const char *msg, MachineBasicBlock &MBB) {
+  assert(&MBB);
+  errs() << '\n';
+  errs() << "*** " << msg << " ***\n"
+         << "- function:    " << MBB.getParent()->getName() << "\n";
+  errs() << "- basic block: BB#" << MBB.getNumber() << ' ' << MBB.getName()
+         << " (" << (const void *)&MBB << ')';
+  errs() << '\n';
+}