Re: [PATCH pahole 0/3] fix handling of bitfields in btf_loader

[Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>]

Em Mon, Feb 25, 2019 at 01:02:39PM -0300, Arnaldo Carvalho de Melo escreveu:
> So yeah, the BTF encoder/decoder is working just fine, the problem is in
> pahole's DWARF code, lemme see...

Please try the patch below, for me btfdiff continues to show no diff for
all types in my vmlinux and now it also produces the same output for
when the first element of a bitfield has its bit_size equal to its
byte_size * 8:

[acme@quaco pahole]$ btfdiff vmlinux
[acme@quaco pahole]$ btfdiff examples/andrii/hsw_tsx_tuning-gcc
[acme@quaco pahole]$ btfdiff examples/andrii/hsw_tsx_tuning-clang
[acme@quaco pahole]$ pahole examples/andrii/hsw_tsx_tuning-clang
union hsw_tsx_tuning {
	struct {
		u32                cycles_last_block;  /*     0     4 */
		u32                hle_abort:1;        /*     4:31  4 */
		u32                rtm_abort:1;        /*     4:30  4 */
		u32                instruction_abort:1; /*     4:29  4 */
		u32                non_instruction_abort:1; /*     4:28  4 */
		u32                retry:1;            /*     4:27  4 */
		u32                data_conflict:1;    /*     4:26  4 */
		u32                capacity_writes:1;  /*     4:25  4 */
		u32                capacity_reads:1;   /*     4:24  4 */
	};                                             /*     0     8 */
	u64                        value;              /*     0     8 */
};
[acme@quaco pahole]$ pahole -F btf examples/andrii/hsw_tsx_tuning-clang
union hsw_tsx_tuning {
	struct {
		u32                cycles_last_block;  /*     0     4 */
		u32                hle_abort:1;        /*     4:31  4 */
		u32                rtm_abort:1;        /*     4:30  4 */
		u32                instruction_abort:1; /*     4:29  4 */
		u32                non_instruction_abort:1; /*     4:28  4 */
		u32                retry:1;            /*     4:27  4 */
		u32                data_conflict:1;    /*     4:26  4 */
		u32                capacity_writes:1;  /*     4:25  4 */
		u32                capacity_reads:1;   /*     4:24  4 */
	};                                             /*     0     8 */
	u64                        value;              /*     0     8 */
};
[acme@quaco pahole]$

diff --git a/dwarf_loader.c b/dwarf_loader.c
index adef437109f6..37477fb23be3 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -607,7 +607,8 @@ static struct variable *variable__new(Dwarf_Die *die, struct cu *cu)
 
 int tag__recode_dwarf_bitfield(struct tag *tag, struct cu *cu, uint16_t bit_size)
 {
-	uint16_t id;
+	int id;
+	uint16_t short_id;
 	struct tag *recoded;
 	/* in all the cases the name is at the same offset */
 	strings_t name = tag__namespace(tag)->name;
@@ -620,7 +621,7 @@ int tag__recode_dwarf_bitfield(struct tag *tag, struct cu *cu, uint16_t bit_size
 		struct tag *type = dtype->tag;
 
 		id = tag__recode_dwarf_bitfield(type, cu, bit_size);
-		if (id == tag->type)
+		if (id < 0)
 			return id;
 
 		struct type *new_typedef = obstack_zalloc(&cu->obstack,
@@ -660,10 +661,9 @@ int tag__recode_dwarf_bitfield(struct tag *tag, struct cu *cu, uint16_t bit_size
 		 * the dwarf_cu as in dwarf there are no such things
 		 * as base_types of less than 8 bits, etc.
 		 */
-		recoded = cu__find_base_type_by_sname_and_size(cu, name, bit_size, &id);
+		recoded = cu__find_base_type_by_sname_and_size(cu, name, bit_size, &short_id);
 		if (recoded != NULL)
-			return id;
-
+			return short_id;
 
 		struct base_type *new_bt = obstack_zalloc(&cu->obstack,
 							  sizeof(*new_bt));
@@ -683,10 +683,9 @@ int tag__recode_dwarf_bitfield(struct tag *tag, struct cu *cu, uint16_t bit_size
 		 * the dwarf_cu as in dwarf there are no such things
 		 * as enumeration_types of less than 8 bits, etc.
 		 */
-		recoded = cu__find_enumeration_by_sname_and_size(cu, name,
-								 bit_size, &id);
+		recoded = cu__find_enumeration_by_sname_and_size(cu, name, bit_size, &short_id);
 		if (recoded != NULL)
-			return id;
+			return short_id;
 
 		struct type *alias = tag__type(tag);
 		struct type *new_enum = obstack_zalloc(&cu->obstack, sizeof(*new_enum));