diff mbox series

[bpf-next,1/2] bpf: Fix compilation warning with -Wparentheses

Message ID 20230728055740.2284534-1-yonghong.song@linux.dev (mailing list archive)
State Accepted
Commit 09fedc731874123e0f6e5e5e3572db0c60378c2a
Delegated to: BPF
Headers show
Series [bpf-next,1/2] bpf: Fix compilation warning with -Wparentheses | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-6 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-7 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-11 success Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-15 success Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-29 success Logs for veristat
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1363 this patch: 1361
netdev/cc_maintainers warning 11 maintainers not CCed: kpsingh@kernel.org martin.lau@linux.dev john.fastabend@gmail.com sdf@google.com song@kernel.org trix@redhat.com nathan@kernel.org llvm@lists.linux.dev jolsa@kernel.org haoluo@google.com ndesaulniers@google.com
netdev/build_clang success Errors and warnings before: 1365 this patch: 1365
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1386 this patch: 1384
netdev/checkpatch warning CHECK: Unnecessary parentheses around 's32_max >= 0' CHECK: Unnecessary parentheses around 's32_min >= 0' CHECK: Unnecessary parentheses around 's64_max >= 0' CHECK: Unnecessary parentheses around 's64_min >= 0'
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for test_progs on s390x with gcc

Commit Message

