linux-kernel - Re: [PATCH v3 0/3] selftests/bpf: Prepare to add -Wsign-compare for bpf selftests

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAEf4Bzaf81OYLTzpN6E4ths_mN2gP29rMYBmbp7P2GqSMj8FbA@mail.gmail.com>
Date: Thu, 25 Sep 2025 16:32:11 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@...il.com>
Cc: andrii@...nel.org, eddyz87@...il.com, ast@...nel.org, daniel@...earbox.net, 
	martin.lau@...ux.dev, song@...nel.org, yonghong.song@...ux.dev, 
	john.fastabend@...il.com, kpsingh@...nel.org, sdf@...ichev.me, 
	haoluo@...gle.com, jolsa@...nel.org, shuah@...nel.org, matttbe@...nel.org, 
	martineau@...nel.org, geliang@...nel.org, davem@...emloft.net, 
	kuba@...nel.org, hawk@...nel.org, linux@...danrome.com, ameryhung@...il.com, 
	toke@...hat.com, houtao1@...wei.com, emil@...alapatis.com, yatsenko@...a.com, 
	isolodrai@...a.com, a.s.protopopov@...il.com, dxu@...uu.xyz, memxor@...il.com, 
	vmalik@...hat.com, bigeasy@...utronix.de, tj@...nel.org, 
	gregkh@...uxfoundation.org, paul@...l-moore.com, 
	bboscaccy@...ux.microsoft.com, James.Bottomley@...senpartnership.com, 
	mrpre@....com, jakub@...udflare.com, bpf@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org, 
	netdev@...r.kernel.org, mptcp@...ts.linux.dev, 
	linux-kernel-mentees@...ts.linuxfoundation.org, skhan@...uxfoundation.org, 
	david.hunter.linux@...il.com
Subject: Re: [PATCH v3 0/3] selftests/bpf: Prepare to add -Wsign-compare for
 bpf selftests

On Thu, Sep 25, 2025 at 2:36 AM Mehdi Ben Hadj Khelifa
<mehdi.benhadjkhelifa@...il.com> wrote:
>
> This series is preparing to add the -Wsign-compare C compilation flag to
> the Makefile for bpf selftests as requested by a TODO to help avoid
> implicit type conversions and have predictable behavior.
>
> Changelog:
>
> Changes from v2:
>
> -Split up the patch into a patch series as suggested by vivek
>
> -Include only changes to variable types with no casting by my mentor
> david
>
> -Removed the -Wsign-compare in Makefile to avoid compilation errors
> until adding casting for rest of comparisons.
>
> Link:https://lore.kernel.org/bpf/20250924195731.6374-1-mehdi.benhadjkhelifa@gmail.com/T/#u
>
> Changes from v1:
>
> - Fix CI failed builds where it failed due to do missing .c and
> .h files in my patch for working in mainline.
>
> Link:https://lore.kernel.org/bpf/20250924162408.815137-1-mehdi.benhadjkhelifa@gmail.com/T/#u
>
> Mehdi Ben Hadj Khelifa (3):
>   selftests/bpf: Prepare to add -Wsign-compare for bpf tests
>   selftests/bpf: Prepare to add -Wsign-compare for bpf tests
>   selftests/bpf: Prepare to add -Wsign-compare for bpf tests
>

I see little value in these transformations. Did we catch any real
issue here? All this type casting and replacement is just churn.

I certainly don't want such churn in libbpf, and I'd leave BPF
selftests as is as well. int vs u64 can have subtle and non-obvious
implications for BPF code generation (for no-alu32 variants
especially), and I think BPF CI already exposed some of those already.

I think we can live without -Wsign-compare just fine.


>  tools/testing/selftests/bpf/progs/test_global_func11.c       | 2 +-
>  tools/testing/selftests/bpf/progs/test_global_func12.c       | 2 +-
>  tools/testing/selftests/bpf/progs/test_global_func13.c       | 2 +-
>  tools/testing/selftests/bpf/progs/test_global_func9.c        | 2 +-
>  tools/testing/selftests/bpf/progs/test_map_init.c            | 2 +-
>  tools/testing/selftests/bpf/progs/test_parse_tcp_hdr_opt.c   | 2 +-
>  .../selftests/bpf/progs/test_parse_tcp_hdr_opt_dynptr.c      | 2 +-
>  tools/testing/selftests/bpf/progs/test_skb_ctx.c             | 2 +-
>  tools/testing/selftests/bpf/progs/test_snprintf.c            | 2 +-
>  tools/testing/selftests/bpf/progs/test_sockmap_strp.c        | 2 +-
>  tools/testing/selftests/bpf/progs/test_tc_tunnel.c           | 2 +-
>  tools/testing/selftests/bpf/progs/test_xdp.c                 | 2 +-
>  tools/testing/selftests/bpf/progs/test_xdp_dynptr.c          | 2 +-
>  tools/testing/selftests/bpf/progs/test_xdp_loop.c            | 2 +-
>  tools/testing/selftests/bpf/progs/test_xdp_noinline.c        | 4 ++--
>  tools/testing/selftests/bpf/progs/uprobe_multi.c             | 4 ++--
>  .../selftests/bpf/progs/uprobe_multi_session_recursive.c     | 5 +++--
>  .../selftests/bpf/progs/verifier_iterating_callbacks.c       | 2 +-
>  18 files changed, 22 insertions(+), 21 deletions(-)
>
> --
> 2.51.0
>