linux-kernel - Re: [PATCH] tools/libbpf: add WERROR option

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87a558obgn.fsf@gentoo.org>
Date: Sun, 13 Jul 2025 07:23:52 +0100
From: Sam James <sam@...too.org>
To: Daniel Borkmann <daniel@...earbox.net>
Cc: bpf@...r.kernel.org,  Andrii Nakryiko <andrii@...nel.org>,  Eduard
 Zingerman <eddyz87@...il.com>,  Alexei Starovoitov <ast@...nel.org>,
  Martin KaFai Lau <martin.lau@...ux.dev>,  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>,  Quentin Monnet <qmo@...nel.org>,
  linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tools/libbpf: add WERROR option

Daniel Borkmann <daniel@...earbox.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
it's fixed by
https://github.com/libbpf/libbpf/commit/715808d3e2d8c54f3001ce3d7fcda0844f765969
(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@...too.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.

>
>>   # Append required CFLAGS
>>   override CFLAGS += -std=gnu89
>>   override CFLAGS += $(EXTRA_WARNINGS) -Wno-switch-enum
>> -override CFLAGS += -Werror -Wall
>> +override CFLAGS += -Wall
>>   override CFLAGS += $(INCLUDES)
>>   override CFLAGS += -fvisibility=hidden
>>   override CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64

sam