[DebugInfo] Support to emit debugInfo for extern variables

[yonghong-song]

Summary

Extern variable usage in BPF is different from traditional
pure user space application. Recent discussion in linux bpf
mailing list has two use cases where debug info types are
required to use extern variables:

• extern types are required to have a suitable interface in libbpf (bpf loader) to provide kernel config parameters to bpf programs. https://lore.kernel.org/bpf/CAEf4BzYCNo5GeVGMhp3fhysQ=_axAf=[e-mail]/T/#t
• extern types are required so kernel bpf verifier can verify program which uses external functions more precisely. This will make later link with actual external function no need to reverify. https://lore.kernel.org/bpf/[e-mail]/T/#m8d5c3e87ffe7f2764e02d722cb0d8cbc136880ed

This patch added clang support to emit debuginfo for extern variables
with a TargetInfo hook to enable it. The debuginfo for the
extern variable is emitted only if that extern variable is
referenced in the current compilation unit.

Currently, only BPF target enables to generate debug info for
extern variables. The emission of such debuginfo is disabled for C++
at this moment since BPF only supports a subset of C language.
Emission with C++ can be enabled later if an appropriate use case
is identified.

-fstandalone-debug permits us to see more debuginfo with the cost
of bloated binary size. This patch did not add emission of extern
variable debug info with -fstandalone-debug. This can be
re-evaluated if there is a real need.

[yonghong-song]

The corresponding BPF patch to consume the debuginfo https://reviews.llvm.org/D70697.

[SouraVX]

Hi @yonghong-song , Thanks for doing this.
I have a doubt, regarding clang behavior, is this relevant to this.

Consider the following test case --

extern global_var;
int main(){}

Now when compiled with clang -g test.c, In file ELF[DWARF] file
their is no, DebugInfo for global_var.
While gcc compiled binary gcc -g test.c. has DebugInfo[DWARF]

DW_TAG_variable
             DW_AT_name      ("global_var")
             DW_AT_decl_file ("/test.c)
             DW_AT_decl_line (1)
             DW_AT_decl_column       (0x0c)
             DW_AT_type      (0x00000039 "int")
             DW_AT_external  (true)
             DW_AT_declaration       (true)

Is this relevant ? Does this patch, adds support for this in clang ? I mean DIGlobalVariable related stuff in clang generated Debug Metadata ? seems to, but still confirming.
Maybe I can use this, to build DWARF related DebugInfo on top of this.

[yonghong-song]

@SouraVX Currently, I filtered all external globals if they are not used. See

for (auto D: ExternalDeclarations) {
  if (!D || D->isInvalidDecl() || D->getPreviousDecl() || !D->isUsed())
    continue;

  Consumer.CompleteExternalDeclaration(D);
}

If I remove !D->isUsed(), I should be able to generate the debuginfo for global_var and eventually in dwarf in your above case.

I tested with gcc 7.3.1, indeed gcc generates dwarf info for global_var as you stated in the above.

What exactly your use case here for these unused external variables?

I would like to hear comments from llvm/clang debuginfo experts before making the change.

[SouraVX]

In D70696#1764043, @yonghong-song wrote:

If I remove !D->isUsed(), I should be able to generate the debuginfo for global_var and eventually in dwarf in your above case.
What exactly your use case here for these unused external variables?

I simplified that test case, too much. Obiviously, for a real situation, that global variable must be defined possibly in some different file, which eventually got linked to final executable. Consider this for a moment -
cat extern.c

int global_var = 2;

cat main.c

extern int global_var;
int main(){
 int local = global_var;
}

Now for above, clang produce debug info for gloabal_var in extern.c fine, Not in main.c for Declaration of gloabl_var. Eventually when producing final binary{a.out}, linker will merge debug_info section of {extern.c and main.c} and we end up with debugging entry in a.out , consequently debugger is also seems to be happy with it.

But I think, it would be nice to have a declaration debug entry for global_var in file{main.o) debugging information contribution. Because it's using this global variable{declared extern}. like in present case!

[umesh.kalappa0]

@SouraVX and @yonghong-song

cat extern.c
int global_var = 2;

compile as an extern.c shared a library ,then final executable a.out doesn't have debugInfo for global_var.

[SouraVX]

In D70696#1764306, @umesh.kalappa0 wrote:

@SouraVX and @yonghong-song

cat extern.c
int global_var = 2;

compile as an extern.c shared a library ,then final executable a.out doesn't have debugInfo for global_var.

Perfect! Now we have a better use case and motivation that we should generate declaration debug entry. for a variable declared as extern.
that will also solves @umesh.kalappa0 above case. a.out will have debuginfo for global_var, since we generated it's declaration debuginfo in primary object main.c.

[aprantl]

clang/lib/Sema/Sema.cpp

1140

clang-format

[probinson]

For the case:

cat def.c
    int global_var = 2;

def.o should have debug info for the definition of global_var.
For the case:

cat noref.c
    extern int global_var;
    int main() {}

I would not expect to see debug info for the declaration of global_var.
For the case:

cat ref.c
    extern int global_var;
    int main() { return global_var; }

I *do* expect to see debug info for the declaration of global_var.

Does bpf require debug info for the declaration of global_var in noref.c ?

[yonghong-song]

clang/lib/Sema/Sema.cpp

1140

Thanks for the review! Will update the patch today after running clang-format.

[dblaikie]

In D70696#1765637, @probinson wrote:

For the case:

cat def.c
    int global_var = 2;

def.o should have debug info for the definition of global_var.
For the case:

cat noref.c
    extern int global_var;
    int main() {}

I would not expect to see debug info for the declaration of global_var.
For the case:

cat ref.c
    extern int global_var;
    int main() { return global_var; }

I *do* expect to see debug info for the declaration of global_var.

FWIW I'd only expect it there with -fstandalone-debug - with -fno-standalone-debug I'd expect this code to rely on the assumption that def.c is also compiled with debug info.

(as it stands today, Clang/LLVM never produces debug info for global_var in ref.c, even with -fstandalone-debug & I'm not too fussed about that, but would be OK if someone wanted to fix/improve that)

Does bpf require debug info for the declaration of global_var in noref.c ?

Yeah, +1, I'm still curious to know more/trying to understand this ^ requirement.

[yonghong-song]

@probinson for the question,

Does bpf require debug info for the declaration of global_var in noref.c ?

No, bpf only cares the referenced external global variables. So my current implementation does not emit debug info
for external global_var in noref.c.

It is just strange that gcc 7.3.1 (did not test, but maybe later gcc versions as well) emits the debuginfo (encoded in dwarf)
even if the external variable is not used in the current compilation unit. Not sure what is the rationale behind it.

[yonghong-song]

@dblaikie Good points. I will guard external variable debug info generation under -fstandalone-debug flag.

[probinson]

In D70696#1765675, @yonghong-song wrote:

It is just strange that gcc 7.3.1 (did not test, but maybe later gcc versions as well) emits the debuginfo (encoded in dwarf)
even if the external variable is not used in the current compilation unit. Not sure what is the rationale behind it.

If it were me, it would be because the declaration is available in the current CU, therefore if the debugger is stopped somewhere in the CU, the symbol should be available to the debugger. Having the external declaration means you don't need to go rooting around in other CUs for the symbol.

This is just one of those trade-offs that producers and consumers make, regarding completeness of information versus bloat of unnecessary information. There is no "right" answer.

[SouraVX]

In D70696#1765675, @yonghong-song wrote:

@probinson for the question,

Does bpf require debug info for the declaration of global_var in noref.c ?

No, bpf only cares the referenced external global variables. So my current implementation does not emit debug info
for external global_var in noref.c.

It is just strange that gcc 7.3.1 (did not test, but maybe later gcc versions as well) emits the debuginfo (encoded in dwarf)
even if the external variable is not used in the current compilation unit. Not sure what is the rationale behind it.

I'm using gcc 9.2.0 -- it's generating, for noref.c{Not used in compilation unit} case also.

[dblaikie]

In D70696#1765714, @yonghong-song wrote:

@dblaikie Good points. I will guard external variable debug info generation under -fstandalone-debug flag.

Oh, I figured given your needs this'd be guarded behind the target being BPF? While I don't mind it being also enabled by -fstandalone-debug (or, if you like, I guess maybe BPF is a "standalone-debug by default" target (like MacOS/Darwin, I think - due to some LLDB limitations at the moment) & maybe that captures all the BPF quirks in this regard?) - though I wouldn't leap to it unless someone else is already interested in that feature in -fstandalone-debug. (I'd be a bit worried about debug info growth and how "is this global variable referenced" is computed - eg: it could be referenced from a dead (uncalled) inline function)

[dblaikie]

In D70696#1765675, @yonghong-song wrote:

@probinson for the question,

Does bpf require debug info for the declaration of global_var in noref.c ?

No, bpf only cares the referenced external global variables. So my current implementation does not emit debug info
for external global_var in noref.c.

It is just strange that gcc 7.3.1 (did not test, but maybe later gcc versions as well) emits the debuginfo (encoded in dwarf)
even if the external variable is not used in the current compilation unit. Not sure what is the rationale behind it.

FWIW, looks like GCC trunk doesn't do this anymore: https://godbolt.org/z/EP5mWF (try it with GCC trunk V GCC 9.2 - 9.2 mentions "glbl_var" and trunk does not).

[probinson]

In D70696#1765671, @dblaikie wrote:

In D70696#1765637, @probinson wrote:

For the case:

cat ref.c
    extern int global_var;
    int main() { return global_var; }

I *do* expect to see debug info for the declaration of global_var.

FWIW I'd only expect it there with -fstandalone-debug - with -fno-standalone-debug I'd expect this code to rely on the assumption that def.c is also compiled with debug info.

When global_var is defined in a separate .so, you might not have the symbol at all. It's helpful to be able to report "that symbol is defined outside this executable" (and IIRC, gdb can look it up by mangled name and actually show it to you anyway). Without even the declaration, you get "that symbol that you can see right there in your source? It doesn't exist, haha." So, I would definitely rather see a declaration for a referenced global.
Even if we currently don't do that.

[dblaikie]

In D70696#1765784, @probinson wrote:

In D70696#1765671, @dblaikie wrote:

In D70696#1765637, @probinson wrote:

For the case:

cat ref.c
    extern int global_var;
    int main() { return global_var; }

I *do* expect to see debug info for the declaration of global_var.

FWIW I'd only expect it there with -fstandalone-debug - with -fno-standalone-debug I'd expect this code to rely on the assumption that def.c is also compiled with debug info.

When global_var is defined in a separate .so, you might not have the symbol at all. It's helpful to be able to report "that symbol is defined outside this executable" (and IIRC, gdb can look it up by mangled name and actually show it to you anyway). Without even the declaration, you get "that symbol that you can see right there in your source? It doesn't exist, haha." So, I would definitely rather see a declaration for a referenced global.
Even if we currently don't do that.

Same would be true of all functions as well though, right? Neither LLVM nor GCC emit declarations for all called functions (& I expect that'd produce significant size growth - I'm worried enough about global variable declarations that are referenced by unused inline functions or other similar things, and pull in complex type hierarchies, etc - let alone all functions called too).

[dblaikie]

In D70696#1765789, @dblaikie wrote:

In D70696#1765784, @probinson wrote:

In D70696#1765671, @dblaikie wrote:

In D70696#1765637, @probinson wrote:

For the case:

cat ref.c
    extern int global_var;
    int main() { return global_var; }

I *do* expect to see debug info for the declaration of global_var.

FWIW I'd only expect it there with -fstandalone-debug - with -fno-standalone-debug I'd expect this code to rely on the assumption that def.c is also compiled with debug info.

When global_var is defined in a separate .so, you might not have the symbol at all. It's helpful to be able to report "that symbol is defined outside this executable" (and IIRC, gdb can look it up by mangled name and actually show it to you anyway). Without even the declaration, you get "that symbol that you can see right there in your source? It doesn't exist, haha." So, I would definitely rather see a declaration for a referenced global.
Even if we currently don't do that.

Same would be true of all functions as well though, right? Neither LLVM nor GCC emit declarations for all called functions (& I expect that'd produce significant size growth - I'm worried enough about global variable declarations that are referenced by unused inline functions or other similar things, and pull in complex type hierarchies, etc - let alone all functions called too).

(but in general, that's what -fstandalone-debug is for: If you want this file's debug info to be complete (for some value of complete) even if other parts of the program (shared libraries, static libraries, however they're done) are compiled without debug info - though I still suspect emitting debug info for all variables and functions called from this file would be a pretty big size cost)

[SouraVX]

@dblaikie , If you try switching to C compiler. then the DW_TAG_variable is their. for C++ . It's not generated. Please check godbolt again. -- though strange

[dblaikie]

In D70696#1765823, @SouraVX wrote:

@dblaikie , If you try switching to C compiler. then the DW_TAG_variable is their. for C++ . It's not generated. Please check godbolt again. -- though strange

Hmm - I don't seem to see it in C either: https://godbolt.org/z/EUufYE (it's present in 9.2 as it is with C++, but not present in GCC trunk)

[SouraVX]

In D70696#1765843, @dblaikie wrote:

In D70696#1765823, @SouraVX wrote:

@dblaikie , If you try switching to C compiler. then the DW_TAG_variable is their. for C++ . It's not generated. Please check godbolt again. -- though strange

Hmm - I don't seem to see it in C either: https://godbolt.org/z/EUufYE (it's present in 9.2 as it is with C++, but not present in GCC trunk)

Ahh,, missed the trunk -- Sorry!

[yonghong-song]

@dblaikie The concern for that users using -fstandalone-debug may suddenly see a bloat of external var debug info is totally valid. Let me just stick to BPF use case for now. If somebody else ever wants this information with -fstandalone-debug, the restriction can be evaluated and relaxed then.

[yonghong-song]

clang-format change (from @aprantl) , add a little details into summary related to recent discussion,
move tests from CodeGenCXX to CodeGen as they are all C tests.

[dblaikie]

Many of the test cases could be collapsed into one file - using different variables that are used, unused, locally or globally declared, etc.

Is this new code only active for C compilations? (does clang reject requests for the bpf target when the input is C++?) I ask due to the concerns around globals used in inline functions where the inline function is unused - though C has inline functions too, so I guess the question stands: Is that a problem? What happens?

Should this be driven by a lower level of code generation - ie: is it OK to only produce debug info descriptions for variables that are referenced in the resulting LLVM IR? (compile time constants wouldn't be described then, for instance - since they won't be code generated, loaded from memory, etc)

Is there somewhere I should be reading about the design requirement for these global variable descriptions to understand the justification for them & the ramifications if there are bugs that cause them not to be emitted?

[yonghong-song]

In D70696#1767616, @dblaikie wrote:

Many of the test cases could be collapsed into one file - using different variables that are used, unused, locally or globally declared, etc.

Okay. Will try to consolidate into one or fewer files. Originally, I am using different files to avoid cases where in the future clang may generate different ordering w.r.t. different global variables.

Is this new code only active for C compilations? (does clang reject requests for the bpf target when the input is C++?) I ask due to the concerns around globals used in inline functions where the inline function is unused - though C has inline functions too, so I guess the question stands: Is that a problem? What happens?

Currently, yes. my implementation only active for C compilation.
In the kernel documentation (https://www.kernel.org/doc/Documentation/networking/filter.txt), we have:

The new instruction set was originally designed with the possible goal in
mind to write programs in "restricted C" and compile into eBPF with a optional
GCC/LLVM backend, so that it can just-in-time map to modern 64-bit CPUs with
minimal performance overhead over two steps, that is, C -> eBPF -> native code.

For LLVM itself, people can compile a C++ program into BPF target. But "officially" we do not
support this. That is why I restricted to C only. For C++ programs, we don't get much usage/tests
from users.

Do you have a concrete example for this? I tried the following:

-bash-4.4$ cat t.h
inline int foo() { extern int g; return g; }
-bash-4.4$ cat t.c
int bar() { return 0; }
-bash-4.4$ clang -target bpf -g -O0 -S -emit-llvm t.c

foo is not used, clang seems smart enough to deduce g is not used, so no debuginfo is emitted in this case.

In general, if an inline function is not used but an external variable is used inside that inline function, the worst case is extra debuginfo for that external variable. Since it is not used, it won't impact bpf loader.

Should this be driven by a lower level of code generation - ie: is it OK to only produce debug info descriptions for variables that are referenced in the resulting LLVM IR? (compile time constants wouldn't be described then, for instance - since they won't be code generated, loaded from memory, etc)

Yes, it is OK to only produce debug info only for variables that are referenced in the resulting LLVM IR. But we are discussing extern variables and no compile time constants here. Maybe I miss something?

Is there somewhere I should be reading about the design requirement for these global variable descriptions to understand the justification for them & the ramifications if there are bugs that cause them not to be emitted?

We do not have design documents yet. The following are two links and I can explain more:

1. https://lore.kernel.org/bpf/CAEf4BzYCNo5GeVGMhp3fhysQ=_axAf=[e-mail]/T/#t

The typical config is at /boot/config-<...> in a linux machine. The config entry typically look like:

CONFIG_CC_IS_GCC=y
CONFIG_GCC_VERSION=40805
CONFIG_INITRAMFS_SOURCE=""

Suppose a bpf program wants to check config value and based on its value to do something, user can write:

extern bool CONFIG_CC_IS_GCC;
extern int CONFIG_GCC_VERSION;
extern char CONFIG_INITRAMFS_SOURCE[20];
...
if (CONFIG_CC_IS_GCC) ...
map_val = CONFIG_GCC_VERSION; 
__builtin_memcpy(map_value, 8, CONFIG_INITRAMFS_SOURCE);

bpfloader will create a data section store all the above info and patch the correct address to the code.
Without extern var type info, it becomes a guess game what type/size the user is using.
Based on precise type information, bpf loader is able to do relocation much easily.

2. https://lore.kernel.org/bpf/[e-mail]/T/#m8d5c3e87ffe7f2764e02d722cb0d8cbc136880ed

This is for bpf program verification.
For example,
bpf_prog1:

foo(...) {
  ... x ... y ...
  z =  bar(x /*struct t * */, y /* int */);
  ...
}

and there is no bar body available yet.
The kernel verifier still able to verify program "foo"
and makes sure type leading to bar for all parameters
are correct.

Later, if there is a program
prog2(struct t *a, int b)
which is verified independently.

The in kernel, prog1 can call prog2 if there parameter types
and return types match. This is the BPF-way dynamic linking.
The types for external used functions can help cut down
verification cost at linking time.

If there is no debug information for these extern variables, the current
proposal is to fail the bpf loader and verifier. User can always workaround
such issues to create bpf maps for the first use case (which is more expensive and not user friendly) and do static
linking before loading into the kernel for the second use case.

[yonghong-song]

consolidated into less test files, added more test cases along the way.

[yonghong-song]

@dblaikie I consolidated test cases as you suggested. Along the way, I also added some more tests. I already answered some of your questions earlier. Please let me know what you think.

[Jac1494]

For some real life case like below we need debuginfo for declaration of global extern variable .

$cat shlib.c
int var;
int test()
{ return var++; }

$cat test.c
extern int test();
extern int var;
int main()
{ var++; printf("%d\n",test()); }

If we debug above case with gdb it is not giving types of variable var.
Because of no variable DIE is there in executable.

(gdb) b main
Breakpoint 1 at 0x40063c: file test.c, line 4.
(gdb) pt var
type = <data variable, no debug info>

To add variable debuginfo we need to merge below patch in code.

diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 9a3bb98..df79d46 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1371,6 +1371,9 @@ public:

   virtual void setAuxTarget(const TargetInfo *Aux) {}

+  /// Whether target allows debuginfo types for decl only variables.
+  virtual bool allowDebugInfoForExternalVar() const { return true; }
+
 protected:
   /// Copy type and layout related info.
   void copyAuxTarget(const TargetInfo *Aux);

diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
index a61c98e..92245c0 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -158,7 +158,11 @@ DIE *DwarfCompileUnit::getOrCreateGlobalVariableDIE(
   if (!GV->isDefinition())
     addFlag(*VariableDIE, dwarf::DW_AT_declaration);
   else
+  {
+    // Add location.
+    addLocationAttribute(VariableDIE, GV, GlobalExprs);
     addGlobalName(GV->getName(), *VariableDIE, DeclContext);
+  }

   if (uint32_t AlignInBytes = GV->getAlignInBytes())
     addUInt(*VariableDIE, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
@@ -167,9 +171,6 @@ DIE *DwarfCompileUnit::getOrCreateGlobalVariableDIE(
   if (MDTuple *TP = GV->getTemplateParams())
     addTemplateParams(*VariableDIE, DINodeArray(TP));

-  // Add location.
-  addLocationAttribute(VariableDIE, GV, GlobalExprs);
-
   return VariableDIE;
 }

[yonghong-song]

@Jac1494 Based on early discussion, we cannot just issue debuginfo for externs when -g is specified as this may cause debuginfo (and final binary) size bloat. One possible way is to enable it when -fstandalone-debug is specified. @dblaikie can you comment on this patch and general possible forward path for extern variable debug info support?

[dblaikie]

In D70696#1774931, @Jac1494 wrote:

For some real life case like below we need debuginfo for declaration of global extern variable .

$cat shlib.c
int var;
int test()
{ return var++; }

$cat test.c
extern int test();
extern int var;
int main()
{ var++; printf("%d\n",test()); }

If we debug above case with gdb it is not giving types of variable var.
Because of no variable DIE is there in executable.

(gdb) b main
Breakpoint 1 at 0x40063c: file test.c, line 4.
(gdb) pt var
type = <data variable, no debug info>

Sounds like this is if you build shlib without debug info?

If you build shlib with debug info, it seems to be fine:

$ clang-tot -shared -o libshlib.so shlib.c -fpic -g
$ clang-tot test.c -lshlib -g -L.
$ LD_LIBRARY_PATH=. gdb ./a.out
(gdb) start
Temporary breakpoint 1, main () at test.c:4
4       { var++; printf("%d\n",test()); }
(gdb) pt var
type = int
(gdb) 

This is the same as if the code were compiled statically & one test.c was compiled with debug info but shlib.c was compiled without debug info - the shared-library-ness doesn't change this situation.

-fstandalone-debug is the flag to use if parts of the program are built without debug info. Yes, that could be used to add global variable declaration to the DWARF & currently isn't - but I think that's a sufficiently nuanced/separate feature built on top of the work in this patch that it should be done separately, rather than trying to have all the design discussion for that in this code review.

This looks fine to me - I think a lot of the testing probably isn't testing "interesting" cases, but up to you.

clang/test/CodeGen/debug-info-extern-basic.c

14–23

What do these tests add? What sort of bugs would be caught by these global function pointer tests that wouldn't be caught by the char tests above them?

clang/test/CodeGen/debug-info-extern-duplicate.c

4–38

Similarly - are these covering novel/distinct codepaths from the other tests? Are there codepaths that are different for multiple declarations?

[yonghong-song]

clang/test/CodeGen/debug-info-extern-basic.c

14–23

These two additional tests to test extern function pointers. One of bpf program use cases specifically need extern function debuginfo type so I added a bunch. I do agree that this may be too much unnecessary and variable tests should cover this.

I will remove all these extern function pointer tests including below one, except one like the above test3() which should be enough.

[yonghong-song]

remove some redundant test cases

[dblaikie]

clang/test/CodeGen/debug-info-extern-basic.c

14–23

Just to explain my thinking a bit more clearly here:

Testing this feature shouldn't be related to how you want to use the feature, but how the feature's designed/what likely places it could have bugs.

If the feature creates types in the same way for every type, and in a way that's already well tested for other types - then I don't think it's useful to test more than one type here.

(imagine, say, testing code generation for function calls - function calls are independent of the expressions that generate their arguments - so it doesn't make sense to test "foo(3)" and "foo(1 + 2)", etc - and of course there are limits to this sort of "orthogonality" analysis, taken too far you don't get enough test coverage, etc - but generally that's what I'm thinking of when I'm thinking about test case reduction, is semi-white-box "could I introduce a bug that would make one of these tests fail and teh other pass" - if it's pretty clear that the two systems are independent, and each independently well tested, I'll only test one or two paths through both of the systems)

[rnk]

Three of these tests have been failing for me locally since this patch landed:

Failing Tests (3):
    Clang :: CodeGen/debug-info-extern-basic.c
    Clang :: CodeGen/debug-info-extern-duplicate.c
    Clang :: CodeGen/debug-info-extern-multi.c

I suspect bots do not see these failures because the BPF target is experimental. I would recommend that you:

• do not use %clang in CodeGen tests, use %clang_cc1
• find a way to make the test run without the BPF target

I'll take a look at fixing forward.

[rnk]

I wasn't able to fix forward because the functionality you added doesn't seem to work, at least not for me, so I went ahead and reverted the change. I considered XFAILing the test, but I am concerned that may cause it to pass unexpectedly on some bot somewhere. In any case, please address the two requests I made (use cc1, avoid requiring BPF target in test) and reland.

[yonghong-song]

@rnk I just submitted a patch https://reviews.llvm.org/D71818 to use %clang_cc1 instead of %clang in the test. Could you check whether this fixed your issue or not?

I cannot remove BPF target requirement as only BPF target can trigger debug info generation for extern variables. This is by design as discussed in this patch review due to concerns it may blow up debug info binary size for existing applications. This may be relaxed in the future through some flags. BPF is used on linux platform only.
As far as I know, BPF is enabled for a lot of bots, ubuntu, x86, ppc, arm, etc. So the coverage of this feature should be fine.

[dblaikie]

In D70696#1794440, @rnk wrote:

Three of these tests have been failing for me locally since this patch landed:

Failing Tests (3):
    Clang :: CodeGen/debug-info-extern-basic.c
    Clang :: CodeGen/debug-info-extern-duplicate.c
    Clang :: CodeGen/debug-info-extern-multi.c

I suspect bots do not see these failures because the BPF target is experimental. I would recommend that you:

• do not use %clang in CodeGen tests, use %clang_cc1
• find a way to make the test run without the BPF target

I'll take a look at fixing forward.

Got any more details on the nature of the failure/repro steps?

In D70696#1794480, @yonghong-song wrote:

@rnk I just submitted a patch https://reviews.llvm.org/D71818 to use %clang_cc1 instead of %clang in the test. Could you check whether this fixed your issue or not?

I cannot remove BPF target requirement as only BPF target can trigger debug info generation for extern variables. This is by design as discussed in this patch review due to concerns it may blow up debug info binary size for existing applications. This may be relaxed in the future through some flags. BPF is used on linux platform only.

If needed/useful, we could add a (just a cc1 flag, not a full driver flag) to support expliictly enabling this feature regardless of target, for testing purposes - but I'm not sure that's needed as yet.

As far as I know, BPF is enabled for a lot of bots, ubuntu, x86, ppc, arm, etc. So the coverage of this feature should be fine.

Could you link to particular bots that have logs showing they ran this test? (perhaps the logs have been retired by now, though - since this patch was reverted :/ - but then at least seeing which bots run BPF tests would be helpful)

[yonghong-song]

Could you link to particular bots that have logs showing they ran this test? (perhaps the logs have been retired by now, though - since this patch was reverted :/ - but then at least seeing which bots run BPF tests would be helpful)

@dblaikie Actually, I do not have direct proof that this test is running on buildbot. I only knew BPF is enabled by a lot of bots. I tried to search through emails when I got from buildbot failures like:

clang-cmake-x86_64-avx2-linux
llvm-clang-win-x-aarch64
llvm-clang-win-x-armv7l
clang-hexagon-elf
clang-with-thin-lto-ubuntu
clang-ppc64be-linux-lnt
sanitizer-x86_64-linux-bootstrap-msan
clang-s390x-linux
sanitizer-x86_64-linux-fuzzer
......

The contents of buildbot failures are all gone and not accessible now. Will definitely check buildbot BPF coverage next time when I saw any issues.

[dblaikie]

In D70696#1795302, @yonghong-song wrote:

Could you link to particular bots that have logs showing they ran this test? (perhaps the logs have been retired by now, though - since this patch was reverted :/ - but then at least seeing which bots run BPF tests would be helpful)

@dblaikie Actually, I do not have direct proof that this test is running on buildbot. I only knew BPF is enabled by a lot of bots. I tried to search through emails when I got from buildbot failures like:

clang-cmake-x86_64-avx2-linux
llvm-clang-win-x-aarch64
llvm-clang-win-x-armv7l
clang-hexagon-elf
clang-with-thin-lto-ubuntu
clang-ppc64be-linux-lnt
sanitizer-x86_64-linux-bootstrap-msan
clang-s390x-linux
sanitizer-x86_64-linux-fuzzer
......

The contents of buildbot failures are all gone and not accessible now. Will definitely check buildbot BPF coverage next time when I saw any issues.

Hmm, yeah, I thought some of the buildbots print out verbose test logs, showing every test execution (even the passing ones) , but I guess I'm mistaken.

Probably worth waiting a few days (maybe until after the holidays/new year) to see if @rnk has some more info that'd help debug the issue before committing again.

(or, given it's the holidasy and not many people are around, some "testing on the bots" might be acceptable (though I guess we already know the bots aren't failing... or you'd have got email, etc... so maybe that doesn't add much value?))

[yonghong-song]

@dblaikie The root cause has been identified by @rnk . A clang plugin is used which requires the following patch:

https://github.com/llvm/llvm-project/commit/5128026467cbc17bfc796d94bc8e40e52a9b0752

which has been merged. The original extern variable debug info type has been relanded. So we are good. Thanks!

[dblaikie]

In D70696#1795427, @yonghong-song wrote:

@dblaikie The root cause has been identified by @rnk . A clang plugin is used which requires the following patch:

https://github.com/llvm/llvm-project/commit/5128026467cbc17bfc796d94bc8e40e52a9b0752

which has been merged. The original extern variable debug info type has been relanded. So we are good. Thanks!

Ah, OK - would be good to follow-up here and mention the commit where this was relanded for the history/records. Thanks!

clang/include/clang/AST/ASTConsumer.h

public:
   ///
   /// The variable declaration itself will be a tentative
   /// definition. If it had an incomplete array type, its type will
   /// have already been changed to an array of size 1. However, the
   /// declaration remains a tentative definition and has not been
   /// modified by the introduction of an implicit zero initializer.
   virtual void CompleteTentativeDefinition(VarDecl *D) {}
 
+  /// CompleteExternalDeclaration - Callback invoked at the end of a translation
+  /// unit to notify the consumer that the given external declaration should be
+  /// completed.
+  virtual void CompleteExternalDeclaration(VarDecl *D) {}
+
   /// Callback invoked when an MSInheritanceAttr has been attached to a
   /// CXXRecordDecl.
   virtual void AssignInheritanceModel(CXXRecordDecl *RD) {}
 
   /// HandleCXXStaticMemberVarInstantiation - Tell the consumer that this
   // variable has been instantiated.
   virtual void HandleCXXStaticMemberVarInstantiation(VarDecl *D) {}
 

clang/include/clang/Basic/TargetInfo.h

 
   /// Check the target is valid after it is fully initialized.
   virtual bool validateTarget(DiagnosticsEngine &Diags) const {
     return true;
   }
 
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
+  /// Whether target allows debuginfo types for decl only variables.
+  virtual bool allowDebugInfoForExternalVar() const { return false; }
+
 protected:
   /// Copy type and layout related info.
   void copyAuxTarget(const TargetInfo *Aux);
   virtual uint64_t getPointerWidthV(unsigned AddrSpace) const {
     return PointerWidth;
   }
   virtual uint64_t getPointerAlignV(unsigned AddrSpace) const {
     return PointerAlign;

clang/include/clang/Sema/Sema.h

public:
 
   typedef LazyVector<VarDecl *, ExternalSemaSource,
                      &ExternalSemaSource::ReadTentativeDefinitions, 2, 2>
     TentativeDefinitionsType;
 
   /// All the tentative definitions encountered in the TU.
   TentativeDefinitionsType TentativeDefinitions;
 
+  /// All the external declarations encoutered and used in the TU.
+  SmallVector<VarDecl *, 4> ExternalDeclarations;
+
   typedef LazyVector<const DeclaratorDecl *, ExternalSemaSource,
                      &ExternalSemaSource::ReadUnusedFileScopedDecls, 2, 2>
     UnusedFileScopedDeclsType;
 
   /// The set of file scoped decls seen so far that have not been used
   /// and must warn if not used. Only contains the first declaration.
   UnusedFileScopedDeclsType UnusedFileScopedDecls;
 

clang/lib/Basic/Targets/BPF.h

bool validateAsmConstraint(const char *&Name,
                              TargetInfo::ConstraintInfo &info) const override {
     return true;
   }
 
   ArrayRef<TargetInfo::GCCRegAlias> getGCCRegAliases() const override {
     return None;
   }
 
+  bool allowDebugInfoForExternalVar() const override { return true; }
+
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override {
     switch (CC) {
     default:
       return CCCR_Warning;
     case CC_C:
     case CC_OpenCLKernel:
       return CCCR_OK;
     }

clang/lib/CodeGen/CGDebugInfo.h

void EmitDeclareOfBlockLiteralArgVariable(const CGBlockInfo &block,
                                             CGBuilderTy &Builder);
 
   /// Emit information about a global variable.
   void EmitGlobalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl);
 
   /// Emit a constant global variable's debug info.
   void EmitGlobalVariable(const ValueDecl *VD, const APValue &Init);
 
+  /// Emit information about an external variable.
+  void EmitExternalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl);
+
   /// Emit C++ using directive.
   void EmitUsingDirective(const UsingDirectiveDecl &UD);
 
   /// Emit the type explicitly casted to.
   void EmitExplicitCastType(QualType Ty);
 
   /// Emit C++ using declaration.
   void EmitUsingDecl(const UsingDecl &UD);

clang/lib/CodeGen/CGDebugInfo.cpp

if (CGM.getLangOpts().CUDA && CGM.getLangOpts().CUDAIsDevice) {
       else if (D->hasAttr<CUDAConstantAttr>())
         AddressSpace =
             CGM.getContext().getTargetAddressSpace(LangAS::cuda_constant);
     }
     AppendAddressSpaceXDeref(AddressSpace, Expr);
 
     GVE = DBuilder.createGlobalVariableExpression(
         DContext, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T, Unit),
-        Var->hasLocalLinkage(),
+        Var->hasLocalLinkage(), true,
         Expr.empty() ? nullptr : DBuilder.createExpression(Expr),
         getOrCreateStaticDataMemberDeclarationOrNull(D), TemplateParameters,
         Align);
     Var->addDebugInfo(GVE);
   }
   DeclCache[D->getCanonicalDecl()].reset(GVE);
 }
 
void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue &Init) {
   if (isa<VarTemplateSpecializationDecl>(VD))
     if (VarD) {
       llvm::DINodeArray parameterNodes = CollectVarTemplateParams(VarD, &*Unit);
       TemplateParameters = parameterNodes.get();
     }
 
   GV.reset(DBuilder.createGlobalVariableExpression(
       DContext, Name, StringRef(), Unit, getLineNumber(VD->getLocation()), Ty,
-      true, InitExpr, getOrCreateStaticDataMemberDeclarationOrNull(VarD),
+      true, true, InitExpr, getOrCreateStaticDataMemberDeclarationOrNull(VarD),
       TemplateParameters, Align));
 }
 
+void CGDebugInfo::EmitExternalVariable(llvm::GlobalVariable *Var,
+                                       const VarDecl *D) {
+  assert(DebugKind >= codegenoptions::LimitedDebugInfo);
+  if (D->hasAttr<NoDebugAttr>())
+    return;
+
+  auto Align = getDeclAlignIfRequired(D, CGM.getContext());
+  llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
+  StringRef Name = D->getName();
+  llvm::DIType *Ty = getOrCreateType(D->getType(), Unit);
+
+  llvm::DIScope *DContext = getDeclContextDescriptor(D);
+  llvm::DIGlobalVariableExpression *GVE =
+      DBuilder.createGlobalVariableExpression(
+          DContext, Name, StringRef(), Unit, getLineNumber(D->getLocation()),
+          Ty, false, false, nullptr, nullptr, nullptr, Align);
+  Var->addDebugInfo(GVE);
+}
+
 llvm::DIScope *CGDebugInfo::getCurrentContextDescriptor(const Decl *D) {
   if (!LexicalBlockStack.empty())
     return LexicalBlockStack.back();
   llvm::DIScope *Mod = getParentModuleOrNull(D);
   return getContextDescriptor(D, Mod ? Mod : TheCU);
 }
 
 void CGDebugInfo::EmitUsingDirective(const UsingDirectiveDecl &UD) {

clang/lib/CodeGen/CodeGenAction.cpp

public:
     void HandleTagDeclRequiredDefinition(const TagDecl *D) override {
       Gen->HandleTagDeclRequiredDefinition(D);
     }
 
     void CompleteTentativeDefinition(VarDecl *D) override {
       Gen->CompleteTentativeDefinition(D);
     }
 
+    void CompleteExternalDeclaration(VarDecl *D) override {
+      Gen->CompleteExternalDeclaration(D);
+    }
+
     void AssignInheritanceModel(CXXRecordDecl *RD) override {
       Gen->AssignInheritanceModel(RD);
     }
 
     void HandleVTable(CXXRecordDecl *RD) override {
       Gen->HandleVTable(RD);
     }
 

clang/lib/CodeGen/CodeGenModule.h

public:
   // passed in function.
   void getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap, GlobalDecl GD);
 
   StringRef getMangledName(GlobalDecl GD);
   StringRef getBlockMangledName(GlobalDecl GD, const BlockDecl *BD);
 
   void EmitTentativeDefinition(const VarDecl *D);
 
+  void EmitExternalDeclaration(const VarDecl *D);
+
   void EmitVTable(CXXRecordDecl *Class);
 
   void RefreshTypeCacheForClass(const CXXRecordDecl *Class);
 
   /// Appends Opts to the "llvm.linker.options" metadata value.
   void AppendLinkerOptions(StringRef Opts);
 
   /// Appends a detect mismatch command to the linker options.
void SetFunctionAttributes(GlobalDecl GD, llvm::Function *F,
                              bool IsIncompleteFunction, bool IsThunk);
 
   void EmitGlobalDefinition(GlobalDecl D, llvm::GlobalValue *GV = nullptr);
 
   void EmitGlobalFunctionDefinition(GlobalDecl GD, llvm::GlobalValue *GV);
   void EmitMultiVersionFunctionDefinition(GlobalDecl GD, llvm::GlobalValue *GV);
 
   void EmitGlobalVarDefinition(const VarDecl *D, bool IsTentative = false);
+  void EmitExternalVarDeclaration(const VarDecl *D);
   void EmitAliasDefinition(GlobalDecl GD);
   void emitIFuncDefinition(GlobalDecl GD);
   void emitCPUDispatchDefinition(GlobalDecl GD);
   void EmitObjCPropertyImplementations(const ObjCImplementationDecl *D);
   void EmitObjCIvarInitializations(ObjCImplementationDecl *D);
 
   // C++ related functions.
 

