* [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings @ 2025-05-08 11:37 Ilya Leoshkevich 2025-05-08 18:38 ` Alexei Starovoitov 0 siblings, 1 reply; 15+ messages in thread From: Ilya Leoshkevich @ 2025-05-08 11:37 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Ilya Leoshkevich clang-21 complains about unused expressions in a few progs. Fix by explicitly casting the respective expressions to void. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tools/testing/selftests/bpf/progs/bpf_arena_spin_lock.h | 4 ++-- tools/testing/selftests/bpf/progs/linked_list_fail.c | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/bpf/progs/bpf_arena_spin_lock.h b/tools/testing/selftests/bpf/progs/bpf_arena_spin_lock.h index d67466c1ff77..f90531cf3ee5 100644 --- a/tools/testing/selftests/bpf/progs/bpf_arena_spin_lock.h +++ b/tools/testing/selftests/bpf/progs/bpf_arena_spin_lock.h @@ -302,7 +302,7 @@ int arena_spin_lock_slowpath(arena_spinlock_t __arena __arg_arena *lock, u32 val * barriers. */ if (val & _Q_LOCKED_MASK) - smp_cond_load_acquire_label(&lock->locked, !VAL, release_err); + (void)smp_cond_load_acquire_label(&lock->locked, !VAL, release_err); /* * take ownership and clear the pending bit. @@ -380,7 +380,7 @@ int arena_spin_lock_slowpath(arena_spinlock_t __arena __arg_arena *lock, u32 val /* Link @node into the waitqueue. */ WRITE_ONCE(prev->next, node); - arch_mcs_spin_lock_contended_label(&node->locked, release_node_err); + (void)arch_mcs_spin_lock_contended_label(&node->locked, release_node_err); /* * While waiting for the MCS lock, the next pointer may have diff --git a/tools/testing/selftests/bpf/progs/linked_list_fail.c b/tools/testing/selftests/bpf/progs/linked_list_fail.c index 6438982b928b..e451726f2ab4 100644 --- a/tools/testing/selftests/bpf/progs/linked_list_fail.c +++ b/tools/testing/selftests/bpf/progs/linked_list_fail.c @@ -219,15 +219,14 @@ int obj_type_id_oor(void *ctx) SEC("?tc") int obj_new_no_composite(void *ctx) { - bpf_obj_new_impl(bpf_core_type_id_local(int), (void *)42); + (void)bpf_obj_new_impl(bpf_core_type_id_local(int), (void *)42); return 0; } SEC("?tc") int obj_new_no_struct(void *ctx) { - - bpf_obj_new(union { int data; unsigned udata; }); + (void)bpf_obj_new(union { int data; unsigned udata; }); return 0; } @@ -252,7 +251,7 @@ int new_null_ret(void *ctx) SEC("?tc") int obj_new_acq(void *ctx) { - bpf_obj_new(struct foo); + (void)bpf_obj_new(struct foo); return 0; } -- 2.49.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-08 11:37 [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings Ilya Leoshkevich @ 2025-05-08 18:38 ` Alexei Starovoitov 2025-05-08 19:21 ` Ilya Leoshkevich 0 siblings, 1 reply; 15+ messages in thread From: Alexei Starovoitov @ 2025-05-08 18:38 UTC (permalink / raw) To: Ilya Leoshkevich Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > clang-21 complains about unused expressions in a few progs. > Fix by explicitly casting the respective expressions to void. ... > if (val & _Q_LOCKED_MASK) > - smp_cond_load_acquire_label(&lock->locked, !VAL, release_err); > + (void)smp_cond_load_acquire_label(&lock->locked, !VAL, release_err); Hmm. I'm on clang-21 too and I don't see them. What warnings do you see ? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-08 18:38 ` Alexei Starovoitov @ 2025-05-08 19:21 ` Ilya Leoshkevich 2025-05-09 16:51 ` Alexei Starovoitov 0 siblings, 1 reply; 15+ messages in thread From: Ilya Leoshkevich @ 2025-05-08 19:21 UTC (permalink / raw) To: Alexei Starovoitov Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On Thu, 2025-05-08 at 11:38 -0700, Alexei Starovoitov wrote: > On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich <iii@linux.ibm.com> > wrote: > > > > clang-21 complains about unused expressions in a few progs. > > Fix by explicitly casting the respective expressions to void. > > ... > > if (val & _Q_LOCKED_MASK) > > - smp_cond_load_acquire_label(&lock->locked, !VAL, > > release_err); > > + (void)smp_cond_load_acquire_label(&lock->locked, > > !VAL, release_err); > > Hmm. I'm on clang-21 too and I don't see them. > What warnings do you see ? In file included from progs/arena_spin_lock.c:7: progs/bpf_arena_spin_lock.h:305:1756: error: expression result unused [-Werror,-Wunused-value] 305 | ({ typeof(_Generic((*&lock->locked), char: (char)0, unsigned char : (unsigned char)0, signed char : (signed char)0, unsigned short : (unsigned short)0, signed short : (signed short)0, unsigned int : (unsigned int)0, signed int : (signed int)0, unsigned long : (unsigned long)0, signed long : (signed long)0, unsigned long long : (unsigned long long)0, signed long long : (signed long long)0, default: (typeof(*&lock->locked))0)) __val = ({ typeof(&lock->locked) __ptr = (&lock->locked); typeof(_Generic((*(&lock->locked)), char: (char)0, unsigned char : (unsigned char)0, signed char : (signed char)0, unsigned short : (unsigned short)0, signed short : (signed short)0, unsigned int : (unsigned int)0, signed int : (signed int)0, unsigned long : (unsigned long)0, signed long : (signed long)0, unsigned long long : (unsigned long long)0, signed long long : (signed long long)0, default: (typeof(*(&lock->locked)))0)) VAL; for (;;) { VAL = (typeof(_Generic((*(&lock->locked)), char: (char)0, unsigned char : (unsigned char)0, signed char : (signed char)0, unsigned short : (unsigned short)0, signed short : (signed short)0, unsigned int : (unsigned int)0, signed int : (signed int)0, unsigned long : (unsigned long)0, signed long : (signed long)0, unsigned long long : (unsigned long long)0, signed long long : (signed long long)0, default: (typeof(*(&lock->locked)))0)))(*(volatile typeof(*__ptr) *)&(*__ptr)); if (!VAL) break; ({ __label__ l_break, l_continue; asm volatile goto("may_goto %l[l_break]" :::: l_break); goto l_continue; l_break: goto release_err; l_continue:; }); ({}); } (typeof(*(&lock- >locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ unsigned long __val; __sync_fetch_and_add(&__val, 0); }); else asm volatile("" ::: "memory"); }); }); (typeof(*(&lock->locked)))__val; }); | ^ ~~~~~ 1 error generated. It started today. Here is the full compiler version: $ clang-21 --version Debian clang version 21.0.0 (++20250501112544+75d1cceb9486- 1~exp1~20250501112558.1422) Target: s390x-unknown-linux-gnu Thread model: posix InstalledDir: /usr/lib/llvm-21/bin Best regards, Ilya ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-08 19:21 ` Ilya Leoshkevich @ 2025-05-09 16:51 ` Alexei Starovoitov 2025-05-12 12:22 ` Ilya Leoshkevich 0 siblings, 1 reply; 15+ messages in thread From: Alexei Starovoitov @ 2025-05-09 16:51 UTC (permalink / raw) To: Ilya Leoshkevich, Kumar Kartikeya Dwivedi Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On Thu, May 8, 2025 at 12:21 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > On Thu, 2025-05-08 at 11:38 -0700, Alexei Starovoitov wrote: > > On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich <iii@linux.ibm.com> > > wrote: > > > > > > clang-21 complains about unused expressions in a few progs. > > > Fix by explicitly casting the respective expressions to void. > > > > ... > > > if (val & _Q_LOCKED_MASK) > > > - smp_cond_load_acquire_label(&lock->locked, !VAL, > > > release_err); > > > + (void)smp_cond_load_acquire_label(&lock->locked, > > > !VAL, release_err); > > > > Hmm. I'm on clang-21 too and I don't see them. > > What warnings do you see ? > > In file included from progs/arena_spin_lock.c:7: > progs/bpf_arena_spin_lock.h:305:1756: error: expression result unused > [-Werror,-Wunused-value] > 305 | ({ typeof(_Generic((*&lock->locked), char: (char)0, unsigned > char : (unsigned char)0, signed char : (signed char)0, unsigned short : > (unsigned short)0, signed short : (signed short)0, unsigned int : > (unsigned int)0, signed int : (signed int)0, unsigned long : (unsigned > long)0, signed long : (signed long)0, unsigned long long : (unsigned > long long)0, signed long long : (signed long long)0, default: > (typeof(*&lock->locked))0)) __val = ({ typeof(&lock->locked) __ptr = > (&lock->locked); typeof(_Generic((*(&lock->locked)), char: (char)0, > unsigned char : (unsigned char)0, signed char : (signed char)0, > unsigned short : (unsigned short)0, signed short : (signed short)0, > unsigned int : (unsigned int)0, signed int : (signed int)0, unsigned > long : (unsigned long)0, signed long : (signed long)0, unsigned long > long : (unsigned long long)0, signed long long : (signed long long)0, > default: (typeof(*(&lock->locked)))0)) VAL; for (;;) { VAL = > (typeof(_Generic((*(&lock->locked)), char: (char)0, unsigned char : > (unsigned char)0, signed char : (signed char)0, unsigned short : > (unsigned short)0, signed short : (signed short)0, unsigned int : > (unsigned int)0, signed int : (signed int)0, unsigned long : (unsigned > long)0, signed long : (signed long)0, unsigned long long : (unsigned > long long)0, signed long long : (signed long long)0, default: > (typeof(*(&lock->locked)))0)))(*(volatile typeof(*__ptr) *)&(*__ptr)); > if (!VAL) break; ({ __label__ l_break, l_continue; asm volatile > goto("may_goto %l[l_break]" :::: l_break); goto l_continue; l_break: > goto release_err; l_continue:; }); ({}); } (typeof(*(&lock- > >locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ unsigned long __val; > __sync_fetch_and_add(&__val, 0); }); else asm volatile("" ::: > "memory"); }); }); (typeof(*(&lock->locked)))__val; }); > | > ^ ~~~~~ > 1 error generated. hmm. The error is impossible to read. Kumar, Do you see a way to silence it differently ? Without adding (void)... Things like: - bpf_obj_new(.. + (void)bpf_obj_new(.. are good to fix, and if we could annotate bpf_obj_new_impl kfunc with __must_check we would have done it, but - arch_mcs_spin_lock... + (void)arch_mcs_spin_lock... is odd. > It started today. > Here is the full compiler version: > > $ clang-21 --version > Debian clang version 21.0.0 (++20250501112544+75d1cceb9486- > 1~exp1~20250501112558.1422) > Target: s390x-unknown-linux-gnu > Thread model: posix > InstalledDir: /usr/lib/llvm-21/bin > > Best regards, > Ilya ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-09 16:51 ` Alexei Starovoitov @ 2025-05-12 12:22 ` Ilya Leoshkevich 2025-05-12 16:41 ` Alexei Starovoitov 0 siblings, 1 reply; 15+ messages in thread From: Ilya Leoshkevich @ 2025-05-12 12:22 UTC (permalink / raw) To: Alexei Starovoitov, Kumar Kartikeya Dwivedi Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On Fri, 2025-05-09 at 09:51 -0700, Alexei Starovoitov wrote: > On Thu, May 8, 2025 at 12:21 PM Ilya Leoshkevich <iii@linux.ibm.com> > wrote: > > > > On Thu, 2025-05-08 at 11:38 -0700, Alexei Starovoitov wrote: > > > On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich > > > <iii@linux.ibm.com> > > > wrote: > > > > > > > > clang-21 complains about unused expressions in a few progs. > > > > Fix by explicitly casting the respective expressions to void. > > > > > > ... > > > > if (val & _Q_LOCKED_MASK) > > > > - smp_cond_load_acquire_label(&lock->locked, > > > > !VAL, > > > > release_err); > > > > + (void)smp_cond_load_acquire_label(&lock- > > > > >locked, > > > > !VAL, release_err); > > > > > > Hmm. I'm on clang-21 too and I don't see them. > > > What warnings do you see ? > > > > In file included from progs/arena_spin_lock.c:7: > > progs/bpf_arena_spin_lock.h:305:1756: error: expression result > > unused > > [-Werror,-Wunused-value] > > 305 | ({ typeof(_Generic((*&lock->locked), char: (char)0, > > unsigned > > char : (unsigned char)0, signed char : (signed char)0, unsigned > > short : > > (unsigned short)0, signed short : (signed short)0, unsigned int : > > (unsigned int)0, signed int : (signed int)0, unsigned long : > > (unsigned > > long)0, signed long : (signed long)0, unsigned long long : > > (unsigned > > long long)0, signed long long : (signed long long)0, default: > > (typeof(*&lock->locked))0)) __val = ({ typeof(&lock->locked) __ptr > > = > > (&lock->locked); typeof(_Generic((*(&lock->locked)), char: (char)0, > > unsigned char : (unsigned char)0, signed char : (signed char)0, > > unsigned short : (unsigned short)0, signed short : (signed short)0, > > unsigned int : (unsigned int)0, signed int : (signed int)0, > > unsigned > > long : (unsigned long)0, signed long : (signed long)0, unsigned > > long > > long : (unsigned long long)0, signed long long : (signed long > > long)0, > > default: (typeof(*(&lock->locked)))0)) VAL; for (;;) { VAL = > > (typeof(_Generic((*(&lock->locked)), char: (char)0, unsigned char : > > (unsigned char)0, signed char : (signed char)0, unsigned short : > > (unsigned short)0, signed short : (signed short)0, unsigned int : > > (unsigned int)0, signed int : (signed int)0, unsigned long : > > (unsigned > > long)0, signed long : (signed long)0, unsigned long long : > > (unsigned > > long long)0, signed long long : (signed long long)0, default: > > (typeof(*(&lock->locked)))0)))(*(volatile typeof(*__ptr) > > *)&(*__ptr)); > > if (!VAL) break; ({ __label__ l_break, l_continue; asm volatile > > goto("may_goto %l[l_break]" :::: l_break); goto l_continue; > > l_break: > > goto release_err; l_continue:; }); ({}); } (typeof(*(&lock- > > > locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ unsigned long > > > __val; > > __sync_fetch_and_add(&__val, 0); }); else asm volatile("" ::: > > "memory"); }); }); (typeof(*(&lock->locked)))__val; }); > > | > > ^ ~~~~~ > > 1 error generated. > > hmm. The error is impossible to read. > > Kumar, > > Do you see a way to silence it differently ? > > Without adding (void)... > > Things like: > - bpf_obj_new(.. > + (void)bpf_obj_new(.. > > are good to fix, and if we could annotate > bpf_obj_new_impl kfunc with __must_check we would have done it, > > but > - arch_mcs_spin_lock... > + (void)arch_mcs_spin_lock... > > is odd. What do you think about moving (void) to the definition of arch_mcs_spin_lock_contended_label()? I can send a v2 if this is better. > > It started today. > > Here is the full compiler version: > > > > $ clang-21 --version > > Debian clang version 21.0.0 (++20250501112544+75d1cceb9486- > > 1~exp1~20250501112558.1422) > > Target: s390x-unknown-linux-gnu > > Thread model: posix > > InstalledDir: /usr/lib/llvm-21/bin > > > > Best regards, > > Ilya ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-12 12:22 ` Ilya Leoshkevich @ 2025-05-12 16:41 ` Alexei Starovoitov 2025-05-12 19:29 ` Kumar Kartikeya Dwivedi 0 siblings, 1 reply; 15+ messages in thread From: Alexei Starovoitov @ 2025-05-12 16:41 UTC (permalink / raw) To: Ilya Leoshkevich Cc: Kumar Kartikeya Dwivedi, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On Mon, May 12, 2025 at 5:22 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > On Fri, 2025-05-09 at 09:51 -0700, Alexei Starovoitov wrote: > > On Thu, May 8, 2025 at 12:21 PM Ilya Leoshkevich <iii@linux.ibm.com> > > wrote: > > > > > > On Thu, 2025-05-08 at 11:38 -0700, Alexei Starovoitov wrote: > > > > On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich > > > > <iii@linux.ibm.com> > > > > wrote: > > > > > > > > > > clang-21 complains about unused expressions in a few progs. > > > > > Fix by explicitly casting the respective expressions to void. > > > > > > > > ... > > > > > if (val & _Q_LOCKED_MASK) > > > > > - smp_cond_load_acquire_label(&lock->locked, > > > > > !VAL, > > > > > release_err); > > > > > + (void)smp_cond_load_acquire_label(&lock- > > > > > >locked, > > > > > !VAL, release_err); > > > > > > > > Hmm. I'm on clang-21 too and I don't see them. > > > > What warnings do you see ? > > > > > > In file included from progs/arena_spin_lock.c:7: > > > progs/bpf_arena_spin_lock.h:305:1756: error: expression result > > > unused > > > [-Werror,-Wunused-value] > > > 305 | ({ typeof(_Generic((*&lock->locked), char: (char)0, > > > unsigned > > > char : (unsigned char)0, signed char : (signed char)0, unsigned > > > short : > > > (unsigned short)0, signed short : (signed short)0, unsigned int : > > > (unsigned int)0, signed int : (signed int)0, unsigned long : > > > (unsigned > > > long)0, signed long : (signed long)0, unsigned long long : > > > (unsigned > > > long long)0, signed long long : (signed long long)0, default: > > > (typeof(*&lock->locked))0)) __val = ({ typeof(&lock->locked) __ptr > > > = > > > (&lock->locked); typeof(_Generic((*(&lock->locked)), char: (char)0, > > > unsigned char : (unsigned char)0, signed char : (signed char)0, > > > unsigned short : (unsigned short)0, signed short : (signed short)0, > > > unsigned int : (unsigned int)0, signed int : (signed int)0, > > > unsigned > > > long : (unsigned long)0, signed long : (signed long)0, unsigned > > > long > > > long : (unsigned long long)0, signed long long : (signed long > > > long)0, > > > default: (typeof(*(&lock->locked)))0)) VAL; for (;;) { VAL = > > > (typeof(_Generic((*(&lock->locked)), char: (char)0, unsigned char : > > > (unsigned char)0, signed char : (signed char)0, unsigned short : > > > (unsigned short)0, signed short : (signed short)0, unsigned int : > > > (unsigned int)0, signed int : (signed int)0, unsigned long : > > > (unsigned > > > long)0, signed long : (signed long)0, unsigned long long : > > > (unsigned > > > long long)0, signed long long : (signed long long)0, default: > > > (typeof(*(&lock->locked)))0)))(*(volatile typeof(*__ptr) > > > *)&(*__ptr)); > > > if (!VAL) break; ({ __label__ l_break, l_continue; asm volatile > > > goto("may_goto %l[l_break]" :::: l_break); goto l_continue; > > > l_break: > > > goto release_err; l_continue:; }); ({}); } (typeof(*(&lock- > > > > locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ unsigned long > > > > __val; > > > __sync_fetch_and_add(&__val, 0); }); else asm volatile("" ::: > > > "memory"); }); }); (typeof(*(&lock->locked)))__val; }); > > > | > > > ^ ~~~~~ > > > 1 error generated. > > > > hmm. The error is impossible to read. > > > > Kumar, > > > > Do you see a way to silence it differently ? > > > > Without adding (void)... > > > > Things like: > > - bpf_obj_new(.. > > + (void)bpf_obj_new(.. > > > > are good to fix, and if we could annotate > > bpf_obj_new_impl kfunc with __must_check we would have done it, > > > > but > > - arch_mcs_spin_lock... > > + (void)arch_mcs_spin_lock... > > > > is odd. > > What do you think about moving (void) to the definition of > arch_mcs_spin_lock_contended_label()? I can send a v2 if this is > better. Kumar, thoughts? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-12 16:41 ` Alexei Starovoitov @ 2025-05-12 19:29 ` Kumar Kartikeya Dwivedi 2025-05-23 11:25 ` Ilya Leoshkevich 0 siblings, 1 reply; 15+ messages in thread From: Kumar Kartikeya Dwivedi @ 2025-05-12 19:29 UTC (permalink / raw) To: Alexei Starovoitov Cc: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On Mon, 12 May 2025 at 12:41, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Mon, May 12, 2025 at 5:22 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > > > On Fri, 2025-05-09 at 09:51 -0700, Alexei Starovoitov wrote: > > > On Thu, May 8, 2025 at 12:21 PM Ilya Leoshkevich <iii@linux.ibm.com> > > > wrote: > > > > > > > > On Thu, 2025-05-08 at 11:38 -0700, Alexei Starovoitov wrote: > > > > > On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich > > > > > <iii@linux.ibm.com> > > > > > wrote: > > > > > > > > > > > > clang-21 complains about unused expressions in a few progs. > > > > > > Fix by explicitly casting the respective expressions to void. > > > > > > > > > > ... > > > > > > if (val & _Q_LOCKED_MASK) > > > > > > - smp_cond_load_acquire_label(&lock->locked, > > > > > > !VAL, > > > > > > release_err); > > > > > > + (void)smp_cond_load_acquire_label(&lock- > > > > > > >locked, > > > > > > !VAL, release_err); > > > > > > > > > > Hmm. I'm on clang-21 too and I don't see them. > > > > > What warnings do you see ? > > > > > > > > In file included from progs/arena_spin_lock.c:7: > > > > progs/bpf_arena_spin_lock.h:305:1756: error: expression result > > > > unused > > > > [-Werror,-Wunused-value] > > > > 305 | ({ typeof(_Generic((*&lock->locked), char: (char)0, > > > > unsigned > > > > char : (unsigned char)0, signed char : (signed char)0, unsigned > > > > short : > > > > (unsigned short)0, signed short : (signed short)0, unsigned int : > > > > (unsigned int)0, signed int : (signed int)0, unsigned long : > > > > (unsigned > > > > long)0, signed long : (signed long)0, unsigned long long : > > > > (unsigned > > > > long long)0, signed long long : (signed long long)0, default: > > > > (typeof(*&lock->locked))0)) __val = ({ typeof(&lock->locked) __ptr > > > > = > > > > (&lock->locked); typeof(_Generic((*(&lock->locked)), char: (char)0, > > > > unsigned char : (unsigned char)0, signed char : (signed char)0, > > > > unsigned short : (unsigned short)0, signed short : (signed short)0, > > > > unsigned int : (unsigned int)0, signed int : (signed int)0, > > > > unsigned > > > > long : (unsigned long)0, signed long : (signed long)0, unsigned > > > > long > > > > long : (unsigned long long)0, signed long long : (signed long > > > > long)0, > > > > default: (typeof(*(&lock->locked)))0)) VAL; for (;;) { VAL = > > > > (typeof(_Generic((*(&lock->locked)), char: (char)0, unsigned char : > > > > (unsigned char)0, signed char : (signed char)0, unsigned short : > > > > (unsigned short)0, signed short : (signed short)0, unsigned int : > > > > (unsigned int)0, signed int : (signed int)0, unsigned long : > > > > (unsigned > > > > long)0, signed long : (signed long)0, unsigned long long : > > > > (unsigned > > > > long long)0, signed long long : (signed long long)0, default: > > > > (typeof(*(&lock->locked)))0)))(*(volatile typeof(*__ptr) > > > > *)&(*__ptr)); > > > > if (!VAL) break; ({ __label__ l_break, l_continue; asm volatile > > > > goto("may_goto %l[l_break]" :::: l_break); goto l_continue; > > > > l_break: > > > > goto release_err; l_continue:; }); ({}); } (typeof(*(&lock- > > > > > locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ unsigned long > > > > > __val; > > > > __sync_fetch_and_add(&__val, 0); }); else asm volatile("" ::: > > > > "memory"); }); }); (typeof(*(&lock->locked)))__val; }); > > > > | > > > > ^ ~~~~~ > > > > 1 error generated. > > > > > > hmm. The error is impossible to read. > > > > > > Kumar, > > > > > > Do you see a way to silence it differently ? > > > > > > Without adding (void)... > > > > > > Things like: > > > - bpf_obj_new(.. > > > + (void)bpf_obj_new(.. > > > > > > are good to fix, and if we could annotate > > > bpf_obj_new_impl kfunc with __must_check we would have done it, > > > > > > but > > > - arch_mcs_spin_lock... > > > + (void)arch_mcs_spin_lock... > > > > > > is odd. > > > > What do you think about moving (void) to the definition of > > arch_mcs_spin_lock_contended_label()? I can send a v2 if this is > > better. > > Kumar, > > thoughts? Sorry for the delay, I was afk. The warning seems a bit aggressive, in the kernel we have users which do and do not use the value and it's fine. I think moving (void) inside the macro is a problem since at least rqspinlock like algorithm would want to inspect the result of the locked bit. No such users exist for now, of course. So maybe we can silence it until we do end up depending on the value. I will give a try with clang-21, but I think probably (void) in the source is better if we do need to silence it. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-12 19:29 ` Kumar Kartikeya Dwivedi @ 2025-05-23 11:25 ` Ilya Leoshkevich 2025-05-24 0:05 ` Yonghong Song 0 siblings, 1 reply; 15+ messages in thread From: Ilya Leoshkevich @ 2025-05-23 11:25 UTC (permalink / raw) To: Kumar Kartikeya Dwivedi, Alexei Starovoitov Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On Mon, 2025-05-12 at 15:29 -0400, Kumar Kartikeya Dwivedi wrote: > On Mon, 12 May 2025 at 12:41, Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: > > > > On Mon, May 12, 2025 at 5:22 AM Ilya Leoshkevich > > <iii@linux.ibm.com> wrote: > > > > > > On Fri, 2025-05-09 at 09:51 -0700, Alexei Starovoitov wrote: > > > > On Thu, May 8, 2025 at 12:21 PM Ilya Leoshkevich > > > > <iii@linux.ibm.com> > > > > wrote: > > > > > > > > > > On Thu, 2025-05-08 at 11:38 -0700, Alexei Starovoitov wrote: > > > > > > On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich > > > > > > <iii@linux.ibm.com> > > > > > > wrote: > > > > > > > > > > > > > > clang-21 complains about unused expressions in a few > > > > > > > progs. > > > > > > > Fix by explicitly casting the respective expressions to > > > > > > > void. > > > > > > > > > > > > ... > > > > > > > if (val & _Q_LOCKED_MASK) > > > > > > > - smp_cond_load_acquire_label(&lock- > > > > > > > >locked, > > > > > > > !VAL, > > > > > > > release_err); > > > > > > > + (void)smp_cond_load_acquire_label(&lock- > > > > > > > > locked, > > > > > > > !VAL, release_err); > > > > > > > > > > > > Hmm. I'm on clang-21 too and I don't see them. > > > > > > What warnings do you see ? > > > > > > > > > > In file included from progs/arena_spin_lock.c:7: > > > > > progs/bpf_arena_spin_lock.h:305:1756: error: expression > > > > > result > > > > > unused > > > > > [-Werror,-Wunused-value] > > > > > 305 | ({ typeof(_Generic((*&lock->locked), char: (char)0, > > > > > unsigned > > > > > char : (unsigned char)0, signed char : (signed char)0, > > > > > unsigned > > > > > short : > > > > > (unsigned short)0, signed short : (signed short)0, unsigned > > > > > int : > > > > > (unsigned int)0, signed int : (signed int)0, unsigned long : > > > > > (unsigned > > > > > long)0, signed long : (signed long)0, unsigned long long : > > > > > (unsigned > > > > > long long)0, signed long long : (signed long long)0, default: > > > > > (typeof(*&lock->locked))0)) __val = ({ typeof(&lock->locked) > > > > > __ptr > > > > > = > > > > > (&lock->locked); typeof(_Generic((*(&lock->locked)), char: > > > > > (char)0, > > > > > unsigned char : (unsigned char)0, signed char : (signed > > > > > char)0, > > > > > unsigned short : (unsigned short)0, signed short : (signed > > > > > short)0, > > > > > unsigned int : (unsigned int)0, signed int : (signed int)0, > > > > > unsigned > > > > > long : (unsigned long)0, signed long : (signed long)0, > > > > > unsigned > > > > > long > > > > > long : (unsigned long long)0, signed long long : (signed long > > > > > long)0, > > > > > default: (typeof(*(&lock->locked)))0)) VAL; for (;;) { VAL = > > > > > (typeof(_Generic((*(&lock->locked)), char: (char)0, unsigned > > > > > char : > > > > > (unsigned char)0, signed char : (signed char)0, unsigned > > > > > short : > > > > > (unsigned short)0, signed short : (signed short)0, unsigned > > > > > int : > > > > > (unsigned int)0, signed int : (signed int)0, unsigned long : > > > > > (unsigned > > > > > long)0, signed long : (signed long)0, unsigned long long : > > > > > (unsigned > > > > > long long)0, signed long long : (signed long long)0, default: > > > > > (typeof(*(&lock->locked)))0)))(*(volatile typeof(*__ptr) > > > > > *)&(*__ptr)); > > > > > if (!VAL) break; ({ __label__ l_break, l_continue; asm > > > > > volatile > > > > > goto("may_goto %l[l_break]" :::: l_break); goto l_continue; > > > > > l_break: > > > > > goto release_err; l_continue:; }); ({}); } (typeof(*(&lock- > > > > > > locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ unsigned > > > > > > long > > > > > > __val; > > > > > __sync_fetch_and_add(&__val, 0); }); else asm volatile("" ::: > > > > > "memory"); }); }); (typeof(*(&lock->locked)))__val; }); > > > > > | > > > > > ^ ~~~~~ > > > > > 1 error generated. > > > > > > > > hmm. The error is impossible to read. > > > > > > > > Kumar, > > > > > > > > Do you see a way to silence it differently ? > > > > > > > > Without adding (void)... > > > > > > > > Things like: > > > > - bpf_obj_new(.. > > > > + (void)bpf_obj_new(.. > > > > > > > > are good to fix, and if we could annotate > > > > bpf_obj_new_impl kfunc with __must_check we would have done it, > > > > > > > > but > > > > - arch_mcs_spin_lock... > > > > + (void)arch_mcs_spin_lock... > > > > > > > > is odd. > > > > > > What do you think about moving (void) to the definition of > > > arch_mcs_spin_lock_contended_label()? I can send a v2 if this is > > > better. > > > > Kumar, > > > > thoughts? > > Sorry for the delay, I was afk. > > The warning seems a bit aggressive, in the kernel we have users which > do and do not use the value and it's fine. > I think moving (void) inside the macro is a problem since at least > rqspinlock like algorithm would want to inspect the result of the > locked bit. > No such users exist for now, of course. So maybe we can silence it > until we do end up depending on the value. > > I will give a try with clang-21, but I think probably (void) in the > source is better if we do need to silence it. Gentle ping. This is still an issue with clang version 21.0.0 (++20250522112647+491619a25003-1~exp1~20250522112819.1465). ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-23 11:25 ` Ilya Leoshkevich @ 2025-05-24 0:05 ` Yonghong Song 2025-05-24 1:01 ` Kumar Kartikeya Dwivedi 0 siblings, 1 reply; 15+ messages in thread From: Yonghong Song @ 2025-05-24 0:05 UTC (permalink / raw) To: Ilya Leoshkevich, Kumar Kartikeya Dwivedi, Alexei Starovoitov Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On 5/23/25 4:25 AM, Ilya Leoshkevich wrote: > On Mon, 2025-05-12 at 15:29 -0400, Kumar Kartikeya Dwivedi wrote: >> On Mon, 12 May 2025 at 12:41, Alexei Starovoitov >> <alexei.starovoitov@gmail.com> wrote: >>> On Mon, May 12, 2025 at 5:22 AM Ilya Leoshkevich >>> <iii@linux.ibm.com> wrote: >>>> On Fri, 2025-05-09 at 09:51 -0700, Alexei Starovoitov wrote: >>>>> On Thu, May 8, 2025 at 12:21 PM Ilya Leoshkevich >>>>> <iii@linux.ibm.com> >>>>> wrote: >>>>>> On Thu, 2025-05-08 at 11:38 -0700, Alexei Starovoitov wrote: >>>>>>> On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich >>>>>>> <iii@linux.ibm.com> >>>>>>> wrote: >>>>>>>> clang-21 complains about unused expressions in a few >>>>>>>> progs. >>>>>>>> Fix by explicitly casting the respective expressions to >>>>>>>> void. >>>>>>> ... >>>>>>>> if (val & _Q_LOCKED_MASK) >>>>>>>> - smp_cond_load_acquire_label(&lock- >>>>>>>>> locked, >>>>>>>> !VAL, >>>>>>>> release_err); >>>>>>>> + (void)smp_cond_load_acquire_label(&lock- >>>>>>>>> locked, >>>>>>>> !VAL, release_err); >>>>>>> Hmm. I'm on clang-21 too and I don't see them. >>>>>>> What warnings do you see ? >>>>>> In file included from progs/arena_spin_lock.c:7: >>>>>> progs/bpf_arena_spin_lock.h:305:1756: error: expression >>>>>> result >>>>>> unused >>>>>> [-Werror,-Wunused-value] >>>>>> 305 | ({ typeof(_Generic((*&lock->locked), char: (char)0, >>>>>> unsigned >>>>>> char : (unsigned char)0, signed char : (signed char)0, >>>>>> unsigned >>>>>> short : >>>>>> (unsigned short)0, signed short : (signed short)0, unsigned >>>>>> int : >>>>>> (unsigned int)0, signed int : (signed int)0, unsigned long : >>>>>> (unsigned >>>>>> long)0, signed long : (signed long)0, unsigned long long : >>>>>> (unsigned >>>>>> long long)0, signed long long : (signed long long)0, default: >>>>>> (typeof(*&lock->locked))0)) __val = ({ typeof(&lock->locked) >>>>>> __ptr >>>>>> = >>>>>> (&lock->locked); typeof(_Generic((*(&lock->locked)), char: >>>>>> (char)0, >>>>>> unsigned char : (unsigned char)0, signed char : (signed >>>>>> char)0, >>>>>> unsigned short : (unsigned short)0, signed short : (signed >>>>>> short)0, >>>>>> unsigned int : (unsigned int)0, signed int : (signed int)0, >>>>>> unsigned >>>>>> long : (unsigned long)0, signed long : (signed long)0, >>>>>> unsigned >>>>>> long >>>>>> long : (unsigned long long)0, signed long long : (signed long >>>>>> long)0, >>>>>> default: (typeof(*(&lock->locked)))0)) VAL; for (;;) { VAL = >>>>>> (typeof(_Generic((*(&lock->locked)), char: (char)0, unsigned >>>>>> char : >>>>>> (unsigned char)0, signed char : (signed char)0, unsigned >>>>>> short : >>>>>> (unsigned short)0, signed short : (signed short)0, unsigned >>>>>> int : >>>>>> (unsigned int)0, signed int : (signed int)0, unsigned long : >>>>>> (unsigned >>>>>> long)0, signed long : (signed long)0, unsigned long long : >>>>>> (unsigned >>>>>> long long)0, signed long long : (signed long long)0, default: >>>>>> (typeof(*(&lock->locked)))0)))(*(volatile typeof(*__ptr) >>>>>> *)&(*__ptr)); >>>>>> if (!VAL) break; ({ __label__ l_break, l_continue; asm >>>>>> volatile >>>>>> goto("may_goto %l[l_break]" :::: l_break); goto l_continue; >>>>>> l_break: >>>>>> goto release_err; l_continue:; }); ({}); } (typeof(*(&lock- >>>>>>> locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ unsigned >>>>>>> long >>>>>>> __val; >>>>>> __sync_fetch_and_add(&__val, 0); }); else asm volatile("" ::: >>>>>> "memory"); }); }); (typeof(*(&lock->locked)))__val; }); >>>>>> | >>>>>> ^ ~~~~~ >>>>>> 1 error generated. >>>>> hmm. The error is impossible to read. >>>>> >>>>> Kumar, >>>>> >>>>> Do you see a way to silence it differently ? >>>>> >>>>> Without adding (void)... >>>>> >>>>> Things like: >>>>> - bpf_obj_new(.. >>>>> + (void)bpf_obj_new(.. >>>>> >>>>> are good to fix, and if we could annotate >>>>> bpf_obj_new_impl kfunc with __must_check we would have done it, >>>>> >>>>> but >>>>> - arch_mcs_spin_lock... >>>>> + (void)arch_mcs_spin_lock... >>>>> >>>>> is odd. >>>> What do you think about moving (void) to the definition of >>>> arch_mcs_spin_lock_contended_label()? I can send a v2 if this is >>>> better. >>> Kumar, >>> >>> thoughts? >> Sorry for the delay, I was afk. >> >> The warning seems a bit aggressive, in the kernel we have users which >> do and do not use the value and it's fine. >> I think moving (void) inside the macro is a problem since at least >> rqspinlock like algorithm would want to inspect the result of the >> locked bit. >> No such users exist for now, of course. So maybe we can silence it >> until we do end up depending on the value. >> >> I will give a try with clang-21, but I think probably (void) in the >> source is better if we do need to silence it. > Gentle ping. > > This is still an issue with clang version 21.0.0 > (++20250522112647+491619a25003-1~exp1~20250522112819.1465). > I cannot reproduce the "unused expressions" error. What is the llvm cmake command line you are using? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-24 0:05 ` Yonghong Song @ 2025-05-24 1:01 ` Kumar Kartikeya Dwivedi 2025-05-24 21:05 ` Ilya Leoshkevich 0 siblings, 1 reply; 15+ messages in thread From: Kumar Kartikeya Dwivedi @ 2025-05-24 1:01 UTC (permalink / raw) To: Yonghong Song Cc: Ilya Leoshkevich, Alexei Starovoitov, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On Sat, 24 May 2025 at 02:06, Yonghong Song <yonghong.song@linux.dev> wrote: > > > > On 5/23/25 4:25 AM, Ilya Leoshkevich wrote: > > On Mon, 2025-05-12 at 15:29 -0400, Kumar Kartikeya Dwivedi wrote: > >> On Mon, 12 May 2025 at 12:41, Alexei Starovoitov > >> <alexei.starovoitov@gmail.com> wrote: > >>> On Mon, May 12, 2025 at 5:22 AM Ilya Leoshkevich > >>> <iii@linux.ibm.com> wrote: > >>>> On Fri, 2025-05-09 at 09:51 -0700, Alexei Starovoitov wrote: > >>>>> On Thu, May 8, 2025 at 12:21 PM Ilya Leoshkevich > >>>>> <iii@linux.ibm.com> > >>>>> wrote: > >>>>>> On Thu, 2025-05-08 at 11:38 -0700, Alexei Starovoitov wrote: > >>>>>>> On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich > >>>>>>> <iii@linux.ibm.com> > >>>>>>> wrote: > >>>>>>>> clang-21 complains about unused expressions in a few > >>>>>>>> progs. > >>>>>>>> Fix by explicitly casting the respective expressions to > >>>>>>>> void. > >>>>>>> ... > >>>>>>>> if (val & _Q_LOCKED_MASK) > >>>>>>>> - smp_cond_load_acquire_label(&lock- > >>>>>>>>> locked, > >>>>>>>> !VAL, > >>>>>>>> release_err); > >>>>>>>> + (void)smp_cond_load_acquire_label(&lock- > >>>>>>>>> locked, > >>>>>>>> !VAL, release_err); > >>>>>>> Hmm. I'm on clang-21 too and I don't see them. > >>>>>>> What warnings do you see ? > >>>>>> In file included from progs/arena_spin_lock.c:7: > >>>>>> progs/bpf_arena_spin_lock.h:305:1756: error: expression > >>>>>> result > >>>>>> unused > >>>>>> [-Werror,-Wunused-value] > >>>>>> 305 | ({ typeof(_Generic((*&lock->locked), char: (char)0, > >>>>>> unsigned > >>>>>> char : (unsigned char)0, signed char : (signed char)0, > >>>>>> unsigned > >>>>>> short : > >>>>>> (unsigned short)0, signed short : (signed short)0, unsigned > >>>>>> int : > >>>>>> (unsigned int)0, signed int : (signed int)0, unsigned long : > >>>>>> (unsigned > >>>>>> long)0, signed long : (signed long)0, unsigned long long : > >>>>>> (unsigned > >>>>>> long long)0, signed long long : (signed long long)0, default: > >>>>>> (typeof(*&lock->locked))0)) __val = ({ typeof(&lock->locked) > >>>>>> __ptr > >>>>>> = > >>>>>> (&lock->locked); typeof(_Generic((*(&lock->locked)), char: > >>>>>> (char)0, > >>>>>> unsigned char : (unsigned char)0, signed char : (signed > >>>>>> char)0, > >>>>>> unsigned short : (unsigned short)0, signed short : (signed > >>>>>> short)0, > >>>>>> unsigned int : (unsigned int)0, signed int : (signed int)0, > >>>>>> unsigned > >>>>>> long : (unsigned long)0, signed long : (signed long)0, > >>>>>> unsigned > >>>>>> long > >>>>>> long : (unsigned long long)0, signed long long : (signed long > >>>>>> long)0, > >>>>>> default: (typeof(*(&lock->locked)))0)) VAL; for (;;) { VAL = > >>>>>> (typeof(_Generic((*(&lock->locked)), char: (char)0, unsigned > >>>>>> char : > >>>>>> (unsigned char)0, signed char : (signed char)0, unsigned > >>>>>> short : > >>>>>> (unsigned short)0, signed short : (signed short)0, unsigned > >>>>>> int : > >>>>>> (unsigned int)0, signed int : (signed int)0, unsigned long : > >>>>>> (unsigned > >>>>>> long)0, signed long : (signed long)0, unsigned long long : > >>>>>> (unsigned > >>>>>> long long)0, signed long long : (signed long long)0, default: > >>>>>> (typeof(*(&lock->locked)))0)))(*(volatile typeof(*__ptr) > >>>>>> *)&(*__ptr)); > >>>>>> if (!VAL) break; ({ __label__ l_break, l_continue; asm > >>>>>> volatile > >>>>>> goto("may_goto %l[l_break]" :::: l_break); goto l_continue; > >>>>>> l_break: > >>>>>> goto release_err; l_continue:; }); ({}); } (typeof(*(&lock- > >>>>>>> locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ unsigned > >>>>>>> long > >>>>>>> __val; > >>>>>> __sync_fetch_and_add(&__val, 0); }); else asm volatile("" ::: > >>>>>> "memory"); }); }); (typeof(*(&lock->locked)))__val; }); > >>>>>> | > >>>>>> ^ ~~~~~ > >>>>>> 1 error generated. > >>>>> hmm. The error is impossible to read. > >>>>> > >>>>> Kumar, > >>>>> > >>>>> Do you see a way to silence it differently ? > >>>>> > >>>>> Without adding (void)... > >>>>> > >>>>> Things like: > >>>>> - bpf_obj_new(.. > >>>>> + (void)bpf_obj_new(.. > >>>>> > >>>>> are good to fix, and if we could annotate > >>>>> bpf_obj_new_impl kfunc with __must_check we would have done it, > >>>>> > >>>>> but > >>>>> - arch_mcs_spin_lock... > >>>>> + (void)arch_mcs_spin_lock... > >>>>> > >>>>> is odd. > >>>> What do you think about moving (void) to the definition of > >>>> arch_mcs_spin_lock_contended_label()? I can send a v2 if this is > >>>> better. > >>> Kumar, > >>> > >>> thoughts? > >> Sorry for the delay, I was afk. > >> > >> The warning seems a bit aggressive, in the kernel we have users which > >> do and do not use the value and it's fine. > >> I think moving (void) inside the macro is a problem since at least > >> rqspinlock like algorithm would want to inspect the result of the > >> locked bit. > >> No such users exist for now, of course. So maybe we can silence it > >> until we do end up depending on the value. > >> > >> I will give a try with clang-21, but I think probably (void) in the > >> source is better if we do need to silence it. > > Gentle ping. > > > > This is still an issue with clang version 21.0.0 > > (++20250522112647+491619a25003-1~exp1~20250522112819.1465). > > > I cannot reproduce the "unused expressions" error. What is the > llvm cmake command line you are using? > Sorry for the delay. I tried just now with clang built from the latest git checkout but I don't see it either. I built it following the steps at https://www.kernel.org/doc/Documentation/bpf/bpf_devel_QA.rst. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-24 1:01 ` Kumar Kartikeya Dwivedi @ 2025-05-24 21:05 ` Ilya Leoshkevich 2025-05-27 5:15 ` Yonghong Song 0 siblings, 1 reply; 15+ messages in thread From: Ilya Leoshkevich @ 2025-05-24 21:05 UTC (permalink / raw) To: Kumar Kartikeya Dwivedi, Yonghong Song Cc: Alexei Starovoitov, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On Sat, 2025-05-24 at 03:01 +0200, Kumar Kartikeya Dwivedi wrote: > On Sat, 24 May 2025 at 02:06, Yonghong Song <yonghong.song@linux.dev> > wrote: > > > > > > > > On 5/23/25 4:25 AM, Ilya Leoshkevich wrote: > > > On Mon, 2025-05-12 at 15:29 -0400, Kumar Kartikeya Dwivedi wrote: > > > > On Mon, 12 May 2025 at 12:41, Alexei Starovoitov > > > > <alexei.starovoitov@gmail.com> wrote: > > > > > On Mon, May 12, 2025 at 5:22 AM Ilya Leoshkevich > > > > > <iii@linux.ibm.com> wrote: > > > > > > On Fri, 2025-05-09 at 09:51 -0700, Alexei Starovoitov > > > > > > wrote: > > > > > > > On Thu, May 8, 2025 at 12:21 PM Ilya Leoshkevich > > > > > > > <iii@linux.ibm.com> > > > > > > > wrote: > > > > > > > > On Thu, 2025-05-08 at 11:38 -0700, Alexei Starovoitov > > > > > > > > wrote: > > > > > > > > > On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich > > > > > > > > > <iii@linux.ibm.com> > > > > > > > > > wrote: > > > > > > > > > > clang-21 complains about unused expressions in a > > > > > > > > > > few > > > > > > > > > > progs. > > > > > > > > > > Fix by explicitly casting the respective > > > > > > > > > > expressions to > > > > > > > > > > void. > > > > > > > > > ... > > > > > > > > > > if (val & _Q_LOCKED_MASK) > > > > > > > > > > - smp_cond_load_acquire_label(&lock- > > > > > > > > > > > locked, > > > > > > > > > > !VAL, > > > > > > > > > > release_err); > > > > > > > > > > + > > > > > > > > > > (void)smp_cond_load_acquire_label(&lock- > > > > > > > > > > > locked, > > > > > > > > > > !VAL, release_err); > > > > > > > > > Hmm. I'm on clang-21 too and I don't see them. > > > > > > > > > What warnings do you see ? > > > > > > > > In file included from progs/arena_spin_lock.c:7: > > > > > > > > progs/bpf_arena_spin_lock.h:305:1756: error: expression > > > > > > > > result > > > > > > > > unused > > > > > > > > [-Werror,-Wunused-value] > > > > > > > > 305 | ({ typeof(_Generic((*&lock->locked), char: > > > > > > > > (char)0, > > > > > > > > unsigned > > > > > > > > char : (unsigned char)0, signed char : (signed char)0, > > > > > > > > unsigned > > > > > > > > short : > > > > > > > > (unsigned short)0, signed short : (signed short)0, > > > > > > > > unsigned > > > > > > > > int : > > > > > > > > (unsigned int)0, signed int : (signed int)0, unsigned > > > > > > > > long : > > > > > > > > (unsigned > > > > > > > > long)0, signed long : (signed long)0, unsigned long > > > > > > > > long : > > > > > > > > (unsigned > > > > > > > > long long)0, signed long long : (signed long long)0, > > > > > > > > default: > > > > > > > > (typeof(*&lock->locked))0)) __val = ({ typeof(&lock- > > > > > > > > >locked) > > > > > > > > __ptr > > > > > > > > = > > > > > > > > (&lock->locked); typeof(_Generic((*(&lock->locked)), > > > > > > > > char: > > > > > > > > (char)0, > > > > > > > > unsigned char : (unsigned char)0, signed char : (signed > > > > > > > > char)0, > > > > > > > > unsigned short : (unsigned short)0, signed short : > > > > > > > > (signed > > > > > > > > short)0, > > > > > > > > unsigned int : (unsigned int)0, signed int : (signed > > > > > > > > int)0, > > > > > > > > unsigned > > > > > > > > long : (unsigned long)0, signed long : (signed long)0, > > > > > > > > unsigned > > > > > > > > long > > > > > > > > long : (unsigned long long)0, signed long long : > > > > > > > > (signed long > > > > > > > > long)0, > > > > > > > > default: (typeof(*(&lock->locked)))0)) VAL; for (;;) { > > > > > > > > VAL = > > > > > > > > (typeof(_Generic((*(&lock->locked)), char: (char)0, > > > > > > > > unsigned > > > > > > > > char : > > > > > > > > (unsigned char)0, signed char : (signed char)0, > > > > > > > > unsigned > > > > > > > > short : > > > > > > > > (unsigned short)0, signed short : (signed short)0, > > > > > > > > unsigned > > > > > > > > int : > > > > > > > > (unsigned int)0, signed int : (signed int)0, unsigned > > > > > > > > long : > > > > > > > > (unsigned > > > > > > > > long)0, signed long : (signed long)0, unsigned long > > > > > > > > long : > > > > > > > > (unsigned > > > > > > > > long long)0, signed long long : (signed long long)0, > > > > > > > > default: > > > > > > > > (typeof(*(&lock->locked)))0)))(*(volatile > > > > > > > > typeof(*__ptr) > > > > > > > > *)&(*__ptr)); > > > > > > > > if (!VAL) break; ({ __label__ l_break, l_continue; asm > > > > > > > > volatile > > > > > > > > goto("may_goto %l[l_break]" :::: l_break); goto > > > > > > > > l_continue; > > > > > > > > l_break: > > > > > > > > goto release_err; l_continue:; }); ({}); } > > > > > > > > (typeof(*(&lock- > > > > > > > > > locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ > > > > > > > > > unsigned > > > > > > > > > long > > > > > > > > > __val; > > > > > > > > __sync_fetch_and_add(&__val, 0); }); else asm > > > > > > > > volatile("" ::: > > > > > > > > "memory"); }); }); (typeof(*(&lock->locked)))__val; }); > > > > > > > > | > > > > > > > > ^ ~~~~~ > > > > > > > > 1 error generated. > > > > > > > hmm. The error is impossible to read. > > > > > > > > > > > > > > Kumar, > > > > > > > > > > > > > > Do you see a way to silence it differently ? > > > > > > > > > > > > > > Without adding (void)... > > > > > > > > > > > > > > Things like: > > > > > > > - bpf_obj_new(.. > > > > > > > + (void)bpf_obj_new(.. > > > > > > > > > > > > > > are good to fix, and if we could annotate > > > > > > > bpf_obj_new_impl kfunc with __must_check we would have > > > > > > > done it, > > > > > > > > > > > > > > but > > > > > > > - arch_mcs_spin_lock... > > > > > > > + (void)arch_mcs_spin_lock... > > > > > > > > > > > > > > is odd. > > > > > > What do you think about moving (void) to the definition of > > > > > > arch_mcs_spin_lock_contended_label()? I can send a v2 if > > > > > > this is > > > > > > better. > > > > > Kumar, > > > > > > > > > > thoughts? > > > > Sorry for the delay, I was afk. > > > > > > > > The warning seems a bit aggressive, in the kernel we have users > > > > which > > > > do and do not use the value and it's fine. > > > > I think moving (void) inside the macro is a problem since at > > > > least > > > > rqspinlock like algorithm would want to inspect the result of > > > > the > > > > locked bit. > > > > No such users exist for now, of course. So maybe we can silence > > > > it > > > > until we do end up depending on the value. > > > > > > > > I will give a try with clang-21, but I think probably (void) in > > > > the > > > > source is better if we do need to silence it. > > > Gentle ping. > > > > > > This is still an issue with clang version 21.0.0 > > > (++20250522112647+491619a25003-1~exp1~20250522112819.1465). > > > > > I cannot reproduce the "unused expressions" error. What is the > > llvm cmake command line you are using? > > > > Sorry for the delay. I tried just now with clang built from the > latest > git checkout but I don't see it either. > I built it following the steps at > https://www.kernel.org/doc/Documentation/bpf/bpf_devel_QA.rst. I use the following make invocation: make CC="ccache gcc" LD=ld.lld-21 O="$PWD/../linux-build-s390x" CLANG="ccache clang-21" LLVM_STRIP=llvm-strip-21 LLC=llc-21 LLD=lld-21 -j128 -C tools/testing/selftests/bpf BPF_GCC= V=1 which results in the following clang invocation: ccache clang-21 -g -Wall -Werror -D__TARGET_ARCH_s390 -mbig-endian - I"$PWD/../../../../.."/linux-build-s390x//tools/include - I"$PWD/../../../../.."/linux/tools/testing/selftests/bpf - I"$PWD/../../../../.."/linux/tools/include/uapi - I"$PWD/../../../../.."/usr/include -std=gnu11 -fno-strict-aliasing - Wno-compare-distinct-pointer-types -idirafter /usr/lib/llvm- 21/lib/clang/21/include -idirafter /usr/local/include -idirafter /usr/include/s390x-linux-gnu -idirafter /usr/include - DENABLE_ATOMICS_TESTS -O2 --target=bpfeb -c progs/arena_spin_lock.c - mcpu=v3 -o "$PWD/../../../../.."/linux-build- s390x//arena_spin_lock.bpf.o I tried dropping ccache, but it did not help. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-24 21:05 ` Ilya Leoshkevich @ 2025-05-27 5:15 ` Yonghong Song 2025-05-27 8:27 ` Ilya Leoshkevich 0 siblings, 1 reply; 15+ messages in thread From: Yonghong Song @ 2025-05-27 5:15 UTC (permalink / raw) To: Ilya Leoshkevich, Kumar Kartikeya Dwivedi Cc: Alexei Starovoitov, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On 5/24/25 2:05 PM, Ilya Leoshkevich wrote: > On Sat, 2025-05-24 at 03:01 +0200, Kumar Kartikeya Dwivedi wrote: >> On Sat, 24 May 2025 at 02:06, Yonghong Song <yonghong.song@linux.dev> >> wrote: >>> >>> >>> On 5/23/25 4:25 AM, Ilya Leoshkevich wrote: >>>> On Mon, 2025-05-12 at 15:29 -0400, Kumar Kartikeya Dwivedi wrote: >>>>> On Mon, 12 May 2025 at 12:41, Alexei Starovoitov >>>>> <alexei.starovoitov@gmail.com> wrote: >>>>>> On Mon, May 12, 2025 at 5:22 AM Ilya Leoshkevich >>>>>> <iii@linux.ibm.com> wrote: >>>>>>> On Fri, 2025-05-09 at 09:51 -0700, Alexei Starovoitov >>>>>>> wrote: >>>>>>>> On Thu, May 8, 2025 at 12:21 PM Ilya Leoshkevich >>>>>>>> <iii@linux.ibm.com> >>>>>>>> wrote: >>>>>>>>> On Thu, 2025-05-08 at 11:38 -0700, Alexei Starovoitov >>>>>>>>> wrote: >>>>>>>>>> On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich >>>>>>>>>> <iii@linux.ibm.com> >>>>>>>>>> wrote: >>>>>>>>>>> clang-21 complains about unused expressions in a >>>>>>>>>>> few >>>>>>>>>>> progs. >>>>>>>>>>> Fix by explicitly casting the respective >>>>>>>>>>> expressions to >>>>>>>>>>> void. >>>>>>>>>> ... >>>>>>>>>>> if (val & _Q_LOCKED_MASK) >>>>>>>>>>> - smp_cond_load_acquire_label(&lock- >>>>>>>>>>>> locked, >>>>>>>>>>> !VAL, >>>>>>>>>>> release_err); >>>>>>>>>>> + >>>>>>>>>>> (void)smp_cond_load_acquire_label(&lock- >>>>>>>>>>>> locked, >>>>>>>>>>> !VAL, release_err); >>>>>>>>>> Hmm. I'm on clang-21 too and I don't see them. >>>>>>>>>> What warnings do you see ? >>>>>>>>> In file included from progs/arena_spin_lock.c:7: >>>>>>>>> progs/bpf_arena_spin_lock.h:305:1756: error: expression >>>>>>>>> result >>>>>>>>> unused >>>>>>>>> [-Werror,-Wunused-value] >>>>>>>>> 305 | ({ typeof(_Generic((*&lock->locked), char: >>>>>>>>> (char)0, >>>>>>>>> unsigned >>>>>>>>> char : (unsigned char)0, signed char : (signed char)0, >>>>>>>>> unsigned >>>>>>>>> short : >>>>>>>>> (unsigned short)0, signed short : (signed short)0, >>>>>>>>> unsigned >>>>>>>>> int : >>>>>>>>> (unsigned int)0, signed int : (signed int)0, unsigned >>>>>>>>> long : >>>>>>>>> (unsigned >>>>>>>>> long)0, signed long : (signed long)0, unsigned long >>>>>>>>> long : >>>>>>>>> (unsigned >>>>>>>>> long long)0, signed long long : (signed long long)0, >>>>>>>>> default: >>>>>>>>> (typeof(*&lock->locked))0)) __val = ({ typeof(&lock- >>>>>>>>>> locked) >>>>>>>>> __ptr >>>>>>>>> = >>>>>>>>> (&lock->locked); typeof(_Generic((*(&lock->locked)), >>>>>>>>> char: >>>>>>>>> (char)0, >>>>>>>>> unsigned char : (unsigned char)0, signed char : (signed >>>>>>>>> char)0, >>>>>>>>> unsigned short : (unsigned short)0, signed short : >>>>>>>>> (signed >>>>>>>>> short)0, >>>>>>>>> unsigned int : (unsigned int)0, signed int : (signed >>>>>>>>> int)0, >>>>>>>>> unsigned >>>>>>>>> long : (unsigned long)0, signed long : (signed long)0, >>>>>>>>> unsigned >>>>>>>>> long >>>>>>>>> long : (unsigned long long)0, signed long long : >>>>>>>>> (signed long >>>>>>>>> long)0, >>>>>>>>> default: (typeof(*(&lock->locked)))0)) VAL; for (;;) { >>>>>>>>> VAL = >>>>>>>>> (typeof(_Generic((*(&lock->locked)), char: (char)0, >>>>>>>>> unsigned >>>>>>>>> char : >>>>>>>>> (unsigned char)0, signed char : (signed char)0, >>>>>>>>> unsigned >>>>>>>>> short : >>>>>>>>> (unsigned short)0, signed short : (signed short)0, >>>>>>>>> unsigned >>>>>>>>> int : >>>>>>>>> (unsigned int)0, signed int : (signed int)0, unsigned >>>>>>>>> long : >>>>>>>>> (unsigned >>>>>>>>> long)0, signed long : (signed long)0, unsigned long >>>>>>>>> long : >>>>>>>>> (unsigned >>>>>>>>> long long)0, signed long long : (signed long long)0, >>>>>>>>> default: >>>>>>>>> (typeof(*(&lock->locked)))0)))(*(volatile >>>>>>>>> typeof(*__ptr) >>>>>>>>> *)&(*__ptr)); >>>>>>>>> if (!VAL) break; ({ __label__ l_break, l_continue; asm >>>>>>>>> volatile >>>>>>>>> goto("may_goto %l[l_break]" :::: l_break); goto >>>>>>>>> l_continue; >>>>>>>>> l_break: >>>>>>>>> goto release_err; l_continue:; }); ({}); } >>>>>>>>> (typeof(*(&lock- >>>>>>>>>> locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ >>>>>>>>>> unsigned >>>>>>>>>> long >>>>>>>>>> __val; >>>>>>>>> __sync_fetch_and_add(&__val, 0); }); else asm >>>>>>>>> volatile("" ::: >>>>>>>>> "memory"); }); }); (typeof(*(&lock->locked)))__val; }); >>>>>>>>> | >>>>>>>>> ^ ~~~~~ >>>>>>>>> 1 error generated. >>>>>>>> hmm. The error is impossible to read. >>>>>>>> >>>>>>>> Kumar, >>>>>>>> >>>>>>>> Do you see a way to silence it differently ? >>>>>>>> >>>>>>>> Without adding (void)... >>>>>>>> >>>>>>>> Things like: >>>>>>>> - bpf_obj_new(.. >>>>>>>> + (void)bpf_obj_new(.. >>>>>>>> >>>>>>>> are good to fix, and if we could annotate >>>>>>>> bpf_obj_new_impl kfunc with __must_check we would have >>>>>>>> done it, >>>>>>>> >>>>>>>> but >>>>>>>> - arch_mcs_spin_lock... >>>>>>>> + (void)arch_mcs_spin_lock... >>>>>>>> >>>>>>>> is odd. >>>>>>> What do you think about moving (void) to the definition of >>>>>>> arch_mcs_spin_lock_contended_label()? I can send a v2 if >>>>>>> this is >>>>>>> better. >>>>>> Kumar, >>>>>> >>>>>> thoughts? >>>>> Sorry for the delay, I was afk. >>>>> >>>>> The warning seems a bit aggressive, in the kernel we have users >>>>> which >>>>> do and do not use the value and it's fine. >>>>> I think moving (void) inside the macro is a problem since at >>>>> least >>>>> rqspinlock like algorithm would want to inspect the result of >>>>> the >>>>> locked bit. >>>>> No such users exist for now, of course. So maybe we can silence >>>>> it >>>>> until we do end up depending on the value. >>>>> >>>>> I will give a try with clang-21, but I think probably (void) in >>>>> the >>>>> source is better if we do need to silence it. >>>> Gentle ping. >>>> >>>> This is still an issue with clang version 21.0.0 >>>> (++20250522112647+491619a25003-1~exp1~20250522112819.1465). >>>> >>> I cannot reproduce the "unused expressions" error. What is the >>> llvm cmake command line you are using? >>> >> Sorry for the delay. I tried just now with clang built from the >> latest >> git checkout but I don't see it either. >> I built it following the steps at >> https://www.kernel.org/doc/Documentation/bpf/bpf_devel_QA.rst. > I use the following make invocation: > > make CC="ccache gcc" LD=ld.lld-21 O="$PWD/../linux-build-s390x" > CLANG="ccache clang-21" LLVM_STRIP=llvm-strip-21 LLC=llc-21 LLD=lld-21 > -j128 -C tools/testing/selftests/bpf BPF_GCC= V=1 > > which results in the following clang invocation: > > ccache clang-21 -g -Wall -Werror -D__TARGET_ARCH_s390 -mbig-endian - > I"$PWD/../../../../.."/linux-build-s390x//tools/include - > I"$PWD/../../../../.."/linux/tools/testing/selftests/bpf - > I"$PWD/../../../../.."/linux/tools/include/uapi - > I"$PWD/../../../../.."/usr/include -std=gnu11 -fno-strict-aliasing - > Wno-compare-distinct-pointer-types -idirafter /usr/lib/llvm- > 21/lib/clang/21/include -idirafter /usr/local/include -idirafter > /usr/include/s390x-linux-gnu -idirafter /usr/include - > DENABLE_ATOMICS_TESTS -O2 --target=bpfeb -c progs/arena_spin_lock.c - > mcpu=v3 -o "$PWD/../../../../.."/linux-build- > s390x//arena_spin_lock.bpf.o > > I tried dropping ccache, but it did not help. Thanks, Ilya. It could be great if you can find out the cmake command lines which eventually builds your clang-21. Once cmake command lines are available, I can build the compiler on x86_64 host and do some checking for it. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-27 5:15 ` Yonghong Song @ 2025-05-27 8:27 ` Ilya Leoshkevich 2025-05-27 21:26 ` Yonghong Song 0 siblings, 1 reply; 15+ messages in thread From: Ilya Leoshkevich @ 2025-05-27 8:27 UTC (permalink / raw) To: Yonghong Song, Kumar Kartikeya Dwivedi Cc: Alexei Starovoitov, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On Mon, 2025-05-26 at 22:15 -0700, Yonghong Song wrote: > > > On 5/24/25 2:05 PM, Ilya Leoshkevich wrote: > > On Sat, 2025-05-24 at 03:01 +0200, Kumar Kartikeya Dwivedi wrote: > > > On Sat, 24 May 2025 at 02:06, Yonghong Song > > > <yonghong.song@linux.dev> > > > wrote: > > > > > > > > > > > > On 5/23/25 4:25 AM, Ilya Leoshkevich wrote: > > > > > On Mon, 2025-05-12 at 15:29 -0400, Kumar Kartikeya Dwivedi > > > > > wrote: > > > > > > On Mon, 12 May 2025 at 12:41, Alexei Starovoitov > > > > > > <alexei.starovoitov@gmail.com> wrote: > > > > > > > On Mon, May 12, 2025 at 5:22 AM Ilya Leoshkevich > > > > > > > <iii@linux.ibm.com> wrote: > > > > > > > > On Fri, 2025-05-09 at 09:51 -0700, Alexei Starovoitov > > > > > > > > wrote: > > > > > > > > > On Thu, May 8, 2025 at 12:21 PM Ilya Leoshkevich > > > > > > > > > <iii@linux.ibm.com> > > > > > > > > > wrote: > > > > > > > > > > On Thu, 2025-05-08 at 11:38 -0700, Alexei > > > > > > > > > > Starovoitov > > > > > > > > > > wrote: > > > > > > > > > > > On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich > > > > > > > > > > > <iii@linux.ibm.com> > > > > > > > > > > > wrote: > > > > > > > > > > > > clang-21 complains about unused expressions in > > > > > > > > > > > > a > > > > > > > > > > > > few > > > > > > > > > > > > progs. > > > > > > > > > > > > Fix by explicitly casting the respective > > > > > > > > > > > > expressions to > > > > > > > > > > > > void. > > > > > > > > > > > ... > > > > > > > > > > > > if (val & _Q_LOCKED_MASK) > > > > > > > > > > > > - > > > > > > > > > > > > smp_cond_load_acquire_label(&lock- > > > > > > > > > > > > > locked, > > > > > > > > > > > > !VAL, > > > > > > > > > > > > release_err); > > > > > > > > > > > > + > > > > > > > > > > > > (void)smp_cond_load_acquire_label(&lock- > > > > > > > > > > > > > locked, > > > > > > > > > > > > !VAL, release_err); > > > > > > > > > > > Hmm. I'm on clang-21 too and I don't see them. > > > > > > > > > > > What warnings do you see ? > > > > > > > > > > In file included from progs/arena_spin_lock.c:7: > > > > > > > > > > progs/bpf_arena_spin_lock.h:305:1756: error: > > > > > > > > > > expression > > > > > > > > > > result > > > > > > > > > > unused > > > > > > > > > > [-Werror,-Wunused-value] > > > > > > > > > > 305 | ({ typeof(_Generic((*&lock->locked), > > > > > > > > > > char: > > > > > > > > > > (char)0, > > > > > > > > > > unsigned > > > > > > > > > > char : (unsigned char)0, signed char : (signed > > > > > > > > > > char)0, > > > > > > > > > > unsigned > > > > > > > > > > short : > > > > > > > > > > (unsigned short)0, signed short : (signed short)0, > > > > > > > > > > unsigned > > > > > > > > > > int : > > > > > > > > > > (unsigned int)0, signed int : (signed int)0, > > > > > > > > > > unsigned > > > > > > > > > > long : > > > > > > > > > > (unsigned > > > > > > > > > > long)0, signed long : (signed long)0, unsigned long > > > > > > > > > > long : > > > > > > > > > > (unsigned > > > > > > > > > > long long)0, signed long long : (signed long > > > > > > > > > > long)0, > > > > > > > > > > default: > > > > > > > > > > (typeof(*&lock->locked))0)) __val = ({ > > > > > > > > > > typeof(&lock- > > > > > > > > > > > locked) > > > > > > > > > > __ptr > > > > > > > > > > = > > > > > > > > > > (&lock->locked); typeof(_Generic((*(&lock- > > > > > > > > > > >locked)), > > > > > > > > > > char: > > > > > > > > > > (char)0, > > > > > > > > > > unsigned char : (unsigned char)0, signed char : > > > > > > > > > > (signed > > > > > > > > > > char)0, > > > > > > > > > > unsigned short : (unsigned short)0, signed short : > > > > > > > > > > (signed > > > > > > > > > > short)0, > > > > > > > > > > unsigned int : (unsigned int)0, signed int : > > > > > > > > > > (signed > > > > > > > > > > int)0, > > > > > > > > > > unsigned > > > > > > > > > > long : (unsigned long)0, signed long : (signed > > > > > > > > > > long)0, > > > > > > > > > > unsigned > > > > > > > > > > long > > > > > > > > > > long : (unsigned long long)0, signed long long : > > > > > > > > > > (signed long > > > > > > > > > > long)0, > > > > > > > > > > default: (typeof(*(&lock->locked)))0)) VAL; for > > > > > > > > > > (;;) { > > > > > > > > > > VAL = > > > > > > > > > > (typeof(_Generic((*(&lock->locked)), char: (char)0, > > > > > > > > > > unsigned > > > > > > > > > > char : > > > > > > > > > > (unsigned char)0, signed char : (signed char)0, > > > > > > > > > > unsigned > > > > > > > > > > short : > > > > > > > > > > (unsigned short)0, signed short : (signed short)0, > > > > > > > > > > unsigned > > > > > > > > > > int : > > > > > > > > > > (unsigned int)0, signed int : (signed int)0, > > > > > > > > > > unsigned > > > > > > > > > > long : > > > > > > > > > > (unsigned > > > > > > > > > > long)0, signed long : (signed long)0, unsigned long > > > > > > > > > > long : > > > > > > > > > > (unsigned > > > > > > > > > > long long)0, signed long long : (signed long > > > > > > > > > > long)0, > > > > > > > > > > default: > > > > > > > > > > (typeof(*(&lock->locked)))0)))(*(volatile > > > > > > > > > > typeof(*__ptr) > > > > > > > > > > *)&(*__ptr)); > > > > > > > > > > if (!VAL) break; ({ __label__ l_break, l_continue; > > > > > > > > > > asm > > > > > > > > > > volatile > > > > > > > > > > goto("may_goto %l[l_break]" :::: l_break); goto > > > > > > > > > > l_continue; > > > > > > > > > > l_break: > > > > > > > > > > goto release_err; l_continue:; }); ({}); } > > > > > > > > > > (typeof(*(&lock- > > > > > > > > > > > locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ > > > > > > > > > > > unsigned > > > > > > > > > > > long > > > > > > > > > > > __val; > > > > > > > > > > __sync_fetch_and_add(&__val, 0); }); else asm > > > > > > > > > > volatile("" ::: > > > > > > > > > > "memory"); }); }); (typeof(*(&lock->locked)))__val; > > > > > > > > > > }); > > > > > > > > > > | > > > > > > > > > > ^ ~~~~~ > > > > > > > > > > 1 error generated. > > > > > > > > > hmm. The error is impossible to read. > > > > > > > > > > > > > > > > > > Kumar, > > > > > > > > > > > > > > > > > > Do you see a way to silence it differently ? > > > > > > > > > > > > > > > > > > Without adding (void)... > > > > > > > > > > > > > > > > > > Things like: > > > > > > > > > - bpf_obj_new(.. > > > > > > > > > + (void)bpf_obj_new(.. > > > > > > > > > > > > > > > > > > are good to fix, and if we could annotate > > > > > > > > > bpf_obj_new_impl kfunc with __must_check we would > > > > > > > > > have > > > > > > > > > done it, > > > > > > > > > > > > > > > > > > but > > > > > > > > > - arch_mcs_spin_lock... > > > > > > > > > + (void)arch_mcs_spin_lock... > > > > > > > > > > > > > > > > > > is odd. > > > > > > > > What do you think about moving (void) to the definition > > > > > > > > of > > > > > > > > arch_mcs_spin_lock_contended_label()? I can send a v2 > > > > > > > > if > > > > > > > > this is > > > > > > > > better. > > > > > > > Kumar, > > > > > > > > > > > > > > thoughts? > > > > > > Sorry for the delay, I was afk. > > > > > > > > > > > > The warning seems a bit aggressive, in the kernel we have > > > > > > users > > > > > > which > > > > > > do and do not use the value and it's fine. > > > > > > I think moving (void) inside the macro is a problem since > > > > > > at > > > > > > least > > > > > > rqspinlock like algorithm would want to inspect the result > > > > > > of > > > > > > the > > > > > > locked bit. > > > > > > No such users exist for now, of course. So maybe we can > > > > > > silence > > > > > > it > > > > > > until we do end up depending on the value. > > > > > > > > > > > > I will give a try with clang-21, but I think probably > > > > > > (void) in > > > > > > the > > > > > > source is better if we do need to silence it. > > > > > Gentle ping. > > > > > > > > > > This is still an issue with clang version 21.0.0 > > > > > (++20250522112647+491619a25003-1~exp1~20250522112819.1465). > > > > > > > > > I cannot reproduce the "unused expressions" error. What is the > > > > llvm cmake command line you are using? > > > > > > > Sorry for the delay. I tried just now with clang built from the > > > latest > > > git checkout but I don't see it either. > > > I built it following the steps at > > > https://www.kernel.org/doc/Documentation/bpf/bpf_devel_QA.rst. > > I use the following make invocation: > > > > make CC="ccache gcc" LD=ld.lld-21 O="$PWD/../linux-build-s390x" > > CLANG="ccache clang-21" LLVM_STRIP=llvm-strip-21 LLC=llc-21 > > LLD=lld-21 > > -j128 -C tools/testing/selftests/bpf BPF_GCC= V=1 > > > > which results in the following clang invocation: > > > > ccache clang-21 -g -Wall -Werror -D__TARGET_ARCH_s390 -mbig-endian > > - > > I"$PWD/../../../../.."/linux-build-s390x//tools/include - > > I"$PWD/../../../../.."/linux/tools/testing/selftests/bpf - > > I"$PWD/../../../../.."/linux/tools/include/uapi - > > I"$PWD/../../../../.."/usr/include -std=gnu11 -fno-strict-aliasing > > - > > Wno-compare-distinct-pointer-types -idirafter /usr/lib/llvm- > > 21/lib/clang/21/include -idirafter /usr/local/include -idirafter > > /usr/include/s390x-linux-gnu -idirafter /usr/include - > > DENABLE_ATOMICS_TESTS -O2 --target=bpfeb -c > > progs/arena_spin_lock.c - > > mcpu=v3 -o "$PWD/../../../../.."/linux-build- > > s390x//arena_spin_lock.bpf.o > > > > I tried dropping ccache, but it did not help. > > Thanks, Ilya. It could be great if you can find out the > cmake command lines which eventually builds your clang-21. > Once cmake command lines are available, I can build > the compiler on x86_64 host and do some checking for it. Hi Yonghong, I don't build it, I take it from apt.llvm.org. It's surprising we don't see this in CI, because it also takes clang from there. If you think this is a compiler and not a code bug, I can debug this myself, because maybe it's reproducible only on s390x. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-27 8:27 ` Ilya Leoshkevich @ 2025-05-27 21:26 ` Yonghong Song 2025-05-27 21:31 ` Yonghong Song 0 siblings, 1 reply; 15+ messages in thread From: Yonghong Song @ 2025-05-27 21:26 UTC (permalink / raw) To: Ilya Leoshkevich, Kumar Kartikeya Dwivedi Cc: Alexei Starovoitov, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On 5/27/25 1:27 AM, Ilya Leoshkevich wrote: > On Mon, 2025-05-26 at 22:15 -0700, Yonghong Song wrote: >> >> On 5/24/25 2:05 PM, Ilya Leoshkevich wrote: >>> On Sat, 2025-05-24 at 03:01 +0200, Kumar Kartikeya Dwivedi wrote: >>>> On Sat, 24 May 2025 at 02:06, Yonghong Song >>>> <yonghong.song@linux.dev> >>>> wrote: >>>>> >>>>> On 5/23/25 4:25 AM, Ilya Leoshkevich wrote: >>>>>> On Mon, 2025-05-12 at 15:29 -0400, Kumar Kartikeya Dwivedi >>>>>> wrote: >>>>>>> On Mon, 12 May 2025 at 12:41, Alexei Starovoitov >>>>>>> <alexei.starovoitov@gmail.com> wrote: >>>>>>>> On Mon, May 12, 2025 at 5:22 AM Ilya Leoshkevich >>>>>>>> <iii@linux.ibm.com> wrote: >>>>>>>>> On Fri, 2025-05-09 at 09:51 -0700, Alexei Starovoitov >>>>>>>>> wrote: >>>>>>>>>> On Thu, May 8, 2025 at 12:21 PM Ilya Leoshkevich >>>>>>>>>> <iii@linux.ibm.com> >>>>>>>>>> wrote: >>>>>>>>>>> On Thu, 2025-05-08 at 11:38 -0700, Alexei >>>>>>>>>>> Starovoitov >>>>>>>>>>> wrote: >>>>>>>>>>>> On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich >>>>>>>>>>>> <iii@linux.ibm.com> >>>>>>>>>>>> wrote: >>>>>>>>>>>>> clang-21 complains about unused expressions in >>>>>>>>>>>>> a >>>>>>>>>>>>> few >>>>>>>>>>>>> progs. >>>>>>>>>>>>> Fix by explicitly casting the respective >>>>>>>>>>>>> expressions to >>>>>>>>>>>>> void. >>>>>>>>>>>> ... >>>>>>>>>>>>> if (val & _Q_LOCKED_MASK) >>>>>>>>>>>>> - >>>>>>>>>>>>> smp_cond_load_acquire_label(&lock- >>>>>>>>>>>>>> locked, >>>>>>>>>>>>> !VAL, >>>>>>>>>>>>> release_err); >>>>>>>>>>>>> + >>>>>>>>>>>>> (void)smp_cond_load_acquire_label(&lock- >>>>>>>>>>>>>> locked, >>>>>>>>>>>>> !VAL, release_err); >>>>>>>>>>>> Hmm. I'm on clang-21 too and I don't see them. >>>>>>>>>>>> What warnings do you see ? >>>>>>>>>>> In file included from progs/arena_spin_lock.c:7: >>>>>>>>>>> progs/bpf_arena_spin_lock.h:305:1756: error: >>>>>>>>>>> expression >>>>>>>>>>> result >>>>>>>>>>> unused >>>>>>>>>>> [-Werror,-Wunused-value] >>>>>>>>>>> 305 | ({ typeof(_Generic((*&lock->locked), >>>>>>>>>>> char: >>>>>>>>>>> (char)0, >>>>>>>>>>> unsigned >>>>>>>>>>> char : (unsigned char)0, signed char : (signed >>>>>>>>>>> char)0, >>>>>>>>>>> unsigned >>>>>>>>>>> short : >>>>>>>>>>> (unsigned short)0, signed short : (signed short)0, >>>>>>>>>>> unsigned >>>>>>>>>>> int : >>>>>>>>>>> (unsigned int)0, signed int : (signed int)0, >>>>>>>>>>> unsigned >>>>>>>>>>> long : >>>>>>>>>>> (unsigned >>>>>>>>>>> long)0, signed long : (signed long)0, unsigned long >>>>>>>>>>> long : >>>>>>>>>>> (unsigned >>>>>>>>>>> long long)0, signed long long : (signed long >>>>>>>>>>> long)0, >>>>>>>>>>> default: >>>>>>>>>>> (typeof(*&lock->locked))0)) __val = ({ >>>>>>>>>>> typeof(&lock- >>>>>>>>>>>> locked) >>>>>>>>>>> __ptr >>>>>>>>>>> = >>>>>>>>>>> (&lock->locked); typeof(_Generic((*(&lock- >>>>>>>>>>>> locked)), >>>>>>>>>>> char: >>>>>>>>>>> (char)0, >>>>>>>>>>> unsigned char : (unsigned char)0, signed char : >>>>>>>>>>> (signed >>>>>>>>>>> char)0, >>>>>>>>>>> unsigned short : (unsigned short)0, signed short : >>>>>>>>>>> (signed >>>>>>>>>>> short)0, >>>>>>>>>>> unsigned int : (unsigned int)0, signed int : >>>>>>>>>>> (signed >>>>>>>>>>> int)0, >>>>>>>>>>> unsigned >>>>>>>>>>> long : (unsigned long)0, signed long : (signed >>>>>>>>>>> long)0, >>>>>>>>>>> unsigned >>>>>>>>>>> long >>>>>>>>>>> long : (unsigned long long)0, signed long long : >>>>>>>>>>> (signed long >>>>>>>>>>> long)0, >>>>>>>>>>> default: (typeof(*(&lock->locked)))0)) VAL; for >>>>>>>>>>> (;;) { >>>>>>>>>>> VAL = >>>>>>>>>>> (typeof(_Generic((*(&lock->locked)), char: (char)0, >>>>>>>>>>> unsigned >>>>>>>>>>> char : >>>>>>>>>>> (unsigned char)0, signed char : (signed char)0, >>>>>>>>>>> unsigned >>>>>>>>>>> short : >>>>>>>>>>> (unsigned short)0, signed short : (signed short)0, >>>>>>>>>>> unsigned >>>>>>>>>>> int : >>>>>>>>>>> (unsigned int)0, signed int : (signed int)0, >>>>>>>>>>> unsigned >>>>>>>>>>> long : >>>>>>>>>>> (unsigned >>>>>>>>>>> long)0, signed long : (signed long)0, unsigned long >>>>>>>>>>> long : >>>>>>>>>>> (unsigned >>>>>>>>>>> long long)0, signed long long : (signed long >>>>>>>>>>> long)0, >>>>>>>>>>> default: >>>>>>>>>>> (typeof(*(&lock->locked)))0)))(*(volatile >>>>>>>>>>> typeof(*__ptr) >>>>>>>>>>> *)&(*__ptr)); >>>>>>>>>>> if (!VAL) break; ({ __label__ l_break, l_continue; >>>>>>>>>>> asm >>>>>>>>>>> volatile >>>>>>>>>>> goto("may_goto %l[l_break]" :::: l_break); goto >>>>>>>>>>> l_continue; >>>>>>>>>>> l_break: >>>>>>>>>>> goto release_err; l_continue:; }); ({}); } >>>>>>>>>>> (typeof(*(&lock- >>>>>>>>>>>> locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ >>>>>>>>>>>> unsigned >>>>>>>>>>>> long >>>>>>>>>>>> __val; >>>>>>>>>>> __sync_fetch_and_add(&__val, 0); }); else asm >>>>>>>>>>> volatile("" ::: >>>>>>>>>>> "memory"); }); }); (typeof(*(&lock->locked)))__val; >>>>>>>>>>> }); >>>>>>>>>>> | >>>>>>>>>>> ^ ~~~~~ >>>>>>>>>>> 1 error generated. >>>>>>>>>> hmm. The error is impossible to read. >>>>>>>>>> >>>>>>>>>> Kumar, >>>>>>>>>> >>>>>>>>>> Do you see a way to silence it differently ? >>>>>>>>>> >>>>>>>>>> Without adding (void)... >>>>>>>>>> >>>>>>>>>> Things like: >>>>>>>>>> - bpf_obj_new(.. >>>>>>>>>> + (void)bpf_obj_new(.. >>>>>>>>>> >>>>>>>>>> are good to fix, and if we could annotate >>>>>>>>>> bpf_obj_new_impl kfunc with __must_check we would >>>>>>>>>> have >>>>>>>>>> done it, >>>>>>>>>> >>>>>>>>>> but >>>>>>>>>> - arch_mcs_spin_lock... >>>>>>>>>> + (void)arch_mcs_spin_lock... >>>>>>>>>> >>>>>>>>>> is odd. >>>>>>>>> What do you think about moving (void) to the definition >>>>>>>>> of >>>>>>>>> arch_mcs_spin_lock_contended_label()? I can send a v2 >>>>>>>>> if >>>>>>>>> this is >>>>>>>>> better. >>>>>>>> Kumar, >>>>>>>> >>>>>>>> thoughts? >>>>>>> Sorry for the delay, I was afk. >>>>>>> >>>>>>> The warning seems a bit aggressive, in the kernel we have >>>>>>> users >>>>>>> which >>>>>>> do and do not use the value and it's fine. >>>>>>> I think moving (void) inside the macro is a problem since >>>>>>> at >>>>>>> least >>>>>>> rqspinlock like algorithm would want to inspect the result >>>>>>> of >>>>>>> the >>>>>>> locked bit. >>>>>>> No such users exist for now, of course. So maybe we can >>>>>>> silence >>>>>>> it >>>>>>> until we do end up depending on the value. >>>>>>> >>>>>>> I will give a try with clang-21, but I think probably >>>>>>> (void) in >>>>>>> the >>>>>>> source is better if we do need to silence it. >>>>>> Gentle ping. >>>>>> >>>>>> This is still an issue with clang version 21.0.0 >>>>>> (++20250522112647+491619a25003-1~exp1~20250522112819.1465). >>>>>> >>>>> I cannot reproduce the "unused expressions" error. What is the >>>>> llvm cmake command line you are using? >>>>> >>>> Sorry for the delay. I tried just now with clang built from the >>>> latest >>>> git checkout but I don't see it either. >>>> I built it following the steps at >>>> https://www.kernel.org/doc/Documentation/bpf/bpf_devel_QA.rst. >>> I use the following make invocation: >>> >>> make CC="ccache gcc" LD=ld.lld-21 O="$PWD/../linux-build-s390x" >>> CLANG="ccache clang-21" LLVM_STRIP=llvm-strip-21 LLC=llc-21 >>> LLD=lld-21 >>> -j128 -C tools/testing/selftests/bpf BPF_GCC= V=1 >>> >>> which results in the following clang invocation: >>> >>> ccache clang-21 -g -Wall -Werror -D__TARGET_ARCH_s390 -mbig-endian >>> - >>> I"$PWD/../../../../.."/linux-build-s390x//tools/include - >>> I"$PWD/../../../../.."/linux/tools/testing/selftests/bpf - >>> I"$PWD/../../../../.."/linux/tools/include/uapi - >>> I"$PWD/../../../../.."/usr/include -std=gnu11 -fno-strict-aliasing >>> - >>> Wno-compare-distinct-pointer-types -idirafter /usr/lib/llvm- >>> 21/lib/clang/21/include -idirafter /usr/local/include -idirafter >>> /usr/include/s390x-linux-gnu -idirafter /usr/include - >>> DENABLE_ATOMICS_TESTS -O2 --target=bpfeb -c >>> progs/arena_spin_lock.c - >>> mcpu=v3 -o "$PWD/../../../../.."/linux-build- >>> s390x//arena_spin_lock.bpf.o >>> >>> I tried dropping ccache, but it did not help. >> Thanks, Ilya. It could be great if you can find out the >> cmake command lines which eventually builds your clang-21. >> Once cmake command lines are available, I can build >> the compiler on x86_64 host and do some checking for it. > Hi Yonghong, I don't build it, I take it from apt.llvm.org. > It's surprising we don't see this in CI, because it also takes > clang from there. If you think this is a compiler and not a code > bug, I can debug this myself, because maybe it's reproducible only on > s390x. I don't think this is a compiler bug. As mentioned by Alexei, __must_check linux/compiler_attributes.h:#define __must_check __attribute__((__warn_unused_result__)) is needed for the compiler to issue an error for unused func return value. I did some further checking on clang source code with a simple example on x86_64 machine: $ cat t.c int bar(void) __attribute__((warn_unused_result)); // int bar(void); int foo(int a) { bar(); return a; } and command line clang -Wall -Werror -g -O2 -c t.c The key related code is at https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaStmt.cpp#L230-L257 // Diagnoses unused expressions that call functions marked [[nodiscard]], // [[gnu::warn_unused_result]] and similar. // Additionally, a DiagID can be provided to emit a warning in additional // contexts (such as for an unused LHS of a comma expression) void DiagnoseUnused(Sema &S, const Expr *E, std::optional<unsigned> DiagID) { bool NoDiscardOnly = !DiagID.has_value(); ...... The following two lines of code is the key: if (!E->isUnusedResultAWarning(WarnExpr, Loc, R1, R2, S.Context)) return; ... With 'int bar(void) __attribute__((warn_unused_result));' the above if stmt will fall through. With 'int bar(void);' the above if stmt will return from DiagnozeUnused() func. For 'return true' case, eventually it emits an error. So we don't have issues with x86. But if s390x emits an error even without __attribute__((warn_unused_result)), I suspect that there is a bug in clang21 frontend with s390x. I assume clang20 will be okay? It is possible that in clang21, s390x clang frontend target specific things may cause clang emit error even without __must_check attribute. If clang20 is okay for s390x, I suggest to file a bug to llvm-project (clang21 frontend). ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings 2025-05-27 21:26 ` Yonghong Song @ 2025-05-27 21:31 ` Yonghong Song 0 siblings, 0 replies; 15+ messages in thread From: Yonghong Song @ 2025-05-27 21:31 UTC (permalink / raw) To: Ilya Leoshkevich, Kumar Kartikeya Dwivedi Cc: Alexei Starovoitov, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf, Heiko Carstens, Vasily Gorbik, Alexander Gordeev On 5/27/25 2:26 PM, Yonghong Song wrote: > > > On 5/27/25 1:27 AM, Ilya Leoshkevich wrote: >> On Mon, 2025-05-26 at 22:15 -0700, Yonghong Song wrote: >>> >>> On 5/24/25 2:05 PM, Ilya Leoshkevich wrote: >>>> On Sat, 2025-05-24 at 03:01 +0200, Kumar Kartikeya Dwivedi wrote: >>>>> On Sat, 24 May 2025 at 02:06, Yonghong Song >>>>> <yonghong.song@linux.dev> >>>>> wrote: >>>>>> >>>>>> On 5/23/25 4:25 AM, Ilya Leoshkevich wrote: >>>>>>> On Mon, 2025-05-12 at 15:29 -0400, Kumar Kartikeya Dwivedi >>>>>>> wrote: >>>>>>>> On Mon, 12 May 2025 at 12:41, Alexei Starovoitov >>>>>>>> <alexei.starovoitov@gmail.com> wrote: >>>>>>>>> On Mon, May 12, 2025 at 5:22 AM Ilya Leoshkevich >>>>>>>>> <iii@linux.ibm.com> wrote: >>>>>>>>>> On Fri, 2025-05-09 at 09:51 -0700, Alexei Starovoitov >>>>>>>>>> wrote: >>>>>>>>>>> On Thu, May 8, 2025 at 12:21 PM Ilya Leoshkevich >>>>>>>>>>> <iii@linux.ibm.com> >>>>>>>>>>> wrote: >>>>>>>>>>>> On Thu, 2025-05-08 at 11:38 -0700, Alexei >>>>>>>>>>>> Starovoitov >>>>>>>>>>>> wrote: >>>>>>>>>>>>> On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich >>>>>>>>>>>>> <iii@linux.ibm.com> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>>> clang-21 complains about unused expressions in >>>>>>>>>>>>>> a >>>>>>>>>>>>>> few >>>>>>>>>>>>>> progs. >>>>>>>>>>>>>> Fix by explicitly casting the respective >>>>>>>>>>>>>> expressions to >>>>>>>>>>>>>> void. >>>>>>>>>>>>> ... >>>>>>>>>>>>>> if (val & _Q_LOCKED_MASK) >>>>>>>>>>>>>> - >>>>>>>>>>>>>> smp_cond_load_acquire_label(&lock- >>>>>>>>>>>>>>> locked, >>>>>>>>>>>>>> !VAL, >>>>>>>>>>>>>> release_err); >>>>>>>>>>>>>> + >>>>>>>>>>>>>> (void)smp_cond_load_acquire_label(&lock- >>>>>>>>>>>>>>> locked, >>>>>>>>>>>>>> !VAL, release_err); >>>>>>>>>>>>> Hmm. I'm on clang-21 too and I don't see them. >>>>>>>>>>>>> What warnings do you see ? >>>>>>>>>>>> In file included from progs/arena_spin_lock.c:7: >>>>>>>>>>>> progs/bpf_arena_spin_lock.h:305:1756: error: >>>>>>>>>>>> expression >>>>>>>>>>>> result >>>>>>>>>>>> unused >>>>>>>>>>>> [-Werror,-Wunused-value] >>>>>>>>>>>> 305 | ({ typeof(_Generic((*&lock->locked), >>>>>>>>>>>> char: >>>>>>>>>>>> (char)0, >>>>>>>>>>>> unsigned >>>>>>>>>>>> char : (unsigned char)0, signed char : (signed >>>>>>>>>>>> char)0, >>>>>>>>>>>> unsigned >>>>>>>>>>>> short : >>>>>>>>>>>> (unsigned short)0, signed short : (signed short)0, >>>>>>>>>>>> unsigned >>>>>>>>>>>> int : >>>>>>>>>>>> (unsigned int)0, signed int : (signed int)0, >>>>>>>>>>>> unsigned >>>>>>>>>>>> long : >>>>>>>>>>>> (unsigned >>>>>>>>>>>> long)0, signed long : (signed long)0, unsigned long >>>>>>>>>>>> long : >>>>>>>>>>>> (unsigned >>>>>>>>>>>> long long)0, signed long long : (signed long >>>>>>>>>>>> long)0, >>>>>>>>>>>> default: >>>>>>>>>>>> (typeof(*&lock->locked))0)) __val = ({ >>>>>>>>>>>> typeof(&lock- >>>>>>>>>>>>> locked) >>>>>>>>>>>> __ptr >>>>>>>>>>>> = >>>>>>>>>>>> (&lock->locked); typeof(_Generic((*(&lock- >>>>>>>>>>>>> locked)), >>>>>>>>>>>> char: >>>>>>>>>>>> (char)0, >>>>>>>>>>>> unsigned char : (unsigned char)0, signed char : >>>>>>>>>>>> (signed >>>>>>>>>>>> char)0, >>>>>>>>>>>> unsigned short : (unsigned short)0, signed short : >>>>>>>>>>>> (signed >>>>>>>>>>>> short)0, >>>>>>>>>>>> unsigned int : (unsigned int)0, signed int : >>>>>>>>>>>> (signed >>>>>>>>>>>> int)0, >>>>>>>>>>>> unsigned >>>>>>>>>>>> long : (unsigned long)0, signed long : (signed >>>>>>>>>>>> long)0, >>>>>>>>>>>> unsigned >>>>>>>>>>>> long >>>>>>>>>>>> long : (unsigned long long)0, signed long long : >>>>>>>>>>>> (signed long >>>>>>>>>>>> long)0, >>>>>>>>>>>> default: (typeof(*(&lock->locked)))0)) VAL; for >>>>>>>>>>>> (;;) { >>>>>>>>>>>> VAL = >>>>>>>>>>>> (typeof(_Generic((*(&lock->locked)), char: (char)0, >>>>>>>>>>>> unsigned >>>>>>>>>>>> char : >>>>>>>>>>>> (unsigned char)0, signed char : (signed char)0, >>>>>>>>>>>> unsigned >>>>>>>>>>>> short : >>>>>>>>>>>> (unsigned short)0, signed short : (signed short)0, >>>>>>>>>>>> unsigned >>>>>>>>>>>> int : >>>>>>>>>>>> (unsigned int)0, signed int : (signed int)0, >>>>>>>>>>>> unsigned >>>>>>>>>>>> long : >>>>>>>>>>>> (unsigned >>>>>>>>>>>> long)0, signed long : (signed long)0, unsigned long >>>>>>>>>>>> long : >>>>>>>>>>>> (unsigned >>>>>>>>>>>> long long)0, signed long long : (signed long >>>>>>>>>>>> long)0, >>>>>>>>>>>> default: >>>>>>>>>>>> (typeof(*(&lock->locked)))0)))(*(volatile >>>>>>>>>>>> typeof(*__ptr) >>>>>>>>>>>> *)&(*__ptr)); >>>>>>>>>>>> if (!VAL) break; ({ __label__ l_break, l_continue; >>>>>>>>>>>> asm >>>>>>>>>>>> volatile >>>>>>>>>>>> goto("may_goto %l[l_break]" :::: l_break); goto >>>>>>>>>>>> l_continue; >>>>>>>>>>>> l_break: >>>>>>>>>>>> goto release_err; l_continue:; }); ({}); } >>>>>>>>>>>> (typeof(*(&lock- >>>>>>>>>>>>> locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ >>>>>>>>>>>>> unsigned >>>>>>>>>>>>> long >>>>>>>>>>>>> __val; >>>>>>>>>>>> __sync_fetch_and_add(&__val, 0); }); else asm >>>>>>>>>>>> volatile("" ::: >>>>>>>>>>>> "memory"); }); }); (typeof(*(&lock->locked)))__val; >>>>>>>>>>>> }); >>>>>>>>>>>> | >>>>>>>>>>>> ^ ~~~~~ >>>>>>>>>>>> 1 error generated. >>>>>>>>>>> hmm. The error is impossible to read. >>>>>>>>>>> >>>>>>>>>>> Kumar, >>>>>>>>>>> >>>>>>>>>>> Do you see a way to silence it differently ? >>>>>>>>>>> >>>>>>>>>>> Without adding (void)... >>>>>>>>>>> >>>>>>>>>>> Things like: >>>>>>>>>>> - bpf_obj_new(.. >>>>>>>>>>> + (void)bpf_obj_new(.. >>>>>>>>>>> >>>>>>>>>>> are good to fix, and if we could annotate >>>>>>>>>>> bpf_obj_new_impl kfunc with __must_check we would >>>>>>>>>>> have >>>>>>>>>>> done it, >>>>>>>>>>> >>>>>>>>>>> but >>>>>>>>>>> - arch_mcs_spin_lock... >>>>>>>>>>> + (void)arch_mcs_spin_lock... >>>>>>>>>>> >>>>>>>>>>> is odd. >>>>>>>>>> What do you think about moving (void) to the definition >>>>>>>>>> of >>>>>>>>>> arch_mcs_spin_lock_contended_label()? I can send a v2 >>>>>>>>>> if >>>>>>>>>> this is >>>>>>>>>> better. >>>>>>>>> Kumar, >>>>>>>>> >>>>>>>>> thoughts? >>>>>>>> Sorry for the delay, I was afk. >>>>>>>> >>>>>>>> The warning seems a bit aggressive, in the kernel we have >>>>>>>> users >>>>>>>> which >>>>>>>> do and do not use the value and it's fine. >>>>>>>> I think moving (void) inside the macro is a problem since >>>>>>>> at >>>>>>>> least >>>>>>>> rqspinlock like algorithm would want to inspect the result >>>>>>>> of >>>>>>>> the >>>>>>>> locked bit. >>>>>>>> No such users exist for now, of course. So maybe we can >>>>>>>> silence >>>>>>>> it >>>>>>>> until we do end up depending on the value. >>>>>>>> >>>>>>>> I will give a try with clang-21, but I think probably >>>>>>>> (void) in >>>>>>>> the >>>>>>>> source is better if we do need to silence it. >>>>>>> Gentle ping. >>>>>>> >>>>>>> This is still an issue with clang version 21.0.0 >>>>>>> (++20250522112647+491619a25003-1~exp1~20250522112819.1465). >>>>>>> >>>>>> I cannot reproduce the "unused expressions" error. What is the >>>>>> llvm cmake command line you are using? >>>>>> >>>>> Sorry for the delay. I tried just now with clang built from the >>>>> latest >>>>> git checkout but I don't see it either. >>>>> I built it following the steps at >>>>> https://www.kernel.org/doc/Documentation/bpf/bpf_devel_QA.rst. >>>> I use the following make invocation: >>>> >>>> make CC="ccache gcc" LD=ld.lld-21 O="$PWD/../linux-build-s390x" >>>> CLANG="ccache clang-21" LLVM_STRIP=llvm-strip-21 LLC=llc-21 >>>> LLD=lld-21 >>>> -j128 -C tools/testing/selftests/bpf BPF_GCC= V=1 >>>> >>>> which results in the following clang invocation: >>>> >>>> ccache clang-21 -g -Wall -Werror -D__TARGET_ARCH_s390 -mbig-endian >>>> - >>>> I"$PWD/../../../../.."/linux-build-s390x//tools/include - >>>> I"$PWD/../../../../.."/linux/tools/testing/selftests/bpf - >>>> I"$PWD/../../../../.."/linux/tools/include/uapi - >>>> I"$PWD/../../../../.."/usr/include -std=gnu11 -fno-strict-aliasing >>>> - >>>> Wno-compare-distinct-pointer-types -idirafter /usr/lib/llvm- >>>> 21/lib/clang/21/include -idirafter /usr/local/include -idirafter >>>> /usr/include/s390x-linux-gnu -idirafter /usr/include - >>>> DENABLE_ATOMICS_TESTS -O2 --target=bpfeb -c >>>> progs/arena_spin_lock.c - >>>> mcpu=v3 -o "$PWD/../../../../.."/linux-build- >>>> s390x//arena_spin_lock.bpf.o >>>> >>>> I tried dropping ccache, but it did not help. >>> Thanks, Ilya. It could be great if you can find out the >>> cmake command lines which eventually builds your clang-21. >>> Once cmake command lines are available, I can build >>> the compiler on x86_64 host and do some checking for it. >> Hi Yonghong, I don't build it, I take it from apt.llvm.org. >> It's surprising we don't see this in CI, because it also takes >> clang from there. If you think this is a compiler and not a code >> bug, I can debug this myself, because maybe it's reproducible only on >> s390x. > > I don't think this is a compiler bug. As mentioned by Alexei, > __must_check Ignore this 'I don't think this is a compiler bug'. This is my early thinking but eventually I think it is likely to be a clang bug with s390x target. > > linux/compiler_attributes.h:#define __must_check > __attribute__((__warn_unused_result__)) > > is needed for the compiler to issue an error for unused func return > value. > [...] > > But if s390x emits an error even without > __attribute__((warn_unused_result)), > I suspect that there is a bug in clang21 frontend with s390x. > I assume clang20 will be okay? > It is possible that in clang21, s390x clang frontend target specific > things > may cause clang emit error even without __must_check attribute. > > If clang20 is okay for s390x, I suggest to file a bug to llvm-project > (clang21 frontend). > > ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2025-05-27 21:31 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-05-08 11:37 [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings Ilya Leoshkevich 2025-05-08 18:38 ` Alexei Starovoitov 2025-05-08 19:21 ` Ilya Leoshkevich 2025-05-09 16:51 ` Alexei Starovoitov 2025-05-12 12:22 ` Ilya Leoshkevich 2025-05-12 16:41 ` Alexei Starovoitov 2025-05-12 19:29 ` Kumar Kartikeya Dwivedi 2025-05-23 11:25 ` Ilya Leoshkevich 2025-05-24 0:05 ` Yonghong Song 2025-05-24 1:01 ` Kumar Kartikeya Dwivedi 2025-05-24 21:05 ` Ilya Leoshkevich 2025-05-27 5:15 ` Yonghong Song 2025-05-27 8:27 ` Ilya Leoshkevich 2025-05-27 21:26 ` Yonghong Song 2025-05-27 21:31 ` Yonghong Song
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox