[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-06-30 Thread Nick Zavaritsky via cfe-commits 

mejedi wrote:

@yonghong-song @efriedma-quic Any chance we could get it merged?

https://github.com/llvm/llvm-project/pull/91310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-07-05 Thread Nick Zavaritsky via cfe-commits 

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 
Date: Sun, 5 May 2024 10:20:52 +
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 ExternalDeclarations;
+  SmallVector ExternalDeclarations;
 
   /// Generally null except when we temporarily switch decl contexts,
   /// like in \see SemaObjC::ActOnObjCTemporaryExitContainerContext.
diff --git a/clang/lib/CodeGen/Ba

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-06-10 Thread Nick Zavaritsky via cfe-commits 

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

>From defe9d0fe64edafdc2e1ff5905c36bf229323692 Mon Sep 17 00:00:00 2001
From: Nick Zavaritsky 
Date: Sun, 5 May 2024 10:20:52 +
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 4d4579fcfd456..4cf6c670b6d9d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2685,7 +2685,7 @@ class Sema final : public SemaBase {
   TentativeDefinitionsType TentativeDefinitions;
 
   /// All the external declarations encoutered and used in the TU.
-  SmallVector ExternalDeclarations;
+  SmallVector ExternalDeclarations;
 
   /// Generally null except when we temporarily switch decl contexts,
   /// like in \see SemaObjC::ActOnObjCTemporaryExitContainerContext.
diff --git a/clang/lib/CodeGen/Ba

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-06-10 Thread Nick Zavaritsky via cfe-commits 

mejedi wrote:

@yonghong-song @dwblaikie 
> Is there some test coverage that shows that unreferenced variables/functions 
> aren't included in the output?

Test added.

https://github.com/llvm/llvm-project/pull/91310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-06-17 Thread Nick Zavaritsky via cfe-commits 

mejedi wrote:

ping

https://github.com/llvm/llvm-project/pull/91310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-05-07 Thread Nick Zavaritsky via cfe-commits 

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

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

>From 6fb034ef2bf370358b1acd1cee30cf2154ef46c3 Mon Sep 17 00:00:00 2001
From: Nick Zavaritsky 
Date: Sun, 5 May 2024 10:20:52 +
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 } };
---
 clang/lib/CodeGen/CGExprConstant.cpp  | 18 ++-
 clang/test/bpf-debug-info-extern-func.c   |  9 
 llvm/lib/Target/BPF/BTFDebug.cpp  | 23 
 llvm/lib/Target/BPF/BTFDebug.h|  4 ++
 llvm/test/CodeGen/BPF/BTF/extern-var-func2.ll | 54 +++
 5 files changed, 106 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/bpf-debug-info-extern-func.c
 create mode 100644 llvm/test/CodeGen/BPF/BTF/extern-var-func2.ll

diff --git a/clang/lib/CodeGen/CGExprConstant.cpp 
b/clang/lib/CodeGen/CGExprConstant.cpp
index 94962091116af..945f2e222b23f 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -1950,8 +1950,22 @@ ConstantLValueEmitter::tryEmitBase(const 
APValue::LValueBase &base) {
 if (D->hasAttr())
   return CGM.GetWeakRefReference(D).getPointer();
 
-if (auto FD = dyn_cast(D))
-  return CGM.GetAddrOfFunction(FD);
+if (auto FD = dyn_cast(D)) {
+  auto *C = CGM.GetAddrOfFunction(FD);
+
+  // we don't normally emit debug info for extern fns referenced via
+  // variable initialisers; BPF needs it since it generates BTF from
+  // debug info and bpftool demands BTF for every symbol linked
+  if (CGM.getTarget().getTriple().isBPF() && FD->getStorageClass() == 
SC_Extern) {
+if (CGDebugInfo *DI = CGM.getModuleDebugInfo()) {
+  auto *Fn = dyn_cast(C);
+  if (Fn && !Fn->getSubprogram())
+DI->EmitFunctionDecl(FD, FD->getLocation(), D->getType(), Fn);
+}
+  }
+
+  return C;
+}
 
 if (auto VD = dyn_cast(D)) {
   // We can never refer to a variable with local storage.
diff --git a/clang/test/bpf-debug-info-extern-func.c 
b/clang/test/bpf-debug-info-extern-func.c
new file mode 100644
index 0..da324630a314d
--- /dev/null
+++ b/clang/test/bpf-debug-info-extern-func.c
@@ -0,0 +1,9 @@
+// RUN: %clang -g -O2 -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/llvm/lib/Target/BPF/BTFDebug.cpp b/llvm/lib/Target/BPF/BTFDebug.cpp
index b6d3b460005c9..7d64f2e04bc26 100644
--- a/llvm/lib/Target/BPF/BTFDebug.cpp
+++ b/llvm/lib/Target/BPF/BTFDebug.cpp
@@ -1494,6 +1494,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(C))
+processFuncPrototypes(Fn);
+  if (auto *CA = dyn_cast(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 11a0c59ba6c90..02181d3013be9 100644
--- a/ll

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-05-07 Thread Nick Zavaritsky via cfe-commits 

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

>From e518bdfb86b06fa8bc447934293623a803a7f630 Mon Sep 17 00:00:00 2001
From: Nick Zavaritsky 
Date: Sun, 5 May 2024 10:20:52 +
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 } };

Signed-off-by: Nick Zavaritsky 
---
 clang/lib/CodeGen/CGExprConstant.cpp  | 18 ++-
 clang/test/bpf-debug-info-extern-func.c   |  9 
 llvm/lib/Target/BPF/BTFDebug.cpp  | 23 
 llvm/lib/Target/BPF/BTFDebug.h|  4 ++
 llvm/test/CodeGen/BPF/BTF/extern-var-func2.ll | 54 +++
 5 files changed, 106 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/bpf-debug-info-extern-func.c
 create mode 100644 llvm/test/CodeGen/BPF/BTF/extern-var-func2.ll

diff --git a/clang/lib/CodeGen/CGExprConstant.cpp 
b/clang/lib/CodeGen/CGExprConstant.cpp
index 94962091116a..945f2e222b23 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -1950,8 +1950,22 @@ ConstantLValueEmitter::tryEmitBase(const 
APValue::LValueBase &base) {
 if (D->hasAttr())
   return CGM.GetWeakRefReference(D).getPointer();
 
-if (auto FD = dyn_cast(D))
-  return CGM.GetAddrOfFunction(FD);
+if (auto FD = dyn_cast(D)) {
+  auto *C = CGM.GetAddrOfFunction(FD);
+
+  // we don't normally emit debug info for extern fns referenced via
+  // variable initialisers; BPF needs it since it generates BTF from
+  // debug info and bpftool demands BTF for every symbol linked
+  if (CGM.getTarget().getTriple().isBPF() && FD->getStorageClass() == 
SC_Extern) {
+if (CGDebugInfo *DI = CGM.getModuleDebugInfo()) {
+  auto *Fn = dyn_cast(C);
+  if (Fn && !Fn->getSubprogram())
+DI->EmitFunctionDecl(FD, FD->getLocation(), D->getType(), Fn);
+}
+  }
+
+  return C;
+}
 
 if (auto VD = dyn_cast(D)) {
   // We can never refer to a variable with local storage.
diff --git a/clang/test/bpf-debug-info-extern-func.c 
b/clang/test/bpf-debug-info-extern-func.c
new file mode 100644
index ..da324630a314
--- /dev/null
+++ b/clang/test/bpf-debug-info-extern-func.c
@@ -0,0 +1,9 @@
+// RUN: %clang -g -O2 -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/llvm/lib/Target/BPF/BTFDebug.cpp b/llvm/lib/Target/BPF/BTFDebug.cpp
index b6d3b460005c..7d64f2e04bc2 100644
--- a/llvm/lib/Target/BPF/BTFDebug.cpp
+++ b/llvm/lib/Target/BPF/BTFDebug.cpp
@@ -1494,6 +1494,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(C))
+processFuncPrototypes(Fn);
+  if (auto *CA = dyn_cast(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 11a0c59ba6c9..02181d3013be 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/B

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-05-07 Thread Nick Zavaritsky via cfe-commits 

mejedi wrote:

CC @yonghong-song 

https://github.com/llvm/llvm-project/pull/91310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-05-07 Thread Nick Zavaritsky via cfe-commits 

mejedi wrote:

cc @anakryiko

https://github.com/llvm/llvm-project/pull/91310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-05-08 Thread Nick Zavaritsky via cfe-commits 


@@ -1950,8 +1950,22 @@ ConstantLValueEmitter::tryEmitBase(const 
APValue::LValueBase &base) {
 if (D->hasAttr())
   return CGM.GetWeakRefReference(D).getPointer();
 
-if (auto FD = dyn_cast(D))
-  return CGM.GetAddrOfFunction(FD);
+if (auto FD = dyn_cast(D)) {
+  auto *C = CGM.GetAddrOfFunction(FD);
+
+  // we don't normally emit debug info for extern fns referenced via
+  // variable initialisers; BPF needs it since it generates BTF from
+  // debug info and bpftool demands BTF for every symbol linked
+  if (CGM.getTarget().getTriple().isBPF() && FD->getStorageClass() == 
SC_Extern) {

mejedi wrote:

@efriedma-quic Thank you for insightful comments! As a newbie, I need more 
guidance.

It is pretty much clear that `allowDebugInfoForExternalRef` is a preferred way 
to check whether we should emit additional debug info. It is currently only 
overriden by BPF target and is being used for exact same purpose we have here.

I need help getting up to speed irt. `CompleteExternalDeclaration` proposal. 
Having grepped through the code, I can see that BPF faced the same challenge 
with `extern` variables. At the moment, there's a vector of `VariableDecl`s 
populated in `SemaDecl.cpp` and later used to attach debug info to variables.

A different mechanism is applied to discovering `extern` functions in need for 
debug info (prior to my patch, `extern` functions referenced through code were 
handled). This is accomplished by code in `CGExpr.cpp`. I am proposing a 
similar extension in `CGExprConstant.cpp`.

Would you like to have functions on `ExternalDeclarations` list instead?

What would be a good spot to get ahold of `FunctionDecl` to put it on 
`ExternalDeclarations` list?

https://github.com/llvm/llvm-project/pull/91310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-05-17 Thread Nick Zavaritsky via cfe-commits 

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

>From 440f62d438810fbd1166a9eb3dbfbe10957fba58 Mon Sep 17 00:00:00 2001
From: Nick Zavaritsky 
Date: Sun, 5 May 2024 10:20:52 +
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 +++
 clang/test/bpf-debug-info-extern-func.c   |  9 
 llvm/lib/Target/BPF/BTFDebug.cpp  | 23 
 llvm/lib/Target/BPF/BTFDebug.h|  4 ++
 llvm/test/CodeGen/BPF/BTF/extern-var-func2.ll | 54 +++
 16 files changed, 128 insertions(+), 26 deletions(-)
 create mode 100644 clang/test/bpf-debug-info-extern-func.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 873592a2d430a..adc716681bfe6 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2611,7 +2611,7 @@ class Sema final : public SemaBase {
   TentativeDefinitionsType TentativeDefinitions;
 
   /// All the external declarations encoutered and used in the TU.
-  SmallVector ExternalDeclarations;
+  SmallVector 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 0fe9929dca2b3..92ff19ab30844 100644
--- a/clang/lib/CodeGen/Backen

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-05-17 Thread Nick Zavaritsky via cfe-commits 

https://github.com/mejedi edited https://github.com/llvm/llvm-project/pull/91310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-05-17 Thread Nick Zavaritsky via cfe-commits 

https://github.com/mejedi edited https://github.com/llvm/llvm-project/pull/91310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-05-17 Thread Nick Zavaritsky via cfe-commits 

https://github.com/mejedi edited https://github.com/llvm/llvm-project/pull/91310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-05-17 Thread Nick Zavaritsky via cfe-commits 


@@ -1950,8 +1950,22 @@ ConstantLValueEmitter::tryEmitBase(const 
APValue::LValueBase &base) {
 if (D->hasAttr())
   return CGM.GetWeakRefReference(D).getPointer();
 
-if (auto FD = dyn_cast(D))
-  return CGM.GetAddrOfFunction(FD);
+if (auto FD = dyn_cast(D)) {
+  auto *C = CGM.GetAddrOfFunction(FD);
+
+  // we don't normally emit debug info for extern fns referenced via
+  // variable initialisers; BPF needs it since it generates BTF from
+  // debug info and bpftool demands BTF for every symbol linked
+  if (CGM.getTarget().getTriple().isBPF() && FD->getStorageClass() == 
SC_Extern) {

mejedi wrote:

@efriedma-quic @yonghong-song I ended up generalising `ExternalDeclarations` to 
include both `VarDecl`s and `FunctionDecl`s.
Since `GetAddrOfFunction` is invoked in so many different contexts, I found it 
difficult to emit debug info only for the subset of calls we care about.
Personally, I find it cleaner to collect external declarations on a list and 
handle them separately rather than plugging directly into codegen. Please let 
me know if I am missing something.
Concerning referenced/all debacle -
 * `Sema::ActOnEndOfTranslationUnit` checks whether a declaration is used (does 
it mean referenced?) before calling `Consumer.CompleteExternalDeclaration`;
 * plugging into codegen ensures that we notice any reference that made it into 
llvm bitcode BUT they could be removed by the optimiser later;
 * BPF backend ensures that only referenced entities make it to BTF.

It looks like both approaches are quite similar?

https://github.com/llvm/llvm-project/pull/91310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-05-17 Thread Nick Zavaritsky via cfe-commits 

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

>From 88f895cc1a761681edd1dff15170ab42d563da49 Mon Sep 17 00:00:00 2001
From: Nick Zavaritsky 
Date: Sun, 5 May 2024 10:20:52 +
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 +++
 clang/test/bpf-debug-info-extern-func.c   |  9 
 llvm/lib/Target/BPF/BTFDebug.cpp  | 23 
 llvm/lib/Target/BPF/BTFDebug.h|  4 ++
 llvm/test/CodeGen/BPF/BTF/extern-var-func2.ll | 54 +++
 16 files changed, 128 insertions(+), 26 deletions(-)
 create mode 100644 clang/test/bpf-debug-info-extern-func.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 873592a2d430a..adc716681bfe6 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2611,7 +2611,7 @@ class Sema final : public SemaBase {
   TentativeDefinitionsType TentativeDefinitions;
 
   /// All the external declarations encoutered and used in the TU.
-  SmallVector ExternalDeclarations;
+  SmallVector 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 0fe9929dca2b3..92ff19ab30844 100644
--- a/clang/lib/CodeGen/Backen

[clang] [llvm] [BPF] Fix linking issues in static map initializers (PR #91310)

2024-05-27 Thread Nick Zavaritsky via cfe-commits 

mejedi wrote:

Ping.

https://github.com/llvm/llvm-project/pull/91310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits