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@xxxxxx>
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@xxxxxx>
> ---
> 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;
>
