maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[Verifier] Constrain where DILocations may be nested 

Summary:
Constrain which metadata nodes are allowed to be, or contain,
DILocations. This ensures that logic for updating DILocations in a
Module is complete.

Currently, !llvm.loop metadata is the only odd duck which contains
nested DILocations. This has caused problems in the past: some passes
forgot to visit the nested locations, leading to subtly broken debug
info and late verification failures.

If there's a compelling reason for some future metadata to nest
DILocations, we'll need to introduce a generic API for updating the
locations attached to an Instruction before relaxing this check.

Reviewers: aprantl, dsanders

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D79245
This commit is contained in:
Vedant Kumar 2020-04-30 21:38:11 -07:00
parent 4ad53177db
commit 8dfe819bcd
2 changed files with 52 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
llvm/lib/IR/Verifier.cpp
View File

@ -397,6 +397,9 @@ public:
}
private:
/// Whether a metadata node is allowed to be, or contain, a DILocation.
enum class AreDebugLocsAllowed { No, Yes };
// Verification methods...
void visitGlobalValue(const GlobalValue &GV);
void visitGlobalVariable(const GlobalVariable &GV);
@ -405,7 +408,7 @@ private:
void visitAliaseeSubExpr(SmallPtrSetImpl<const GlobalAlias *> &Visited,
const GlobalAlias &A, const Constant &C);
void visitNamedMDNode(const NamedMDNode &NMD);
void visitMDNode(const MDNode &MD);
void visitMDNode(const MDNode &MD, AreDebugLocsAllowed AllowLocs);
void visitMetadataAsValue(const MetadataAsValue &MD, Function *F);
void visitValueAsMetadata(const ValueAsMetadata &MD, Function *F);
void visitComdat(const Comdat &C);
@ -780,11 +783,11 @@ void Verifier::visitNamedMDNode(const NamedMDNode &NMD) {
if (!MD)
continue;
visitMDNode(*MD);
visitMDNode(*MD, AreDebugLocsAllowed::Yes);
}
}
void Verifier::visitMDNode(const MDNode &MD) {
void Verifier::visitMDNode(const MDNode &MD, AreDebugLocsAllowed AllowLocs) {
// Only visit each node once. Metadata can be mutually recursive, so this
// avoids infinite recursion here, as well as being an optimization.
if (!MDNodes.insert(&MD).second)
@ -807,8 +810,10 @@ void Verifier::visitMDNode(const MDNode &MD) {
continue;
Assert(!isa<LocalAsMetadata>(Op), "Invalid operand for global metadata!",
&MD, Op);
AssertDI(!isa<DILocation>(Op) || AllowLocs == AreDebugLocsAllowed::Yes,
"DILocation not allowed within this metadata node", &MD, Op);
if (auto *N = dyn_cast<MDNode>(Op)) {
visitMDNode(*N);
visitMDNode(*N, AllowLocs);
continue;
}
if (auto *V = dyn_cast<ValueAsMetadata>(Op)) {
@ -851,7 +856,7 @@ void Verifier::visitValueAsMetadata(const ValueAsMetadata &MD, Function *F) {
void Verifier::visitMetadataAsValue(const MetadataAsValue &MDV, Function *F) {
Metadata *MD = MDV.getMetadata();
if (auto *N = dyn_cast<MDNode>(MD)) {
visitMDNode(*N);
visitMDNode(*N, AreDebugLocsAllowed::No);
return;
}
@ -2313,7 +2318,7 @@ void Verifier::visitFunction(const Function &F) {
"function declaration may not have a !prof attachment", &F);
// Verify the metadata itself.
visitMDNode(*I.second);
visitMDNode(*I.second, AreDebugLocsAllowed::Yes);
}
Assert(!F.hasPersonalityFn(),
"Function declaration shouldn't have a personality routine", &F);
@ -2337,6 +2342,7 @@ void Verifier::visitFunction(const Function &F) {
// Visit metadata attachments.
for (const auto &I : MDs) {
// Verify that the attachment is legal.
auto AllowLocs = AreDebugLocsAllowed::No;
switch (I.first) {
default:
break;
@ -2351,6 +2357,7 @@ void Verifier::visitFunction(const Function &F) {
AssertDI(!AttachedTo || AttachedTo == &F,
"DISubprogram attached to more than one function", SP, &F);
AttachedTo = &F;
AllowLocs = AreDebugLocsAllowed::Yes;
break;
}
case LLVMContext::MD_prof:
@ -2361,7 +2368,7 @@ void Verifier::visitFunction(const Function &F) {
}
// Verify the metadata itself.
visitMDNode(*I.second);
visitMDNode(*I.second, AllowLocs);
}
}
@ -4307,7 +4314,7 @@ void Verifier::visitInstruction(Instruction &I) {
if (MDNode *N = I.getDebugLoc().getAsMDNode()) {
AssertDI(isa<DILocation>(N), "invalid !dbg metadata attachment", &I, N);
visitMDNode(*N);
visitMDNode(*N, AreDebugLocsAllowed::Yes);
}
if (auto *DII = dyn_cast<DbgVariableIntrinsic>(&I)) {
@ -4315,6 +4322,17 @@ void Verifier::visitInstruction(Instruction &I) {
verifyNotEntryValue(*DII);
}
SmallVector<std::pair<unsigned, MDNode *>, 4> MDs;
I.getAllMetadata(MDs);
for (auto Attachment : MDs) {
unsigned Kind = Attachment.first;
auto AllowLocs =
(Kind == LLVMContext::MD_dbg || Kind == LLVMContext::MD_loop)
? AreDebugLocsAllowed::Yes
: AreDebugLocsAllowed::No;
visitMDNode(*Attachment.second, AllowLocs);
}
InstsInThisBlock.insert(&I);
}

26
llvm/test/Verifier/dilocation-in-wrong-place.ll Normal file
View File

@ -0,0 +1,26 @@
; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
; CHECK: DILocation not allowed within this metadata node
; CHECK-NEXT: [[unknownMD:![0-9]+]] = distinct !{[[unknownMD]], [[dbgMD:![0-9]+]]}
; CHECK-NEXT: [[dbgMD]] = !DILocation
define void @f() !dbg !5 {
ret void, !dbg !10, !unknown_md !11
}
!llvm.dbg.cu = !{!0}
!llvm.debugify = !{!3, !3}
!llvm.module.flags = !{!4}
!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
!1 = !DIFile(filename: "loop.ll", directory: "/")
!2 = !{}
!3 = !{i32 1}
!4 = !{i32 2, !"Debug Info Version", i32 3}
!5 = distinct !DISubprogram(name: "f", linkageName: "f", scope: null, file: !1, line: 1, type: !6, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !7)
!6 = !DISubroutineType(types: !2)
!7 = !{!8}
!8 = !DILocalVariable(name: "1", scope: !5, file: !1, line: 1, type: !9)
!9 = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_unsigned)
!10 = !DILocation(line: 1, column: 1, scope: !5)
!11 = !{!11, !10}