Re: [PATCH bpf-next v3 1/2] selftests/bpf: Annotate bpf_obj_new_impl() with __must_check

[Yonghong Song <yonghong.song@linux.dev>]

On 8/27/25 12:46 PM, Ilya Leoshkevich wrote:
> The verifier requires that pointers returned by bpf_obj_new_impl() are
> either dropped or stored in a map. Therefore programs that do not use
> its return values will fail to load. Make the compiler point out these
> issues. Adjust selftests that check that the verifier does indeed spot
> these bugs.
>
> Note that now there two different bpf_obj_new_impl() declarations: one
> with __must_check from bpf_experimental.h, and one without from
> vmlinux.h. According to the GCC doc [1] this is fine and has the
> desired effect:
>
>      Compatible attribute specifications on distinct declarations of the
>      same function are merged.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc-12.4.0/gcc/Function-Attributes.html
>
> Link: https://lore.kernel.org/bpf/CAADnVQL6Q+QRv3_JwEd26biwGpFYcwD_=BjBJWLAtpgOP9CKRw@mail.gmail.com/
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   .../testing/selftests/bpf/bpf_experimental.h  |  6 ++++-
>   .../selftests/bpf/progs/linked_list_fail.c    | 23 +++++++++++++++----
>   2 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
> index da7e230f2781..a8f206f4fdb9 100644
> --- a/tools/testing/selftests/bpf/bpf_experimental.h
> +++ b/tools/testing/selftests/bpf/bpf_experimental.h
> @@ -8,6 +8,10 @@
>   
>   #define __contains(name, node) __attribute__((btf_decl_tag("contains:" #name ":" #node)))
>   
> +#ifndef __must_check
> +#define __must_check __attribute__((__warn_unused_result__))
> +#endif

As you mentioned in Patch 2, we definitely has an issue with clang. I tried the following
experiments with latest master branch.

$ cat run3.sh
echo ".c => .i => .o"

clang  -g -Wall -Werror -D__TARGET_ARCH_x86 -mlittle-endian \
   -I/home/yhs/work/bpf-next/tools/testing/selftests/bpf/tools/include \
   -I/home/yhs/work/bpf-next/tools/testing/selftests/bpf \
   -I/home/yhs/work/bpf-next/tools/include/uapi \
   -I/home/yhs/work/bpf-next/tools/testing/selftests/usr/include \
   -std=gnu11 -fno-strict-aliasing -Wno-compare-distinct-pointer-types \
   -idirafter /home/yhs/work/llvm-project/llvm/build.21/Release/lib/clang/21/include \
   -idirafter /usr/local/include -idirafter /usr/include    -DENABLE_ATOMICS_TESTS \
   -O2 --target=bpfel -E progs/linked_list_fail.c -mcpu=v4 \
   -o /home/yhs/work/bpf-next/tools/testing/selftests/bpf/linked_list_fail.i
clang  -g -Wall -Werror -mlittle-endian -std=gnu11 -fno-strict-aliasing \
   -Wno-compare-distinct-pointer-types -O2 --target=bpfel -c linked_list_fail.i \
   -mcpu=v4 \
   -o /home/yhs/work/bpf-next/tools/testing/selftests/bpf/cpuv4/linked_list_fail.bpf.o

echo ".c => .o"

clang  -g -Wall -Werror -D__TARGET_ARCH_x86 -mlittle-endian \
   -I/home/yhs/work/bpf-next/tools/testing/selftests/bpf/tools/include \
   -I/home/yhs/work/bpf-next/tools/testing/selftests/bpf \
   -I/home/yhs/work/bpf-next/tools/include/uapi \
   -I/home/yhs/work/bpf-next/tools/testing/selftests/usr/include \
   -std=gnu11 -fno-strict-aliasing -Wno-compare-distinct-pointer-types \
   -idirafter /home/yhs/work/llvm-project/llvm/build.21/Release/lib/clang/21/include \
   -idirafter /usr/local/include -idirafter /usr/include    -DENABLE_ATOMICS_TESTS \
   -O2 --target=bpfel -c progs/linked_list_fail.c -mcpu=v4 \
   -o /home/yhs/work/bpf-next/tools/testing/selftests/bpf/cpuv4/linked_list_fail.bpf.o
$ ./run3.sh
.c => .i => .o
progs/linked_list_fail.c:230:3: error: expression result unused [-Werror,-Wunused-value]
   230 |  ((union { int data; unsigned udata; } *)bpf_obj_new_impl(__builtin_btf_type_id(*((typeof(union { int data; unsigned udata; }) *) 0), BPF_TYPE_ID_LOCAL), ((void *)0)));
       |   ^                                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
progs/linked_list_fail.c:255:3: error: expression result unused [-Werror,-Wunused-value]
   255 |  ((struct foo *)bpf_obj_new_impl(__builtin_btf_type_id(*((typeof(struct foo) *) 0), BPF_TYPE_ID_LOCAL), ((void *)0)));
       |   ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.
.c => .o
$

The clang compilation command line is from selftest build with V=1.

Compiling from .c to .o is okay, but from .c => .i => .o will have
errors due to unused value.

I think you do not need __must_check here. '-Wall -Werror' seems already covered this.
You can also mention the clang bug in the commit message. The fix will be below:

diff --git a/tools/testing/selftests/bpf/progs/linked_list_fail.c b/tools/testing/selftests/bpf/progs/linked_list_fail.c
index 6438982b928b..35616b5c9b9e 100644
--- a/tools/testing/selftests/bpf/progs/linked_list_fail.c
+++ b/tools/testing/selftests/bpf/progs/linked_list_fail.c
@@ -227,7 +227,7 @@ 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 +252,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;
  }

I think this probably will address your icecc issue.


> +
>   /* Description
>    *	Allocates an object of the type represented by 'local_type_id' in
>    *	program BTF. User may use the bpf_core_type_id_local macro to pass the
> @@ -20,7 +24,7 @@
>    *	A pointer to an object of the type corresponding to the passed in
>    *	'local_type_id', or NULL on failure.
>    */
> -extern void *bpf_obj_new_impl(__u64 local_type_id, void *meta) __ksym;
> +extern __must_check void *bpf_obj_new_impl(__u64 local_type_id, void *meta) __ksym;
>   
>   /* Convenience macro to wrap over bpf_obj_new_impl */
>   #define bpf_obj_new(type) ((type *)bpf_obj_new_impl(bpf_core_type_id_local(type), NULL))
> diff --git a/tools/testing/selftests/bpf/progs/linked_list_fail.c b/tools/testing/selftests/bpf/progs/linked_list_fail.c
> index 6438982b928b..1e30d103e1c7 100644
> --- a/tools/testing/selftests/bpf/progs/linked_list_fail.c
> +++ b/tools/testing/selftests/bpf/progs/linked_list_fail.c
> @@ -212,22 +212,33 @@ int map_compat_raw_tp_w(void *ctx)
>   SEC("?tc")
>   int obj_type_id_oor(void *ctx)
>   {
> -	bpf_obj_new_impl(~0UL, NULL);
> +	void *f;
> +
> +	f = bpf_obj_new_impl(~0UL, NULL);
> +	(void)f;
> +
>   	return 0;
>   }
>   
>   SEC("?tc")
>   int obj_new_no_composite(void *ctx)
>   {
> -	bpf_obj_new_impl(bpf_core_type_id_local(int), (void *)42);
> +	void *f;
> +
> +	f = bpf_obj_new_impl(bpf_core_type_id_local(int), (void *)42);
> +	(void)f;
> +
>   	return 0;
>   }
>   
>   SEC("?tc")
>   int obj_new_no_struct(void *ctx)
>   {
> +	void *f;
> +
> +	f = bpf_obj_new(union { int data; unsigned udata; });
> +	(void)f;
>   
> -	bpf_obj_new(union { int data; unsigned udata; });
>   	return 0;
>   }
>   
> @@ -252,7 +263,11 @@ int new_null_ret(void *ctx)
>   SEC("?tc")
>   int obj_new_acq(void *ctx)
>   {
> -	bpf_obj_new(struct foo);
> +	void *f;
> +
> +	f = bpf_obj_new(struct foo);
> +	(void)f;
> +
>   	return 0;
>   }
>