[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAEf4Bzb+KhWDkJEmCK_RW0UYc4C_CbmAWFYvoAqhBHBTarpeHw@mail.gmail.com>
Date: Tue, 6 May 2025 16:34:25 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Ihor Solodrai <ihor.solodrai@...ux.dev>
Cc: Holger Hoffstätte <holger@...lied-asynchrony.com>,
Alexei Starovoitov <alexei.starovoitov@...il.com>, Quentin Monnet <qmo@...nel.org>,
Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>,
Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>,
Jiri Olsa <jolsa@...nel.org>, bpf <bpf@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] bpftool: build bpf bits with -std=gnu11
On Tue, May 6, 2025 at 3:34 PM Ihor Solodrai <ihor.solodrai@...ux.dev> wrote:
>
> On 2025-05-06 3:23 p.m., Andrii Nakryiko wrote:
> > On Tue, May 6, 2025 at 2:41 PM Ihor Solodrai <ihor.solodrai@...ux.dev> wrote:
> >>
> >> On 2025-05-06 2:04 p.m., Andrii Nakryiko wrote:
> >> > On Sun, May 4, 2025 at 3:24 AM Holger Hoffstätte
> >> > <holger@...lied-asynchrony.com> wrote:
> >> >>
> >> >> On 2025-05-03 04:36, Alexei Starovoitov wrote:
> >> >>> On Fri, May 2, 2025 at 2:53 AM Holger Hoffstätte
> >> >>> <holger@...lied-asynchrony.com> wrote:
> >> >>>>
> >> >>>> On 2025-05-02 11:26, Quentin Monnet wrote:
> >> >>>>> On 02/05/2025 09:57, Holger Hoffstätte wrote:
> >> >>>>>> A gcc-15-based bpf toolchain defaults to C23 and fails to
> >> compile various
> >> >>>>>> kernel headers due to their use of a custom 'bool' type.
> >> >>>>>> Explicitly using -std=gnu11 works with both clang and bpf-toolchain.
> >> >>>>>>
> >> >>>>>> Signed-off-by: Holger Hoffstätte <holger@...lied-asynchrony.com>
> >> >>>>>
> >> >>>>> Thanks! I tested that it still works with clang.
> >> >>>>>
> >> >>>>> Acked-by: Quentin Monnet <qmo@...nel.org>
> >> >>>>
> >> >>>> Thanks!
> >> >>>>
> >> >>>>> I didn't manage to compile with gcc, though. I tried with gcc
> >> 15.1.1 but
> >> >>>>> option '--target=bpf' is apparently unrecognised by the gcc
> >> version on
> >> >>>>> my setup.
> >> >>>>>
> >> >>>>> Out of curiosity, how did you build using gcc for the skeleton?
> >> Was it
> >> >>>>> enough to run "CLANG=gcc make"? Does it pass the clang-bpf-co-re
> >> build
> >> >>>>> probe successfully?
> >> >>>>
> >> >>>> I'm on Gentoo where we have a gcc-14/15 based "bpf-toolchain" package,
> >> >>>> which is just gcc configured & packaged for the bpf target.
> >> >>>> Our bpftool package can be built with clang (default) or without, in
> >> >>>> which case it depend on the bpf-toolchain. The idea is to gradually
> >> >>>> allow bpf/xdp tooling to build/run without requiring clang.
> >> >>>>
> >> >>>> The --target definition is conditional and removed when not using
> >> clang:
> >> >>>>
> >> https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-util/bpftool/bpftool-7.5.0.ebuild?id=bf70fbf7b0dc97fbc97af579954ea81a8df36113#n94
> >> >>>>
> >> >>>> The bug for building with the new gcc-15 based toolchain where this
> >> >>>> patch originated is here: https://bugs.gentoo.org/955156
> >> >>>
> >> >>> So you're fixing this build error:
> >> >>>
> >> >>> bpf-unknown-none-gcc \
> >> >>> -I. \
> >> >>>
> >> -I/var/tmp/portage/dev-util/bpftool-7.5.0/work/bpftool-libbpf-v7.5.0-sources/include/uapi/
> >> >>> \
> >> >>>
> >> -I/var/tmp/portage/dev-util/bpftool-7.5.0/work/bpftool-libbpf-v7.5.0-sources/src/bootstrap/libbpf/include
> >> >>> \
> >> >>> -g -O2 -Wall -fno-stack-protector \
> >> >>> -c skeleton/profiler.bpf.c -o profiler.bpf.o
> >> >>> In file included from skeleton/profiler.bpf.c:3:
> >> >>> ./vmlinux.h:5: warning: ignoring '#pragma clang attribute'
> >> [-Wunknown-pragmas]
> >> >>> 5 | #pragma clang attribute push
> >> >>> (__attribute__((preserve_access_index)), apply_to = record)
> >> >>> ./vmlinux.h:9845:9: error: cannot use keyword 'false' as
> >> enumeration constant
> >> >>> 9845 | false = 0,
> >> >>> | ^~~~~
> >> >>> ./vmlinux.h:9845:9: note: 'false' is a keyword with '-std=c23' onwards
> >> >>> ./vmlinux.h:31137:15: error: 'bool' cannot be defined via 'typedef'
> >> >>> 31137 | typedef _Bool bool;
> >> >>> | ^~~~
> >> >>>
> >> >>> with -std=gnu11 flag and
> >> >>
> >> >> Yes, correct. This is the same as all over the kernel or the bpf tests
> >> >> for handling C23. I fully understand that this particular patch is only
> >> >> one piece of the puzzle.
> >> >>
> >> >
> >> > What's the best way to detect (at compile time) whether bool, false,
> >> > and true are treated as reserved keywords? To solve this properly
> >> > vmlinux.h would have to be adjusted by vmlinux.h to avoid emitting
> >> > bool/false/true *iff* compiler version/mode doesn't like that
> >>
> >> I ran into this when adding GCC BPF to CI [1].
> >>
> >> One can do something like:
> >>
> >> #if __STDC_VERSION__ < 202311L
> >> enum {
> >> false = 0,
> >> true = 1,
> >> };
> >> #endif
> >>
> >> But in case of vmlinux.h this would require hacking bpftool, and so for
> >> selftests/bpf we decided to pass -std=gnu11 [2].
> >
> > We can adjust btf_dump_is_blacklisted() to ignore bool typedef
> > (unconditionally), and we'll need to ignore anon enum with false/true
> > (which is annoying), and then bpftool will unconditionally add the
> > above block plus typedef _Bool bool.
> >
> > Would that work?
>
> I think yes, but then the question is why do all this work in bpftool
> instead of passing -std=gnu11 to the compiler? Especially given that
because -std=gnu11 is a mitigation, not a solution (at least for this
issue). The issue is that vmlinux.h (as it is right now) is not
compatible with C23 standard, which artificially limits what BPF users
can use with their .bpf.c code *just because of vmlinux.h*. Why? We
should find a solution to not depend on -std=gnu11 workarounds.
So this is not about bpftool and BPF selftests specifically, it's
about anyone using vmlinux.h.
> kernel is built with such flags:
>
> $ grep -r --include="[Mm]akefile" 'std=gnu'
>
[...]
> >
> >>
> >> [1]
> >> https://lore.kernel.org/bpf/CAADnVQKNqdLW1bpvCpVV3yNizwra0cCkBnAbsNp3rTmi8WFcvQ@mail.gmail.com/
> >> [2]
> >> https://lore.kernel.org/bpf/20250107235813.2964472-1-ihor.solodrai@pm.me/
> >>
> >> >
> >> >>> ignoring an important warning ?
> >> >>
> >> >> Nobody is (or was) ignoring the warning - it was under discussion when
> >> >> I posted the patch. After reaching out to Oracle to verify, we have now
> >> >> added the BPF_NO_PRESERVE_ACCESS_INDEX define when building with
> >> gcc-bpf;
> >> >> this resolves the warning, just like in the bpf self-tests.
> >> >>
> >> >> You are right that such an addition to the in-kernel bpftool build is
> >> >> still missing. If you have a suggestion on how best to do that via the
> >> >> existing Makefile I'm all ears.
> >> >>
> >> >> As for the remaining warnings - we are also very aware of the ongoing
> >> >> upstream work to support btf_type_tag:
> >> >> https://gcc.gnu.org/pipermail/gcc-patches/2025-April/682340.html.
> >> >>
> >> >>> End result: partially functional bpftool,
> >> >>> and users will have no idea why some features of bpftool are not
> >> working.
> >> >>
> >> >> First of all this is never shipped to any users; using gcc-bpf requires
> >> >> active opt-in by developers or users, and now also warns that such a
> >> setup
> >> >> may result in unexpected bugs due to ongoing work in both Linux and
> >> bpftool.
> >> >> Like I said before, by default everyone builds with clang and that
> >> is also
> >> >> true for our distributed binaries.
> >> >>
> >> >> If you think adding the -std=gnu11 bit is inappropriate at this time
> >> then
> >> >> just ignore this patch for now. Sooner or later the bpftool build
> >> will have
> >> >> to be adapted with BPF_CFLAGS (liek in the selftests) and hopefuilly an
> >> >> abstracted BPF_CC so that we no longer have to pretend to be clang when
> >> >> using gcc.
> >> >>
> >> >> cheers
> >> >> Holger
>
Powered by blists - more mailing lists