clang/lib/CodeGen/CodeGenModule.cpp

if (!MustBeEmitted(D) && !GV) {
       DeferredDecls[MangledName] = D;
       return;
   }
 
   // The tentative definition is the only definition.
   EmitGlobalVarDefinition(D);
 }
 
+void CodeGenModule::EmitExternalDeclaration(const VarDecl *D) {
+  EmitExternalVarDeclaration(D);
+}
+
 CharUnits CodeGenModule::GetTargetTypeStoreSize(llvm::Type *Ty) const {
   return Context.toCharUnitsFromBits(
       getDataLayout().getTypeStoreSizeInBits(Ty));
 }
 
 LangAS CodeGenModule::GetGlobalVarAddressSpace(const VarDecl *D) {
   LangAS AddrSpace = LangAS::Default;
   if (LangOpts.OpenCL) {
void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D,
   SanitizerMD->reportGlobalToASan(GV, *D, NeedsGlobalCtor);
 
   // Emit global variable debug information.
   if (CGDebugInfo *DI = getModuleDebugInfo())
     if (getCodeGenOpts().getDebugInfo() >= codegenoptions::LimitedDebugInfo)
       DI->EmitGlobalVariable(GV, D);
 }
 
+void CodeGenModule::EmitExternalVarDeclaration(const VarDecl *D) {
+  if (CGDebugInfo *DI = getModuleDebugInfo())
+    if (getCodeGenOpts().getDebugInfo() >= codegenoptions::LimitedDebugInfo) {
+      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);
+    }
+}
+
 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>())
     return true;
 

