Re: [PATCH dwarves v4 0/6] btf_encoder: emit type tags for bpf_arena pointers

Ihor Solodrai
2025-03-24T18:07:10Z

On 3/23/25 4:11 AM, Alan Maguire wrote:
> [...]
>
> hi Ihor, I took a look at the series and merged it with latest next
> branch; results are in
>
> https://web.git.kernel.org/pub/scm/devel/pahole/pahole.git/log/?h=next.attributes-v4
>
> ...if you want to take a look.
>
> There are a few small things I think that it would be good to resolve
> before landing this.
>
> First, when testing this with -DLIBBPF_EMBEDDED=OFF and a packaged
> libbpf 1.5 - which means we wouldn't have the latest attributes-related
> libbpf function; I saw:
>
> BTF .tmp_vmlinux1.btf.o
> btf__add_type_attr is not available, is libbpf < 1.6?
> error: failed to encode function 'bbr_cwnd_event': invalid proto
> Failed to encode BTF
> NM .tmp_vmlinux1.syms

Hi Alan. Thanks for testing. This is my mistake, I should've checked
for attributes feature here:

@@ -731,6 +812,10 @@ static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct f

assert(ftype != NULL || state != NULL);

+ if (is_kfunc_state(state) && encoder->tag_kfuncs)
+ if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
+ return -1;

>
> ...and we got no BTF as a result. Ideally we'd like pahole to encode but
> without the attributes feature if not available. Related, we also report
> features that are not present, i.e. attributes with
> "--supported_btf_features". So I propose that we make use of the weak
> declarations being NULL in an optional feature check function. It is
> optionally declared for a feature, and if declared must return true if
> the feature is available.
>
> Something like the below works (it's in the next.attributes-v4 branch
> too for reference) and it resolves the issue of BTF generation failure
> and accurate supported_btf_features reporting. What do you think? Thanks!

I think failing fast is a good approach here: check that all
requested/default features are available on startup.

>
> Alan
>
> From: Alan Maguire <alan.maguire@oracle.com>
> Date: Sun, 23 Mar 2025 11:06:18 +0000
> Subject: [PATCH] pahole: add a BTF feature check function
>
> It is used to see if functions that BTF features rely on are
> really there; weak declarations mean they will be NULL if not
> in non-embedded linked libbpf.
>
> This gives us more accurate --supported_btf_features reporting also.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
> pahole.c | 39 +++++++++++++++++++++++++++++++++------
> 1 file changed, 33 insertions(+), 6 deletions(-)
>

Acked-by: Ihor Solodrai <ihor.solodrai@linux.dev>

> diff --git a/pahole.c b/pahole.c
> index 4a2b1ce..8304ba4 100644
> --- a/pahole.c
> +++ b/pahole.c
> @@ -1183,10 +1183,31 @@ ARGP_PROGRAM_VERSION_HOOK_DEF =
> dwarves_print_version;
> * floats, etc. This ensures backwards compatibility.
> */
> #define BTF_DEFAULT_FEATURE(name, alias, initial_value) \
> - { #name, #alias, &conf_load.alias, initial_value, true }
> + { #name, #alias, &conf_load.alias, initial_value, true, NULL }
> +
> +#define BTF_DEFAULT_FEATURE_CHECK(name, alias, initial_value,
> feature_check) \
> + { #name, #alias, &conf_load.alias, initial_value, true, feature_check }
>
> #define BTF_NON_DEFAULT_FEATURE(name, alias, initial_value) \
> - { #name, #alias, &conf_load.alias, initial_value, false }
> + { #name, #alias, &conf_load.alias, initial_value, false, NULL }
> +
> +#define BTF_NON_DEFAULT_FEATURE_CHECK(name, alias, initial_value,
> feature_check) \
> + { #name, #alias, &conf_load.alias, initial_value, false, feature_check }
> +
> +static bool enum64_check(void)
> +{
> + return btf__add_enum64 != NULL;
> +}
> +
> +static bool distilled_base_check(void)
> +{
> + return btf__distill_base != NULL;
> +}
> +
> +static bool attributes_check(void)
> +{
> + return btf__add_type_attr != NULL;
> +}
>
> struct btf_feature {
> const char *name;
> @@ -1196,20 +1217,23 @@ struct btf_feature {
> bool default_enabled; /* some nonstandard features may not
> * be enabled for --btf_features=default
> */
> + bool (*feature_check)(void);
> } btf_features[] = {
> BTF_DEFAULT_FEATURE(encode_force, btf_encode_force, false),
> BTF_DEFAULT_FEATURE(var, skip_encoding_btf_vars, true),
> BTF_DEFAULT_FEATURE(float, btf_gen_floats, false),
> BTF_DEFAULT_FEATURE(decl_tag, skip_encoding_btf_decl_tag, true),
> BTF_DEFAULT_FEATURE(type_tag, skip_encoding_btf_type_tag, true),
> - BTF_DEFAULT_FEATURE(enum64, skip_encoding_btf_enum64, true),
> + BTF_DEFAULT_FEATURE_CHECK(enum64, skip_encoding_btf_enum64, true,
> enum64_check),
> BTF_DEFAULT_FEATURE(optimized_func, btf_gen_optimized, false),
> BTF_DEFAULT_FEATURE(consistent_func,
> skip_encoding_btf_inconsistent_proto, false),
> BTF_DEFAULT_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false),
> BTF_NON_DEFAULT_FEATURE(reproducible_build, reproducible_build, false),
> - BTF_NON_DEFAULT_FEATURE(distilled_base, btf_gen_distilled_base, false),
> + BTF_NON_DEFAULT_FEATURE_CHECK(distilled_base, btf_gen_distilled_base,
> false,
> + distilled_base_check),
> BTF_NON_DEFAULT_FEATURE(global_var, encode_btf_global_vars, false),
> - BTF_NON_DEFAULT_FEATURE(attributes, btf_attributes, false),
> + BTF_NON_DEFAULT_FEATURE_CHECK(attributes, btf_attributes, false,
> + attributes_check),
> };
>
> #define BTF_MAX_FEATURE_STR 1024
> @@ -1248,7 +1272,8 @@ static void enable_btf_feature(struct btf_feature
> *feature)
> /* switch "initial-off" features on, and "initial-on" features
> * off; i.e. negate the initial value.
> */
> - *feature->conf_value = !feature->initial_value;
> + if (!feature->feature_check || feature->feature_check())
> + *feature->conf_value = !feature->initial_value;
> }
>
> static void show_supported_btf_features(FILE *output)
> @@ -1256,6 +1281,8 @@ static void show_supported_btf_features(FILE *output)
> int i;
>
> for (i = 0; i < ARRAY_SIZE(btf_features); i++) {
> + if (btf_features[i].feature_check && !btf_features[i].feature_check())
> + continue;
> if (i > 0)
> fprintf(output, ",");
> fprintf(output, "%s", btf_features[i].name);