Page MenuHomePhabricator

[DebugInfo] Support to emit debugInfo for extern variables
ClosedPublic

Authored by yonghong-song on Nov 25 2019, 2:08 PM.

Details

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:

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.

Diff Detail

Event Timeline

yonghong-song created this revision.Nov 25 2019, 2:08 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 25 2019, 2:08 PM

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

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.

@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.

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 added a subscriber: umesh.kalappa0.EditedNov 30 2019, 6:25 AM

@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 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 added inline comments.Dec 2 2019, 10:24 AM
clang/lib/Sema/Sema.cpp
1140

clang-format

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 marked an inline comment as done.Dec 2 2019, 11:22 AM
yonghong-song added inline comments.
clang/lib/Sema/Sema.cpp
1140

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

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.

@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.

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

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.

@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 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)

@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).

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.

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).

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)

@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 , 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)

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

@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 edited the summary of this revision. (Show Details)

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.

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?

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=23PtwaZs-yAyafmXC9g@mail.gmail.com/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.

  1. https://lore.kernel.org/bpf/87eez4odqp.fsf@toke.dk/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.

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

@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.

ast added a subscriber: ast.Dec 6 2019, 6:42 PM
Jac1494 added a subscriber: Jac1494.EditedDec 9 2019, 4:13 AM

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;
 }

@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 accepted this revision.Dec 9 2019, 4:50 PM

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
15–24

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
5–39

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

This revision is now accepted and ready to land.Dec 9 2019, 4:50 PM
yonghong-song marked an inline comment as done.Dec 9 2019, 5:18 PM
yonghong-song added inline comments.
clang/test/CodeGen/debug-info-extern-basic.c
15–24

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.

remove some redundant test cases

dblaikie added inline comments.Dec 9 2019, 6:01 PM
clang/test/CodeGen/debug-info-extern-basic.c
15–24

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)

This revision was automatically updated to reflect the committed changes.
rnk added a subscriber: rnk.Dec 22 2019, 12:40 PM

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 added a comment.Dec 22 2019, 12:57 PM

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.

@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.

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?

@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)

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.

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?))

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