Skip to content

Conversation

kernel-patches-daemon-bpf-rc[bot]
Copy link

Pull request for series with
subject: selftests/bpf: Change variable types for -Wsign-compare
version: 4
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1013549

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 7361c86
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1013549
version: 4

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 04a8995
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1013549
version: 4

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 04a8995
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1013549
version: 4

@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 96d31df
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1013549
version: 4

This is a follow up patch for commit 495d2d8("selftests/bpf: Attempt
to build BPF programs with -Wsign-compare") from Alexei Starovoitov[1]
to be able to enable -Wsign-compare C compilation flag for clang since
-Wall doesn't add it and BPF programs are built with clang.This has the
benefit to catch problematic comparisons in future tests as quoted from
the commit message:"
  int i = -1;
  unsigned int j = 1;
  if (i < j) // this is false.

  long i = -1;
  unsigned int j = 1;
  if (i < j) // this is true.

C standard for reference:

- If either operand is unsigned long the other shall be converted to
unsigned long.

- Otherwise, if one operand is a long int and the other unsigned int,
then if a long int can represent all the values of an unsigned int,
the unsigned int shall be converted to a long int;
otherwise both operands shall be converted to unsigned long int.

- Otherwise, if either operand is long, the other shall be
converted to long.

- Otherwise, if either operand is unsigned, the other shall be
converted to unsigned.

Unfortunately clang's -Wsign-compare is very noisy.
It complains about (s32)a == (u32)b which is safe and doen't
have surprising behavior."

This specific patch supresses the following warnings when
-Wsign-compare is enabled:

1 warning generated.

progs/bpf_iter_bpf_percpu_array_map.c:35:16: warning: comparison of
integers of different signs: 'int' and 'const volatile __u32'
(aka 'const volatile unsigned int') [-Wsign-compare]
   35 |         for (i = 0; i < num_cpus; i++) {
      |                     ~ ^ ~~~~~~~~

1 warning generated.

progs/bpf_qdisc_fifo.c:93:2: warning: comparison of integers of
different signs: 'int' and '__u32'
(aka 'unsigned int') [-Wsign-compare]
   93 |         bpf_for(i, 0, sch->q.qlen) {
      |         ^       ~     ~~~~~~~~~~~

Should be noted that many more similar changes are still needed in order
to be able to enable the -Wsign-compare flag since -Werror is enabled and
would cause compilation of bpf selftests to fail.

[1].
Link:torvalds/linux@495d2d8

Signed-off-by: Mehdi Ben Hadj Khelifa <[email protected]>
@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: e758657
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1013549
version: 4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants