[DebugInfo] Fix assertion for extern void type

[yonghong-song]

Summary

Commit d77ae1552fc2 ("[DebugInfo] Support to emit debugInfo
for extern variables") added support to emit debuginfo
for extern variables. Currently, only BPF target enables to
emit debuginfo for extern variables.

But if the extern variable has "void" type, the compilation will
fail.

-bash-4.4$ cat t.c 
extern void bla;
void *test() {
  void *x = &bla;
  return x;
}
-bash-4.4$ clang -target bpf -g -O2 -S t.c 
missing global variable type
!1 = distinct !DIGlobalVariable(name: "bla", scope: !2, file: !3, line: 1,
                                isLocal: false, isDefinition: false)
... 
fatal error: error in backend: Broken module found, compilation aborted!
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace,
    preprocessed source, and associated run script.
Stack dump:
...

The IR requires a DIGlobalVariable must have a valid type and the
"void" type does not generate any type, hence the above fatal error.

Note that if the extern variable is defined as "const void", the
compilation will succeed.

-bash-4.4$ cat t.c
extern const void bla;
const void *test() {
  const void *x = &bla;
  return x;
}
-bash-4.4$ clang -target bpf -g -O2 -S t.c
-bash-4.4$ cat t.ll
...
!1 = distinct !DIGlobalVariable(name: "bla", scope: !2, file: !3, line: 1,
                                type: !6, isLocal: false, isDefinition: false)
!6 = !DIDerivedType(tag: DW_TAG_const_type, baseType: null)
...

It might be the expected behavior not to generate debuginfo for
extern "void" variables. But the fatal error (with assert on) is not
nice.

Since currently, "const void extern_var" is supported by the
debug info, it is natural that "void extern_var" should also
be supported. This patch disabled assertion of "void extern_var"
in IR verifier and add proper guarding when emiting potential
null debug info type to dwarf types.

[anakryiko]

For my purposes, having extern var BTF only for extern const void is just fine. Not crashing Clang for extern void is great as well :)

[dblaikie]

