On Wed, Mar 29, 2023 at 1:59 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > On 3/29/23 12:12 PM, James Hilliard wrote: > > On Wed, Mar 29, 2023 at 11:03 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > >> > >> On 3/27/23 8:51 PM, James Hilliard wrote: > >>>> diff --git a/tools/testing/selftests/bpf/progs/bench_local_storage_create.c b/tools/testing/selftests/bpf/progs/bench_local_storage_create.c > >>>> index 2814bab54d28..7c851c9d5e47 100644 > >>>> --- a/tools/testing/selftests/bpf/progs/bench_local_storage_create.c > >>>> +++ b/tools/testing/selftests/bpf/progs/bench_local_storage_create.c > >>>> @@ -22,6 +22,13 @@ struct { > >>>> __type(value, struct storage); > >>>> } sk_storage_map SEC(".maps"); > >>>> > >>>> +struct { > >>>> + __uint(type, BPF_MAP_TYPE_TASK_STORAGE); > >>>> + __uint(map_flags, BPF_F_NO_PREALLOC); > >>>> + __type(key, int); > >>>> + __type(value, struct storage); > >>>> +} task_storage_map SEC(".maps"); > >>>> + > >>>> SEC("raw_tp/kmalloc") > >>>> int BPF_PROG(kmalloc, unsigned long call_site, const void *ptr, > >>>> size_t bytes_req, size_t bytes_alloc, gfp_t gfp_flags, > >>>> @@ -32,6 +39,24 @@ int BPF_PROG(kmalloc, unsigned long call_site, const void *ptr, > >>>> return 0; > >>>> } > >>>> > >>>> +SEC("tp_btf/sched_process_fork") > >>>> +int BPF_PROG(fork, struct task_struct *parent, struct task_struct *child) > >>> > >>> Apparently fork is a built-in function in bpf-gcc: > >> > >> It is also failing in a plain C program > >> > >> #> gcc -Werror=builtin-declaration-mismatch -o test test.c > >> test.c:14:35: error: conflicting types for built-in function ‘fork’; expected > >> ‘int(void)’ [-Werror=builtin-declaration-mismatch] > >> 14 | int __attribute__((__noinline__)) fork(long x, long y) > >> | ^~~~ > >> cc1: some warnings being treated as errors > >> > >> #> clang -o test test.c > >> succeed > >> > >> I am not too attached to the name but it seems something should be addressed in > >> the gcc instead. > > > > Hmm, so it looks like it's marked as a builtin here: > > https://github.com/gcc-mirror/gcc/blob/releases/gcc-12.1.0/gcc/builtins.def#L875 > > > > The macro for that is here: > > https://github.com/gcc-mirror/gcc/blob/releases/gcc-12.1.0/gcc/builtins.def#L104-L111 > > > > Which has this comment: > > /* Like DEF_LIB_BUILTIN, except that the function is not one that is > > specified by ANSI/ISO C. So, when we're being fully conformant we > > ignore the version of these builtins that does not begin with > > __builtin. */ > > > > Looks like this builtin was originally added here: > > https://github.com/gcc-mirror/gcc/commit/d1c38823924506d389ca58d02926ace21bdf82fa > > > > Based on this issue it looks like fork is treated as a builtin for > > libgcov support: > > https://gcc.gnu.org/bugzilla//show_bug.cgi?id=82457 > > > > So from my understanding fork is a gcc builtin when building with -std=gnu11 > > but is not a builtin when building with -std=c11. > > That sounds like there is a knob to turn this behavior on and off. Do the same > for the bpf target? I don't think we want to have to do that. > > > > > So it looks like fork is translated to __gcov_fork when -std=gnu* is set which > > is why we get this error. > > > > As this appears to be intended behavior for gcc I think the best option is > > to just rename the function so that we don't run into issues when building > > with gnu extensions like -std=gnu11. > > Is it sure 'fork' is the only culprit? If not, it is better to address it > properly because this unnecessary name change is annoying when switching bpf > prog from clang to gcc. Like changing the name in this .c here has to make > another change to the .c in the prog_tests/ directory. We've fixed a similar issue in the past by renaming to avoid a conflict with the builtin: https://github.com/torvalds/linux/commit/ab0350c743d5c93fd88742f02b3dff12168ab435 > > > > >> > >>> > >>> In file included from progs/bench_local_storage_create.c:6: > >>> progs/bench_local_storage_create.c:43:14: error: conflicting types for > >>> built-in function 'fork'; expected 'int(void)' > >>> [-Werror=builtin-declaration-mismatch] > >>> 43 | int BPF_PROG(fork, struct task_struct *parent, struct > >>> task_struct *child) > >>> | ^~~~ > >>> > >>> I haven't been able to find this documented anywhere however. > >> >