BPF programs explicitly initialise global variables to 0 to make sure
clang (v10 or older) do not put the variables in the common section.
Skip "initialise globals to 0" check for BPF programs to elimiate error
messages like:
ERROR: do not initialise globals to 0
#19: FILE: samples/bpf/tracex1_kern.c:21:
Cc: Andy Whitcroft <[email protected]>
Cc: Joe Perches <[email protected]>
Signed-off-by: Song Liu <[email protected]>
---
Changes v3 => v4:
1. Yet more fixes to regexes.
Changes v2 => v3:
1. Fix regex.
Changes v1 => v2:
1. Add function exclude_global_initialisers() to keep the code clean.
---
scripts/checkpatch.pl | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1afe3af1cc097..f5b7ec9170999 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2428,6 +2428,15 @@ sub get_raw_comment {
return $comment;
}
+sub exclude_global_initialisers {
+ my ($realfile) = @_;
+
+ # Do not check for BPF programs (tools/testing/selftests/bpf/progs/*.c, samples/bpf/*_kern.c, *.bpf.c).
+ return $realfile =~ m@^tools/testing/selftests/bpf/progs/.*\.c$@ ||
+ $realfile =~ m@^samples/bpf/.*_kern\.c$@ ||
+ $realfile =~ m@/bpf/.*\.bpf\.c$@;
+}
+
sub process {
my $filename = shift;
@@ -4323,7 +4332,8 @@ sub process {
}
# check for global initialisers.
- if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/) {
+ if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/ &&
+ !exclude_global_initialisers($realfile)) {
if (ERROR("GLOBAL_INITIALISERS",
"do not initialise globals to $1\n" . $herecurr) &&
$fix) {
--
2.24.1
On Tue, 2021-02-09 at 13:19 -0800, Song Liu wrote:
> BPF programs explicitly initialise global variables to 0 to make sure
> clang (v10 or older) do not put the variables in the common section.
Acked-by: Joe Perches <[email protected]>
So the patch is OK now, but I have a question about the concept:
Do you mean that these initialized to 0 global variables
should go into bss or another section?
Perhaps it'd be useful to somehow mark variables into specific
sections rather than bss when initialized to 0 and data when not
initialized to 0.
$ clang --version
clang version 10.0.0 (git://github.com/llvm/llvm-project.git 305b961f64b75e73110e309341535f6d5a48ed72)
Target: x86_64-unknown-linux-gnu
Thread model: posix
$ cat t_common.c
int a = 0;
int b = 1;
int foo_a(void)
{
return a;
}
int foo_b(void)
{
return b;
}
$ clang -c -O3 t_common.c
$ objdump -x t_common.o
t_common.o: file format elf64-x86-64
t_common.o
architecture: i386:x86-64, flags 0x00000011:
HAS_RELOC, HAS_SYMS
start address 0x0000000000000000
Sections:
Idx Name Size VMA LMA File off Algn
0 .text 00000017 0000000000000000 0000000000000000 00000040 2**4
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
1 .bss 00000004 0000000000000000 0000000000000000 00000058 2**2
ALLOC
2 .data 00000004 0000000000000000 0000000000000000 00000058 2**2
CONTENTS, ALLOC, LOAD, DATA
3 .comment 00000068 0000000000000000 0000000000000000 0000005c 2**0
CONTENTS, READONLY
4 .note.GNU-stack 00000000 0000000000000000 0000000000000000 000000c4 2**0
CONTENTS, READONLY
5 .eh_frame 00000040 0000000000000000 0000000000000000 000000c8 2**3
CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA
6 .llvm_addrsig 00000000 0000000000000000 0000000000000000 00000210 2**0
CONTENTS, READONLY, EXCLUDE
SYMBOL TABLE:
0000000000000000 l df *ABS* 0000000000000000 t_common.c
0000000000000000 l d .text 0000000000000000 .text
0000000000000000 g O .bss 0000000000000004 a
0000000000000000 g O .data 0000000000000004 b
0000000000000000 g F .text 0000000000000007 foo_a
0000000000000010 g F .text 0000000000000007 foo_b
RELOCATION RECORDS FOR [.text]:
OFFSET TYPE VALUE
0000000000000002 R_X86_64_PC32 a-0x0000000000000004
0000000000000012 R_X86_64_PC32 b-0x0000000000000004
RELOCATION RECORDS FOR [.eh_frame]:
OFFSET TYPE VALUE
0000000000000020 R_X86_64_PC32 .text
0000000000000034 R_X86_64_PC32 .text+0x0000000000000010
Perhaps instead something like:
$ cat t_common_bpf.c
__attribute__((__section__("bpf"))) int a = 0;
__attribute__((__section__("bpf"))) int b = 1;
int foo_a(void)
{
return a;
}
int foo_b(void)
{
return b;
}
$ clang -c -O3 t_common_bpf.c
$ objdump -x t_common_bpf.o
t_common_bpf.o: file format elf64-x86-64
t_common_bpf.o
architecture: i386:x86-64, flags 0x00000011:
HAS_RELOC, HAS_SYMS
start address 0x0000000000000000
Sections:
Idx Name Size VMA LMA File off Algn
0 .text 00000017 0000000000000000 0000000000000000 00000040 2**4
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
1 bpf 00000008 0000000000000000 0000000000000000 00000058 2**2
CONTENTS, ALLOC, LOAD, DATA
2 .comment 00000068 0000000000000000 0000000000000000 00000060 2**0
CONTENTS, READONLY
3 .note.GNU-stack 00000000 0000000000000000 0000000000000000 000000c8 2**0
CONTENTS, READONLY
4 .eh_frame 00000040 0000000000000000 0000000000000000 000000c8 2**3
CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA
5 .llvm_addrsig 00000000 0000000000000000 0000000000000000 00000210 2**0
CONTENTS, READONLY, EXCLUDE
SYMBOL TABLE:
0000000000000000 l df *ABS* 0000000000000000 t_common_bpf.c
0000000000000000 l d .text 0000000000000000 .text
0000000000000000 g O bpf 0000000000000004 a
0000000000000004 g O bpf 0000000000000004 b
0000000000000000 g F .text 0000000000000007 foo_a
0000000000000010 g F .text 0000000000000007 foo_b
RELOCATION RECORDS FOR [.text]:
OFFSET TYPE VALUE
0000000000000002 R_X86_64_PC32 a-0x0000000000000004
0000000000000012 R_X86_64_PC32 b-0x0000000000000004
RELOCATION RECORDS FOR [.eh_frame]:
OFFSET TYPE VALUE
0000000000000020 R_X86_64_PC32 .text
0000000000000034 R_X86_64_PC32 .text+0x0000000000000010
> On Feb 9, 2021, at 6:10 PM, Joe Perches <[email protected]> wrote:
>
> On Tue, 2021-02-09 at 13:19 -0800, Song Liu wrote:
>> BPF programs explicitly initialise global variables to 0 to make sure
>> clang (v10 or older) do not put the variables in the common section.
>
> Acked-by: Joe Perches <[email protected]>
>
> So the patch is OK now, but I have a question about the concept:
>
> Do you mean that these initialized to 0 global variables
> should go into bss or another section?
We want these variables to go to bss.
> Perhaps it'd be useful to somehow mark variables into specific
> sections rather than bss when initialized to 0 and data when not
> initialized to 0.
Currently, libbpf expects zero initialized global data in bss. This
convention works well so far. Is there any reason that we specify
section for global data?
Thanks,
Song
>
> $ clang --version
> clang version 10.0.0 (git://github.com/llvm/llvm-project.git 305b961f64b75e73110e309341535f6d5a48ed72)
> Target: x86_64-unknown-linux-gnu
> Thread model: posix
>
> $ cat t_common.c
> int a = 0;
> int b = 1;
>
> int foo_a(void)
> {
> return a;
> }
>
> int foo_b(void)
> {
> return b;
> }
>
> $ clang -c -O3 t_common.c
>
> $ objdump -x t_common.o
>
> t_common.o: file format elf64-x86-64
> t_common.o
> architecture: i386:x86-64, flags 0x00000011:
> HAS_RELOC, HAS_SYMS
> start address 0x0000000000000000
>
> Sections:
> Idx Name Size VMA LMA File off Algn
> 0 .text 00000017 0000000000000000 0000000000000000 00000040 2**4
> CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
> 1 .bss 00000004 0000000000000000 0000000000000000 00000058 2**2
> ALLOC
> 2 .data 00000004 0000000000000000 0000000000000000 00000058 2**2
> CONTENTS, ALLOC, LOAD, DATA
> 3 .comment 00000068 0000000000000000 0000000000000000 0000005c 2**0
> CONTENTS, READONLY
> 4 .note.GNU-stack 00000000 0000000000000000 0000000000000000 000000c4 2**0
> CONTENTS, READONLY
> 5 .eh_frame 00000040 0000000000000000 0000000000000000 000000c8 2**3
> CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA
> 6 .llvm_addrsig 00000000 0000000000000000 0000000000000000 00000210 2**0
> CONTENTS, READONLY, EXCLUDE
> SYMBOL TABLE:
> 0000000000000000 l df *ABS* 0000000000000000 t_common.c
> 0000000000000000 l d .text 0000000000000000 .text
> 0000000000000000 g O .bss 0000000000000004 a
> 0000000000000000 g O .data 0000000000000004 b
> 0000000000000000 g F .text 0000000000000007 foo_a
> 0000000000000010 g F .text 0000000000000007 foo_b
>
>
> RELOCATION RECORDS FOR [.text]:
> OFFSET TYPE VALUE
> 0000000000000002 R_X86_64_PC32 a-0x0000000000000004
> 0000000000000012 R_X86_64_PC32 b-0x0000000000000004
>
>
> RELOCATION RECORDS FOR [.eh_frame]:
> OFFSET TYPE VALUE
> 0000000000000020 R_X86_64_PC32 .text
> 0000000000000034 R_X86_64_PC32 .text+0x0000000000000010
>
>
> Perhaps instead something like:
>
> $ cat t_common_bpf.c
> __attribute__((__section__("bpf"))) int a = 0;
> __attribute__((__section__("bpf"))) int b = 1;
>
> int foo_a(void)
> {
> return a;
> }
>
> int foo_b(void)
> {
> return b;
> }
>
> $ clang -c -O3 t_common_bpf.c
>
> $ objdump -x t_common_bpf.o
>
> t_common_bpf.o: file format elf64-x86-64
> t_common_bpf.o
> architecture: i386:x86-64, flags 0x00000011:
> HAS_RELOC, HAS_SYMS
> start address 0x0000000000000000
>
> Sections:
> Idx Name Size VMA LMA File off Algn
> 0 .text 00000017 0000000000000000 0000000000000000 00000040 2**4
> CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
> 1 bpf 00000008 0000000000000000 0000000000000000 00000058 2**2
> CONTENTS, ALLOC, LOAD, DATA
> 2 .comment 00000068 0000000000000000 0000000000000000 00000060 2**0
> CONTENTS, READONLY
> 3 .note.GNU-stack 00000000 0000000000000000 0000000000000000 000000c8 2**0
> CONTENTS, READONLY
> 4 .eh_frame 00000040 0000000000000000 0000000000000000 000000c8 2**3
> CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA
> 5 .llvm_addrsig 00000000 0000000000000000 0000000000000000 00000210 2**0
> CONTENTS, READONLY, EXCLUDE
> SYMBOL TABLE:
> 0000000000000000 l df *ABS* 0000000000000000 t_common_bpf.c
> 0000000000000000 l d .text 0000000000000000 .text
> 0000000000000000 g O bpf 0000000000000004 a
> 0000000000000004 g O bpf 0000000000000004 b
> 0000000000000000 g F .text 0000000000000007 foo_a
> 0000000000000010 g F .text 0000000000000007 foo_b
>
>
> RELOCATION RECORDS FOR [.text]:
> OFFSET TYPE VALUE
> 0000000000000002 R_X86_64_PC32 a-0x0000000000000004
> 0000000000000012 R_X86_64_PC32 b-0x0000000000000004
>
>
> RELOCATION RECORDS FOR [.eh_frame]:
> OFFSET TYPE VALUE
> 0000000000000020 R_X86_64_PC32 .text
> 0000000000000034 R_X86_64_PC32 .text+0x0000000000000010
>
>
>
>
On Wed, 2021-02-10 at 04:07 +0000, Song Liu wrote:
> > On Feb 9, 2021, at 6:10 PM, Joe Perches <[email protected]> wrote:
> > On Tue, 2021-02-09 at 13:19 -0800, Song Liu wrote:
> > > BPF programs explicitly initialise global variables to 0 to make sure
> > > clang (v10 or older) do not put the variables in the common section.
> >
> > Acked-by: Joe Perches <[email protected]>
> >
> > So the patch is OK now, but I have a question about the concept:
> >
> > Do you mean that these initialized to 0 global variables
> > should go into bss or another section?
>
> We want these variables to go to bss.
OK, then the patch is fine.
> > Perhaps it'd be useful to somehow mark variables into specific
> > sections rather than bss when initialized to 0 and data when not
> > initialized to 0.
>
> Currently, libbpf expects zero initialized global data in bss. This
> convention works well so far. Is there any reason that we specify
> section for global data?
There's no need I know of.
cheers, Joe