Skip to content

Commit ae0d224

Browse files
mejedimejedi
authored
[BPF] Fix linking issues in static map initializers (#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: ```c 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.
1 parent 2bc474b commit ae0d224

17 files changed

+139
-26
lines changed

‎clang/include/clang/AST/ASTConsumer.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ namespace clang {
2323
class ASTDeserializationListener; // layering violation because void* is ugly
2424
class SemaConsumer; // layering violation required for safe SemaConsumer
2525
class TagDecl;
26+
class DeclaratorDecl;
2627
class VarDecl;
2728
class FunctionDecl;
2829
class ImportDecl;
@@ -105,7 +106,7 @@ class ASTConsumer {
105106
/// CompleteExternalDeclaration - Callback invoked at the end of a translation
106107
/// unit to notify the consumer that the given external declaration should be
107108
/// completed.
108-
virtual void CompleteExternalDeclaration(VarDecl *D) {}
109+
virtual void CompleteExternalDeclaration(DeclaratorDecl *D) {}
109110

110111
/// Callback invoked when an MSInheritanceAttr has been attached to a
111112
/// CXXRecordDecl.

‎clang/include/clang/Frontend/MultiplexConsumer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ class MultiplexConsumer : public SemaConsumer {
6767
void HandleTopLevelDeclInObjCContainer(DeclGroupRef D) override;
6868
void HandleImplicitImportDecl(ImportDecl *D) override;
6969
void CompleteTentativeDefinition(VarDecl *D) override;
70-
void CompleteExternalDeclaration(VarDecl *D) override;
70+
void CompleteExternalDeclaration(DeclaratorDecl *D) override;
7171
void AssignInheritanceModel(CXXRecordDecl *RD) override;
7272
void HandleVTable(CXXRecordDecl *RD) override;
7373
ASTMutationListener *GetASTMutationListener() override;

‎clang/include/clang/Sema/Sema.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3098,7 +3098,7 @@ class Sema final : public SemaBase {
30983098
TentativeDefinitionsType TentativeDefinitions;
30993099

31003100
/// All the external declarations encoutered and used in the TU.
3101-
SmallVector<VarDecl *, 4> ExternalDeclarations;
3101+
SmallVector<DeclaratorDecl *, 4> ExternalDeclarations;
31023102

31033103
/// Generally null except when we temporarily switch decl contexts,
31043104
/// like in \see SemaObjC::ActOnObjCTemporaryExitContainerContext.

‎clang/lib/CodeGen/BackendConsumer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ class BackendConsumer : public ASTConsumer {
107107
void HandleTagDeclDefinition(TagDecl *D) override;
108108
void HandleTagDeclRequiredDefinition(const TagDecl *D) override;
109109
void CompleteTentativeDefinition(VarDecl *D) override;
110-
void CompleteExternalDeclaration(VarDecl *D) override;
110+
void CompleteExternalDeclaration(DeclaratorDecl *D) override;
111111
void AssignInheritanceModel(CXXRecordDecl *RD) override;
112112
void HandleVTable(CXXRecordDecl *RD) override;
113113

‎clang/lib/CodeGen/CGExpr.cpp

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3141,21 +3141,8 @@ LValue CodeGenFunction::EmitDeclRefLValue(const DeclRefExpr *E) {
31413141
return LV;
31423142
}
31433143

3144-
if (const auto *FD = dyn_cast<FunctionDecl>(ND)) {
3145-
LValue LV = EmitFunctionDeclLValue(*this, E, FD);
3146-
3147-
// Emit debuginfo for the function declaration if the target wants to.
3148-
if (getContext().getTargetInfo().allowDebugInfoForExternalRef()) {
3149-
if (CGDebugInfo *DI = CGM.getModuleDebugInfo()) {
3150-
auto *Fn =
3151-
cast<llvm::Function>(LV.getPointer(*this)->stripPointerCasts());
3152-
if (!Fn->getSubprogram())
3153-
DI->EmitFunctionDecl(FD, FD->getLocation(), T, Fn);
3154-
}
3155-
}
3156-
3157-
return LV;
3158-
}
3144+
if (const auto *FD = dyn_cast<FunctionDecl>(ND))
3145+
return EmitFunctionDeclLValue(*this, E, FD);
31593146

31603147
// FIXME: While we're emitting a binding from an enclosing scope, all other
31613148
// DeclRefExprs we see should be implicitly treated as if they also refer to

‎clang/lib/CodeGen/CodeGenAction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ void BackendConsumer::CompleteTentativeDefinition(VarDecl *D) {
376376
Gen->CompleteTentativeDefinition(D);
377377
}
378378

379-
void BackendConsumer::CompleteExternalDeclaration(VarDecl *D) {
379+
void BackendConsumer::CompleteExternalDeclaration(DeclaratorDecl *D) {
380380
Gen->CompleteExternalDeclaration(D);
381381
}
382382

‎clang/lib/CodeGen/CodeGenModule.cpp

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5185,8 +5185,11 @@ void CodeGenModule::EmitTentativeDefinition(const VarDecl *D) {
51855185
EmitGlobalVarDefinition(D);
51865186
}
51875187

5188-
void CodeGenModule::EmitExternalDeclaration(const VarDecl *D) {
5189-
EmitExternalVarDeclaration(D);
5188+
void CodeGenModule::EmitExternalDeclaration(const DeclaratorDecl *D) {
5189+
if (auto const *V = dyn_cast<const VarDecl>(D))
5190+
EmitExternalVarDeclaration(V);
5191+
if (auto const *FD = dyn_cast<const FunctionDecl>(D))
5192+
EmitExternalFunctionDeclaration(FD);
51905193
}
51915194

51925195
CharUnits CodeGenModule::GetTargetTypeStoreSize(llvm::Type *Ty) const {
@@ -5622,6 +5625,18 @@ void CodeGenModule::EmitExternalVarDeclaration(const VarDecl *D) {
56225625
}
56235626
}
56245627

5628+
void CodeGenModule::EmitExternalFunctionDeclaration(const FunctionDecl *FD) {
5629+
if (CGDebugInfo *DI = getModuleDebugInfo())
5630+
if (getCodeGenOpts().hasReducedDebugInfo()) {
5631+
auto *Ty = getTypes().ConvertType(FD->getType());
5632+
StringRef MangledName = getMangledName(FD);
5633+
auto *Fn = dyn_cast<llvm::Function>(
5634+
GetOrCreateLLVMFunction(MangledName, Ty, FD, /* ForVTable */ false));
5635+
if (!Fn->getSubprogram())
5636+
DI->EmitFunctionDecl(FD, FD->getLocation(), FD->getType(), Fn);
5637+
}
5638+
}
5639+
56255640
static bool isVarDeclStrongDefinition(const ASTContext &Context,
56265641
CodeGenModule &CGM, const VarDecl *D,
56275642
bool NoCommon) {

‎clang/lib/CodeGen/CodeGenModule.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1338,7 +1338,7 @@ class CodeGenModule : public CodeGenTypeCache {
13381338

13391339
void EmitTentativeDefinition(const VarDecl *D);
13401340

1341-
void EmitExternalDeclaration(const VarDecl *D);
1341+
void EmitExternalDeclaration(const DeclaratorDecl *D);
13421342

13431343
void EmitVTable(CXXRecordDecl *Class);
13441344

@@ -1690,6 +1690,7 @@ class CodeGenModule : public CodeGenTypeCache {
16901690

16911691
void EmitGlobalVarDefinition(const VarDecl *D, bool IsTentative = false);
16921692
void EmitExternalVarDeclaration(const VarDecl *D);
1693+
void EmitExternalFunctionDeclaration(const FunctionDecl *D);
16931694
void EmitAliasDefinition(GlobalDecl GD);
16941695
void emitIFuncDefinition(GlobalDecl GD);
16951696
void emitCPUDispatchDefinition(GlobalDecl GD);

‎clang/lib/CodeGen/ModuleBuilder.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ namespace {
310310
Builder->EmitTentativeDefinition(D);
311311
}
312312

313-
void CompleteExternalDeclaration(VarDecl *D) override {
313+
void CompleteExternalDeclaration(DeclaratorDecl *D) override {
314314
Builder->EmitExternalDeclaration(D);
315315
}
316316

‎clang/lib/Frontend/MultiplexConsumer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ void MultiplexConsumer::CompleteTentativeDefinition(VarDecl *D) {
357357
Consumer->CompleteTentativeDefinition(D);
358358
}
359359

360-
void MultiplexConsumer::CompleteExternalDeclaration(VarDecl *D) {
360+
void MultiplexConsumer::CompleteExternalDeclaration(DeclaratorDecl *D) {
361361
for (auto &Consumer : Consumers)
362362
Consumer->CompleteExternalDeclaration(D);
363363
}

‎clang/lib/Interpreter/IncrementalParser.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ class IncrementalASTConsumer final : public ASTConsumer {
7979
void CompleteTentativeDefinition(VarDecl *D) override final {
8080
Consumer->CompleteTentativeDefinition(D);
8181
}
82-
void CompleteExternalDeclaration(VarDecl *D) override final {
82+
void CompleteExternalDeclaration(DeclaratorDecl *D) override final {
8383
Consumer->CompleteExternalDeclaration(D);
8484
}
8585
void AssignInheritanceModel(CXXRecordDecl *RD) override final {

‎clang/lib/Sema/SemaDecl.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10818,6 +10818,14 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
1081810818
break;
1081910819
}
1082010820

10821+
// Similar to no_builtin logic above, at this point of the code
10822+
// FunctionDecl::isThisDeclarationADefinition() always returns `false`
10823+
// because Sema::ActOnStartOfFunctionDef has not been called yet.
10824+
if (Context.getTargetInfo().allowDebugInfoForExternalRef() &&
10825+
!NewFD->isInvalidDecl() &&
10826+
D.getFunctionDefinitionKind() == FunctionDefinitionKind::Declaration)
10827+
ExternalDeclarations.push_back(NewFD);
10828+
1082110829
return NewFD;
1082210830
}
1082310831

‎clang/test/CodeGen/bpf-debug-info-extern-func.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// RUN: %clang -g -target bpf -S -emit-llvm %s -o - | FileCheck %s
2+
//
3+
// When linking BPF object files via bpftool, BTF info is required for
4+
// every symbol. BTF is generated from debug info. Ensure that debug info
5+
// is emitted for extern functions referenced via variable initializers.
6+
//
7+
// CHECK: !DISubprogram(name: "fn"
8+
extern void fn(void);
9+
void (*pfn) (void) = &fn;

‎clang/test/CodeGen/bpf-debug-info-unref.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// RUN: %clang -g -target bpf -S -emit-llvm %s -o - | FileCheck %s
2+
//
3+
// No debug info is produced for unreferenced functions.
4+
// CHECK-NOT: !DISubprogram
5+
void unref(void);
6+
void unref2(typeof(unref));
7+
8+
// No debug info for unused extern variables as well.
9+
// CHECK-NOT: !DiGlobalVariable
10+
extern int unused;
11+
extern int unused2[sizeof(unused)];

‎llvm/lib/Target/BPF/BTFDebug.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,6 +1495,29 @@ void BTFDebug::processGlobals(bool ProcessingMapDef) {
14951495

14961496
DataSecEntries[std::string(SecName)]->addDataSecEntry(VarId,
14971497
Asm->getSymbol(&Global), Size);
1498+
1499+
if (Global.hasInitializer())
1500+
processGlobalInitializer(Global.getInitializer());
1501+
}
1502+
}
1503+
1504+
/// Process global variable initializer in pursuit for function
1505+
/// pointers. Add discovered (extern) functions to BTF. Some (extern)
1506+
/// functions might have been missed otherwise. Every symbol needs BTF
1507+
/// info when linking with bpftool. Primary use case: "static"
1508+
/// initialization of BPF maps.
1509+
///
1510+
/// struct {
1511+
/// __uint(type, BPF_MAP_TYPE_PROG_ARRAY);
1512+
/// ...
1513+
/// } prog_map SEC(".maps") = { .values = { extern_func } };
1514+
///
1515+
void BTFDebug::processGlobalInitializer(const Constant *C) {
1516+
if (auto *Fn = dyn_cast<Function>(C))
1517+
processFuncPrototypes(Fn);
1518+
if (auto *CA = dyn_cast<ConstantAggregate>(C)) {
1519+
for (unsigned I = 0, N = CA->getNumOperands(); I < N; ++I)
1520+
processGlobalInitializer(CA->getOperand(I));
14981521
}
14991522
}
15001523

‎llvm/lib/Target/BPF/BTFDebug.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,10 @@ class BTFDebug : public DebugHandlerBase {
352352
/// Generate types and variables for globals.
353353
void processGlobals(bool ProcessingMapDef);
354354

355+
/// Process global variable initializer in pursuit for function
356+
/// pointers.
357+
void processGlobalInitializer(const Constant *C);
358+
355359
/// Generate types for function prototypes.
356360
void processFuncPrototypes(const Function *);
357361

‎llvm/test/CodeGen/BPF/BTF/extern-var-func2.ll

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
; RUN: llc -march=bpfel -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
2+
; RUN: llc -march=bpfeb -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
3+
;
4+
; Source code:
5+
; extern int elsewhere(void);
6+
; struct {
7+
; void *values[];
8+
; } prog_map = { .values = { elsewhere } };
9+
; Compilation flag:
10+
; clang -target bpf -O2 -g -S -emit-llvm test.c
11+
; ModuleID = 'b.c'
12+
13+
@prog_map = dso_local local_unnamed_addr global { [1 x ptr] } { [1 x ptr] [ptr @elsewhere] }, align 8, !dbg !0
14+
15+
declare !dbg !17 dso_local i32 @elsewhere() #0
16+
17+
; CHECK: .long 0 # BTF_KIND_FUNC_PROTO(id = 6)
18+
; CHECK-NEXT: .long 218103808 # 0xd000000
19+
; CHECK-NEXT: .long 7
20+
; CHECK-NEXT: .long 37 # BTF_KIND_INT(id = 7)
21+
; CHECK-NEXT: .long 16777216 # 0x1000000
22+
; CHECK-NEXT: .long 4
23+
; CHECK-NEXT: .long 16777248 # 0x1000020
24+
; CHECK-NEXT: .long 41 # BTF_KIND_FUNC(id = 8)
25+
; CHECK-NEXT: .long 201326594 # 0xc000002
26+
; CHECK-NEXT: .long 6
27+
28+
attributes #0 = { "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
29+
30+
!llvm.dbg.cu = !{!2}
31+
!llvm.module.flags = !{!12, !13, !14, !15}
32+
!llvm.ident = !{!16}
33+
34+
!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
35+
!1 = distinct !DIGlobalVariable(name: "prog_map", scope: !2, file: !3, line: 4, type: !5, isLocal: false, isDefinition: true)
36+
!2 = distinct !DICompileUnit(language: DW_LANG_C11, file: !3, producer: "clang version 19.0.0git ([email protected]:llvm/llvm-project.git 0390a6803608e3a5314315b73740c2d3f5a5723f)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false, nameTableKind: None)
37+
!3 = !DIFile(filename: "b.c", directory: "/home/nickz/llvm-project.git", checksumkind: CSK_MD5, checksum: "41cc17375f1261a0e072590833492553")
38+
!4 = !{!0}
39+
!5 = distinct !DICompositeType(tag: DW_TAG_structure_type, file: !3, line: 2, elements: !6)
40+
!6 = !{!7}
41+
!7 = !DIDerivedType(tag: DW_TAG_member, name: "values", scope: !5, file: !3, line: 3, baseType: !8)
42+
!8 = !DICompositeType(tag: DW_TAG_array_type, baseType: !9, elements: !10)
43+
!9 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 64)
44+
!10 = !{!11}
45+
!11 = !DISubrange(count: -1)
46+
!12 = !{i32 7, !"Dwarf Version", i32 5}
47+
!13 = !{i32 2, !"Debug Info Version", i32 3}
48+
!14 = !{i32 1, !"wchar_size", i32 4}
49+
!15 = !{i32 7, !"frame-pointer", i32 2}
50+
!16 = !{!"clang version 19.0.0git ([email protected]:llvm/llvm-project.git 0390a6803608e3a5314315b73740c2d3f5a5723f)"}
51+
!17 = !DISubprogram(name: "elsewhere", scope: !3, file: !3, line: 1, type: !18, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
52+
!18 = !DISubroutineType(types: !19)
53+
!19 = !{!20}
54+
!20 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)

0 commit comments

Comments
 (0)