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

[Ilya Leoshkevich <iii@linux.ibm.com>]

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