On 11/12/2024 02:12, Eduard Zingerman wrote:
> btf_loader issues a warning when it sees several BTF_DECL_TAGs
> pointing to the same type. Such situation is possible in practice
> after patch [0], that marks certain functions with kfunc and
> bpf_fastcall tags. E.g.:
>
> $ pfunct vmlinux -F btf -f bpf_rdonly_cast
> WARNING: still unsuported BTF_KIND_DECL_TAG(bpf_fastcall) for bpf_cast_to_kern_ctx already with attribute (bpf_kfunc), ignoring
> WARNING: still unsuported BTF_KIND_DECL_TAG(bpf_fastcall) for bpf_rdonly_cast already with attribute (bpf_kfunc), ignoring
> bpf_kfunc void * bpf_rdonly_cast(const void * obj__ign, u32 btf_id__k);
>
> This commit extends 'struct tag' to allow attaching multiple
> attributes. Define 'struct attributes' as follows:
>
> struct attributes {
> uint64_t cnt;
> const char *values[];
> };
>
> In order to avoid adding counter field in 'struct tag',
> as not many instances of 'struct tag' would have attributes.
>
> Same command after this patch:
>
> $ pfunct vmlinux -F btf -f bpf_rdonly_cast
> bpf_kfunc bpf_fastcall void * bpf_rdonly_cast(const void * obj__ign, u32 btf_id__k);
>
> [0] https://lore.kernel.org/dwarves/094b626d44e817240ae8e44b6f7933b13c26d879.camel@xxxxxxxxx/T/#m8a6cb49a99d1b2ba38d616495a540ae8fc5f3a76
>
> Reported-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Closes: https://lore.kernel.org/dwarves/Z1dFXVFYmQ-nHSVO@x1/
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
Looks good to me. I _think_ we're safe enough in assuming the tag
ordering "bpf_kfunc bpf_fastcall" in the btf_functions.sh test, right?
Reviewed-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
Tested-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
> ---
> btf_loader.c | 31 +++++++++++++++++++++++--------
> dwarf_loader.c | 2 +-
> dwarves.c | 3 +++
> dwarves.h | 8 +++++++-
> dwarves_fprintf.c | 6 ++++--
> tests/btf_functions.sh | 2 +-
> 6 files changed, 39 insertions(+), 13 deletions(-)
>
> diff --git a/btf_loader.c b/btf_loader.c
> index 4814f29..af9e1db 100644
> --- a/btf_loader.c
> +++ b/btf_loader.c
> @@ -459,9 +459,28 @@ static int create_new_tag(struct cu *cu, int type, const struct btf_type *tp, ui
> return 0;
> }
>
> +static struct attributes *attributes__realloc(struct attributes *attributes, const char *value)
> +{
> + struct attributes *result;
> + uint64_t cnt;
> + size_t sz;
> +
> + cnt = attributes ? attributes->cnt : 0;
> + sz = sizeof(*attributes) + (cnt + 1) * sizeof(*attributes->values);
> + result = realloc(attributes, sz);
> + if (!result)
> + return NULL;
> + if (!attributes)
> + result->cnt = 0;
> + result->values[cnt] = value;
> + result->cnt++;
> + return result;
> +}
> +
> static int process_decl_tag(struct cu *cu, const struct btf_type *tp)
> {
> struct tag *tag = cu__type(cu, tp->type);
> + struct attributes *tmp;
>
> if (tag == NULL)
> tag = cu__function(cu, tp->type);
> @@ -475,15 +494,11 @@ static int process_decl_tag(struct cu *cu, const struct btf_type *tp)
> }
>
> const char *attribute = cu__btf_str(cu, tp->name_off);
> + tmp = attributes__realloc(tag->attributes, attribute);
> + if (!tmp)
> + return -ENOMEM;
>
> - if (tag->attribute != NULL) {
> - char bf[128];
> -
> - fprintf(stderr, "WARNING: still unsuported BTF_KIND_DECL_TAG(%s) for %s already with attribute (%s), ignoring\n",
> - attribute, tag__name(tag, cu, bf, sizeof(bf), NULL), tag->attribute);
> - } else {
> - tag->attribute = attribute;
> - }
> + tag->attributes = tmp;
>
> return 0;
> }
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index 598fde4..34376b2 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -513,7 +513,7 @@ static void tag__init(struct tag *tag, struct cu *cu, Dwarf_Die *die)
>
> dwarf_tag__set_attr_type(dtag, abstract_origin, die, DW_AT_abstract_origin);
> tag->recursivity_level = 0;
> - tag->attribute = NULL;
> + tag->attributes = NULL;
>
> if (cu->extra_dbg_info) {
> pthread_mutex_lock(&libdw__lock);
> diff --git a/dwarves.c b/dwarves.c
> index ae512b9..f970dd2 100644
> --- a/dwarves.c
> +++ b/dwarves.c
> @@ -210,6 +210,9 @@ void tag__delete(struct tag *tag, struct cu *cu)
>
> assert(list_empty(&tag->node));
>
> + if (tag->attributes)
> + free(tag->attributes);
> +
> switch (tag->tag) {
> case DW_TAG_union_type:
> type__delete(tag__type(tag), cu); break;
> diff --git a/dwarves.h b/dwarves.h
> index 1cb0d62..0a4d5a2 100644
> --- a/dwarves.h
> +++ b/dwarves.h
> @@ -516,10 +516,16 @@ int cu__for_all_tags(struct cu *cu,
> struct cu *cu, void *cookie),
> void *cookie);
>
> +struct attributes {
> + uint64_t cnt;
> + const char *values[];
> +};
> +
> /** struct tag - basic representation of a debug info element
> * @priv - extra data, for instance, DWARF offset, id, decl_{file,line}
> * @top_level -
> * @shared_tags: used by struct namespace
> + * @attributes - attributes specified by BTF_DECL_TAGs targeting this tag
> */
> struct tag {
> struct list_head node;
> @@ -530,7 +536,7 @@ struct tag {
> bool has_btf_type_tag:1;
> bool shared_tags:1;
> uint8_t recursivity_level;
> - const char *attribute;
> + struct attributes *attributes;
> };
>
> // To use with things like type->type_enum == perf_event_type+perf_user_event_type
> diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> index e16a6b4..c3e7f3c 100644
> --- a/dwarves_fprintf.c
> +++ b/dwarves_fprintf.c
> @@ -1405,9 +1405,11 @@ static size_t function__fprintf(const struct tag *tag, const struct cu *cu,
> struct ftype *ftype = func->btf ? tag__ftype(cu__type(cu, func->proto.tag.type)) : &func->proto;
> size_t printed = 0;
> bool inlined = !conf->strip_inline && function__declared_inline(func);
> + int i;
>
> - if (tag->attribute)
> - printed += fprintf(fp, "%s ", tag->attribute);
> + if (tag->attributes)
> + for (i = 0; i < tag->attributes->cnt; ++i)
> + printed += fprintf(fp, "%s ", tag->attributes->values[i]);
>
> if (func->virtuality == DW_VIRTUALITY_virtual ||
> func->virtuality == DW_VIRTUALITY_pure_virtual)
> diff --git a/tests/btf_functions.sh b/tests/btf_functions.sh
> index 61f8a00..c92e5ae 100755
> --- a/tests/btf_functions.sh
> +++ b/tests/btf_functions.sh
> @@ -66,7 +66,7 @@ pfunct --all --no_parm_names --format_path=dwarf $vmlinux | \
> sort|uniq > $outdir/dwarf.funcs
> # all functions from BTF (removing bpf_kfunc prefix where found)
> pfunct --all --no_parm_names --format_path=btf $outdir/vmlinux.btf 2>/dev/null|\
> - awk '{ gsub("^bpf_kfunc ",""); print $0}'|sort|uniq > $outdir/btf.funcs
> + awk '{ gsub("^(bpf_kfunc |bpf_fastcall )+",""); print $0}'|sort|uniq > $outdir/btf.funcs
>
> exact=0
> inline=0
