Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Song Liu <songliubraving@fb.com>
To:     Joe Perches <joe@perches.com>
CC:     open list <linux-kernel@vger.kernel.org>,
        bpf <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
Thread-Topic: [PATCH v4] checkpatch: do not apply "initialise globals to 0"
 check to BPF progs
Thread-Index: AQHW/ylj1Y/C3YR/QkyVJIzS8tmtH6pQpXMAgAAgx4A=
Date:   Wed, 10 Feb 2021 04:07:42 +0000
Message-ID: <F5609C13-E5D7-47C2-94C2-F4C2443352C4@fb.com>
References: <20210209211954.490077-1-songliubraving@fb.com>
 <2b4805f6ca2b44f4195b6fdba4f82d5e90ab1989.camel@perches.com>
In-Reply-To: <2b4805f6ca2b44f4195b6fdba4f82d5e90ab1989.camel@perches.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <420B9578234366419CFE1B2A6E1121EA@namprd15.prod.outlook.com>
X-MS-Exchange-CrossTenant-Network-Message-Id: 85fec6b6-5903-4cf1-61a0-08d8cd7969e7
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Feb 2021 04:07:42.3424
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: rD68zrV9rF3Lav9QmQY8Kl4uStiptykWNd/4tJJ651uvmpv/XZKQ6PHF3Y4vtKYoQkmJKPS9nPuqGrk7a4sV8Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR15MB3255
X-OriginatorOrg: fb.com
Content-Transfer-Encoding: quoted-printable
X-Proofpoint-UnRewURL: 0 URL was un-rewritten
MIME-Version: 1.0
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.369,18.0.737
 definitions=2021-02-09_08:2021-02-09,2021-02-09 signatures=0
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 mlxscore=0
 lowpriorityscore=0 phishscore=0 clxscore=1015 bulkscore=0 suspectscore=0
 malwarescore=0 mlxlogscore=912 adultscore=0 spamscore=0 priorityscore=1501
 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2009150000 definitions=main-2102100041
X-FB-Internal: deliver
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Feb 9, 2021, at 6:10 PM, Joe Perches <joe@perches.com> wrote:
>=20
> 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.
>=20
> Acked-by: Joe Perches <joe@perches.com>
>=20
> So the patch is OK now, but I have a question about the concept:
>=20
> 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.=20

> 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=20
convention works well so far. Is there any reason that we specify=20
section for global data?=20

Thanks,
Song

>=20
> $ clang --version
> clang version 10.0.0 (git://github.com/llvm/llvm-project.git 305b961f64b7=
5e73110e309341535f6d5a48ed72)
> Target: x86_64-unknown-linux-gnu
> Thread model: posix
>=20
> $ cat t_common.c
> int a =3D 0;
> int b =3D 1;
>=20
> int foo_a(void)
> {
> 	return a;
> }
>=20
> int foo_b(void)
> {
> 	return b;
> }
>=20
> $ clang -c -O3 t_common.c
>=20
> $ objdump -x t_common.o=20
>=20
> t_common.o:     file format elf64-x86-64
> t_common.o
> architecture: i386:x86-64, flags 0x00000011:
> HAS_RELOC, HAS_SYMS
> start address 0x0000000000000000
>=20
> 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
>=20
>=20
> RELOCATION RECORDS FOR [.text]:
> OFFSET           TYPE              VALUE=20
> 0000000000000002 R_X86_64_PC32     a-0x0000000000000004
> 0000000000000012 R_X86_64_PC32     b-0x0000000000000004
>=20
>=20
> RELOCATION RECORDS FOR [.eh_frame]:
> OFFSET           TYPE              VALUE=20
> 0000000000000020 R_X86_64_PC32     .text
> 0000000000000034 R_X86_64_PC32     .text+0x0000000000000010
>=20
>=20
> Perhaps instead something like:
>=20
> $ cat t_common_bpf.c
> __attribute__((__section__("bpf"))) int a =3D 0;
> __attribute__((__section__("bpf"))) int b =3D 1;
>=20
> int foo_a(void)
> {
> 	return a;
> }
>=20
> int foo_b(void)
> {
> 	return b;
> }
>=20
> $ clang -c -O3 t_common_bpf.c
>=20
> $ objdump -x t_common_bpf.o=20
>=20
> 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
>=20
> 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
>=20
>=20
> RELOCATION RECORDS FOR [.text]:
> OFFSET           TYPE              VALUE=20
> 0000000000000002 R_X86_64_PC32     a-0x0000000000000004
> 0000000000000012 R_X86_64_PC32     b-0x0000000000000004
>=20
>=20
> RELOCATION RECORDS FOR [.eh_frame]:
> OFFSET           TYPE              VALUE=20
> 0000000000000020 R_X86_64_PC32     .text
> 0000000000000034 R_X86_64_PC32     .text+0x0000000000000010
>=20
>=20
>=20
>=20