[PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[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

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Alexei Starovoitov]

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 ?

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Ilya Leoshkevich]

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

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Alexei Starovoitov]

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

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Ilya Leoshkevich]

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

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Alexei Starovoitov]

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?

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Kumar Kartikeya Dwivedi]

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.

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Ilya Leoshkevich]

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).

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Yonghong Song]

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?

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Kumar Kartikeya Dwivedi]

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.

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Ilya Leoshkevich]

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.

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Yonghong Song]

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.

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Ilya Leoshkevich]

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.

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Yonghong Song]

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).

Re: [PATCH bpf-next] selftests/bpf: Fix "expression result unused" warnings

[Yonghong Song]

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).
>
>