From 88116fe71ab0953859fd3f32027149b5ee6d3a16 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Mon, 13 Apr 2015 18:53:11 +0000 Subject: [PATCH] Reapply "Verifier: Check for incompatible bit piece expressions" This reverts commit r234717, reapplying r234698 (in spirit). As described in r234717, the original `Verifier` check had a use-after-free. Instead of storing pointers to "interesting" debug info intrinsics whose bit piece expressions should be verified once we have typerefs, do a second traversal. I've added a testcase to catch the `llc` crasher. Original commit message: Verifier: Check for incompatible bit piece expressions Convert an assertion into a `Verifier` check. Bit piece expressions must fit inside the variable, and mustn't be the entire variable. Catching this in the verifier will help us find bugs sooner, and makes `DIVariable::getSizeInBits()` dead code. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@234776 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/DebugInfo.h | 3 - lib/CodeGen/AsmPrinter/DwarfDebug.cpp | 10 +-- lib/IR/DebugInfo.cpp | 13 ---- lib/IR/Verifier.cpp | 99 +++++++++++++++++++++++-- test/DebugInfo/X86/deleted-bit-piece.ll | 46 ++++++++++++ 5 files changed, 139 insertions(+), 32 deletions(-) create mode 100644 test/DebugInfo/X86/deleted-bit-piece.ll diff --git a/include/llvm/IR/DebugInfo.h b/include/llvm/IR/DebugInfo.h index 64d9f4d95bd..32cea6f3487 100644 --- a/include/llvm/IR/DebugInfo.h +++ b/include/llvm/IR/DebugInfo.h @@ -735,9 +735,6 @@ public: /// \brief Check if this is an inlined function argument. bool isInlinedFnArgument(const Function *CurFn); - /// \brief Return the size reported by the variable's type. - unsigned getSizeInBits(const DITypeIdentifierMap &Map); - void printExtendedName(raw_ostream &OS) const; }; diff --git a/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/lib/CodeGen/AsmPrinter/DwarfDebug.cpp index 76e019bf8e9..2998d1fbf30 100644 --- a/lib/CodeGen/AsmPrinter/DwarfDebug.cpp +++ b/lib/CodeGen/AsmPrinter/DwarfDebug.cpp @@ -1535,15 +1535,7 @@ void DebugLocEntry::finalize(const AsmPrinter &AP, Offset += PieceOffset-Offset; } Offset += PieceSize; - -#ifndef NDEBUG - DIVariable Var = Piece.getVariable(); - unsigned VarSize = Var.getSizeInBits(TypeIdentifierMap); - assert(PieceSize+PieceOffset <= VarSize - && "piece is larger than or outside of variable"); - assert(PieceSize != VarSize - && "piece covers entire variable"); -#endif + emitDebugLocValue(AP, TypeIdentifierMap, Streamer, Piece, PieceOffset); } } else { diff --git a/lib/IR/DebugInfo.cpp b/lib/IR/DebugInfo.cpp index b0bd8223294..7797a026211 100644 --- a/lib/IR/DebugInfo.cpp +++ b/lib/IR/DebugInfo.cpp @@ -33,19 +33,6 @@ using namespace llvm; using namespace llvm::dwarf; -/// \brief Return the size reported by the variable's type. -unsigned DIVariable::getSizeInBits(const DITypeIdentifierMap &Map) { - DIType Ty = getType().resolve(Map); - // Follow derived types until we reach a type that - // reports back a size. - while (isa(Ty) && !Ty.getSizeInBits()) { - DIDerivedType DT = cast(Ty); - Ty = DT.getTypeDerivedFrom().resolve(Map); - } - assert(Ty.getSizeInBits() && "type with size 0"); - return Ty.getSizeInBits(); -} - //===----------------------------------------------------------------------===// // Simple Descriptor Constructors and other Methods //===----------------------------------------------------------------------===// diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp index 9fb6f9a1a7e..ba1a8e8b762 100644 --- a/lib/IR/Verifier.cpp +++ b/lib/IR/Verifier.cpp @@ -178,8 +178,8 @@ class Verifier : public InstVisitor, VerifierSupport { /// \brief Keep track of the metadata nodes that have been checked already. SmallPtrSet MDNodes; - /// \brief Track string-based type references. - SmallDenseMap TypeRefs; + /// \brief Track unresolved string-based type references. + SmallDenseMap UnresolvedTypeRefs; /// \brief The personality function referenced by the LandingPadInsts. /// All LandingPadInsts within the same function must use the same @@ -407,6 +407,9 @@ private: // Module-level debug info verification... void verifyTypeRefs(); + template + void verifyBitPieceExpression(const DbgInfoIntrinsic &I, + const MapTy &TypeRefs); void visitUnresolvedTypeRef(const MDString *S, const MDNode *N); }; } // End anonymous namespace @@ -702,7 +705,7 @@ bool Verifier::isValidUUID(const MDNode &N, const Metadata *MD) { // Keep track of names of types referenced via UUID so we can check that they // actually exist. - TypeRefs.insert(std::make_pair(S, &N)); + UnresolvedTypeRefs.insert(std::make_pair(S, &N)); return true; } @@ -3386,6 +3389,71 @@ void Verifier::visitDbgIntrinsic(StringRef Kind, DbgIntrinsicTy &DII) { BB ? BB->getParent() : nullptr, Var, VarIA, Loc, LocIA); } +template +static uint64_t getVariableSize(const MDLocalVariable &V, const MapTy &Map) { + // Be careful of broken types (checked elsewhere). + const Metadata *RawType = V.getRawType(); + while (RawType) { + // Try to get the size directly. + if (auto *T = dyn_cast(RawType)) + if (uint64_t Size = T->getSizeInBits()) + return Size; + + if (auto *DT = dyn_cast(RawType)) { + // Look at the base type. + RawType = DT->getRawBaseType(); + continue; + } + + if (auto *S = dyn_cast(RawType)) { + // Don't error on missing types (checked elsewhere). + RawType = Map.lookup(S); + continue; + } + + // Missing type or size. + break; + } + + // Fail gracefully. + return 0; +} + +template +void Verifier::verifyBitPieceExpression(const DbgInfoIntrinsic &I, + const MapTy &TypeRefs) { + MDLocalVariable *V; + MDExpression *E; + if (auto *DVI = dyn_cast(&I)) { + V = dyn_cast_or_null(DVI->getRawVariable()); + E = dyn_cast_or_null(DVI->getRawExpression()); + } else { + auto *DDI = cast(&I); + V = dyn_cast_or_null(DDI->getRawVariable()); + E = dyn_cast_or_null(DDI->getRawExpression()); + } + + // We don't know whether this intrinsic verified correctly. + if (!V || !E || !E->isValid()) + return; + + // Nothing to do if this isn't a bit piece expression. + if (!E->isBitPiece()) + return; + + // If there's no size, the type is broken, but that should be checked + // elsewhere. + uint64_t VarSize = getVariableSize(*V, TypeRefs); + if (!VarSize) + return; + + unsigned PieceSize = E->getBitPieceSize(); + unsigned PieceOffset = E->getBitPieceOffset(); + Assert(PieceSize + PieceOffset <= VarSize, + "piece is larger than or outside of variable", &I, V, E); + Assert(PieceSize != VarSize, "piece covers entire variable", &I, V, E); +} + void Verifier::visitUnresolvedTypeRef(const MDString *S, const MDNode *N) { // This is in its own function so we get an error for each bad type ref (not // just the first). @@ -3397,18 +3465,35 @@ void Verifier::verifyTypeRefs() { if (!CUs) return; - // Visit all the compile units again to check the type references. + // Visit all the compile units again to map the type references. + SmallDenseMap TypeRefs; for (auto *CU : CUs->operands()) if (auto Ts = cast(CU)->getRetainedTypes()) for (MDType *Op : Ts) if (auto *T = dyn_cast(Op)) - TypeRefs.erase(T->getRawIdentifier()); - if (TypeRefs.empty()) + if (auto *S = T->getRawIdentifier()) { + UnresolvedTypeRefs.erase(S); + TypeRefs.insert(std::make_pair(S, T)); + } + + // Verify debug info intrinsic bit piece expressions. This needs a second + // pass through the intructions, since we haven't built TypeRefs yet when + // verifying functions, and simply queuing the DbgInfoIntrinsics to evaluate + // later/now would queue up some that could be later deleted. + for (const Function &F : *M) + for (const BasicBlock &BB : F) + for (const Instruction &I : BB) + if (auto *DII = dyn_cast(&I)) + verifyBitPieceExpression(*DII, TypeRefs); + + // Return early if all typerefs were resolved. + if (UnresolvedTypeRefs.empty()) return; // Sort the unresolved references by name so the output is deterministic. typedef std::pair TypeRef; - SmallVector Unresolved(TypeRefs.begin(), TypeRefs.end()); + SmallVector Unresolved(UnresolvedTypeRefs.begin(), + UnresolvedTypeRefs.end()); std::sort(Unresolved.begin(), Unresolved.end(), [](const TypeRef &LHS, const TypeRef &RHS) { return LHS.first->getString() < RHS.first->getString(); diff --git a/test/DebugInfo/X86/deleted-bit-piece.ll b/test/DebugInfo/X86/deleted-bit-piece.ll new file mode 100644 index 00000000000..b8ae9b16f50 --- /dev/null +++ b/test/DebugInfo/X86/deleted-bit-piece.ll @@ -0,0 +1,46 @@ +; RUN: llc < %s | FileCheck %s +; This is mainly a crasher for the revert in r234717. A debug info intrinsic +; that gets deleted can't have its bit piece expression verified after it's +; deleted. + +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-apple-macosx10.8.0" + +; CHECK: __Z3foov: +; CHECK: retq + +define void @_Z3foov() { +entry: + br i1 undef, label %exit, label %bb + +bb: ; preds = %entry + call void @llvm.dbg.value(metadata i8* undef, i64 0, metadata !15, metadata !16), !dbg !17 + br label %exit + +exit: ; preds = %bb, %entry + ret void +} + +declare void @llvm.dbg.value(metadata, i64, metadata, metadata) + +!llvm.module.flags = !{!0, !1} +!llvm.dbg.cu = !{!2} + +!0 = !{i32 2, !"Dwarf Version", i32 2} +!1 = !{i32 2, !"Debug Info Version", i32 3} +!2 = !MDCompileUnit(language: DW_LANG_C_plus_plus, file: !3, isOptimized: true, runtimeVersion: 0, emissionKind: 1, enums: !4, retainedTypes: !5, subprograms: !11, globals: !4, imports: !4) +!3 = !MDFile(filename: "foo.cpp", directory: "/path/to/dir") +!4 = !{} +!5 = !{!6} +!6 = !MDCompositeType(tag: DW_TAG_structure_type, name: "Class", size: 64, align: 64, elements: !7, identifier: "_ZT5Class") +!7 = !{!8, !10} +!8 = !MDDerivedType(tag: DW_TAG_member, name: "a", scope: !"_ZT5Class", baseType: !9, size: 32, align: 32) +!9 = !MDBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed) +!10 = !MDDerivedType(tag: DW_TAG_member, name: "b", scope: !"_ZT5Class", baseType: !9, size: 32, align: 32) +!11 = !{!12} +!12 = !MDSubprogram(name: "foo", scope: null, file: !3, type: !13, isLocal: false, isDefinition: true, isOptimized: false, function: void ()* @_Z3foov) +!13 = !MDSubroutineType(types: !14) +!14 = !{null} +!15 = !MDLocalVariable(tag: DW_TAG_auto_variable, name: "v", scope: !12, type: !"_ZT5Class") +!16 = !MDExpression(DW_OP_bit_piece, 32, 32) +!17 = !MDLocation(line: 2755, column: 9, scope: !12)