On Fri, Oct 21, 2022 at 04:02:03PM +0100, Alan Maguire wrote:
> BTF deduplication was throwing some strange results, where core kernel
> data types were failing to deduplicate due to the return values
> of function type members being void (0) instead of the actual type
> (unsigned int). An example of this can be seen below, where
> "struct dst_ops" was failing to deduplicate between kernel and
> module:
>
> struct dst_ops {
> short unsigned int family;
> unsigned int gc_thresh;
> int (*gc)(struct dst_ops *);
> struct dst_entry * (*check)(struct dst_entry *, __u32);
> unsigned int (*default_advmss)(const struct dst_entry *);
> unsigned int (*mtu)(const struct dst_entry *);
> ...
>
> struct dst_ops___2 {
> short unsigned int family;
> unsigned int gc_thresh;
> int (*gc)(struct dst_ops___2 *);
> struct dst_entry___2 * (*check)(struct dst_entry___2 *, __u32);
> void (*default_advmss)(const struct dst_entry___2 *);
> void (*mtu)(const struct dst_entry___2 *);
> ...
>
> This was seen with
>
> bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
>
> ...which rewrites the return value as 0 (void) when it is marked
> as matching DW_TAG_unspecified_type:
>
> static int32_t btf_encoder__tag_type(struct btf_encoder *encoder, uint32_t type_id_off, uint32_t tag_type)
> {
> if (tag_type == 0)
> return 0;
>
> if (encoder->cu->unspecified_type.tag && tag_type == encoder->cu->unspecified_type.type) {
> // No provision for encoding this, turn it into void.
> return 0;
> }
>
> return type_id_off + tag_type;
> }
>
> However the odd thing was that on further examination, the unspecified type
> was not being set, so why was this logic being tripped? Futher debugging
> showed that the encoder->cu->unspecified_type.tag value was garbage, and
> the type id happened to collide with "unsigned int"; as a result we
> were replacing unsigned ints with void return values, and since this
> was being done to function type members in structs, it triggered a
> type mismatch which failed deduplication between kernel and module.
>
> The fix is simply to calloc() the cu in cu__new() instead.
>
> Fixes: bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
> Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
awesome, this fixes the missing dedup I was seeing
with current pahole:
$ bpftool btf dump file ./vmlinux.test | grep "STRUCT 'task_struct'" | wc -l
69
with this patch:
$ bpftool btf dump file ./vmlinux.test | grep "STRUCT 'task_struct'" | wc -l
1
Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx>
thanks,
jirka
> ---
> dwarves.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/dwarves.c b/dwarves.c
> index fbebc1d..424381d 100644
> --- a/dwarves.c
> +++ b/dwarves.c
> @@ -626,7 +626,7 @@ struct cu *cu__new(const char *name, uint8_t addr_size,
> const unsigned char *build_id, int build_id_len,
> const char *filename, bool use_obstack)
> {
> - struct cu *cu = malloc(sizeof(*cu) + build_id_len);
> + struct cu *cu = calloc(1, sizeof(*cu) + build_id_len);
>
> if (cu != NULL) {
> uint32_t void_id;
> --
> 2.31.1
>
