On Wed, 2025-08-27 at 22:54 +0200, Ilya Leoshkevich wrote: > On Wed, 2025-08-27 at 13:05 -0700, Andrii Nakryiko wrote: > > On Wed, Aug 27, 2025 at 11:34 AM Ilya Leoshkevich > > <iii@xxxxxxxxxxxxx> > > wrote: > > > > > > On Wed, 2025-08-27 at 10:32 -0700, Andrii Nakryiko wrote: > > > > On Wed, Aug 27, 2025 at 6:05 AM Ilya Leoshkevich > > > > <iii@xxxxxxxxxxxxx> > > > > 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. > > > > > > > > > > Link: > > > > > https://lore.kernel.org/bpf/CAADnVQL6Q+QRv3_JwEd26biwGpFYcwD_=BjBJWLAtpgOP9CKRw@xxxxxxxxxxxxxx/ > > > > > Suggested-by: Alexei Starovoitov <ast@xxxxxxxxxx> > > > > > Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx> > > > > > --- > > > > > tools/lib/bpf/bpf_helpers.h | 4 > > > > > ++++ > > > > > tools/testing/selftests/bpf/bpf_experimental.h | 2 +- > > > > > tools/testing/selftests/bpf/progs/linked_list_fail.c | 8 > > > > > ++++- > > > > > --- > > > > > 3 files changed, 9 insertions(+), 5 deletions(-) > > [...] > > > > > > /* When utilizing vmlinux.h with BPF CO-RE, user BPF > > > > > programs > > > > > can't include > > > > > * any system-level headers (such as stddef.h, > > > > > linux/version.h, > > > > > etc), and > > > > > * commonly-used macros like NULL and KERNEL_VERSION aren't > > > > > available through > > > > > diff --git a/tools/testing/selftests/bpf/bpf_experimental.h > > > > > b/tools/testing/selftests/bpf/bpf_experimental.h > > > > > index da7e230f2781..e5ef4792da42 100644 > > > > > --- a/tools/testing/selftests/bpf/bpf_experimental.h > > > > > +++ b/tools/testing/selftests/bpf/bpf_experimental.h > > > > > @@ -20,7 +20,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; > > > > > > > > bpf_obj_new_impl will generally come from vmlinux.h nowadays, > > > > and > > > > that > > > > one won't have __must_check annotation, is that a problem? > > > > > > It should be fine according to [1]: > > > > > > Compatible attribute specifications on distinct declarations of > > > the > > > same function are merged. > > > > > > I will add this to the commit message in v3. > > > > Sure, for BPF selftests it will work. My question was broader, for > > anyone using bpf_obj_new in the wild, they won't have __must_check > > annotation from vmlinux.h (and I doubt they will manually add it > > like > > we do here for BPF selftests), so if that's important, I guess we > > need > > to think how to wire that up so that it happens automatically > > through > > vmlinux.h. > > > > "It's not that important to bother" is a fine answer as well :) > > I see. Seems like it's tough: > > - The attribute is not available in DWARF > - But we could introduce KF_MUST_CHECK flag > - Which pahole would extract from .BTF_ids and convert to > a btf_decl_tag > - This will make pahole depend on .BTF_ids format though, which > might > be undesirable Hm, I should have checked that before hitting "send": apparently pahole already parses both .BTF_ids and __BTF_ID__set8__*. Still, DW_TAG_GNU_annotation looks like a better long-term solution. > - Then bpftool would convert this btf_decl_tag to __must_check > > Seems like they are attempting to upstream the new > DW_TAG_GNU_annotation right now [1], if that lands and is available > for non-BPF targets, we could put > __attribute((btf_decl_tag("must_check"))) on kernel's > bpf_obj_new_impl() and directly access it from bpftool. > > So for now I would propose to limit the solution to selftests. > > [1] https://gcc.gnu.org/pipermail/gcc-patches/2025-August/692445.html > > > > [1] > > > https://gcc.gnu.org/onlinedocs/gcc-12.4.0/gcc/Function-Attributes.html > > > > > > [...]
