On Sun, Apr 28, 2024 at 4:26 AM Jose E. Marchesi
<jose.marchesi@xxxxxxxxxx> wrote:
>
> The macro bpf_ksym_exists is defined in bpf_helpers.h as:
>
> #define bpf_ksym_exists(sym) ({ \
> _Static_assert(!__builtin_constant_p(!!sym), #sym " should be marked as __weak"); \
> !!sym; \
> })
>
> The purpose of the macro is to determine whether a given symbol has
> been defined, given the address of the object associated with the
> symbol. It also has a compile-time check to make sure the object
> whose address is passed to the macro has been declared as weak, which
> makes the check on `sym' meaningful.
>
> As it happens, the check for weak doesn't work in GCC in all cases,
> because __builtin_constant_p not always folds at parse time when
> optimizing. This is because optimizations that happen later in the
> compilation process, like inlining, may make a previously non-constant
> expression a constant. This results in errors like the following when
> building the selftests with GCC:
>
> bpf_helpers.h:190:24: error: expression in static assertion is not constant
> 190 | _Static_assert(!__builtin_constant_p(!!sym), #sym " should be marked as __weak"); \
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Fortunately recent versions of GCC support a __builtin_has_attribute
> that can be used to directly check for the __weak__ attribute. This
> patch changes bpf_helpers.h to use that builtin when building with a
> recent enough GCC, and to omit the check if GCC is too old to support
> the builtin.
>
> The macro used for GCC becomes:
>
> #define bpf_ksym_exists(sym) ({ \
> _Static_assert(__builtin_has_attribute (*sym, __weak__), #sym " should be marked as __weak"); \
> !!sym; \
> })
>
> Note that since bpf_ksym_exists is designed to get the address of the
> object associated with symbol SYM, we pass *sym to
> __builtin_has_attribute instead of sym. When an expression is passed
> to __builtin_has_attribute then it is the type of the passed
> expression that is checked for the specified attribute. The
> expression itself is not evaluated. This accommodates well with the
> existing usages of the macro:
>
> - For function objects:
>
> struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym __weak;
> [...]
> bpf_ksym_exists(bpf_task_acquire)
>
> - For variable objects:
>
> extern const struct rq runqueues __ksym __weak; /* typed */
> [...]
> bpf_ksym_exists(&runqueues)
>
> Note also that BPF support was added in GCC 10 and support for
> __builtin_has_attribute in GCC 9.
>
> Locally tested in bpf-next master branch.
> No regressions.
>
> Signed-of-by: Jose E. Marchesi <jose.marchesi@xxxxxxxxxx>
> Cc: Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>
> Cc: david.faust@xxxxxxxxxx
> Cc: cupertino.miranda@xxxxxxxxxx
> ---
> tools/lib/bpf/bpf_helpers.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> index 62e1c0cc4a59..a720636a87d9 100644
> --- a/tools/lib/bpf/bpf_helpers.h
> +++ b/tools/lib/bpf/bpf_helpers.h
> @@ -186,10 +186,19 @@ enum libbpf_tristate {
> #define __kptr __attribute__((btf_type_tag("kptr")))
> #define __percpu_kptr __attribute__((btf_type_tag("percpu_kptr")))
>
> +#if defined (__clang__)
> #define bpf_ksym_exists(sym) ({ \
> _Static_assert(!__builtin_constant_p(!!sym), #sym " should be marked as __weak"); \
> !!sym; \
> })
> +#elif __GNUC__ > 8
> +#define bpf_ksym_exists(sym) ({ \
> + _Static_assert(__builtin_has_attribute (*sym, __weak__), #sym " should be marked as __weak"); \
> + !!sym; \
> +})
I wrapped _Static_assert() to keep it under 100 characters (and fix
one unaligned '\' while at it). Also, the patch prefix should be
"libbpf: " as this is purely a libbpf header. Applied to bpf-next,
thanks.
> +#else
> +#define bpf_ksym_exists(sym) !!sym
> +#endif
>
> #define __arg_ctx __attribute__((btf_decl_tag("arg:ctx")))
> #define __arg_nonnull __attribute((btf_decl_tag("arg:nonnull")))
> --
> 2.30.2
>
>
