All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH pahole 0/2] fix two more DWARF/BTF discrepancies
@ 2019-02-26  0:08 Andrii Nakryiko
  2019-02-26  0:08 ` [PATCH pahole 1/2] btf_loader: simplify fixup code by relying on BTF data more Andrii Nakryiko
  2019-02-26  0:08 ` [PATCH pahole 2/2] btf_encoder: don't special case packed enums Andrii Nakryiko
  0 siblings, 2 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2019-02-26  0:08 UTC (permalink / raw)
  To: acme, yhs, ast, dwarves, bpf, andrii.nakryiko; +Cc: Andrii Nakryiko

This patchset fixes two more issues where DWARF/BTF encoding/loading
produce different result:
  - patch #1 fixes incorrect handling of "unknown" base types;
  - patch #2 fixes incorrect handling of packed enums during encoding.

Andrii Nakryiko (2):
  btf_loader: simplify fixup code by relying on BTF data more
  btf_encoder: don't special case packed enums

 btf_encoder.c |  9 --------
 btf_loader.c  | 59 ++++++++++-----------------------------------------
 2 files changed, 11 insertions(+), 57 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH pahole 1/2] btf_loader: simplify fixup code by relying on BTF data more
  2019-02-26  0:08 [PATCH pahole 0/2] fix two more DWARF/BTF discrepancies Andrii Nakryiko
@ 2019-02-26  0:08 ` Andrii Nakryiko
  2019-02-26  0:08 ` [PATCH pahole 2/2] btf_encoder: don't special case packed enums Andrii Nakryiko
  1 sibling, 0 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2019-02-26  0:08 UTC (permalink / raw)
  To: acme, yhs, ast, dwarves, bpf, andrii.nakryiko; +Cc: Andrii Nakryiko

btf_loader relies on guessing base integral type size for enums and
integers, which is unreliable. There doesn't seem to be a need for
that, as all this information could be extracted from BTF information.