clang/lib/CodeGen/ModuleBuilder.cpp

public:
 
     void CompleteTentativeDefinition(VarDecl *D) override {
       if (Diags.hasErrorOccurred())
         return;
 
       Builder->EmitTentativeDefinition(D);
     }
 
+    void CompleteExternalDeclaration(VarDecl *D) override {
+      Builder->EmitExternalDeclaration(D);
+    }
+
     void HandleVTable(CXXRecordDecl *RD) override {
       if (Diags.hasErrorOccurred())
         return;
 
       Builder->EmitVTable(RD);
     }
   };
 }

clang/lib/Sema/Sema.cpp

for (TentativeDefinitionsType::iterator
     // No initialization is performed for a tentative definition.
     CheckCompleteVariableDeclaration(VD);
 
     // Notify the consumer that we've completed a tentative definition.
     if (!VD->isInvalidDecl())
       Consumer.CompleteTentativeDefinition(VD);
   }
 
+  for (auto D : ExternalDeclarations) {
+    if (!D || D->isInvalidDecl() || D->getPreviousDecl() || !D->isUsed())
+      continue;
+
+    Consumer.CompleteExternalDeclaration(D);
+  }
+
   // If there were errors, disable 'unused' warnings since they will mostly be
   // noise. Don't warn for a use from a module: either we should warn on all
   // file-scope declarations in modules or not at all, but whether the
   // declaration is used is immaterial.
   if (!Diags.hasErrorOccurred() && TUKind != TU_Module) {
     // Output warning for unused file scoped decls.
     for (UnusedFileScopedDeclsType::iterator
            I = UnusedFileScopedDecls.begin(ExternalSource),

clang/lib/Sema/SemaDecl.cpp

case VarDecl::DeclarationOnly:
                                  AbstractVariableType))
         Var->setInvalidDecl();
       if (!Type->isDependentType() && !Var->isInvalidDecl() &&
           Var->getStorageClass() == SC_PrivateExtern) {
         Diag(Var->getLocation(), diag::warn_private_extern);
         Diag(Var->getLocation(), diag::note_private_extern);
       }
 
