[llvm] r234776 - Reapply "Verifier: Check for incompatible bit piece expressions"
Duncan P. N. Exon Smith
dexonsmith at apple.com
Mon Apr 13 11:53:11 PDT 2015
Author: dexonsmith
Date: Mon Apr 13 13:53:11 2015
New Revision: 234776
URL: http://llvm.org/viewvc/llvm-project?rev=234776&view=rev
Log:
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.
Added:
llvm/trunk/test/DebugInfo/X86/deleted-bit-piece.ll
Modified:
llvm/trunk/include/llvm/IR/DebugInfo.h
llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
llvm/trunk/lib/IR/DebugInfo.cpp
llvm/trunk/lib/IR/Verifier.cpp
Modified: llvm/trunk/include/llvm/IR/DebugInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfo.h?rev=234776&r1=234775&r2=234776&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/DebugInfo.h (original)
+++ llvm/trunk/include/llvm/IR/DebugInfo.h Mon Apr 13 13:53:11 2015
@@ -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;
};
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=234776&r1=234775&r2=234776&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Mon Apr 13 13:53:11 2015
@@ -1535,15 +1535,7 @@ void DebugLocEntry::finalize(const AsmPr
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 {
Modified: llvm/trunk/lib/IR/DebugInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=234776&r1=234775&r2=234776&view=diff
==============================================================================
--- llvm/trunk/lib/IR/DebugInfo.cpp (original)
+++ llvm/trunk/lib/IR/DebugInfo.cpp Mon Apr 13 13:53:11 2015
@@ -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<MDDerivedType>(Ty) && !Ty.getSizeInBits()) {
- DIDerivedType DT = cast<MDDerivedType>(Ty);
- Ty = DT.getTypeDerivedFrom().resolve(Map);
- }
- assert(Ty.getSizeInBits() && "type with size 0");
- return Ty.getSizeInBits();
-}
-
//===----------------------------------------------------------------------===//
// Simple Descriptor Constructors and other Methods
//===----------------------------------------------------------------------===//
Modified: llvm/trunk/lib/IR/Verifier.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=234776&r1=234775&r2=234776&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Verifier.cpp (original)
+++ llvm/trunk/lib/IR/Verifier.cpp Mon Apr 13 13:53:11 2015
@@ -178,8 +178,8 @@ class Verifier : public InstVisitor<Veri
/// \brief Keep track of the metadata nodes that have been checked already.
SmallPtrSet<const Metadata *, 32> MDNodes;
- /// \brief Track string-based type references.
- SmallDenseMap<const MDString *, const MDNode *, 32> TypeRefs;
+ /// \brief Track unresolved string-based type references.
+ SmallDenseMap<const MDString *, const MDNode *, 32> 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 <class MapTy>
+ 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
// 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(StringR
BB ? BB->getParent() : nullptr, Var, VarIA, Loc, LocIA);
}
+template <class MapTy>
+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<MDType>(RawType))
+ if (uint64_t Size = T->getSizeInBits())
+ return Size;
+
+ if (auto *DT = dyn_cast<MDDerivedType>(RawType)) {
+ // Look at the base type.
+ RawType = DT->getRawBaseType();
+ continue;
+ }
+
+ if (auto *S = dyn_cast<MDString>(RawType)) {
+ // Don't error on missing types (checked elsewhere).
+ RawType = Map.lookup(S);
+ continue;
+ }
+
+ // Missing type or size.
+ break;
+ }
+
+ // Fail gracefully.
+ return 0;
+}
+
+template <class MapTy>
+void Verifier::verifyBitPieceExpression(const DbgInfoIntrinsic &I,
+ const MapTy &TypeRefs) {
+ MDLocalVariable *V;
+ MDExpression *E;
+ if (auto *DVI = dyn_cast<DbgValueInst>(&I)) {
+ V = dyn_cast_or_null<MDLocalVariable>(DVI->getRawVariable());
+ E = dyn_cast_or_null<MDExpression>(DVI->getRawExpression());
+ } else {
+ auto *DDI = cast<DbgDeclareInst>(&I);
+ V = dyn_cast_or_null<MDLocalVariable>(DDI->getRawVariable());
+ E = dyn_cast_or_null<MDExpression>(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<const MDString *, const MDType *, 32> TypeRefs;
for (auto *CU : CUs->operands())
if (auto Ts = cast<MDCompileUnit>(CU)->getRetainedTypes())
for (MDType *Op : Ts)
if (auto *T = dyn_cast<MDCompositeType>(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<DbgInfoIntrinsic>(&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<const MDString *, const MDNode *> TypeRef;
- SmallVector<TypeRef, 32> Unresolved(TypeRefs.begin(), TypeRefs.end());
+ SmallVector<TypeRef, 32> Unresolved(UnresolvedTypeRefs.begin(),
+ UnresolvedTypeRefs.end());
std::sort(Unresolved.begin(), Unresolved.end(),
[](const TypeRef &LHS, const TypeRef &RHS) {
return LHS.first->getString() < RHS.first->getString();
Added: llvm/trunk/test/DebugInfo/X86/deleted-bit-piece.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/deleted-bit-piece.ll?rev=234776&view=auto
==============================================================================
--- llvm/trunk/test/DebugInfo/X86/deleted-bit-piece.ll (added)
+++ llvm/trunk/test/DebugInfo/X86/deleted-bit-piece.ll Mon Apr 13 13:53:11 2015
@@ -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)
More information about the llvm-commits
mailing list