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@xxxxxxxxxxxxxx/
Suggested-by: Alexei Starovoitov <ast@xxxxxxxxxx>
Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
---
.../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;
}
