https://github.com/mejedi updated 
https://github.com/llvm/llvm-project/pull/91310

>From a9682a1eefc360c9f0e517601d0d8cc4b5e9f16f Mon Sep 17 00:00:00 2001
From: Nick Zavaritsky <nick.zavarit...@emnify.com>
Date: Sun, 5 May 2024 10:20:52 +0000
Subject: [PATCH] [BPF] Fix linking issues in static map initializers

When BPF object files are linked with bpftool, every symbol must be
accompanied by BTF info. Ensure that extern functions referenced by
global variable initializers are included in BTF.

The primary motivation is "static" initialization of PROG maps:

  extern int elsewhere(struct xdp_md *);

  struct {
    __uint(type, BPF_MAP_TYPE_PROG_ARRAY);
    __uint(max_entries, 1);
    __type(key, int);
    __type(value, int);
    __array(values, int (struct xdp_md *));
  } prog_map SEC(".maps") = { .values = { elsewhere } };

BPF backend needs debug info to produce BTF. Debug info is not
normally generated for external variables and functions. Previously, it
was solved differently for variables (collecting variable declarations
in ExternalDeclarations vector) and functions (logic invoked during
codegen in CGExpr.cpp).

This patch generalises ExternalDefclarations to include both function
and variable declarations. This change ensures that function references
are not missed no matter the context. Previously external functions
referenced in constant expressions lacked debug info.
---
 clang/include/clang/AST/ASTConsumer.h         |  3 +-
 .../clang/Frontend/MultiplexConsumer.h        |  2 +-
 clang/include/clang/Sema/Sema.h               |  2 +-
 clang/lib/CodeGen/BackendConsumer.h           |  2 +-
 clang/lib/CodeGen/CGExpr.cpp                  | 17 +-----
 clang/lib/CodeGen/CodeGenAction.cpp           |  2 +-
 clang/lib/CodeGen/CodeGenModule.cpp           | 19 ++++++-
 clang/lib/CodeGen/CodeGenModule.h             |  3 +-
 clang/lib/CodeGen/ModuleBuilder.cpp           |  2 +-
 clang/lib/Frontend/MultiplexConsumer.cpp      |  2 +-
 clang/lib/Interpreter/IncrementalParser.cpp   |  2 +-
 clang/lib/Sema/SemaDecl.cpp                   |  8 +++
 .../test/CodeGen/bpf-debug-info-extern-func.c |  9 ++++
 clang/test/CodeGen/bpf-debug-info-unref.c     | 11 ++++
 llvm/lib/Target/BPF/BTFDebug.cpp              | 23 ++++++++
 llvm/lib/Target/BPF/BTFDebug.h                |  4 ++
 llvm/test/CodeGen/BPF/BTF/extern-var-func2.ll | 54 +++++++++++++++++++
 17 files changed, 139 insertions(+), 26 deletions(-)
 create mode 100644 clang/test/CodeGen/bpf-debug-info-extern-func.c
 create mode 100644 clang/test/CodeGen/bpf-debug-info-unref.c
 create mode 100644 llvm/test/CodeGen/BPF/BTF/extern-var-func2.ll

