[Verifier] Fix !dbg validation if Scope is the Subprogram

[loladiro]

Summary

We are inserting both Scope and SP into the Seen map and check whether
it was already there in which case we skip the validation (the idea
being that we already checked this Subprogram before). However,
if (Scope == SP) as MDNodes, then inserting the Scope, will trigger
the Seen check causing us to incorrectly not validate this !dbg
attachment. Fix this by not performing the SP Seen check if Scope == SP

[loladiro]

Small formatting fix

[dblaikie]

This seems like a slightly weird algorithm

We check if the debug location itself is seen, then if the immediate scope
is seen, then we walk the entire scope chain until we get to the top level
subprogram and check if that's already seen. Why not check the intermediate
scopes too, if checking the first and last are both worthwhile...

& if we actually check everything along the chain, then maybe this should
fall out more naturally?

[loladiro]

I'm not sure the added complexity of walking the chain and checking everything would be worth it. It seems like that would just be manually inlining getInlinedAtScope + doing the memoization. It seems like a reasonable assumption that the scope declared on one instruction would also be declared on instructions nearby, while that's not necessarily the case for intermediate ones (plus if it is we'll put it in the list when we get there).

[loladiro]

I would appreciate some guidance on how to go forward here. Walking and caching the intermediate chains doesn't make this significantly simpler, so the primary questions is whether the caching is worth it at all. Unfortunately, I don't have any large enough test cases with meaningfully nested inlining structures to be able to make such measurements here.

llvm/trunk/lib/IR/Verifier.cpp

for (auto &I : BB) {
       if (!Seen.insert(DL).second)
         continue;
 
       DILocalScope *Scope = DL->getInlinedAtScope();
       if (Scope && !Seen.insert(Scope).second)
         continue;
 
       DISubprogram *SP = Scope ? Scope->getSubprogram() : nullptr;
-      if (SP && !Seen.insert(SP).second)
+
+      // Scope and SP could be the same MDNode and we don't want to skip
+      // validation in that case
+      if (SP && ((Scope != SP) && !Seen.insert(SP).second))
         continue;
 
       // FIXME: Once N is canonical, check "SP == &N".
       Assert(SP->describes(&F),
              "!dbg attachment points at wrong subprogram for function", N, &F,
              &I, DL, Scope, SP);
     }
 }

llvm/trunk/test/CodeGen/X86/2010-05-28-Crash.ll

 
 declare i32 @zoo(...)
 
 declare void @llvm.dbg.value(metadata, i64, metadata, metadata) nounwind readnone
 
 define i32 @bar(i32 %x) nounwind optsize ssp !dbg !8 {
 entry:
   tail call void @llvm.dbg.value(metadata i32 %x, i64 0, metadata !7, metadata !DIExpression()), !dbg !DILocation(scope: !8)
-  tail call void @llvm.dbg.value(metadata i32 1, i64 0, metadata !0, metadata !DIExpression()) nounwind, !dbg !DILocation(scope: !1)
+  tail call void @llvm.dbg.value(metadata i32 1, i64 0, metadata !0, metadata !DIExpression()) nounwind, !dbg !DILocation(scope: !1, inlinedAt: !DILocation(scope: !8))
   %0 = tail call i32 (...) @zoo(i32 1) nounwind, !dbg !12 ; <i32> [#uses=1]
   %1 = add nsw i32 %0, %x, !dbg !13               ; <i32> [#uses=1]
   ret i32 %1, !dbg !13
 }
 
 !llvm.dbg.cu = !{!3}
 !llvm.module.flags = !{!20}
 

llvm/trunk/test/Verifier/func-dbg.ll

+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+
+define i32 @foo() !dbg !4 {
+entry:
+  ret i32 0, !dbg !6
+}
+
+define i32 @bar() !dbg !5 {
+entry:
+; CHECK: !dbg attachment points at wrong subprogram for function
+  ret i32 0, !dbg !6
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!7, !8}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: 0, enums: !2, retainedTypes: !2, subprograms: !3, globals: !2, imports: !2)
+!1 = !DIFile(filename: "dwarf-test.c", directory: "test")
+!2 = !{}
+!3 = !{!4, !5}
+!4 = distinct !DISubprogram(name: "foo", scope: !0, isDefinition: true)
+!5 = distinct !DISubprogram(name: "bar", scope: !0, isDefinition: true)
+!6 = !DILocation(line: 7, scope: !4)
+!7 = !{i32 2, !"Dwarf Version", i32 3}
+!8 = !{i32 1, !"Debug Info Version", i32 3}