Em Mon, Dec 31, 2018 at 06:03:27AM +0000, Andrii Nakryiko escreveu:
> Union, when loaded, were created as 'struct type', while structs were
> created as 'struct class', that contained 'struct type' within it. When
> processing, though, unions and structs were treated the same with
> assumption that they both are represented by 'struct class'. This patch
> fixes issue by making sure unions and structs are created the same,
> modulo DW_TAG_structure_type vs DW_TAG_union_type tag.
>
> Without fix:
>
> andriin@devvm$ ~/local/pahole/build/pahole -F dwarf -C fpregs_state ~/local/linux/net/ipv4/tcp.o
> union fpregs_state {
> struct fxregs_state fxsave; /* 0 512 */
> struct swregs_state soft; /* 0 136 */
> struct xregs_state xsave; /* 0 576 */
> u8 __padding[4096]; /* 0 4096 */
> };
> andriin@devvm$ ~/local/pahole/build/pahole -F btf -C fpregs_state ~/local/linux/net/ipv4/tcp.o
> Segmentation fault (core dumped)
I couldn't reproduce this here:
[acme@quaco pahole]$ pahole -J ~/git/build/v4.20-rc5/net/ipv4/tcp.o
[acme@quaco pahole]$ pahole -F btf -C fpregs_state ~/git/build/v4.20-rc5/net/ipv4/tcp.o
union fpregs_state {
struct fregs_state fsave; /* 0 112 */
struct fxregs_state fxsave; /* 0 512 */
struct swregs_state soft; /* 0 136 */
struct xregs_state xsave; /* 0 576 */
u8 __padding[4096]; /* 0 4096 */
};
[acme@quaco pahole]$
Can you try with what in my current tree? I think these address it:
e066ed0431b6 (HEAD -> master, acme.korg/master) pahole: Fix logic inversion wrt finding holes in structs
6ebb50f47b19 pahole: Do not apply 'struct class' filters to 'struct type'
c9afdf88411d dwarves: Check if the tag is a 'struct class' in class__find_holes()
And I merged 6ebb50f47b19 with e066ed0431b6 to preserve bisectability.
So update the branch and this is the fix:
commit 2c4569a952e0910a4a19d87d636f0887f5f7ed4d
Author: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
Date: Fri Dec 21 14:20:58 2018 -0300
pahole: Do not apply 'struct class' filters to 'struct type'
Now unions are handled as well in pahole, so bail out before checking
'struct class' members for struct specific filters when handling
non-structs in class__filter()
Fixes: 31664d60ad41 ("pahole: Show tagged enums as well when no class is specified")
Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
- Arnaldo
>
> With fix:
>
> andriin@devvm$ ~/local/pahole/build/pahole -F dwarf -C fpregs_state ~/local/linux/net/ipv4/tcp.o
> union fpregs_state {
> struct fxregs_state fxsave; /* 0 512 */
> struct swregs_state soft; /* 0 136 */
> struct xregs_state xsave; /* 0 576 */
> u8 __padding[4096]; /* 0 4096 */
> };
> andriin@devvm$ ~/local/pahole/build/pahole -F btf -C fpregs_state ~/local/linux/net/ipv4/tcp.o
> union fpregs_state {
> struct fregs_state fsave; /* 0 112 */
> struct fxregs_state fxsave; /* 0 512 */
> struct swregs_state soft; /* 0 136 */
> struct xregs_state xsave; /* 0 576 */
> u8 __padding[4096]; /* 0 4096 */
> };
>
> Note that there is a difference between DWARF and BTF loader outputs:
> DWARF one seems to always skip first member of union type. It's not the case
> for structs, though. This issue will be investigated and fixed separately.
>
> Signed-off-by: Andrii Nakryiko <andriin@xxxxxx>
> ---
> btf_loader.c | 39 ++++++++++++---------------------------
> 1 file changed, 12 insertions(+), 27 deletions(-)
>
> diff --git a/btf_loader.c b/btf_loader.c
> index dae21d5..f04b12a 100644
> --- a/btf_loader.c
> +++ b/btf_loader.c
> @@ -1,4 +1,4 @@
> -/*
> +/*
> * btf_loader.c
> *
> * Copyright (C) 2018 Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> @@ -121,12 +121,12 @@ static struct type *type__new(uint16_t tag, strings_t name, size_t size)
> return type;
> }
>
> -static struct class *class__new(strings_t name, size_t size)
> +static struct class *class__new(strings_t name, size_t size, uint16_t tag)
> {
> struct class *class = tag__alloc(sizeof(*class));
>
> if (class != NULL) {
> - type__init(&class->type, DW_TAG_structure_type, name, size);
> + type__init(&class->type, tag, name, size);
> INIT_LIST_HEAD(&class->vtable);
> }
>
> @@ -205,7 +205,10 @@ static int create_members(struct btf *btf, void *ptr, int vlen, struct type *cla
> static int create_new_class(struct btf *btf, void *ptr, int vlen, struct btf_type *tp, uint64_t size, long id)
> {
> strings_t name = btf__get32(btf, &tp->name_off);
> - struct class *class = class__new(name, size);
> + uint16_t type_tag = BTF_INFO_KIND(tp->info) == BTF_KIND_STRUCT
> + ? DW_TAG_structure_type
> + : DW_TAG_union_type;
> + struct class *class = class__new(name, size, type_tag);
> int member_size = create_members(btf, ptr, vlen, &class->type);
>
> if (member_size < 0)
> @@ -219,25 +222,6 @@ out_free:
> return -ENOMEM;
> }
>
> -static int create_new_union(struct btf *btf, void *ptr,
> - int vlen, struct btf_type *tp,
> - uint64_t size, long id)
> -{
> - strings_t name = btf__get32(btf, &tp->name_off);
> - struct type *un = type__new(DW_TAG_union_type, name, size);
> - int member_size = create_members(btf, ptr, vlen, un);
> -
> - if (member_size < 0)
> - goto out_free;
> -
> - cu__add_tag(btf->priv, &un->namespace.tag, &id);
> -
> - return (vlen * member_size);
> -out_free:
> - type__delete(un, btf->priv);
> - return -ENOMEM;
> -}
> -
> static struct enumerator *enumerator__new(strings_t name, uint32_t value)
> {
> struct enumerator *en = tag__alloc(sizeof(*en));
> @@ -302,7 +286,10 @@ static int create_new_subroutine_type(struct btf *btf, void *ptr,
> static int create_new_forward_decl(struct btf *btf, struct btf_type *tp, uint64_t size, long id)
> {
> strings_t name = btf__get32(btf, &tp->name_off);
> - struct class *fwd = class__new(name, size);
> + uint16_t type_tag = BTF_INFO_KIND(tp->info)
> + ? DW_TAG_union_type
> + : DW_TAG_structure_type;
> + struct class *fwd = class__new(name, size, type_tag);
>
> if (fwd == NULL)
> return -ENOMEM;
> @@ -384,10 +371,8 @@ static int btf__load_types(struct btf *btf)
> vlen = create_new_base_type(btf, ptr, type_ptr, type_index);
> } else if (type == BTF_KIND_ARRAY) {
> vlen = create_new_array(btf, ptr, type_index);
> - } else if (type == BTF_KIND_STRUCT) {
> + } else if (type == BTF_KIND_STRUCT || type == BTF_KIND_UNION) {
> vlen = create_new_class(btf, ptr, vlen, type_ptr, size, type_index);
> - } else if (type == BTF_KIND_UNION) {
> - vlen = create_new_union(btf, ptr, vlen, type_ptr, size, type_index);
> } else if (type == BTF_KIND_ENUM) {
> vlen = create_new_enumeration(btf, ptr, vlen, type_ptr, size, type_index);
> } else if (type == BTF_KIND_FWD) {
> --
> 2.17.1
--
- Arnaldo