diff --git a/clang/include/clang/AST/ASTConsumer.h 
b/clang/include/clang/AST/ASTConsumer.h
index ebcd8059284d8..447f2592d2359 100644
--- a/clang/include/clang/AST/ASTConsumer.h
+++ b/clang/include/clang/AST/ASTConsumer.h
@@ -23,6 +23,7 @@ namespace clang {
   class ASTDeserializationListener; // layering violation because void* is ugly
   class SemaConsumer; // layering violation required for safe SemaConsumer
   class TagDecl;
+  class DeclaratorDecl;
   class VarDecl;
   class FunctionDecl;
   class ImportDecl;
@@ -105,7 +106,7 @@ class ASTConsumer {
   /// 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) {}
+  virtual void CompleteExternalDeclaration(DeclaratorDecl *D) {}
 
   /// Callback invoked when an MSInheritanceAttr has been attached to a
   /// CXXRecordDecl.
diff --git a/clang/include/clang/Frontend/MultiplexConsumer.h 
b/clang/include/clang/Frontend/MultiplexConsumer.h
index 4ed0d86d3cdfb..e49e3392d1f31 100644
--- a/clang/include/clang/Frontend/MultiplexConsumer.h
+++ b/clang/include/clang/Frontend/MultiplexConsumer.h
@@ -67,7 +67,7 @@ class MultiplexConsumer : public SemaConsumer {
   void HandleTopLevelDeclInObjCContainer(DeclGroupRef D) override;
   void HandleImplicitImportDecl(ImportDecl *D) override;
   void CompleteTentativeDefinition(VarDecl *D) override;
-  void CompleteExternalDeclaration(VarDecl *D) override;
+  void CompleteExternalDeclaration(DeclaratorDecl *D) override;
   void AssignInheritanceModel(CXXRecordDecl *RD) override;
   void HandleVTable(CXXRecordDecl *RD) override;
   ASTMutationListener *GetASTMutationListener() override;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index fb3a5d25c635c..75a80540dbcbf 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3098,7 +3098,7 @@ class Sema final : public SemaBase {
   TentativeDefinitionsType TentativeDefinitions;
 
   /// All the external declarations encoutered and used in the TU.
-  SmallVector<VarDecl *, 4> ExternalDeclarations;
+  SmallVector<DeclaratorDecl *, 4> ExternalDeclarations;
 
   /// Generally null except when we temporarily switch decl contexts,
   /// like in \see SemaObjC::ActOnObjCTemporaryExitContainerContext.
diff --git a/clang/lib/CodeGen/BackendConsumer.h 
b/clang/lib/CodeGen/BackendConsumer.h
index a648bd314e501..a023d29cbd1d7 100644
--- a/clang/lib/CodeGen/BackendConsumer.h
+++ b/clang/lib/CodeGen/BackendConsumer.h
@@ -107,7 +107,7 @@ class BackendConsumer : public ASTConsumer {
   void HandleTagDeclDefinition(TagDecl *D) override;
   void HandleTagDeclRequiredDefinition(const TagDecl *D) override;
   void CompleteTentativeDefinition(VarDecl *D) override;
-  void CompleteExternalDeclaration(VarDecl *D) override;
+  void CompleteExternalDeclaration(DeclaratorDecl *D) override;
   void AssignInheritanceModel(CXXRecordDecl *RD) override;
   void HandleVTable(CXXRecordDecl *RD) override;
 
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 23e5deee32581..039f60c774591 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3141,21 +3141,8 @@ LValue CodeGenFunction::EmitDeclRefLValue(const 
DeclRefExpr *E) {
     return LV;
   }
 
-  if (const auto *FD = dyn_cast<FunctionDecl>(ND)) {
-    LValue LV = EmitFunctionDeclLValue(*this, E, FD);
-
-    // Emit debuginfo for the function declaration if the target wants to.
-    if (getContext().getTargetInfo().allowDebugInfoForExternalRef()) {
-      if (CGDebugInfo *DI = CGM.getModuleDebugInfo()) {
-        auto *Fn =
-            cast<llvm::Function>(LV.getPointer(*this)->stripPointerCasts());
-        if (!Fn->getSubprogram())
-          DI->EmitFunctionDecl(FD, FD->getLocation(), T, Fn);
-      }
-    }
-
-    return LV;
-  }
+  if (const auto *FD = dyn_cast<FunctionDecl>(ND))
+    return EmitFunctionDeclLValue(*this, E, FD);
 
   // FIXME: While we're emitting a binding from an enclosing scope, all other
   // DeclRefExprs we see should be implicitly treated as if they also refer to
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp 
b/clang/lib/CodeGen/CodeGenAction.cpp
index 0b92c5318a5c2..e87226e60297c 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -376,7 +376,7 @@ void BackendConsumer::CompleteTentativeDefinition(VarDecl 
*D) {
   Gen->CompleteTentativeDefinition(D);
 }
 
-void BackendConsumer::CompleteExternalDeclaration(VarDecl *D) {
+void BackendConsumer::CompleteExternalDeclaration(DeclaratorDecl *D) {
   Gen->CompleteExternalDeclaration(D);
 }
 
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 99e986d371cac..dc9dd034dee7b 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5185,8 +5185,11 @@ void CodeGenModule::EmitTentativeDefinition(const 
VarDecl *D) {
   EmitGlobalVarDefinition(D);
 }
 
-void CodeGenModule::EmitExternalDeclaration(const VarDecl *D) {
-  EmitExternalVarDeclaration(D);
+void CodeGenModule::EmitExternalDeclaration(const DeclaratorDecl *D) {
+  if (auto const *V = dyn_cast<const VarDecl>(D))
+    EmitExternalVarDeclaration(V);
+  if (auto const *FD = dyn_cast<const FunctionDecl>(D))
+    EmitExternalFunctionDeclaration(FD);
 }
 
 CharUnits CodeGenModule::GetTargetTypeStoreSize(llvm::Type *Ty) const {
@@ -5622,6 +5625,18 @@ void CodeGenModule::EmitExternalVarDeclaration(const 
VarDecl *D) {
     }
 }
 
+void CodeGenModule::EmitExternalFunctionDeclaration(const FunctionDecl *FD) {
+  if (CGDebugInfo *DI = getModuleDebugInfo())
+    if (getCodeGenOpts().hasReducedDebugInfo()) {
+      auto *Ty = getTypes().ConvertType(FD->getType());
+      StringRef MangledName = getMangledName(FD);
+      auto *Fn = dyn_cast<llvm::Function>(
+          GetOrCreateLLVMFunction(MangledName, Ty, FD, /* ForVTable */ false));
+      if (!Fn->getSubprogram())
+        DI->EmitFunctionDecl(FD, FD->getLocation(), FD->getType(), Fn);
+    }
+}
+
 static bool isVarDeclStrongDefinition(const ASTContext &Context,
                                       CodeGenModule &CGM, const VarDecl *D,
                                       bool NoCommon) {
diff --git a/clang/lib/CodeGen/CodeGenModule.h 
b/clang/lib/CodeGen/CodeGenModule.h
index 4796d421aaa69..0444f9f8449f8 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1338,7 +1338,7 @@ class CodeGenModule : public CodeGenTypeCache {
 
   void EmitTentativeDefinition(const VarDecl *D);
 
-  void EmitExternalDeclaration(const VarDecl *D);
+  void EmitExternalDeclaration(const DeclaratorDecl *D);
 
   void EmitVTable(CXXRecordDecl *Class);
 
@@ -1690,6 +1690,7 @@ class CodeGenModule : public CodeGenTypeCache {
 
   void EmitGlobalVarDefinition(const VarDecl *D, bool IsTentative = false);
   void EmitExternalVarDeclaration(const VarDecl *D);
+  void EmitExternalFunctionDeclaration(const FunctionDecl *D);
   void EmitAliasDefinition(GlobalDecl GD);
   void emitIFuncDefinition(GlobalDecl GD);
   void emitCPUDispatchDefinition(GlobalDecl GD);
diff --git a/clang/lib/CodeGen/ModuleBuilder.cpp 
b/clang/lib/CodeGen/ModuleBuilder.cpp
index df85295cfb2e2..d4e0ab0339a8b 100644
--- a/clang/lib/CodeGen/ModuleBuilder.cpp
+++ b/clang/lib/CodeGen/ModuleBuilder.cpp
@@ -310,7 +310,7 @@ namespace {
       Builder->EmitTentativeDefinition(D);
     }
 
-    void CompleteExternalDeclaration(VarDecl *D) override {
+    void CompleteExternalDeclaration(DeclaratorDecl *D) override {
       Builder->EmitExternalDeclaration(D);
     }
 
diff --git a/clang/lib/Frontend/MultiplexConsumer.cpp 
b/clang/lib/Frontend/MultiplexConsumer.cpp
index 8fdc7f55a5003..651c55aeed540 100644
--- a/clang/lib/Frontend/MultiplexConsumer.cpp
+++ b/clang/lib/Frontend/MultiplexConsumer.cpp
@@ -357,7 +357,7 @@ void MultiplexConsumer::CompleteTentativeDefinition(VarDecl 
*D) {
     Consumer->CompleteTentativeDefinition(D);
 }
 
-void MultiplexConsumer::CompleteExternalDeclaration(VarDecl *D) {
+void MultiplexConsumer::CompleteExternalDeclaration(DeclaratorDecl *D) {
   for (auto &Consumer : Consumers)
     Consumer->CompleteExternalDeclaration(D);
 }
diff --git a/clang/lib/Interpreter/IncrementalParser.cpp 
b/clang/lib/Interpreter/IncrementalParser.cpp
index a8d0294fb6151..b7c809c45098c 100644
--- a/clang/lib/Interpreter/IncrementalParser.cpp
+++ b/clang/lib/Interpreter/IncrementalParser.cpp
@@ -79,7 +79,7 @@ class IncrementalASTConsumer final : public ASTConsumer {
   void CompleteTentativeDefinition(VarDecl *D) override final {
     Consumer->CompleteTentativeDefinition(D);
   }
-  void CompleteExternalDeclaration(VarDecl *D) override final {
+  void CompleteExternalDeclaration(DeclaratorDecl *D) override final {
     Consumer->CompleteExternalDeclaration(D);
   }
   void AssignInheritanceModel(CXXRecordDecl *RD) override final {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index b3bfdacb01790..aa44608035538 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -10818,6 +10818,14 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, 
DeclContext *DC,
       break;
     }
 
+  // Similar to no_builtin logic above, at this point of the code
+  // FunctionDecl::isThisDeclarationADefinition() always returns `false`
+  // because Sema::ActOnStartOfFunctionDef has not been called yet.
+  if (Context.getTargetInfo().allowDebugInfoForExternalRef() &&
+      !NewFD->isInvalidDecl() &&
+      D.getFunctionDefinitionKind() == FunctionDefinitionKind::Declaration)
+    ExternalDeclarations.push_back(NewFD);
+
   return NewFD;
 }
 
diff --git a/clang/test/CodeGen/bpf-debug-info-extern-func.c 
b/clang/test/CodeGen/bpf-debug-info-extern-func.c
new file mode 100644
index 0000000000000..e87c8bea3a878
--- /dev/null
+++ b/clang/test/CodeGen/bpf-debug-info-extern-func.c
@@ -0,0 +1,9 @@
+// RUN: %clang -g -target bpf -S -emit-llvm %s -o - | FileCheck %s
+//
+// When linking BPF object files via bpftool, BTF info is required for
+// every symbol. BTF is generated from debug info. Ensure that debug info
+// is emitted for extern functions referenced via variable initializers.
+//
+// CHECK: !DISubprogram(name: "fn"
+extern void fn(void);
+void (*pfn) (void) = &fn;
diff --git a/clang/test/CodeGen/bpf-debug-info-unref.c 
b/clang/test/CodeGen/bpf-debug-info-unref.c
new file mode 100644
index 0000000000000..91f761ec3b77f
--- /dev/null
+++ b/clang/test/CodeGen/bpf-debug-info-unref.c
@@ -0,0 +1,11 @@
+// RUN: %clang -g -target bpf -S -emit-llvm %s -o - | FileCheck %s
+//
+// No debug info is produced for unreferenced functions.
+// CHECK-NOT: !DISubprogram
+void unref(void);
+void unref2(typeof(unref));
+
+// No debug info for unused extern variables as well.
+// CHECK-NOT: !DiGlobalVariable
+extern int unused;
+extern int unused2[sizeof(unused)];
diff --git a/llvm/lib/Target/BPF/BTFDebug.cpp b/llvm/lib/Target/BPF/BTFDebug.cpp
index 34581e3b6286a..4d847abea731d 100644
--- a/llvm/lib/Target/BPF/BTFDebug.cpp
+++ b/llvm/lib/Target/BPF/BTFDebug.cpp
@@ -1495,6 +1495,29 @@ void BTFDebug::processGlobals(bool ProcessingMapDef) {
 
     DataSecEntries[std::string(SecName)]->addDataSecEntry(VarId,
         Asm->getSymbol(&Global), Size);
+
+    if (Global.hasInitializer())
+      processGlobalInitializer(Global.getInitializer());
+  }
+}
+
+/// Process global variable initializer in pursuit for function
+/// pointers. Add discovered (extern) functions to BTF. Some (extern)
+/// functions might have been missed otherwise. Every symbol needs BTF
+/// info when linking with bpftool. Primary use case: "static"
+/// initialization of BPF maps.
+///
+/// struct {
+///   __uint(type, BPF_MAP_TYPE_PROG_ARRAY);
+///   ...
+/// } prog_map SEC(".maps") = { .values = { extern_func } };
+///
+void BTFDebug::processGlobalInitializer(const Constant *C) {
+  if (auto *Fn = dyn_cast<Function>(C))
+    processFuncPrototypes(Fn);
+  if (auto *CA = dyn_cast<ConstantAggregate>(C)) {
+    for (unsigned I = 0, N = CA->getNumOperands(); I < N; ++I)
+      processGlobalInitializer(CA->getOperand(I));
   }
 }
 
diff --git a/llvm/lib/Target/BPF/BTFDebug.h b/llvm/lib/Target/BPF/BTFDebug.h
index 3ef4a85299b65..b24a79de74efa 100644
--- a/llvm/lib/Target/BPF/BTFDebug.h
+++ b/llvm/lib/Target/BPF/BTFDebug.h
@@ -352,6 +352,10 @@ class BTFDebug : public DebugHandlerBase {
   /// Generate types and variables for globals.
   void processGlobals(bool ProcessingMapDef);
 
+  /// Process global variable initializer in pursuit for function
+  /// pointers.
+  void processGlobalInitializer(const Constant *C);
+
   /// Generate types for function prototypes.
   void processFuncPrototypes(const Function *);
 
diff --git a/llvm/test/CodeGen/BPF/BTF/extern-var-func2.ll 
b/llvm/test/CodeGen/BPF/BTF/extern-var-func2.ll
new file mode 100644
index 0000000000000..700486d9f3515
--- /dev/null
+++ b/llvm/test/CodeGen/BPF/BTF/extern-var-func2.ll
@@ -0,0 +1,54 @@
+; RUN: llc -march=bpfel -filetype=asm -o - %s | FileCheck 
-check-prefixes=CHECK %s
+; RUN: llc -march=bpfeb -filetype=asm -o - %s | FileCheck 
-check-prefixes=CHECK %s
+;
+; Source code:
+;   extern int elsewhere(void);
+;   struct {
+;     void *values[];
+;   } prog_map = { .values = { elsewhere } };
+; Compilation flag:
+;   clang -target bpf -O2 -g -S -emit-llvm test.c
+; ModuleID = 'b.c'
+
+@prog_map = dso_local local_unnamed_addr global { [1 x ptr] } { [1 x ptr] [ptr 
@elsewhere] }, align 8, !dbg !0
+
+declare !dbg !17 dso_local i32 @elsewhere() #0
+
+; CHECK:             .long   0                               # 
BTF_KIND_FUNC_PROTO(id = 6)
+; CHECK-NEXT:        .long   218103808                       # 0xd000000
+; CHECK-NEXT:        .long   7
+; CHECK-NEXT:        .long   37                              # BTF_KIND_INT(id 
= 7)
+; CHECK-NEXT:        .long   16777216                        # 0x1000000
+; CHECK-NEXT:        .long   4
+; CHECK-NEXT:        .long   16777248                        # 0x1000020
+; CHECK-NEXT:        .long   41                              # 
BTF_KIND_FUNC(id = 8)
+; CHECK-NEXT:        .long   201326594                       # 0xc000002
+; CHECK-NEXT:        .long   6
+
+attributes #0 = { "frame-pointer"="all" "no-trapping-math"="true" 
"stack-protector-buffer-size"="8" }
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!12, !13, !14, !15}
+!llvm.ident = !{!16}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "prog_map", scope: !2, file: !3, line: 
4, type: !5, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C11, file: !3, producer: "clang 
version 19.0.0git (g...@github.com:llvm/llvm-project.git 
0390a6803608e3a5314315b73740c2d3f5a5723f)", isOptimized: true, runtimeVersion: 
0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false, 
nameTableKind: None)
+!3 = !DIFile(filename: "b.c", directory: "/home/nickz/llvm-project.git", 
checksumkind: CSK_MD5, checksum: "41cc17375f1261a0e072590833492553")
+!4 = !{!0}
+!5 = distinct !DICompositeType(tag: DW_TAG_structure_type, file: !3, line: 2, 
elements: !6)
+!6 = !{!7}
+!7 = !DIDerivedType(tag: DW_TAG_member, name: "values", scope: !5, file: !3, 
line: 3, baseType: !8)
+!8 = !DICompositeType(tag: DW_TAG_array_type, baseType: !9, elements: !10)
+!9 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 64)
+!10 = !{!11}
+!11 = !DISubrange(count: -1)
+!12 = !{i32 7, !"Dwarf Version", i32 5}
+!13 = !{i32 2, !"Debug Info Version", i32 3}
+!14 = !{i32 1, !"wchar_size", i32 4}
+!15 = !{i32 7, !"frame-pointer", i32 2}
+!16 = !{!"clang version 19.0.0git (g...@github.com:llvm/llvm-project.git 
0390a6803608e3a5314315b73740c2d3f5a5723f)"}
+!17 = !DISubprogram(name: "elsewhere", scope: !3, file: !3, line: 1, type: 
!18, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
+!18 = !DISubroutineType(types: !19)
+!19 = !{!20}
+!20 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to