Yonghong Song July 28, 2023, 5:57 a.m. UTC
The kernel test robot reported compilation warnings when -Wparentheses is
added to KBUILD_CFLAGS with gcc compiler. The following is the error message:

  .../bpf-next/kernel/bpf/verifier.c: In function ‘coerce_reg_to_size_sx’:
  .../bpf-next/kernel/bpf/verifier.c:5901:14:
    error: suggest parentheses around comparison in operand of ‘==’ [-Werror=parentheses]
    if (s64_max >= 0 == s64_min >= 0) {
        ~~~~~~~~^~~~
  .../bpf-next/kernel/bpf/verifier.c: In function ‘coerce_subreg_to_size_sx’:
  .../bpf-next/kernel/bpf/verifier.c:5965:14:
    error: suggest parentheses around comparison in operand of ‘==’ [-Werror=parentheses]
    if (s32_min >= 0 == s32_max >= 0) {
        ~~~~~~~~^~~~

To fix the issue, add proper parentheses for the above '>=' condition
to silence the warning/error.

I tried a few clang compilers like clang16 and clang18 and they do not emit
such warnings with -Wparentheses.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202307281133.wi0c4SqG-lkp@intel.com/
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 kernel/bpf/core.c     | 4 ++--
 kernel/bpf/verifier.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Jiri Olsa July 28, 2023, 10:25 a.m. UTC | #1
On Thu, Jul 27, 2023 at 10:57:40PM -0700, Yonghong Song wrote:
> The kernel test robot reported compilation warnings when -Wparentheses is
> added to KBUILD_CFLAGS with gcc compiler. The following is the error message:
> 
>   .../bpf-next/kernel/bpf/verifier.c: In function ‘coerce_reg_to_size_sx’:
>   .../bpf-next/kernel/bpf/verifier.c:5901:14:
>     error: suggest parentheses around comparison in operand of ‘==’ [-Werror=parentheses]
>     if (s64_max >= 0 == s64_min >= 0) {
>         ~~~~~~~~^~~~
>   .../bpf-next/kernel/bpf/verifier.c: In function ‘coerce_subreg_to_size_sx’:
>   .../bpf-next/kernel/bpf/verifier.c:5965:14:
>     error: suggest parentheses around comparison in operand of ‘==’ [-Werror=parentheses]
>     if (s32_min >= 0 == s32_max >= 0) {
>         ~~~~~~~~^~~~
> 
> To fix the issue, add proper parentheses for the above '>=' condition
> to silence the warning/error.
> 
> I tried a few clang compilers like clang16 and clang18 and they do not emit
> such warnings with -Wparentheses.

I just hit it with gcc and this fixes it for me

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202307281133.wi0c4SqG-lkp@intel.com/
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>  kernel/bpf/core.c     | 4 ++--
>  kernel/bpf/verifier.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index db0b631908c2..baccdec22f19 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -1877,7 +1877,7 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
>  		case 1:
>  			AX = abs((s32)DST);
>  			do_div(AX, abs((s32)SRC));
> -			if ((s32)DST < 0 == (s32)SRC < 0)
> +			if (((s32)DST < 0) == ((s32)SRC < 0))
>  				DST = (u32)AX;
>  			else
>  				DST = (u32)-AX;
> @@ -1904,7 +1904,7 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
>  		case 1:
>  			AX = abs((s32)DST);
>  			do_div(AX, abs((s32)IMM));
> -			if ((s32)DST < 0 == (s32)IMM < 0)
> +			if (((s32)DST < 0) == ((s32)IMM < 0))
>  				DST = (u32)AX;
>  			else
>  				DST = (u32)-AX;
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 0b1ada93582b..e7b1af016841 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5898,7 +5898,7 @@ static void coerce_reg_to_size_sx(struct bpf_reg_state *reg, int size)
>  	s64_min = min(init_s64_max, init_s64_min);
>  
>  	/* both of s64_max/s64_min positive or negative */
> -	if (s64_max >= 0 == s64_min >= 0) {
> +	if ((s64_max >= 0) == (s64_min >= 0)) {
>  		reg->smin_value = reg->s32_min_value = s64_min;
>  		reg->smax_value = reg->s32_max_value = s64_max;
>  		reg->umin_value = reg->u32_min_value = s64_min;
> @@ -5962,7 +5962,7 @@ static void coerce_subreg_to_size_sx(struct bpf_reg_state *reg, int size)
>  	s32_max = max(init_s32_max, init_s32_min);
>  	s32_min = min(init_s32_max, init_s32_min);
>  
> -	if (s32_min >= 0 == s32_max >= 0) {
> +	if ((s32_min >= 0) == (s32_max >= 0)) {
>  		reg->s32_min_value = s32_min;
>  		reg->s32_max_value = s32_max;
>  		reg->u32_min_value = (u32)s32_min;
> -- 
> 2.34.1
> 
>
patchwork-bot+netdevbpf@kernel.org July 28, 2023, 4 p.m. UTC | #2
Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Thu, 27 Jul 2023 22:57:40 -0700 you wrote:
> The kernel test robot reported compilation warnings when -Wparentheses is
> added to KBUILD_CFLAGS with gcc compiler. The following is the error message:
> 
>   .../bpf-next/kernel/bpf/verifier.c: In function ‘coerce_reg_to_size_sx’:
>   .../bpf-next/kernel/bpf/verifier.c:5901:14:
>     error: suggest parentheses around comparison in operand of ‘==’ [-Werror=parentheses]
>     if (s64_max >= 0 == s64_min >= 0) {
>         ~~~~~~~~^~~~
>   .../bpf-next/kernel/bpf/verifier.c: In function ‘coerce_subreg_to_size_sx’:
>   .../bpf-next/kernel/bpf/verifier.c:5965:14:
>     error: suggest parentheses around comparison in operand of ‘==’ [-Werror=parentheses]
>     if (s32_min >= 0 == s32_max >= 0) {
>         ~~~~~~~~^~~~
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/2] bpf: Fix compilation warning with -Wparentheses
    https://git.kernel.org/bpf/bpf-next/c/09fedc731874
  - [bpf-next,2/2] selftests/bpf: Enable test test_progs-cpuv4 for gcc build kernel
    https://git.kernel.org/bpf/bpf-next/c/a76584fc9ff6

You are awesome, thank you!
diff mbox series

Patch

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index db0b631908c2..baccdec22f19 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1877,7 +1877,7 @@  static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
 		case 1:
 			AX = abs((s32)DST);
 			do_div(AX, abs((s32)SRC));
-			if ((s32)DST < 0 == (s32)SRC < 0)
+			if (((s32)DST < 0) == ((s32)SRC < 0))
 				DST = (u32)AX;
 			else
 				DST = (u32)-AX;
@@ -1904,7 +1904,7 @@  static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
 		case 1:
 			AX = abs((s32)DST);
 			do_div(AX, abs((s32)IMM));
-			if ((s32)DST < 0 == (s32)IMM < 0)
+			if (((s32)DST < 0) == ((s32)IMM < 0))
 				DST = (u32)AX;
 			else
 				DST = (u32)-AX;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0b1ada93582b..e7b1af016841 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5898,7 +5898,7 @@  static void coerce_reg_to_size_sx(struct bpf_reg_state *reg, int size)
 	s64_min = min(init_s64_max, init_s64_min);
 
 	/* both of s64_max/s64_min positive or negative */
-	if (s64_max >= 0 == s64_min >= 0) {
+	if ((s64_max >= 0) == (s64_min >= 0)) {
 		reg->smin_value = reg->s32_min_value = s64_min;
 		reg->smax_value = reg->s32_max_value = s64_max;
 		reg->umin_value = reg->u32_min_value = s64_min;
@@ -5962,7 +5962,7 @@  static void coerce_subreg_to_size_sx(struct bpf_reg_state *reg, int size)
 	s32_max = max(init_s32_max, init_s32_min);
 	s32_min = min(init_s32_max, init_s32_min);
 
-	if (s32_min >= 0 == s32_max >= 0) {
+	if ((s32_min >= 0) == (s32_max >= 0)) {
 		reg->s32_min_value = s32_min;
 		reg->s32_max_value = s32_max;
 		reg->u32_min_value = (u32)s32_min;