mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Song Liu <songliubraving@fb.com>, linux-kernel@vger.kernel.org
Cc: bpf@vger.kernel.org, Andy Whitcroft <apw@canonical.com>
Subject: Re: [PATCH v4] checkpatch: do not apply "initialise globals to 0" check to BPF progs
Date: Tue, 09 Feb 2021 18:10:22 -0800	[thread overview]
Message-ID: <2b4805f6ca2b44f4195b6fdba4f82d5e90ab1989.camel@perches.com> (raw)
In-Reply-To: <20210209211954.490077-1-songliubraving@fb.com>

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?

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





  reply	other threads:[~2021-02-10  2:33 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-09 21:19 Song Liu
2021-02-10  2:10 ` Joe Perches [this message]
2021-02-10  4:07   ` Song Liu
2021-02-10  4:12     ` Joe Perches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2b4805f6ca2b44f4195b6fdba4f82d5e90ab1989.camel@perches.com \
    --to=joe@perches.com \
    --cc=apw@canonical.com \
    --cc=bpf@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=songliubraving@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®