[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!

clang/lib/CodeGen/CodeGenModule.cpp

void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D,
 
   // Emit global variable debug information.
   if (CGDebugInfo *DI = getModuleDebugInfo())
     if (getCodeGenOpts().hasReducedDebugInfo())
       DI->EmitGlobalVariable(GV, D);
 }
 
 void CodeGenModule::EmitExternalVarDeclaration(const VarDecl *D) {
-  if (CGDebugInfo *DI = getModuleDebugInfo())
-    if (getCodeGenOpts().hasReducedDebugInfo()) {
-      QualType ASTTy = D->getType();
-      llvm::Type *Ty = getTypes().ConvertTypeForMem(D->getType());
-      llvm::PointerType *PTy =
-          llvm::PointerType::get(Ty, getContext().getTargetAddressSpace(ASTTy));
-      llvm::Constant *GV = GetOrCreateLLVMGlobal(D->getName(), PTy, D);
-      DI->EmitExternalVariable(
-          cast<llvm::GlobalVariable>(GV->stripPointerCasts()), D);
-    }
+  if (CGDebugInfo *DI = getModuleDebugInfo()) {
+    if (!getCodeGenOpts().hasReducedDebugInfo())
+      return;
+
+    // The DIGlobalVariable must have a type, skip the generation
+    // if the underlying type is void without any qualifiers.
+    if (!ASTTy.hasQualifiers() && ASTTy.getTypePtr()->isVoidType())
+      return;
+
 }
 
 static bool isVarDeclStrongDefinition(const ASTContext &Context,
                                       CodeGenModule &CGM, const VarDecl *D,
                                       bool NoCommon) {
   // Don't give variables common linkage if -fno-common was specified unless it
   // was overridden by a NoCommon attribute.
   if ((NoCommon || D->hasAttr<NoCommonAttr>()) && !D->hasAttr<CommonAttr>())