verify-di: Implement DebugInfoVerifier

Implement DebugInfoVerifier, which steals verification relying on
DebugInfoFinder from Verifier.

  - Adds LegacyDebugInfoVerifierPassPass, a ModulePass which wraps
    DebugInfoVerifier.  Uses -verify-di command-line flag.

  - Change verifyModule() to invoke DebugInfoVerifier as well as
    Verifier.

  - Add a call to createDebugInfoVerifierPass() wherever there was a
    call to createVerifierPass().

This implementation as a module pass should sidestep efficiency issues,
allowing us to turn debug info verification back on.

<rdar://problem/15500563>

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@206300 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/IR/Verifier.h

@@ -28,6 +28,7 @@ namespace llvm {
 
 class Function;
 class FunctionPass;
+class ModulePass;
 class Module;
 class PreservedAnalyses;
 class raw_ostream;
@@ -58,6 +59,18 @@ bool verifyModule(const Module &M, raw_ostream *OS = nullptr);
 /// Note that this creates a pass suitable for the legacy pass manager. It has nothing to do with \c VerifierPass.
 FunctionPass *createVerifierPass(bool FatalErrors = true);
 
+/// \brief Create a debug-info verifier pass.
+///
+/// Check a module for validity of debug info. This is essentially a pass
+/// wrapped around the debug-info parts of \a verifyModule().  When the pass
+/// detects a verification error it is always printed to stderr, and by default
+/// they are fatal. You can override that by passing \c false to \p
+/// FatalErrors.
+///
+/// Note that this creates a pass suitable for the legacy pass manager. It has
+/// nothing to do with \c VerifierPass.
+ModulePass *createDebugInfoVerifierPass(bool FatalErrors = true);
+
 class VerifierPass {
   bool FatalErrors;
 

include/llvm/InitializePasses.h

@@ -103,6 +103,7 @@ void initializeDAHPass(PassRegistry&);
 void initializeDCEPass(PassRegistry&);
 void initializeDSEPass(PassRegistry&);
 void initializeDebugIRPass(PassRegistry&);
+void initializeDebugInfoVerifierLegacyPassPass(PassRegistry &);
 void initializeDeadInstEliminationPass(PassRegistry&);
 void initializeDeadMachineInstructionElimPass(PassRegistry&);
 void initializeDelinearizationPass(PassRegistry &);

lib/CodeGen/Passes.cpp

@@ -384,8 +384,10 @@ void TargetPassConfig::addIRPasses() {
 
   // Before running any passes, run the verifier to determine if the input
   // coming from the front-end and/or optimizer is valid.
-  if (!DisableVerify)
+  if (!DisableVerify) {
     addPass(createVerifierPass());
+    addPass(createDebugInfoVerifierPass());
+  }
 
   // Run loop strength reduction before anything else.
   if (getOptLevel() != CodeGenOpt::None && !DisableLSR) {
@@ -443,6 +445,12 @@ void TargetPassConfig::addCodeGenPrepare() {
 void TargetPassConfig::addISelPrepare() {
   addPreISel();
 
+  // Need to verify DebugInfo *before* creating the stack protector analysis.
+  // It's a function pass, and verifying between it and its users causes a
+  // crash.
+  if (!DisableVerify)
+    addPass(createDebugInfoVerifierPass());
+
   addPass(createStackProtectorPass(TM));
 
   if (PrintISelInput)

lib/IR/Verifier.cpp

@@ -61,6 +61,7 @@
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/InstIterator.h"
 #include "llvm/IR/InstVisitor.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/LLVMContext.h"
@@ -160,9 +161,6 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
   /// personality function.
   const Value *PersonalityFn;
 
-  /// \brief Finder keeps track of all debug info MDNodes in a Module.
-  DebugInfoFinder Finder;
-
 public:
   explicit Verifier(raw_ostream &OS = dbgs())
       : VerifierSupport(OS), Context(nullptr), DL(nullptr),
@@ -196,24 +194,18 @@ public:
     // FIXME: It's really gross that we have to cast away constness here.
     DT.recalculate(const_cast<Function &>(F));
 
-    Finder.reset();
     Broken = false;
     // FIXME: We strip const here because the inst visitor strips const.
     visit(const_cast<Function &>(F));
     InstsInThisBlock.clear();
     PersonalityFn = nullptr;
 
-    if (VerifyDebugInfo)
-      // Verify Debug Info.
-      verifyDebugInfo();
-
     return !Broken;
   }
 
   bool verify(const Module &M) {
     this->M = &M;
     Context = &M.getContext();
-    Finder.reset();
     Broken = false;
 
     // Scan through, checking all of the external function's linkage now...
@@ -241,13 +233,6 @@ public:
     visitModuleFlags(M);
     visitModuleIdents(M);
 
-    if (VerifyDebugInfo) {
-      Finder.reset();
-      Finder.processModule(M);
-      // Verify Debug Info.
-      verifyDebugInfo();
-    }
-
     return !Broken;
   }
 
@@ -332,8 +317,21 @@ private:
 
   void VerifyBitcastType(const Value *V, Type *DestTy, Type *SrcTy);
   void VerifyConstantExprBitcastType(const ConstantExpr *CE);
+};
+class DebugInfoVerifier : public VerifierSupport {
+public:
+  explicit DebugInfoVerifier(raw_ostream &OS = dbgs()) : VerifierSupport(OS) {}
 
+  bool verify(const Module &M) {
+    this->M = &M;
+    verifyDebugInfo();
+    return !Broken;
+  }
+
+private:
   void verifyDebugInfo();
+  void processInstructions(DebugInfoFinder &Finder);
+  void processCallInst(DebugInfoFinder &Finder, const CallInst &CI);
 };
 } // End anonymous namespace
 
@@ -2109,11 +2107,6 @@ void Verifier::visitInstruction(Instruction &I) {
   MDNode *MD = I.getMetadata(LLVMContext::MD_range);
   Assert1(!MD || isa<LoadInst>(I), "Ranges are only for loads!", &I);
 
-  if (VerifyDebugInfo) {
-    MD = I.getMetadata(LLVMContext::MD_dbg);
-    Finder.processLocation(*M, DILocation(MD));
-  }
-
   InstsInThisBlock.insert(&I);
 }
 
@@ -2313,17 +2306,7 @@ void Verifier::visitIntrinsicFunctionCall(Intrinsic::ID ID, CallInst &CI) {
     MDNode *MD = cast<MDNode>(CI.getArgOperand(0));
     Assert1(MD->getNumOperands() == 1,
                 "invalid llvm.dbg.declare intrinsic call 2", &CI);
-    if (VerifyDebugInfo)
-      Finder.processDeclare(*M, cast<DbgDeclareInst>(&CI));
   } break;
-  case Intrinsic::dbg_value: { //llvm.dbg.value
-    if (VerifyDebugInfo) {
-      Assert1(CI.getArgOperand(0) && isa<MDNode>(CI.getArgOperand(0)),
-              "invalid llvm.dbg.value intrinsic call 1", &CI);
-      Finder.processValue(*M, cast<DbgValueInst>(&CI));
-    }
-    break;
-  }
   case Intrinsic::memcpy:
   case Intrinsic::memmove:
   case Intrinsic::memset:
@@ -2385,25 +2368,51 @@ void Verifier::visitIntrinsicFunctionCall(Intrinsic::ID ID, CallInst &CI) {
   }
 }
 
-void Verifier::verifyDebugInfo() {
+void DebugInfoVerifier::verifyDebugInfo() {
+  if (!VerifyDebugInfo)
+    return;
+
+  DebugInfoFinder Finder;
+  Finder.processModule(*M);
+  processInstructions(Finder);
+
   // Verify Debug Info.
-  if (VerifyDebugInfo) {
-    for (DICompileUnit CU : Finder.compile_units()) {
-      Assert1(CU.Verify(), "DICompileUnit does not Verify!", CU);
+  for (DICompileUnit CU : Finder.compile_units())
+    Assert1(CU.Verify(), "DICompileUnit does not Verify!", CU);
+  for (DISubprogram S : Finder.subprograms())
+    Assert1(S.Verify(), "DISubprogram does not Verify!", S);
+  for (DIGlobalVariable GV : Finder.global_variables())
+    Assert1(GV.Verify(), "DIGlobalVariable does not Verify!", GV);
+  for (DIType T : Finder.types())
+    Assert1(T.Verify(), "DIType does not Verify!", T);
+  for (DIScope S : Finder.scopes())
+    Assert1(S.Verify(), "DIScope does not Verify!", S);
+}
+
+void DebugInfoVerifier::processInstructions(DebugInfoFinder &Finder) {
+  for (const Function &F : *M)
+    for (auto I = inst_begin(&F), E = inst_end(&F); I != E; ++I) {
+      if (MDNode *MD = I->getMetadata(LLVMContext::MD_dbg))
+        Finder.processLocation(*M, DILocation(MD));
+      if (const CallInst *CI = dyn_cast<CallInst>(&*I))
+        processCallInst(Finder, *CI);
     }
-    for (DISubprogram S : Finder.subprograms()) {
-      Assert1(S.Verify(), "DISubprogram does not Verify!", S);
-    }
-    for (DIGlobalVariable GV : Finder.global_variables()) {
-      Assert1(GV.Verify(), "DIGlobalVariable does not Verify!", GV);
-    }
-    for (DIType T : Finder.types()) {
-      Assert1(T.Verify(), "DIType does not Verify!", T);
-    }
-    for (DIScope S : Finder.scopes()) {
-      Assert1(S.Verify(), "DIScope does not Verify!", S);
-    }
-  }
+}
+
+void DebugInfoVerifier::processCallInst(DebugInfoFinder &Finder,
+                                        const CallInst &CI) {
+  if (Function *F = CI.getCalledFunction())
+    if (Intrinsic::ID ID = (Intrinsic::ID)F->getIntrinsicID())
+      switch (ID) {
+      case Intrinsic::dbg_declare:
+        Finder.processDeclare(*M, cast<DbgDeclareInst>(&CI));
+        break;
+      case Intrinsic::dbg_value:
+        Finder.processValue(*M, cast<DbgValueInst>(&CI));
+        break;
+      default:
+        break;
+      }
 }
 
 //===----------------------------------------------------------------------===//
@@ -2433,7 +2442,8 @@ bool llvm::verifyModule(const Module &M, raw_ostream *OS) {
 
   // Note that this function's return value is inverted from what you would
   // expect of a function called "verify".
-  return !V.verify(M) || Broken;
+  DebugInfoVerifier DIV(OS ? *OS : NullStr);
+  return !V.verify(M) || !DIV.verify(M) || Broken;
 }
 
 namespace {
@@ -2469,15 +2479,48 @@ struct VerifierLegacyPass : public FunctionPass {
     AU.setPreservesAll();
   }
 };
+struct DebugInfoVerifierLegacyPass : public ModulePass {
+  static char ID;
+
+  DebugInfoVerifier V;
+  bool FatalErrors;
+
+  DebugInfoVerifierLegacyPass() : ModulePass(ID), FatalErrors(true) {
+    initializeDebugInfoVerifierLegacyPassPass(*PassRegistry::getPassRegistry());
+  }
+  explicit DebugInfoVerifierLegacyPass(bool FatalErrors)
+      : ModulePass(ID), V(dbgs()), FatalErrors(FatalErrors) {
+    initializeDebugInfoVerifierLegacyPassPass(*PassRegistry::getPassRegistry());
+  }
+
+  bool runOnModule(Module &M) override {
+    if (!V.verify(M) && FatalErrors)
+      report_fatal_error("Broken debug info found, compilation aborted!");
+
+    return false;
+  }
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.setPreservesAll();
+  }
+};
 }
 
 char VerifierLegacyPass::ID = 0;
 INITIALIZE_PASS(VerifierLegacyPass, "verify", "Module Verifier", false, false)
 
+char DebugInfoVerifierLegacyPass::ID = 0;
+INITIALIZE_PASS(DebugInfoVerifierLegacyPass, "verify-di", "Debug Info Verifier",
+                false, false)
+
 FunctionPass *llvm::createVerifierPass(bool FatalErrors) {
   return new VerifierLegacyPass(FatalErrors);
 }
 
+ModulePass *llvm::createDebugInfoVerifierPass(bool FatalErrors) {
+  return new DebugInfoVerifierLegacyPass(FatalErrors);
+}
+
 PreservedAnalyses VerifierPass::run(Module *M) {
   if (verifyModule(*M, &dbgs()) && FatalErrors)
     report_fatal_error("Broken module found, compilation aborted!");

lib/LTO/LTOCodeGenerator.cpp

@@ -399,6 +399,7 @@ void LTOCodeGenerator::applyScopeRestrictions() {
   // Start off with a verification pass.
   PassManager passes;
   passes.add(createVerifierPass());
+  passes.add(createDebugInfoVerifierPass());
 
   // mark which symbols can not be internalized
   Mangler Mangler(TargetMach->getDataLayout());
@@ -471,6 +472,7 @@ bool LTOCodeGenerator::generateObjectFile(raw_ostream &out,
 
   // Start off with a verification pass.
   passes.add(createVerifierPass());
+  passes.add(createDebugInfoVerifierPass());
 
   // Add an appropriate DataLayout instance for this module...
   mergedModule->setDataLayout(TargetMach->getDataLayout());
@@ -492,6 +494,7 @@ bool LTOCodeGenerator::generateObjectFile(raw_ostream &out,
 
   // Make sure everything is still good.
   passes.add(createVerifierPass());
+  passes.add(createDebugInfoVerifierPass());
 
   PassManager codeGenPasses;
 

lib/Transforms/Scalar/Scalar.cpp

@@ -181,6 +181,7 @@ void LLVMAddDemoteMemoryToRegisterPass(LLVMPassManagerRef PM) {
 
 void LLVMAddVerifierPass(LLVMPassManagerRef PM) {
   unwrap(PM)->add(createVerifierPass());
+  // FIXME: should this also add createDebugInfoVerifierPass()?
 }
 
 void LLVMAddCorrelatedValuePropagationPass(LLVMPassManagerRef PM) {

tools/bugpoint/CrashDebugger.cpp

@@ -410,6 +410,7 @@ bool ReduceCrashingInstructions::TestInsts(std::vector<const Instruction*>
   // Verify that this is still valid.
   PassManager Passes;
   Passes.add(createVerifierPass());
+  Passes.add(createDebugInfoVerifierPass());
   Passes.run(*M);
 
   // Try running on the hacked up program...

tools/llvm-stress/llvm-stress.cpp

@@ -713,6 +713,7 @@ int main(int argc, char **argv) {
 
   PassManager Passes;
   Passes.add(createVerifierPass());
+  Passes.add(createDebugInfoVerifierPass());
   Passes.add(createPrintModulePass(Out->os()));
   Passes.run(*M.get());
   Out->keep();

tools/opt/opt.cpp

@@ -191,7 +191,10 @@ static inline void addPass(PassManagerBase &PM, Pass *P) {
   PM.add(P);
 
   // If we are verifying all of the intermediate steps, add the verifier...
-  if (VerifyEach) PM.add(createVerifierPass());
+  if (VerifyEach) {
+    PM.add(createVerifierPass());
+    PM.add(createDebugInfoVerifierPass());
+  }
 }
 
 /// AddOptimizationPasses - This routine adds optimization passes
@@ -201,7 +204,8 @@ static inline void addPass(PassManagerBase &PM, Pass *P) {
 /// OptLevel - Optimization Level
 static void AddOptimizationPasses(PassManagerBase &MPM,FunctionPassManager &FPM,
                                   unsigned OptLevel, unsigned SizeLevel) {
-  FPM.add(createVerifierPass());                  // Verify that input is correct
+  FPM.add(createVerifierPass());          // Verify that input is correct
+  MPM.add(createDebugInfoVerifierPass()); // Verify that debug info is correct
 
   PassManagerBuilder Builder;
   Builder.OptLevel = OptLevel;
@@ -240,6 +244,9 @@ static void AddStandardCompilePasses(PassManagerBase &PM) {
   if (StripDebug)
     addPass(PM, createStripSymbolsPass(true));
 
+  // Verify debug info only after it's (possibly) stripped.
+  PM.add(createDebugInfoVerifierPass());
+
   if (DisableOptimizations) return;
 
   // -std-compile-opts adds the same module passes as -O3.
@@ -257,6 +264,9 @@ static void AddStandardLinkPasses(PassManagerBase &PM) {
   if (StripDebug)
     addPass(PM, createStripSymbolsPass(true));
 
+  // Verify debug info only after it's (possibly) stripped.
+  PM.add(createDebugInfoVerifierPass());
+
   if (DisableOptimizations) return;
 
   PassManagerBuilder Builder;
@@ -600,8 +610,10 @@ int main(int argc, char **argv) {
   }
 
   // Check that the module is well formed on completion of optimization
-  if (!NoVerify && !VerifyEach)
+  if (!NoVerify && !VerifyEach) {
     Passes.add(createVerifierPass());
+    Passes.add(createDebugInfoVerifierPass());
+  }
 
   // Write bitcode or assembly to the output as the last step...
   if (!NoOutput && !AnalyzeOnly) {

unittests/Bitcode/BitReaderTest.cpp

@@ -59,6 +59,7 @@ TEST(BitReaderTest, MaterializeFunctionsForBlockAddr) { // PR11677
   std::unique_ptr<Module> m(ModuleOrErr.get());
   PassManager passes;
   passes.add(createVerifierPass());
+  passes.add(createDebugInfoVerifierPass());
   passes.run(*m);
 }