Re: [PATCH] libbpf: fix helper macro C++ compatibility

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] libbpf: C++ build failure in bpf_for() and bpf_repeat() due to (void *)0
- [Low] libbpf: missing extern C linkage on bpf_iter_num_*() in bpf_helpers.h
--

commit 7e0b0adc8984c99351ff40e7c423f83f7d25ccab
Author: Eduard Zateev <hackerowskiy@gmail.com>

libbpf: fix helper macro C++ compatibility

This commit replaces GNU named variadic macro parameters in bpf_helpers.h with
__VA_ARGS__ to avoid C++26 variadic macro warnings. It also adds explicit
integer and typed pointer casts to helper macros and generated helper stubs so
that bpf_helpers.h and bpf_helper_defs.h can be used from C++.

> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> index b37b727ef216f..6c9fdb69661ea 100644
> --- a/tools/lib/bpf/bpf_helpers.h
> +++ b/tools/lib/bpf/bpf_helpers.h

[ ... ]

> @@ -346,7 +367,7 @@ enum libbpf_tristate {
>  		   __bpf_printk /*1*/, __bpf_printk /*0*/)
>  
>  /* Helper macro to print out debug messages */
> -#define bpf_printk(fmt, args...) ___bpf_pick_printk(args)(fmt, ##args)
> +#define bpf_printk(fmt, ...) ___bpf_pick_printk(__VA_ARGS__)(fmt, ##__VA_ARGS__)
>  
>  struct bpf_iter_num;
>  

[Severity: Low]
When tools/lib/bpf/bpf_helpers.h is included directly in a C++ translation
unit without an outer extern "C" block, these kfunc declarations immediately
below struct bpf_iter_num have C++ linkage:

extern int bpf_iter_num_new(struct bpf_iter_num *it, int start, int end) __weak __ksym;
extern int *bpf_iter_num_next(struct bpf_iter_num *it) __weak __ksym;
extern void bpf_iter_num_destroy(struct bpf_iter_num *it) __weak __ksym;

When bpf_for_each(num, cur, start, end) is expanded in C++, the compiler emits
mangled ELF symbol names for bpf_iter_num_new(), bpf_iter_num_next(), and
bpf_iter_num_destroy() in .symtab, which causes bpf_object__collect_externs()
 -> find_extern_btf_id() and kernel BTF kfunc resolution in libbpf to fail
with -ENOENT.

Should bpf_helpers.h wrap its extern declarations in an #ifdef __cplusplus
extern "C" guard, similar to the other headers in tools/lib/bpf/?

> @@ -382,16 +403,18 @@ extern void bpf_iter_num_destroy(struct bpf_iter_num *it) __weak __ksym;
>   * extension: __attribute__((cleanup(<func>))), supported by both GCC and
>   * Clang.
>   */
> -#define bpf_for_each(type, cur, args...) for (							\
> +#define bpf_for_each(type, cur, ...) for (							\
>  	/* initialize and define destructor */							\
>  	struct bpf_iter_##type ___it __attribute__((aligned(8), /* enforce, just in case */,	\
>  						    cleanup(bpf_iter_##type##_destroy))),	\
>  	/* ___p pointer is just to call bpf_iter_##type##_new() *once* to init ___it */		\
>  			       *___p __attribute__((unused)) = (				\
> -					bpf_iter_##type##_new(&___it, ##args),			\
> +						bpf_iter_##type##_new(&___it, \
> +								       ##__VA_ARGS__), \
>  	/* this is a workaround for Clang bug: it currently doesn't emit BTF */			\
>  	/* for bpf_iter_##type##_destroy() when used from cleanup() attribute */		\
> -					(void)bpf_iter_##type##_destroy, (void *)0);		\
> +					(void)bpf_iter_##type##_destroy, \
> +					(struct bpf_iter_##type *)0); \

[Severity: Low]
Here bpf_for_each() is updated to initialize ___p with
(struct bpf_iter_##type *)0 instead of (void *)0, since C++ rejects implicit
conversion from void * to a typed pointer.

However, the companion macros bpf_for() and bpf_repeat() immediately below in
tools/lib/bpf/bpf_helpers.h still initialize struct bpf_iter_num *___p with
(void *)0:

#define bpf_for(i, start, end) for (								\
	...
			    *___p __attribute__((unused)) = (					\
				bpf_iter_num_new(&___it, (start), (end)),			\
				(void)bpf_iter_num_destroy, (void *)0);				\

and:

#define bpf_repeat(N) for (									\
	...
			    *___p __attribute__((unused)) = (					\
				bpf_iter_num_new(&___it, 0, (N)),				\
				(void)bpf_iter_num_destroy, (void *)0);				\

Would expanding bpf_for() or bpf_repeat() in C++ still fail to compile unless
those initializers are also cast to (struct bpf_iter_num *)0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024035902.44566-1-hackerowskiy@gmail.com?part=1