Patches without tests shouldn't be approved - and at least the usual/rough metric I use for patch approval is (most often - unless I'm reviewing the work of the code owner in any area who wants a second opinion, etc) - I approve it if I'd be willing to commit a similar patch without approval (with post-commit review - ie: I'm fairly sure this is the right direction and folks will agree with me, or minor changes can be handled in post-commit iteration). Otherwise "approval" gets muddied as to whether it means "you can now commit this" versus "I think this is good but you should wait for someone more authoritative to say 'yes' before it's committed" (usually folks on the project call out the latter explicitly - sometimes using Phab's "approve" sometimes not).

In any case: I think if we're going to support "const void" we should support "void" too & fix the verifier to allow this & make sure LLVM produces the correct debug info for it (which is probably a DW_TAG_variable without a DW_AT_type attribute (same as a function with a void return type has no DW_AT_type attribute) - and check that GCC does the same thing

[yonghong-song]

Thanks, @dblaikie

I think if we're going to support "const void" we should support "void" too & fix the verifier to allow this & make sure LLVM produces the correct debug info for it (which is probably a DW_TAG_variable without a DW_AT_type attribute (same as a function with a void return type has no DW_AT_type attribute) - and check that GCC does the same thing.

Your suggestion makes sense, I will try to fix the fatal error according to what you suggested.

[yonghong-song]

instead of skipping generating debuginfo for "extern void var_name", do generate debuginfo and skip emitting dwarf types for it.
cc @dblaikie

[dblaikie]

Looks good - thanks!

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

if (auto *SDMDecl = GV->getStaticDataMemberDeclaration()) {
     // If the global variable's type is different from the one in the class
     // member type, assume that it's more specific and also emit it.
     if (GTy != SDMDecl->getBaseType())
       addType(*VariableDIE, GTy);
   } else {
     DeclContext = GV->getScope();
     // Add name and type.
     addString(*VariableDIE, dwarf::DW_AT_name, GV->getDisplayName());
+    if (GTy)
       addType(*VariableDIE, GTy);
 
     // Add scoping info.
     if (!GV->isLocalToUnit())
       addFlag(*VariableDIE, dwarf::DW_AT_external);
 
     // Add line number info.
     addSourceLine(*VariableDIE, GV);
   }

llvm/lib/IR/Verifier.cpp

 }
 
 void Verifier::visitDIGlobalVariable(const DIGlobalVariable &N) {
   // Checks common to all variables.
   visitDIVariable(N);
 
   AssertDI(N.getTag() == dwarf::DW_TAG_variable, "invalid tag", &N);
   AssertDI(isType(N.getRawType()), "invalid type ref", &N, N.getRawType());
+  // Assert only if the global variable is not an extern
+  if (N.isDefinition())
     AssertDI(N.getType(), "missing global variable type", &N);
   if (auto *Member = N.getRawStaticDataMemberDeclaration()) {
     AssertDI(isa<DIDerivedType>(Member),
              "invalid static data member declaration", &N, Member);
   }
 }
 
 void Verifier::visitDILocalVariable(const DILocalVariable &N) {
   // Checks common to all variables.

llvm/test/DebugInfo/BPF/extern-void.ll

+; RUN: llc -march=bpfel -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+; RUN: llc -march=bpfeb -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+;
+; Source code:
+;   extern void bla1;
+;   void *test1() {
+;     void *x = &bla1;
+;     return x;
+;   }
+;
+;   extern const void bla2;
+;   const void *test2() {
+;     const void *x = &bla2;
+;     return x;
+;   }
+; Compilation flag:
+;   clang -target bpf -O2 -g -S -emit-llvm t.c
+
+@bla1 = external dso_local global i8, align 1, !dbg !0
+@bla2 = external dso_local constant i8, align 1, !dbg !6
+
+; Function Attrs: norecurse nounwind readnone
+define dso_local nonnull i8* @test1() local_unnamed_addr #0 !dbg !13 {
+entry:
+  call void @llvm.dbg.value(metadata i8* @bla1, metadata !18, metadata !DIExpression()), !dbg !19
+  ret i8* @bla1, !dbg !20
+}
+
+; Function Attrs: norecurse nounwind readnone
+define dso_local nonnull i8* @test2() local_unnamed_addr #0 !dbg !21 {
+entry:
+  call void @llvm.dbg.value(metadata i8* @bla2, metadata !26, metadata !DIExpression()), !dbg !27
+  ret i8* @bla2, !dbg !28
+}
+
+; CHECK:        .quad bla1
+; CHECK-NEXT:   DW_TAG_variable
+;
+; CHECK:        .quad   bla2
+; CHECK-NEXT:   DW_TAG_const_type
+; CHECK-NEXT:   DW_TAG_subprogram
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { norecurse nounwind readnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!9, !10, !11}
+!llvm.ident = !{!12}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "bla1", scope: !2, file: !3, line: 1, isLocal: false, isDefinition: false)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project.git 8a8c6913a931e8bbd119012f4badd81155a0f48a)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "t.c", directory: "/home/yhs/tmp3")
+!4 = !{}
+!5 = !{!0, !6}
+!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
+!7 = distinct !DIGlobalVariable(name: "bla2", scope: !2, file: !3, line: 7, type: !8, isLocal: false, isDefinition: false)
+!8 = !DIDerivedType(tag: DW_TAG_const_type, baseType: null)
+!9 = !{i32 7, !"Dwarf Version", i32 4}
+!10 = !{i32 2, !"Debug Info Version", i32 3}
+!11 = !{i32 1, !"wchar_size", i32 4}
+!12 = !{!"clang version 11.0.0 (https://github.com/llvm/llvm-project.git 8a8c6913a931e8bbd119012f4badd81155a0f48a)"}
+!13 = distinct !DISubprogram(name: "test1", scope: !3, file: !3, line: 2, type: !14, scopeLine: 2, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !17)
+!14 = !DISubroutineType(types: !15)
+!15 = !{!16}
+!16 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 64)
+!17 = !{!18}
+!18 = !DILocalVariable(name: "x", scope: !13, file: !3, line: 3, type: !16)
+!19 = !DILocation(line: 0, scope: !13)
+!20 = !DILocation(line: 4, column: 3, scope: !13)
+!21 = distinct !DISubprogram(name: "test2", scope: !3, file: !3, line: 8, type: !22, scopeLine: 8, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !25)
+!22 = !DISubroutineType(types: !23)
+!23 = !{!24}
+!24 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !8, size: 64)
+!25 = !{!26}
+!26 = !DILocalVariable(name: "x", scope: !21, file: !3, line: 9, type: !24)
+!27 = !DILocation(line: 0, scope: !21)
+!28 = !DILocation(line: 10, column: 3, scope: !21)

llvm/test/DebugInfo/BPF/lit.local.cfg

+if not 'BPF' in config.root.targets:
+    config.unsupported = True