[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

Jac1494 added a comment. 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 extern int test(); extern int var; int main() { var++; printf("%d\n",test()); } If we debug above case with gdb

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song added a comment. @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 thi

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. In D70696#1774931 , @Jac1494 wrote: > For some real life case like below we need debuginfo for declaration of > global extern variable . > > $cat s

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song marked an inline comment as done. yonghong-song added inline comments. Comment at: clang/test/CodeGen/debug-info-extern-basic.c:14-23 +extern int (*foo)(int); +int test3() { + return foo(0); +} + +int test4() { + extern int (*foo2)(int); dblaikie

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song updated this revision to Diff 232980. yonghong-song added a comment. remove some redundant test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70696/new/ https://reviews.llvm.org/D70696 Files: clang/include/clang/AST/ASTConsum

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

dblaikie added inline comments. Comment at: clang/test/CodeGen/debug-info-extern-basic.c:14-23 +extern int (*foo)(int); +int test3() { + return foo(0); +} + +int test4() { + extern int (*foo2)(int); yonghong-song wrote: > dblaikie wrote: > > What do these tests

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

This revision was automatically updated to reflect the committed changes. Closed by commit rGd77ae1552fc2: [DebugInfo] Support to emit debugInfo for extern variables (authored by yonghong-song). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70696/new

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

rnk added a comment. 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 s

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

rnk added a comment. 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,

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song added a comment. @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 fo

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

dblaikie added a comment. 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-exter

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song added a comment. > 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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

dblaikie added a comment. 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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song added a comment. @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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

dblaikie added a comment. 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/5128026467cbc17

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song created this revision. yonghong-song added reviewers: dblaikie, aprantl, RKSimon. yonghong-song added a project: debug-info. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. Extern variable usage in BPF is different from traditional

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song added a comment. The corresponding BPF patch to consume the debuginfo https://reviews.llvm.org/D70697. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70696/new/ https://reviews.llvm.org/D70696 __

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

SouraVX added a comment. 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, DebugInf

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song added a comment. @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 r

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

SouraVX added a comment. 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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

umesh.kalappa0 added a comment. @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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

SouraVX added a comment. 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.

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

aprantl added inline comments. Comment at: clang/lib/Sema/Sema.cpp:1140 + for (auto D: ExternalDeclarations) { +if (!D || D->isInvalidDecl() || D->getPreviousDecl() || !D->isUsed()) clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

probinson added a comment. 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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song marked an inline comment as done. yonghong-song added inline comments. Comment at: clang/lib/Sema/Sema.cpp:1140 + for (auto D: ExternalDeclarations) { +if (!D || D->isInvalidDecl() || D->getPreviousDecl() || !D->isUsed()) aprantl wrote: > cla

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

dblaikie added a comment. 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_va

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song added a comment. @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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song added a comment. @dblaikie Good points. I will guard external variable debug info generation under `-fstandalone-debug` flag. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70696/new/ https://reviews.llvm.org/D70696 ___

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

probinson added a comment. 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 cur

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

SouraVX added a comment. 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 curre

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

dblaikie added a comment. 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 bein

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

dblaikie added a comment. 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 curr

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

probinson added a comment. In D70696#1765671 , @dblaikie wrote: > In D70696#1765637 , @probinson wrote: > > > For the case: > > > > cat ref.c > > extern int global_var; > > int main() { return global_v

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

dblaikie added a comment. In D70696#1765784 , @probinson wrote: > In D70696#1765671 , @dblaikie wrote: > > > In D70696#1765637 , @probinson > > wrote: > > > > > For the case

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

dblaikie added a comment. In D70696#1765789 , @dblaikie wrote: > In D70696#1765784 , @probinson wrote: > > > In D70696#1765671 , @dblaikie > > wrote: > > > > > In D70696#176

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

SouraVX added a comment. @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 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70696/new/ htt

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

SouraVX added a comment. 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 g

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

dblaikie added a comment. 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 s

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song added a comment. @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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song updated this revision to Diff 231786. yonghong-song edited the summary of this revision. yonghong-song added a comment. 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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

dblaikie added a comment. 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 d

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song added a comment. 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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song updated this revision to Diff 232426. yonghong-song added a comment. consolidated into less test files, added more test cases along the way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70696/new/ https://reviews.llvm.org/D70696 Fil

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

yonghong-song added a comment. @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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev