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
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=23ptwazs-yayafmx...@mail.gmail.com/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/87eez4odqp....@toke.dk/T/#m8d5c3e87ffe7f2764e02d722cb0d8cbc136880ed

This patch added clang support to emit debuginfo for extern variables
with a TargetInfo hook to enable it. 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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70696

Files:
  clang/include/clang/AST/ASTConsumer.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/Targets/BPF.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ModuleBuilder.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGenCXX/debug-info-extern-duplicate-2.c
  clang/test/CodeGenCXX/debug-info-extern-duplicate.c
  clang/test/CodeGenCXX/debug-info-extern-unused.c
  clang/test/CodeGenCXX/debug-info-extern-var-char-2.c
  clang/test/CodeGenCXX/debug-info-extern-var-char.c
  clang/test/CodeGenCXX/debug-info-extern-var-func.c
  clang/test/CodeGenCXX/debug-info-extern-var-multi.c
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/unittests/Transforms/Utils/CloningTest.cpp

Index: llvm/unittests/Transforms/Utils/CloningTest.cpp
===================================================================
--- llvm/unittests/Transforms/Utils/CloningTest.cpp
+++ llvm/unittests/Transforms/Utils/CloningTest.cpp
@@ -764,7 +764,7 @@
 
     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);
Index: llvm/lib/IR/DebugInfo.cpp
===================================================================
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -1290,7 +1290,7 @@
   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));
 }
 
Index: llvm/lib/IR/DIBuilder.cpp
===================================================================
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -639,13 +639,14 @@
 
 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();
Index: llvm/include/llvm/IR/DIBuilder.h
===================================================================
--- llvm/include/llvm/IR/DIBuilder.h
+++ llvm/include/llvm/IR/DIBuilder.h
@@ -581,7 +581,7 @@
     ///                    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);
 
Index: clang/test/CodeGenCXX/debug-info-extern-var-multi.c
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-extern-var-multi.c
@@ -0,0 +1,13 @@
+// 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;
+}
+
+// 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: ![[Tch]] = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
Index: clang/test/CodeGenCXX/debug-info-extern-var-func.c
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-extern-var-func.c
@@ -0,0 +1,13 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+extern int (*foo)(int);
+int test() {
+  return foo(0);
+}
+
+// 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)
Index: clang/test/CodeGenCXX/debug-info-extern-var-char.c
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-extern-var-char.c
@@ -0,0 +1,10 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+extern char ch;
+int test() {
+  return ch;
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "ch",{{.*}} type: ![[T:[0-9]+]], isLocal: false, isDefinition: false
+// CHECK: ![[T]] = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
Index: clang/test/CodeGenCXX/debug-info-extern-var-char-2.c
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-extern-var-char-2.c
@@ -0,0 +1,10 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+int test() {
+  extern char ch;
+  return ch;
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "ch",{{.*}} type: ![[T:[0-9]+]], isLocal: false, isDefinition: false
+// CHECK: ![[T]] = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
Index: clang/test/CodeGenCXX/debug-info-extern-unused.c
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-extern-unused.c
@@ -0,0 +1,9 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+extern char ch;
+int test() {
+  return 0;
+}
+
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch"
Index: clang/test/CodeGenCXX/debug-info-extern-duplicate.c
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-extern-duplicate.c
@@ -0,0 +1,12 @@
+// 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;
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "ch",{{.*}} type: ![[T:[0-9]+]], isLocal: false, isDefinition: false
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch"
+// CHECK: ![[T]] = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
Index: clang/test/CodeGenCXX/debug-info-extern-duplicate-2.c
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-extern-duplicate-2.c
@@ -0,0 +1,12 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+extern char ch;
+int test() {
+  extern char ch;
+  return ch;
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "ch",{{.*}} type: ![[T:[0-9]+]], isLocal: false, isDefinition: false
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch"
+// CHECK: ![[T]] = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12149,6 +12149,11 @@
         Diag(Var->getLocation(), diag::note_private_extern);
       }
 
+      if (Context.getTargetInfo().allowDebugInfoForExternalVar() &&
+          !Var->isInvalidDecl() &&
+          !getLangOpts().CPlusPlus)
+        ExternalDeclarations.push_back(Var);
+
       return;
 
     case VarDecl::TentativeDefinition:
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1137,6 +1137,13 @@
       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
Index: clang/lib/CodeGen/ModuleBuilder.cpp
===================================================================
--- clang/lib/CodeGen/ModuleBuilder.cpp
+++ clang/lib/CodeGen/ModuleBuilder.cpp
@@ -290,6 +290,10 @@
       Builder->EmitTentativeDefinition(D);
     }
 
+    void CompleteExternalDeclaration(VarDecl *D) override {
+      Builder->EmitExternalDeclaration(D);
+    }
+
     void HandleVTable(CXXRecordDecl *RD) override {
       if (Diags.hasErrorOccurred())
         return;
Index: clang/lib/CodeGen/CodeGenModule.h
===================================================================
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1163,6 +1163,8 @@
 
   void EmitTentativeDefinition(const VarDecl *D);
 
+  void EmitExternalDeclaration(const VarDecl *D);
+
   void EmitVTable(CXXRecordDecl *Class);
 
   void RefreshTypeCacheForClass(const CXXRecordDecl *Class);
@@ -1398,6 +1400,7 @@
   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);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3714,6 +3714,10 @@
   EmitGlobalVarDefinition(D);
 }
 
+void CodeGenModule::EmitExternalDeclaration(const VarDecl *D) {
+  EmitExternalVarDeclaration(D);
+}
+
 CharUnits CodeGenModule::GetTargetTypeStoreSize(llvm::Type *Ty) const {
   return Context.toCharUnitsFromBits(
       getDataLayout().getTypeStoreSizeInBits(Ty));
@@ -4097,6 +4101,19 @@
       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) {
Index: clang/lib/CodeGen/CodeGenAction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -331,6 +331,10 @@
       Gen->CompleteTentativeDefinition(D);
     }
 
+    void CompleteExternalDeclaration(VarDecl *D) override {
+      Gen->CompleteExternalDeclaration(D);
+    }
+
     void AssignInheritanceModel(CXXRecordDecl *RD) override {
       Gen->AssignInheritanceModel(RD);
     }
Index: clang/lib/CodeGen/CGDebugInfo.h
===================================================================
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -477,6 +477,9 @@
   /// 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);
 
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===================================================================
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4487,7 +4487,7 @@
 
     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);
@@ -4590,10 +4590,29 @@
 
   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();
Index: clang/lib/Basic/Targets/BPF.h
===================================================================
--- clang/lib/Basic/Targets/BPF.h
+++ clang/lib/Basic/Targets/BPF.h
@@ -76,6 +76,8 @@
     return None;
   }
 
+  bool allowDebugInfoForExternalVar() const override { return true; }
+
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override {
     switch (CC) {
     default:
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -666,6 +666,9 @@
   /// 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;
Index: clang/include/clang/Basic/TargetInfo.h
===================================================================
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1395,6 +1395,9 @@
 
   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);
Index: clang/include/clang/AST/ASTConsumer.h
===================================================================
--- clang/include/clang/AST/ASTConsumer.h
+++ clang/include/clang/AST/ASTConsumer.h
@@ -102,6 +102,11 @@
   /// 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) {}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to