+      if (Context.getTargetInfo().allowDebugInfoForExternalVar() &&
+          !Var->isInvalidDecl() && !getLangOpts().CPlusPlus)
+        ExternalDeclarations.push_back(Var);
+
       return;
 
     case VarDecl::TentativeDefinition:
       // File scope. C99 6.9.2p2: A declaration of an identifier for an
       // object that has file scope without an initializer, and without a
       // storage-class specifier or with the storage-class specifier "static",
       // constitutes a tentative definition. Note: A tentative definition with
       // external linkage is valid (C99 6.2.2p5).

clang/test/CodeGen/debug-info-extern-basic.c

+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+extern char ch;
+int test() {
+  return ch;
+}
+
+int test2() {
+  extern char ch2;
+  return ch2;
+}
+
+extern int (*foo)(int);
+int test3() {
+  return foo(0);
+}
+
+int test4() {
+  extern int (*foo2)(int);
+  return foo2(0);
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "ch",{{.*}} type: ![[CHART:[0-9]+]], isLocal: false, isDefinition: false
+// CHECK: distinct !DIGlobalVariable(name: "ch2",{{.*}} type: ![[CHART]], isLocal: false, isDefinition: false
+// CHECK: ![[CHART]] = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+
+// CHECK: distinct !DIGlobalVariable(name: "foo",{{.*}} type: ![[FUNC:[0-9]+]], isLocal: false, isDefinition: false)
+// CHECK: ![[FUNC]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[SUB:[0-9]+]], size: 64)
+// CHECK: ![[SUB]] = !DISubroutineType(types: ![[TYPES:[0-9]+]])
+// CHECK: ![[TYPES]] = !{![[BASET:[0-9]+]], ![[BASET]]}
+// CHECK: ![[BASET]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+
+// CHECK: distinct !DIGlobalVariable(name: "foo2",{{.*}} type: ![[FUNC]], isLocal: false, isDefinition: false)

clang/test/CodeGen/debug-info-extern-duplicate.c

+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+extern char ch;
+extern char ch;
+int test() {
+  return ch;
+}
+
+extern char ch2;
+int test2() {
+  extern char ch2;
+  return ch2;
+}
+
+int test3() {
+  extern char ch3;
+  extern char ch3;
+  return ch3;
+}
+
+extern int (*foo)(int);
+extern int (*foo)(int);
+int test4() {
+  return foo(0);
+}
+
+extern int (*foo2)(int);
+int test5() {
+  extern int (*foo2)(int);
+  return foo2(0);
+}
+
+int test6() {
+  extern int (*foo3)(int);
+  extern int (*foo3)(int);
+  return foo3(0);
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "ch",{{.*}} type: ![[T:[0-9]+]], isLocal: false, isDefinition: false
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch"
+
+// CHECK: distinct !DIGlobalVariable(name: "ch2",{{.*}} type: ![[T]], isLocal: false, isDefinition: false
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch2"
+
+// CHECK: distinct !DIGlobalVariable(name: "ch3",{{.*}} type: ![[T]], isLocal: false, isDefinition: false
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch2"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch3"
+
+// CHECK: distinct !DIGlobalVariable(name: "foo",{{.*}} type: ![[T2:[0-9]+]], isLocal: false, isDefinition: false
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch2"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch3"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "foo"
+
+// CHECK: distinct !DIGlobalVariable(name: "foo2",{{.*}} type: ![[T2]], isLocal: false, isDefinition: false
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch2"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch3"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "foo"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "foo2"
+
+// CHECK: distinct !DIGlobalVariable(name: "foo3",{{.*}} type: ![[T2]], isLocal: false, isDefinition: false
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch2"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch3"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "foo"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "foo2"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "foo3"

clang/test/CodeGen/debug-info-extern-multi.c

+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+extern char ch;
+int test() {
+  extern short sh;
+  return ch + sh;
+}
+
+extern char (*foo)(char);
+int test2() {
+  return foo(0) + ch;
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "ch",{{.*}} type: ![[Tch:[0-9]+]], isLocal: false, isDefinition: false
+// CHECK: distinct !DIGlobalVariable(name: "sh",{{.*}} type: ![[Tsh:[0-9]+]], isLocal: false, isDefinition: false
+// CHECK: ![[Tsh]] = !DIBasicType(name: "short", size: 16, encoding: DW_ATE_signed)
+
+// CHECK: distinct !DIGlobalVariable(name: "foo",{{.*}} type: ![[Tptr:[0-9]+]], isLocal: false, isDefinition: false
+// ![[Tptr]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[Tsub:[0-9]+]], size: 64)
+// ![[Tsub]] = !DISubroutineType(types: ![[Tproto:[0-9]+]])
+// ![[Tproto]] = !{![[Tch]], ![[Tch]]}
+// CHECK: ![[Tch]] = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)

clang/test/CodeGen/debug-info-extern-unused.c

+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+extern char ch;
+int test() {
+  return 0;
+}
+
+int test2() {
+  extern char ch2;
+  return 0;
+}
+
+extern int (*foo)(int);
+int test3() {
+  return 0;
+}
+
+int test4() {
+  extern int (*foo2)(int);
+  return 0;
+}
+
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch2"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "foo"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "foo2"

llvm/include/llvm/IR/DIBuilder.h

public:
     ///                      externally visible or not.
     /// \param Expr        The location of the global relative to the attached
     ///                    GlobalVariable.
     /// \param Decl        Reference to the corresponding declaration.
     /// \param AlignInBits Variable alignment(or 0 if no alignment attr was
     ///                    specified)
     DIGlobalVariableExpression *createGlobalVariableExpression(
         DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *File,
-        unsigned LineNo, DIType *Ty, bool isLocalToUnit,
+        unsigned LineNo, DIType *Ty, bool isLocalToUnit, bool isDefined = true,
         DIExpression *Expr = nullptr, MDNode *Decl = nullptr,
         MDTuple *templateParams = nullptr, uint32_t AlignInBits = 0);
 
     /// Identical to createGlobalVariable
     /// except that the resulting DbgNode is temporary and meant to be RAUWed.
     DIGlobalVariable *createTempGlobalVariableFwdDecl(
         DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *File,
         unsigned LineNo, DIType *Ty, bool isLocalToUnit, MDNode *Decl = nullptr,

llvm/lib/IR/DIBuilder.cpp

if (auto *CT =
           dyn_cast_or_null<DICompositeType>(getNonCompileUnitScope(Context)))
     assert(CT->getIdentifier().empty() &&
            "Context of a global variable should not be a type with identifier");
 #endif
 }
 
 DIGlobalVariableExpression *DIBuilder::createGlobalVariableExpression(
     DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *F,
-    unsigned LineNumber, DIType *Ty, bool isLocalToUnit, DIExpression *Expr,
+    unsigned LineNumber, DIType *Ty, bool isLocalToUnit,
+    bool isDefined, DIExpression *Expr,
     MDNode *Decl, MDTuple *templateParams, uint32_t AlignInBits) {
   checkGlobalVariableScope(Context);
 
   auto *GV = DIGlobalVariable::getDistinct(
       VMContext, cast_or_null<DIScope>(Context), Name, LinkageName, F,
-      LineNumber, Ty, isLocalToUnit, true, cast_or_null<DIDerivedType>(Decl),
+      LineNumber, Ty, isLocalToUnit, isDefined, cast_or_null<DIDerivedType>(Decl),
       templateParams, AlignInBits);
   if (!Expr)
     Expr = createExpression();
   auto *N = DIGlobalVariableExpression::get(VMContext, GV, Expr);
   AllGVs.push_back(N);
   return N;
 }
 

llvm/lib/IR/DebugInfo.cpp

 LLVMMetadataRef LLVMDIBuilderCreateGlobalVariableExpression(
     LLVMDIBuilderRef Builder, LLVMMetadataRef Scope, const char *Name,
     size_t NameLen, const char *Linkage, size_t LinkLen, LLVMMetadataRef File,
     unsigned LineNo, LLVMMetadataRef Ty, LLVMBool LocalToUnit,
     LLVMMetadataRef Expr, LLVMMetadataRef Decl, uint32_t AlignInBits) {
   return wrap(unwrap(Builder)->createGlobalVariableExpression(
       unwrapDI<DIScope>(Scope), {Name, NameLen}, {Linkage, LinkLen},
       unwrapDI<DIFile>(File), LineNo, unwrapDI<DIType>(Ty), LocalToUnit,
-      unwrap<DIExpression>(Expr), unwrapDI<MDNode>(Decl),
+      true, unwrap<DIExpression>(Expr), unwrapDI<MDNode>(Decl),
       nullptr, AlignInBits));
 }
 
 LLVMMetadataRef LLVMDIGlobalVariableExpressionGetVariable(LLVMMetadataRef GVE) {
   return wrap(unwrapDI<DIGlobalVariableExpression>(GVE)->getVariable());
 }
 
 LLVMMetadataRef LLVMDIGlobalVariableExpressionGetExpression(

llvm/unittests/Transforms/Utils/CloningTest.cpp

void CreateOldModule() {
     GV->addDebugInfo(GVExpression);
 
     // DIGlobalVariableExpression not attached to any global variable
     auto Expr = DBuilder.createExpression(
         ArrayRef<uint64_t>{dwarf::DW_OP_constu, 42U, dwarf::DW_OP_stack_value});
 
     DBuilder.createGlobalVariableExpression(
         Subprogram, "unattached", "unattached", File, 1,
-        DBuilder.createNullPtrType(), false, Expr);
+        DBuilder.createNullPtrType(), false, true, Expr);
 
     auto *Entry = BasicBlock::Create(C, "", F);
     IBuilder.SetInsertPoint(Entry);
     IBuilder.CreateRetVoid();
 
     // Finalize the debug info
     DBuilder.finalize();
   }