* [PATCH v2 pahole 1/2] btf_loader: simplify fixup code by relying on BTF data more
2019-02-26 1:19 [PATCH v2 pahole 0/2] fix two more DWARF/BTF discrepancies Andrii Nakryiko
@ 2019-02-26 1:19 ` Andrii Nakryiko
2019-02-26 1:19 ` [PATCH v2 pahole 2/2] btf_encoder: don't special case packed enums Andrii Nakryiko
1 sibling, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2019-02-26 1:19 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] 4+ messages in thread* [PATCH v2 pahole 2/2] btf_encoder: don't special case packed enums
2019-02-26 1:19 [PATCH v2 pahole 0/2] fix two more DWARF/BTF discrepancies Andrii Nakryiko
2019-02-26 1:19 ` [PATCH v2 pahole 1/2] btf_loader: simplify fixup code by relying on BTF data more Andrii Nakryiko
@ 2019-02-26 1:19 ` Andrii Nakryiko
2019-02-26 14:07 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 4+ messages in thread
From: Andrii Nakryiko @ 2019-02-26 1:19 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.
Original commit tried to generate correct struct bitfield member type if
the member is an enum. This was dated before kind_flag implementation.
Later, kind_flag support was added and now 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" makes handling it easier to handle packed enums
correctly.
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
Fixes: b18354f64cc2 ("btf: Generate correct struct bitfield member types")
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
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;
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread