Re: [PATCH] libbpf: fix BTF dedup to support recursive typedef definitions

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2025-11-07 at 16:34 +0100, paulhoussel2@gmail.com wrote:

[...]

> @@ -4936,10 +4963,77 @@ static int btf_dedup_struct_types(struct btf_dedup *d)
>  	return 0;
>  }
>
> +/*
> + * Deduplicate typedef types.
> + *
> + * Similar as for struct/union types, for each typedef type its type
> + * signature hash is calculated, taking into account type's name
> + * and its size, but ignoring type ID's referenced from fields,
> + * because they might not be deduped completely until after
> + * reference types deduplication phase.
> + */
> +static int btf_dedup_typedef_type(struct btf_dedup *d, __u32 type_id)
> +{

This function is effectively identical to btf_dedup_struct_type(),
the only things that differ are calls to hash and equal funcs:

	 	t = btf_type_by_id(d->btf, type_id);
	 	kind = btf_kind(t);

	-	if (kind != BTF_KIND_STRUCT && kind != BTF_KIND_UNION)
	+	if (kind != BTF_KIND_TYPEDEF)
	 		return 0;
	-
	-	h = btf_hash_struct(t);
	+	h = btf_hash_typedef(t);
	 	for_each_dedup_cand(d, hash_entry, h) {
	 		__u32 cand_id = hash_entry->value;
	 		int eq;

	 		cand_type = btf_type_by_id(d->btf, cand_id);
	-		if (!btf_shallow_equal_struct(t, cand_type))
	+		if (!btf_equal_typedef(t, cand_type))
	 			continue;

	 		btf_dedup_clear_hypot_map(d);

I don't like coping the logic related to hypot map maintenance and
d->hypot_adjust_canon flat processing.

Instead of copy-pasting, could you please modify btf_dedup_struct_type()?
- extend the type check to allow BTF_KIND_TYPEDEF;
- replace calls to btf_hash_struct() and btf_shallow_equal_struct()
  with calls to functions that select btf_hash_struct() /
  btf_hash_typedef() etc basing on the type?

Also, could you please add tests?
See tools/testing/selftests/bpf/prog_tests/btf.c.

> +	struct btf_type *cand_type, *t;
> +	struct hashmap_entry *hash_entry;
> +	/* if we don't find equivalent type, then we are canonical */
> +	__u32 new_id = type_id;
> +	__u16 kind;
> +	long h;
> +
> +	if (d->map[type_id] <= BTF_MAX_NR_TYPES)
> +		return 0;
> +
> +	t = btf_type_by_id(d->btf, type_id);
> +	kind = btf_kind(t);
> +
> +	if (kind != BTF_KIND_TYPEDEF)
> +		return 0;
> +	h = btf_hash_typedef(t);
> +	for_each_dedup_cand(d, hash_entry, h) {
> +		__u32 cand_id = hash_entry->value;
> +		int eq;
> +
> +		cand_type = btf_type_by_id(d->btf, cand_id);
> +		if (!btf_equal_typedef(t, cand_type))
> +			continue;
> +
> +		btf_dedup_clear_hypot_map(d);
> +		eq = btf_dedup_is_equiv(d, type_id, cand_id);
> +		if (eq < 0)
> +			return eq;
> +		if (!eq)
> +			continue;
> +		btf_dedup_merge_hypot_map(d);
> +		if (d->hypot_adjust_canon) /* not really equivalent */
> +			continue;
> +		new_id = cand_id;
> +		break;
> +	}
> +
> +	d->map[type_id] = new_id;
> +	if (type_id == new_id && btf_dedup_table_add(d, h, type_id))
> +		return -ENOMEM;
> +
> +	return 0;
> +}

[...]