From: Duncan P. N. Exon Smith Date: Tue, 15 Apr 2014 16:27:38 +0000 (+0000) Subject: verify-di: Implement DebugInfoVerifier X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=commitdiff_plain;h=32791b02facda5b879f2bcf7dd3f1faea1e4782d 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@206300 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/IR/Verifier.h b/include/llvm/IR/Verifier.h index 68caa96f186..0272e206f37 100644 --- a/include/llvm/IR/Verifier.h +++ b/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; diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h index 9b9f2345ad2..bfa5530522d 100644 --- a/include/llvm/InitializePasses.h +++ b/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 &); diff --git a/lib/CodeGen/Passes.cpp b/lib/CodeGen/Passes.cpp index 370fcb16144..b3f719841db 100644 --- a/lib/CodeGen/Passes.cpp +++ b/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) diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp index 1f977e8470e..887ca7d48eb 100644 --- a/lib/IR/Verifier.cpp +++ b/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, 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(F)); - Finder.reset(); Broken = false; // FIXME: We strip const here because the inst visitor strips const. visit(const_cast(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(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(CI.getArgOperand(0)); Assert1(MD->getNumOperands() == 1, "invalid llvm.dbg.declare intrinsic call 2", &CI); - if (VerifyDebugInfo) - Finder.processDeclare(*M, cast(&CI)); } break; - case Intrinsic::dbg_value: { //llvm.dbg.value - if (VerifyDebugInfo) { - Assert1(CI.getArgOperand(0) && isa(CI.getArgOperand(0)), - "invalid llvm.dbg.value intrinsic call 1", &CI); - Finder.processValue(*M, cast(&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 (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); + 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(&*I)) + processCallInst(Finder, *CI); } - } +} + +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(&CI)); + break; + case Intrinsic::dbg_value: + Finder.processValue(*M, cast(&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!"); diff --git a/lib/LTO/LTOCodeGenerator.cpp b/lib/LTO/LTOCodeGenerator.cpp index 8df42772d5a..da74c57c589 100644 --- a/lib/LTO/LTOCodeGenerator.cpp +++ b/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; diff --git a/lib/Transforms/Scalar/Scalar.cpp b/lib/Transforms/Scalar/Scalar.cpp index e950ebacd84..09167b9e82a 100644 --- a/lib/Transforms/Scalar/Scalar.cpp +++ b/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) { diff --git a/tools/bugpoint/CrashDebugger.cpp b/tools/bugpoint/CrashDebugger.cpp index bdaa6c9c89d..c646ff49b1c 100644 --- a/tools/bugpoint/CrashDebugger.cpp +++ b/tools/bugpoint/CrashDebugger.cpp @@ -410,6 +410,7 @@ bool ReduceCrashingInstructions::TestInsts(std::vector // Verify that this is still valid. PassManager Passes; Passes.add(createVerifierPass()); + Passes.add(createDebugInfoVerifierPass()); Passes.run(*M); // Try running on the hacked up program... diff --git a/tools/llvm-stress/llvm-stress.cpp b/tools/llvm-stress/llvm-stress.cpp index 18f1e6ca1be..5e42bb9fcbf 100644 --- a/tools/llvm-stress/llvm-stress.cpp +++ b/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(); diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index 5a198816e67..8f958bbd0aa 100644 --- a/tools/opt/opt.cpp +++ b/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) { diff --git a/unittests/Bitcode/BitReaderTest.cpp b/unittests/Bitcode/BitReaderTest.cpp index ba030230a20..b6a3e9a7b75 100644 --- a/unittests/Bitcode/BitReaderTest.cpp +++ b/unittests/Bitcode/BitReaderTest.cpp @@ -59,6 +59,7 @@ TEST(BitReaderTest, MaterializeFunctionsForBlockAddr) { // PR11677 std::unique_ptr m(ModuleOrErr.get()); PassManager passes; passes.add(createVerifierPass()); + passes.add(createDebugInfoVerifierPass()); passes.run(*m); }