Before:
$ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/libc-2.28.so.debug
base_type__name_to_size: base_type _Float128
class__fixup_btf_bitfields: unknown base type name "_Float128"!
base_type__name_to_size: base_type _Float128
class__fixup_btf_bitfields: unknown base type name "_Float128"!
base_type__name_to_size: base_type _Float128
class__fixup_btf_bitfields: unknown base type name "_Float128"!
base_type__name_to_size: base_type _Float128
class__fixup_btf_bitfields: unknown base type name "_Float128"!
base_type__name_to_size: base_type _Float128
class__fixup_btf_bitfields: unknown base type name "_Float128"!
base_type__name_to_size: base_type _Float128
class__fixup_btf_bitfields: unknown base type name "_Float128"!
--- /tmp/btfdiff.dwarf.aV4wSL   2019-02-25 13:31:54.787923673 -0800
+++ /tmp/btfdiff.btf.22NQmJ     2019-02-25 13:31:54.802923668 -0800
@@ -461,9 +461,15 @@ struct La_x86_64_retval {
        uint64_t                   lrv_rdx;              /*     8     8 */
        La_x86_64_xmm              lrv_xmm0;             /*    16    16 */
        La_x86_64_xmm              lrv_xmm1;             /*    32    16 */
-       long double                lrv_st0;              /*    48    16 */
+       long double                lrv_st0;              /*    48     8 */
+
+       /* XXX 8 bytes hole, try to pack */
+
        /* --- cacheline 1 boundary (64 bytes) --- */
-       long double                lrv_st1;              /*    64    16 */
+       long double                lrv_st1;              /*    64     8 */
+
+       /* XXX 8 bytes hole, try to pack */
+
        La_x86_64_vector           lrv_vector0;          /*    80    64 */
        /* --- cacheline 2 boundary (128 bytes) was 16 bytes ago --- */
        La_x86_64_vector           lrv_vector1;          /*   144    64 */
@@ -472,6 +478,7 @@ struct La_x86_64_retval {
        __int128                   lrv_bnd1;             /*   224    16 */

        /* size: 240, cachelines: 4, members: 10 */
+       /* sum members: 224, holes: 2, sum holes: 16 */
        /* last cacheline: 48 bytes */
 };
 struct r_debug {
@@ -2044,7 +2051,7 @@ union ieee754_float {
        } ieee_nan;                                    /*     0     4 */
 };
 union ieee854_long_double {
-       long double                d;                  /*     0    16 */
+       long double                d;                  /*     0     8 */
        struct {
                unsigned int       mantissa1:32;       /*     0: 0  4 */
                unsigned int       mantissa0:32;       /*     4: 0  4 */
@@ -2141,7 +2148,7 @@ struct ucontext_t {
        /* last cacheline: 8 bytes */
 };
 union ieee854_float128 {
-       _Float128                  d;                  /*     0    16 */
+       _Float128                  d;                  /*     0     0 */
        struct {
                unsigned int       mantissa3:32;       /*     0: 0  4 */
                unsigned int       mantissa2:32;       /*     4: 0  4 */
@@ -2219,7 +2226,7 @@ union printf_arg {
        long unsigned int          pa_u_long_int;      /*     0     8 */
        long long unsigned int     pa_u_long_long_int; /*     0     8 */
        double                     pa_double;          /*     0     8 */
-       long double                pa_long_double;     /*     0    16 */
+       long double                pa_long_double;     /*     0     8 */
        const char  *              pa_string;          /*     0     8 */
        const wchar_t  *           pa_wstring;         /*     0     8 */
        void *                     pa_pointer;         /*     0     8 */

$ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/libc-2.28.so.debug
<empty output>

Still good for kernel image:
$ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/vmlinux4
<empty output>

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 btf_loader.c | 59 ++++++++++------------------------------------------
 1 file changed, 11 insertions(+), 48 deletions(-)

diff --git a/btf_loader.c b/btf_loader.c
index 62e7e30..3b4fb40 100644
--- a/btf_loader.c
+++ b/btf_loader.c
@@ -463,64 +463,27 @@ static int class__fixup_btf_bitfields(struct tag *tag, struct cu *cu, struct btf
 			continue;
 
 		pos->bitfield_offset = 0;
+		pos->byte_size = tag__size(type, cu);
+		pos->bit_size = pos->byte_size * 8;
 
-		uint16_t type_bit_size;
-		size_t integral_bit_size;
-
-		switch (type->tag) {
-		case DW_TAG_enumeration_type:
-			type_bit_size = tag__type(type)->size;
-			/* Best we can do to check if this is a packed enum */
-			if (is_power_of_2(type_bit_size))
-				integral_bit_size = roundup(type_bit_size, 8);
-			else
-				integral_bit_size = sizeof(int) * 8;
-			break;
-		case DW_TAG_base_type: {
-			struct base_type *bt = tag__base_type(type);
-			char name[256];
-			type_bit_size = bt->bit_size;
-			integral_bit_size = base_type__name_to_size(bt, cu);
-			if (integral_bit_size == 0) {
-				fprintf(stderr, "%s: unknown base type name \"%s\"!\n",
-					__func__, base_type__name(bt, cu, name,
-								  sizeof(name)));
-			}
-		}
-			break;
-		default:
-			pos->byte_size = tag__size(type, cu);
-			pos->bit_size = pos->byte_size * 8;
+		/* bitfield fixup is needed for enums and base types only */
+		if (type->tag != DW_TAG_base_type && type->tag != DW_TAG_enumeration_type)
 			continue;
-		}
 
-		/*
-		 * XXX: integral_bit_size can be zero if base_type__name_to_size doesn't
-		 * know about the base_type name, so one has to add there when
-		 * such base_type isn't found. pahole will put zero on the
-		 * struct output so it should be easy to spot the name when
-		 * such unlikely thing happens.
-		 */
-		pos->byte_size = integral_bit_size / 8;
-		pos->bit_size = type_bit_size;
-
-		if (integral_bit_size == 0) {
-			pos->bit_size = 0;
+		/* if BTF data is incorrect and has size == 0, skip field, 
+		 * instead of crashing */
+		if (pos->byte_size == 0) {
 			continue;
 		}
 
 		if (pos->bitfield_size) {
 			/* bitfields seem to be always aligned, no matter the packing */
-			pos->byte_offset = pos->bit_offset / integral_bit_size * integral_bit_size / 8;
-		} else {
-			pos->byte_offset = pos->bit_offset / 8;
-		}
-
-
-		if (pos->bitfield_size) {
+			pos->byte_offset = pos->bit_offset / pos->bit_size * pos->bit_size / 8;
 			pos->bitfield_offset = pos->bit_offset - pos->byte_offset * 8;
 			if (!btfe->is_big_endian)
-				pos->bitfield_offset = integral_bit_size - pos->bitfield_offset - pos->bitfield_size;
+				pos->bitfield_offset = pos->bit_size - pos->bitfield_offset - pos->bitfield_size;
+		} else {
+			pos->byte_offset = pos->bit_offset / 8;
 		}
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH pahole 2/2] btf_encoder: don't special case packed enums
  2019-02-26  0:08 [PATCH pahole 0/2] fix two more DWARF/BTF discrepancies Andrii Nakryiko
  2019-02-26  0:08 ` [PATCH pahole 1/2] btf_loader: simplify fixup code by relying on BTF data more Andrii Nakryiko
@ 2019-02-26  0:08 ` Andrii Nakryiko
  2019-02-26  1:00   ` Yonghong Song
  1 sibling, 1 reply; 5+ messages in thread
From: Andrii Nakryiko @ 2019-02-26  0:08 UTC (permalink / raw)
  To: acme, yhs, ast, dwarves, bpf, andrii.nakryiko; +Cc: Andrii Nakryiko

BTF data can represent packed enums correctly without any special
handling from pahole side. Previously pahole's own `enum vscope` would
be omitted causing problems.

Repro:

$ cat test/packed_enum.c
enum packed_enum {
        VALUE1,
        VALUE2,
        VALUE3
} __attribute__((packed));

struct s {
        int x;
        enum packed_enum e;
        int y;
};

int main()
{
        struct s s;
        return 0;
}

$ gcc -g -c test/packed_enum.c -o test/packed_enum.o

$ ~/local/pahole/build/pahole -JV test/packed_enum.o
File test/packed_enum.o:
[1] INT (anon) size=1 bit_offset=0 nr_bits=8 encoding=SIGNED
[2] STRUCT s kind_flag=0 size=12 vlen=3
        x type_id=3 bits_offset=0
        e type_id=1 bits_offset=32
        y type_id=3 bits_offset=64
[3] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED

$ ~/local/pahole/build/pahole -F dwarf test/packed_enum.o
struct s {
        int                        x;                    /*     0     4 */
        enum packed_enum           e;                    /*     4     1 */

        /* XXX 3 bytes hole, try to pack */

        int                        y;                    /*     8     4 */

        /* size: 12, cachelines: 1, members: 3 */
        /* sum members: 9, holes: 1, sum holes: 3 */
        /* last cacheline: 12 bytes */
};

$ ~/local/pahole/build/pahole -F btf test/packed_enum.o
struct s {
        int                        x;                    /*     0     4 */
        nameless base type!        e;                    /*     4     1 */

        /* XXX 3 bytes hole, try to pack */

        int                        y;                    /*     8     4 */

        /* size: 12, cachelines: 1, members: 3 */
        /* sum members: 9, holes: 1, sum holes: 3 */
        /* last cacheline: 12 bytes */
};

Notice how pahole's log doesn't have a mention of encoding 'packed_enum'
(anonymous integer is generated instead), which causes 'nameless base
type!' output above.

Fix this change:

$ ~/local/pahole/build/pahole -JV test/packed_enum.o
File test/packed_enum.o:
[1] ENUM packed_enum size=1 vlen=3
        VALUE1 val=0
        VALUE2 val=1
        VALUE3 val=2
[2] STRUCT s kind_flag=0 size=12 vlen=3
        x type_id=3 bits_offset=0
        e type_id=1 bits_offset=32
        y type_id=3 bits_offset=64
[3] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED

$ ~/local/pahole/build/pahole -F btf test/packed_enum.o
struct s {
        int                        x;                    /*     0     4 */
        enum packed_enum           e;                    /*     4     1 */

        /* XXX 3 bytes hole, try to pack */

        int                        y;                    /*     8     4 */

        /* size: 12, cachelines: 1, members: 3 */
        /* sum members: 9, holes: 1, sum holes: 3 */
        /* last cacheline: 12 bytes */
};

$ PAHOLE=~/local/pahole/build/pahole ./btfdiff test/packed_enum.o

Also verified on pahole, kernel and glibc:

$ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/pahole.debug
$ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/libc-2.28.so.debug
$ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/vmlinux4

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 btf_encoder.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 9cf0831..d8dc368 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -116,15 +116,6 @@ static int32_t enumeration_type__encode(struct btf_elf *btfe, struct tag *tag)
 	struct enumerator *pos;
 	int32_t type_id;
 
-	/* if enumerator bit_size is not 32, generate an int type instead. */
-	if (etype->size != 32) {
-		struct base_type bt = {};
-
-		bt.bit_size = etype->size;
-		bt.is_signed = true;
-		return btf_elf__add_base_type(btfe, &bt);
-	}
-
 	type_id = btf_elf__add_enum(btfe, etype->namespace.name, etype->size, etype->nr_members);
 	if (type_id < 0)
 		return type_id;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH pahole 2/2] btf_encoder: don't special case packed enums
  2019-02-26  0:08 ` [PATCH pahole 2/2] btf_encoder: don't special case packed enums Andrii Nakryiko
@ 2019-02-26  1:00   ` Yonghong Song
  2019-02-26  1:03     ` Andrii Nakryiko
  0 siblings, 1 reply; 5+ messages in thread
From: Yonghong Song @ 2019-02-26  1:00 UTC (permalink / raw)
  To: Andrii Nakryiko, acme, Alexei Starovoitov, dwarves, bpf, andrii.nakryiko



On 2/25/19 4:08 PM, Andrii Nakryiko wrote:
> BTF data can represent packed enums correctly without any special
> handling from pahole side. Previously pahole's own `enum vscope` would
> be omitted causing problems.
> 
> Repro:
> 
> $ cat test/packed_enum.c
> enum packed_enum {
>          VALUE1,
>          VALUE2,
>          VALUE3
> } __attribute__((packed));
> 
> struct s {
>          int x;
>          enum packed_enum e;
>          int y;
> };
> 
> int main()
> {
>          struct s s;
>          return 0;
> }
> 
> $ gcc -g -c test/packed_enum.c -o test/packed_enum.o
> 
> $ ~/local/pahole/build/pahole -JV test/packed_enum.o
> File test/packed_enum.o:
> [1] INT (anon) size=1 bit_offset=0 nr_bits=8 encoding=SIGNED
> [2] STRUCT s kind_flag=0 size=12 vlen=3
>          x type_id=3 bits_offset=0
>          e type_id=1 bits_offset=32
>          y type_id=3 bits_offset=64
> [3] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
> 
> $ ~/local/pahole/build/pahole -F dwarf test/packed_enum.o
> struct s {
>          int                        x;                    /*     0     4 */
>          enum packed_enum           e;                    /*     4     1 */
> 
>          /* XXX 3 bytes hole, try to pack */
> 
>          int                        y;                    /*     8     4 */
> 
>          /* size: 12, cachelines: 1, members: 3 */
>          /* sum members: 9, holes: 1, sum holes: 3 */
>          /* last cacheline: 12 bytes */
> };
> 
> $ ~/local/pahole/build/pahole -F btf test/packed_enum.o
> struct s {
>          int                        x;                    /*     0     4 */
>          nameless base type!        e;                    /*     4     1 */
> 
>          /* XXX 3 bytes hole, try to pack */
> 
>          int                        y;                    /*     8     4 */
> 
>          /* size: 12, cachelines: 1, members: 3 */
>          /* sum members: 9, holes: 1, sum holes: 3 */
>          /* last cacheline: 12 bytes */
> };
> 
> Notice how pahole's log doesn't have a mention of encoding 'packed_enum'
> (anonymous integer is generated instead), which causes 'nameless base
> type!' output above.
> 
> Fix this change:
> 
> $ ~/local/pahole/build/pahole -JV test/packed_enum.o
> File test/packed_enum.o:
> [1] ENUM packed_enum size=1 vlen=3
>          VALUE1 val=0
>          VALUE2 val=1
>          VALUE3 val=2
> [2] STRUCT s kind_flag=0 size=12 vlen=3
>          x type_id=3 bits_offset=0
>          e type_id=1 bits_offset=32
>          y type_id=3 bits_offset=64
> [3] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
> 
> $ ~/local/pahole/build/pahole -F btf test/packed_enum.o
> struct s {
>          int                        x;                    /*     0     4 */
>          enum packed_enum           e;                    /*     4     1 */
> 
>          /* XXX 3 bytes hole, try to pack */
> 
>          int                        y;                    /*     8     4 */
> 
>          /* size: 12, cachelines: 1, members: 3 */
>          /* sum members: 9, holes: 1, sum holes: 3 */
>          /* last cacheline: 12 bytes */
> };
> 
> $ PAHOLE=~/local/pahole/build/pahole ./btfdiff test/packed_enum.o
> 
> Also verified on pahole, kernel and glibc:
> 
> $ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/pahole.debug
> $ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/libc-2.28.so.debug
> $ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/vmlinux4
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Thanks for the fix.
Maybe add:
Fixes: b18354f64cc2 ("btf: Generate correct struct bitfield member types")

In commit message, add what Commit b18354f64cc2 tries to do
and why it is not needed any more.
  It tries to generate correct struct bitfield member type if the
  member is an enum. This is dated before kind_flag implementation.
  later kind_flag supported is added and pahole always generates
  BTF with kind_flag = 1 for structures with bitfield,
  where bitfield size is encoded in btf_member,
  so this workaround is not needed any more.
  Removing this "hack" can make handling packed enum correctly too.

With this change,
   Acked-by: Yonghong Song <yhs@fb.com>

> ---
>   btf_encoder.c | 9 ---------
>   1 file changed, 9 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 9cf0831..d8dc368 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -116,15 +116,6 @@ static int32_t enumeration_type__encode(struct btf_elf *btfe, struct tag *tag)
>   	struct enumerator *pos;
>   	int32_t type_id;
>   
> -	/* if enumerator bit_size is not 32, generate an int type instead. */
> -	if (etype->size != 32) {
> -		struct base_type bt = {};
> -
> -		bt.bit_size = etype->size;
> -		bt.is_signed = true;
> -		return btf_elf__add_base_type(btfe, &bt);
> -	}
> -
>   	type_id = btf_elf__add_enum(btfe, etype->namespace.name, etype->size, etype->nr_members);
>   	if (type_id < 0)
>   		return type_id;
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH pahole 2/2] btf_encoder: don't special case packed enums
  2019-02-26  1:00   ` Yonghong Song
@ 2019-02-26  1:03     ` Andrii Nakryiko
  0 siblings, 0 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2019-02-26  1:03 UTC (permalink / raw)
  To: Yonghong Song; +Cc: Andrii Nakryiko, acme, Alexei Starovoitov, dwarves, bpf

On Mon, Feb 25, 2019 at 5:00 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 2/25/19 4:08 PM, Andrii Nakryiko wrote:
> > BTF data can represent packed enums correctly without any special
> > handling from pahole side. Previously pahole's own `enum vscope` would
> > be omitted causing problems.
> >
> > Repro:
> >
> > $ cat test/packed_enum.c
> > enum packed_enum {
> >          VALUE1,
> >          VALUE2,
> >          VALUE3
> > } __attribute__((packed));
> >
> > struct s {
> >          int x;
> >          enum packed_enum e;
> >          int y;
> > };
> >
> > int main()
> > {
> >          struct s s;
> >          return 0;
> > }
> >
> > $ gcc -g -c test/packed_enum.c -o test/packed_enum.o
> >
> > $ ~/local/pahole/build/pahole -JV test/packed_enum.o
> > File test/packed_enum.o:
> > [1] INT (anon) size=1 bit_offset=0 nr_bits=8 encoding=SIGNED
> > [2] STRUCT s kind_flag=0 size=12 vlen=3
> >          x type_id=3 bits_offset=0
> >          e type_id=1 bits_offset=32
> >          y type_id=3 bits_offset=64
> > [3] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
> >
> > $ ~/local/pahole/build/pahole -F dwarf test/packed_enum.o
> > struct s {
> >          int                        x;                    /*     0     4 */
> >          enum packed_enum           e;                    /*     4     1 */
> >
> >          /* XXX 3 bytes hole, try to pack */
> >
> >          int                        y;                    /*     8     4 */
> >
> >          /* size: 12, cachelines: 1, members: 3 */
> >          /* sum members: 9, holes: 1, sum holes: 3 */
> >          /* last cacheline: 12 bytes */
> > };
> >
> > $ ~/local/pahole/build/pahole -F btf test/packed_enum.o
> > struct s {
> >          int                        x;                    /*     0     4 */
> >          nameless base type!        e;                    /*     4     1 */
> >
> >          /* XXX 3 bytes hole, try to pack */
> >
> >          int                        y;                    /*     8     4 */
> >
> >          /* size: 12, cachelines: 1, members: 3 */
> >          /* sum members: 9, holes: 1, sum holes: 3 */
> >          /* last cacheline: 12 bytes */
> > };
> >
> > Notice how pahole's log doesn't have a mention of encoding 'packed_enum'
> > (anonymous integer is generated instead), which causes 'nameless base
> > type!' output above.
> >
> > Fix this change:
> >
> > $ ~/local/pahole/build/pahole -JV test/packed_enum.o
> > File test/packed_enum.o:
> > [1] ENUM packed_enum size=1 vlen=3
> >          VALUE1 val=0
> >          VALUE2 val=1
> >          VALUE3 val=2
> > [2] STRUCT s kind_flag=0 size=12 vlen=3
> >          x type_id=3 bits_offset=0
> >          e type_id=1 bits_offset=32
> >          y type_id=3 bits_offset=64
> > [3] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
> >
> > $ ~/local/pahole/build/pahole -F btf test/packed_enum.o
> > struct s {
> >          int                        x;                    /*     0     4 */
> >          enum packed_enum           e;                    /*     4     1 */
> >
> >          /* XXX 3 bytes hole, try to pack */
> >
> >          int                        y;                    /*     8     4 */
> >
> >          /* size: 12, cachelines: 1, members: 3 */
> >          /* sum members: 9, holes: 1, sum holes: 3 */
> >          /* last cacheline: 12 bytes */
> > };
> >
> > $ PAHOLE=~/local/pahole/build/pahole ./btfdiff test/packed_enum.o
> >
> > Also verified on pahole, kernel and glibc:
> >
> > $ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/pahole.debug
> > $ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/libc-2.28.so.debug
> > $ PAHOLE=~/local/pahole/build/pahole ./btfdiff ~/local/btf/vmlinux4
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>
> Thanks for the fix.
> Maybe add:
> Fixes: b18354f64cc2 ("btf: Generate correct struct bitfield member types")
>
> In commit message, add what Commit b18354f64cc2 tries to do
> and why it is not needed any more.
>   It tries to generate correct struct bitfield member type if the
>   member is an enum. This is dated before kind_flag implementation.
>   later kind_flag supported is added and pahole always generates
>   BTF with kind_flag = 1 for structures with bitfield,
>   where bitfield size is encoded in btf_member,
>   so this workaround is not needed any more.
>   Removing this "hack" can make handling packed enum correctly too.
>
> With this change,
>    Acked-by: Yonghong Song <yhs@fb.com>

Will do! Thanks for providing context. I was guessing this must have
been done before BTF had good support for bitfields.

>
> > ---
> >   btf_encoder.c | 9 ---------
> >   1 file changed, 9 deletions(-)
> >
> > diff --git a/btf_encoder.c b/btf_encoder.c
> > index 9cf0831..d8dc368 100644
> > --- a/btf_encoder.c
> > +++ b/btf_encoder.c
> > @@ -116,15 +116,6 @@ static int32_t enumeration_type__encode(struct btf_elf *btfe, struct tag *tag)
> >       struct enumerator *pos;
> >       int32_t type_id;
> >
> > -     /* if enumerator bit_size is not 32, generate an int type instead. */
> > -     if (etype->size != 32) {
> > -             struct base_type bt = {};
> > -
> > -             bt.bit_size = etype->size;
> > -             bt.is_signed = true;
> > -             return btf_elf__add_base_type(btfe, &bt);
> > -     }
> > -
> >       type_id = btf_elf__add_enum(btfe, etype->namespace.name, etype->size, etype->nr_members);
> >       if (type_id < 0)
> >               return type_id;
> >

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-02-26  1:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-26  0:08 [PATCH pahole 0/2] fix two more DWARF/BTF discrepancies Andrii Nakryiko
2019-02-26  0:08 ` [PATCH pahole 1/2] btf_loader: simplify fixup code by relying on BTF data more Andrii Nakryiko
2019-02-26  0:08 ` [PATCH pahole 2/2] btf_encoder: don't special case packed enums Andrii Nakryiko
2019-02-26  1:00   ` Yonghong Song
2019-02-26  1:03     ` Andrii Nakryiko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.