reland "[DebugInfo] Support to emit debugInfo for extern variables"

[yonghong-song]

Summary

Commit d77ae1552fc21a9f3877f3ed7e13d631f517c825
("[DebugInfo] Support to emit debugInfo for extern variables")
added deebugInfo for extern variables for BPF target.
The commit is reverted by 891e25b02d760d0de18c7d46947913b3166047e7
as the committed tests using %clang instead of %clang_cc1 causing
test failed in certain scenarios as reported by Reid Kleckner.

This patch fixed the tests by using %clang_cc1.

[yonghong-song]

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 D70696 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 only at linux now.
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.

[rnk]

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

2

Clang does not require a registered target to emit LLVM IR for a target. Please remove this, so that these tests actually run and cover the code under test in most configurations.

[yonghong-song]

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

2

Let me try.

[yonghong-song]

Removed REQUIRES: bpf-registered-target. This is not needed any more if using %clang_cc1. Tested by build clang with and without BPF target.

[rnk]

I patched this in, and the tests don't pass for me locally, so we're back to where we started. Go ahead and land it, I guess. I'm kind of not happy that we had to poke another virtual method through the ASTConsumer interface to make this happen.

[yonghong-song]

@rnk I somehow cannot reproduce the issue locally. If you can let me know how to exactly reproduce the issue, I might be able to also help with some kind of solution, although frankly speaking, I am not a clang expert.

[rnk]

+@rsmith @rjmccall
For more meaningful feedback on the code structure.

clang/include/clang/Sema/Sema.h

671–672

This name seems a bit vague. This is a member of Sema, essentially a global variable, for this very corner-case feature (BPF). I'm not convinced that you couldn't have done this without changing Sema.

[rnk]

You forgot to update MultiplexConsumer:

diff --git a/clang/include/clang/Frontend/MultiplexConsumer.h b/clang/include/clang/Frontend/MultiplexConsumer.h
index ca6ed8310ae..3054e184281 100644
--- a/clang/include/clang/Frontend/MultiplexConsumer.h
+++ b/clang/include/clang/Frontend/MultiplexConsumer.h
@@ -65,6 +65,7 @@ public:
   void HandleTopLevelDeclInObjCContainer(DeclGroupRef D) override;
   void HandleImplicitImportDecl(ImportDecl *D) override;
   void CompleteTentativeDefinition(VarDecl *D) override;
+  void CompleteExternalDeclaration(VarDecl *D) override;
   void AssignInheritanceModel(CXXRecordDecl *RD) override;
   void HandleVTable(CXXRecordDecl *RD) override;
   ASTMutationListener *GetASTMutationListener() override;
diff --git a/clang/lib/Frontend/MultiplexConsumer.cpp b/clang/lib/Frontend/MultiplexConsumer.cpp
index 04e896296c9..5abbb3a235b 100644
--- a/clang/lib/Frontend/MultiplexConsumer.cpp
+++ b/clang/lib/Frontend/MultiplexConsumer.cpp
@@ -322,6 +322,11 @@ void MultiplexConsumer::CompleteTentativeDefinition(VarDecl *D) {
     Consumer->CompleteTentativeDefinition(D);
 }
 
+void MultiplexConsumer::CompleteExternalDeclaration(VarDecl *D) {
+  for (auto &Consumer : Consumers)
+    Consumer->CompleteExternalDeclaration(D);
+}
+
 void MultiplexConsumer::AssignInheritanceModel(CXXRecordDecl *RD) {
   for (auto &Consumer : Consumers)
     Consumer->AssignInheritanceModel(RD);

That seems to fix this. I'm curious why my build of clang uses this.

[rnk]

Ah, I see, these tests will fail if you link in some clang plugins, which I do for Chrome. So, this only affects me. Anyway, I'll fix it.

[yonghong-song]

@rnk Great to know the reason. Thanks!