Re: [PATCH] tools/libbpf: add WERROR option

[Sam James <sam@gentoo.org>]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Sat, Jul 12, 2025 at 11:24 PM Sam James <sam@gentoo.org> wrote:
>>
>> Daniel Borkmann <daniel@iogearbox.net> writes:
>>
>> > On 7/5/25 12:43 PM, Sam James wrote:
>> >> Check the 'WERROR' variable and suppress adding '-Werror' if WERROR=0.
>> >> This mirrors what tools/perf and other directories in tools do to
>> >> handle
>> >> -Werror rather than adding it unconditionally.
>> >
>> > Could you also add to the commit desc why you need it? Are there particular
>> > warnings you specifically need to suppress when building under gentoo?
>>
>> Sure. In this case, it was https://bugs.gentoo.org/959293 where I think
>
> I don't recall it was reported on bpf mailing list.
>
>> it's fixed by
>> https://github.com/libbpf/libbpf/commit/715808d3e2d8c54f3001ce3d7fcda0844f765969
>
> and looks like it was fixed by accident, so..

I'll note that I've sent patches that have been merged for these
before. It's just that they're sensitive to optimisation level and prone
to false positives. Especially with e.g. -Og.

>
>> (and the corresponding commit in the kernel tree proper). Backporting
>> that was a bit too big for our tastes.
>>
>> The real issue is just that -Werror when we have users who might be
>> testing with in-development compilers or with alternative options
>> results in a build failure when you didn't expect one.
>>
>> >
>> >> Signed-off-by: Sam James <sam@gentoo.org>
>> >> ---
>> >>   tools/lib/bpf/Makefile | 7 ++++++-
>> >>   1 file changed, 6 insertions(+), 1 deletion(-)
>> >> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
>> >> index 168140f8e646..9563d37265da 100644
>> >> --- a/tools/lib/bpf/Makefile
>> >> +++ b/tools/lib/bpf/Makefile
>> >> @@ -77,10 +77,15 @@ else
>> >>     CFLAGS := -g -O2
>> >>   endif
>> >>   +# Treat warnings as errors unless directed not to
>> >> +ifneq ($(WERROR),0)
>> >> +  CFLAGS += -Werror
>> >> +endif
>> >
>> > Should we also add sth similar to tools/bpf/bpftool/Makefile and by default
>> > enforce with -Werror with the option to disable?
>>
>> Yes, that sounds good to me, though I was nervous of stumbling onto a
>> philosophical debate about -Werror and wasn't sure what y'all preferred
>> :)
>>
>> I can send v2 with an updated commit message and this change. I'll wait
>> a bit for further comments based on my two replies here.
>
> No.
> We want Werror to be there by default and it shouldn't be trivial to turn off,
> so that people report and fix issues with new compilers.
> Like in this case, looks like it was a legitimate error of
> in-development gcc-16.
> If it was reported to us and turned out to be not a libbpf issue than
> gentoo should have reported it back to gcc devs to make sure they don't
> add bogus warnings to the compiler. Win-win.
>
> You're right, in many ways it is a philosophical debate.
> We cleaned up libbpf and selftests/bpf from warnings and
> we don't want them to reappear. So we don't want an easy way
> to silence them. Report issues instead.

OK then.