From 3ec16b419f8101ea195b7cc05ee152b9156becea Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Sat, 11 Apr 2015 19:58:35 +0000 Subject: [PATCH] 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@234698 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 | 96 +++++++++++++++++++++++++-- 4 files changed, 90 insertions(+), 32 deletions(-) diff --git a/include/llvm/IR/DebugInfo.h b/include/llvm/IR/DebugInfo.h index 33bb225db19..668c29ac77a 100644 --- a/include/llvm/IR/DebugInfo.h +++ b/include/llvm/IR/DebugInfo.h @@ -740,9 +740,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 1fbc53c195b..19b799e0892 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 ba50ecb0064..30fd7fd2099 100644 --- a/lib/IR/Verifier.cpp +++ b/lib/IR/Verifier.cpp @@ -178,8 +178,11 @@ 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 Track queue of bit piece expressions to verify. + SmallVector QueuedBitPieceExpressions; /// \brief The personality function referenced by the LandingPadInsts. /// All LandingPadInsts within the same function must use the same @@ -407,6 +410,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 +708,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; } @@ -3375,6 +3381,11 @@ void Verifier::visitDbgIntrinsic(StringRef Kind, DbgIntrinsicTy &DII) { "invalid llvm.dbg." + Kind + " intrinsic expression", &DII, DII.getRawExpression()); + // Queue up bit piece expressions to be verified once we can resolve + // typerefs. + if (DII.getExpression()->isValid() && DII.getExpression()->isBitPiece()) + QueuedBitPieceExpressions.push_back(&DII); + // Ignore broken !dbg attachments; they're checked elsewhere. if (MDNode *N = DII.getDebugLoc().getAsMDNode()) if (!isa(N)) @@ -3390,6 +3401,66 @@ 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 = DVI->getVariable(); + E = DVI->getExpression(); + } else { + auto *DDI = cast(&I); + V = DDI->getVariable(); + E = DDI->getExpression(); + } + + assert(V && E->isValid() && E->isBitPiece() && + "Expected valid bitpieces here"); + + // 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). @@ -3401,18 +3472,29 @@ 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 intrinsic bit piece expressions. + for (auto *DII : QueuedBitPieceExpressions) + 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(); -- 2.34.1