From ea8c1595246e81ba4273e6975c740d99705581d2 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Mon, 13 Apr 2015 00:06:28 +0000 Subject: [PATCH] Revert "Verifier: Check for incompatible bit piece expressions" This reverts commit r234698. This caused a use-after-free: `QueuedBitPieceExpressions` holds onto references to `DbgInfoIntrinsic`s and references them past where they're deleted (this is because the verifier is run as a function pass, and then `verifyTypeRefs()` is called during `doFinalization()`). I'll include a reduced crasher for `llc` when I recommit the check. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@234717 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, 32 insertions(+), 90 deletions(-) diff --git a/include/llvm/IR/DebugInfo.h b/include/llvm/IR/DebugInfo.h index 32cea6f3487..64d9f4d95bd 100644 --- a/include/llvm/IR/DebugInfo.h +++ b/include/llvm/IR/DebugInfo.h @@ -735,6 +735,9 @@ 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 2998d1fbf30..76e019bf8e9 100644 --- a/lib/CodeGen/AsmPrinter/DwarfDebug.cpp +++ b/lib/CodeGen/AsmPrinter/DwarfDebug.cpp @@ -1535,7 +1535,15 @@ 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 7797a026211..b0bd8223294 100644 --- a/lib/IR/DebugInfo.cpp +++ b/lib/IR/DebugInfo.cpp @@ -33,6 +33,19 @@ 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 a8eef378dce..9fb6f9a1a7e 100644 --- a/lib/IR/Verifier.cpp +++ b/lib/IR/Verifier.cpp @@ -178,11 +178,8 @@ class Verifier : public InstVisitor, VerifierSupport { /// \brief Keep track of the metadata nodes that have been checked already. SmallPtrSet MDNodes; - /// \brief Track unresolved string-based type references. - SmallDenseMap UnresolvedTypeRefs; - - /// \brief Track queue of bit piece expressions to verify. - SmallVector QueuedBitPieceExpressions; + /// \brief Track string-based type references. + SmallDenseMap TypeRefs; /// \brief The personality function referenced by the LandingPadInsts. /// All LandingPadInsts within the same function must use the same @@ -410,9 +407,6 @@ 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 @@ -708,7 +702,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. - UnresolvedTypeRefs.insert(std::make_pair(S, &N)); + TypeRefs.insert(std::make_pair(S, &N)); return true; } @@ -3377,11 +3371,6 @@ 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)) @@ -3397,66 +3386,6 @@ 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). @@ -3468,29 +3397,18 @@ void Verifier::verifyTypeRefs() { if (!CUs) return; - // Visit all the compile units again to map the type references. - SmallDenseMap TypeRefs; + // Visit all the compile units again to check the type references. for (auto *CU : CUs->operands()) if (auto Ts = cast(CU)->getRetainedTypes()) for (MDType *Op : Ts) if (auto *T = dyn_cast(Op)) - 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()) + TypeRefs.erase(T->getRawIdentifier()); + if (TypeRefs.empty()) return; // Sort the unresolved references by name so the output is deterministic. typedef std::pair TypeRef; - SmallVector Unresolved(UnresolvedTypeRefs.begin(), - UnresolvedTypeRefs.end()); + SmallVector Unresolved(TypeRefs.begin(), TypeRefs.end()); std::sort(Unresolved.begin(), Unresolved.end(), [](const TypeRef &LHS, const TypeRef &RHS) { return LHS.first->getString() < RHS.first->getString(); -- 2.34.1