Re: [PATCH v4] checkpatch: do not apply "initialise globals to 0" check to BPF progs

[Song Liu <songliubraving@fb.com>]

> On Feb 9, 2021, at 6:10 PM, Joe Perches <joe@perches.com> 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 <joe@perches.com>
> 
> 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
> 
> 
> 
>