On Thu, Jan 10, 2019 at 10:10 AM Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> wrote:
>
> Em Thu, Jan 10, 2019 at 02:50:32PM -0300, Arnaldo Carvalho de Melo escreveu:
> > So, the size for an enumeration in BTF is in bytes, not in bits, as in
> > CTF, fixed with the patch below, looking at the other cases now.
>
> We're down to packed data structs, probably in the BTF encoder, see
> below.
>
> $ diff -u /tmp/tcp.o.dwarf.c /tmp/tcp.o.btf.c
> --- /tmp/tcp.o.dwarf.c 2019-01-10 12:10:26.890521322 -0300
> +++ /tmp/tcp.o.btf.c 2019-01-10 14:46:04.584743337 -0300
> @@ -585,11 +585,17 @@
> __u16 vesapm_off; /* 48 2 */
> __u16 pages; /* 50 2 */
> __u16 vesa_attributes; /* 52 2 */
> - __u32 capabilities; /* 54 4 */
> - __u32 ext_lfb_base; /* 58 4 */
> + __u32 capabilities; /* 52 4 */
> + __u32 ext_lfb_base; /* 56 4 */
> +
> + /* XXX 2 bytes hole, try to pack */
> +
> __u8 _reserved[2]; /* 62 2 */
>
> Were these 4 extra bytes came from? The original struct is:
>
> /*
> * These are set up by the setup-routine at boot-time:
> */
>
> struct screen_info {
> __u8 orig_x; /* 0x00 */
> __u8 orig_y; /* 0x01 */
> __u16 ext_mem_k; /* 0x02 */
> __u16 orig_video_page; /* 0x04 */
> __u8 orig_video_mode; /* 0x06 */
> __u8 orig_video_cols; /* 0x07 */
> __u8 flags; /* 0x08 */
> __u8 unused2; /* 0x09 */
> __u16 orig_video_ega_bx;/* 0x0a */
> __u16 unused3; /* 0x0c */
> __u8 orig_video_lines; /* 0x0e */
> __u8 orig_video_isVGA; /* 0x0f */
> __u16 orig_video_points;/* 0x10 */
>
> /* VESA graphic mode -- linear frame buffer */
> __u16 lfb_width; /* 0x12 */
> __u16 lfb_height; /* 0x14 */
> __u16 lfb_depth; /* 0x16 */
> __u32 lfb_base; /* 0x18 */
> __u32 lfb_size; /* 0x1c */
> __u16 cl_magic, cl_offset; /* 0x20 */
> __u16 lfb_linelength; /* 0x24 */
> __u8 red_size; /* 0x26 */
> __u8 red_pos; /* 0x27 */
> __u8 green_size; /* 0x28 */
> __u8 green_pos; /* 0x29 */
> __u8 blue_size; /* 0x2a */
> __u8 blue_pos; /* 0x2b */
> __u8 rsvd_size; /* 0x2c */
> __u8 rsvd_pos; /* 0x2d */
> __u16 vesapm_seg; /* 0x2e */
> __u16 vesapm_off; /* 0x30 */
> __u16 pages; /* 0x32 */
> __u16 vesa_attributes; /* 0x34 */
> __u32 capabilities; /* 0x36 */
> __u32 ext_lfb_base; /* 0x3a */
> __u8 _reserved[2]; /* 0x3e */
> } __attribute__((packed));
>
> $ pahole --hex -F dwarf -C screen_info ~/git/build/v4.20-rc5/net/ipv4/tcp.o
> struct screen_info {
> __u8 orig_x; /* 0 0x1 */
> __u8 orig_y; /* 0x1 0x1 */
> __u16 ext_mem_k; /* 0x2 0x2 */
> __u16 orig_video_page; /* 0x4 0x2 */
> __u8 orig_video_mode; /* 0x6 0x1 */
> __u8 orig_video_cols; /* 0x7 0x1 */
> __u8 flags; /* 0x8 0x1 */
> __u8 unused2; /* 0x9 0x1 */
> __u16 orig_video_ega_bx; /* 0xa 0x2 */
> __u16 unused3; /* 0xc 0x2 */
> __u8 orig_video_lines; /* 0xe 0x1 */
> __u8 orig_video_isVGA; /* 0xf 0x1 */
> __u16 orig_video_points; /* 0x10 0x2 */
> __u16 lfb_width; /* 0x12 0x2 */
> __u16 lfb_height; /* 0x14 0x2 */
> __u16 lfb_depth; /* 0x16 0x2 */
> __u32 lfb_base; /* 0x18 0x4 */
> __u32 lfb_size; /* 0x1c 0x4 */
> __u16 cl_magic; /* 0x20 0x2 */
> __u16 cl_offset; /* 0x22 0x2 */
> __u16 lfb_linelength; /* 0x24 0x2 */
> __u8 red_size; /* 0x26 0x1 */
> __u8 red_pos; /* 0x27 0x1 */
> __u8 green_size; /* 0x28 0x1 */
> __u8 green_pos; /* 0x29 0x1 */
> __u8 blue_size; /* 0x2a 0x1 */
> __u8 blue_pos; /* 0x2b 0x1 */
> __u8 rsvd_size; /* 0x2c 0x1 */
> __u8 rsvd_pos; /* 0x2d 0x1 */
> __u16 vesapm_seg; /* 0x2e 0x2 */
> __u16 vesapm_off; /* 0x30 0x2 */
> __u16 pages; /* 0x32 0x2 */
> __u16 vesa_attributes; /* 0x34 0x2 */
> __u32 capabilities; /* 0x36 0x4 */
> __u32 ext_lfb_base; /* 0x3a 0x4 */
> __u8 _reserved[2]; /* 0x3e 0x2 */
>
> /* size: 64, cachelines: 1, members: 36 */
> };
>
> the dwarf loader/pahole printer seem to be doing the right thing, the
> BTF encoder, it seems, has issues with attribute((packed)) structs, see
> below.
>
> [acme@quaco pahole]$ pahole --hex -F dwarf -C screen_info ~/git/build/v4.20-rc5/net/ipv4/tcp.o > /tmp/dwarf
> [acme@quaco pahole]$ pahole --hex -F btf -C screen_info ~/git/build/v4.20-rc5/net/ipv4/tcp.o > /tmp/btf
> [acme@quaco pahole]$ diff -u /tmp/dwarf /tmp/btf
> --- /tmp/dwarf 2019-01-10 15:00:14.946723885 -0300
> +++ /tmp/btf 2019-01-10 15:00:18.669757798 -0300
> @@ -32,9 +32,15 @@
> __u16 vesapm_off; /* 0x30 0x2 */
> __u16 pages; /* 0x32 0x2 */
> __u16 vesa_attributes; /* 0x34 0x2 */
> - __u32 capabilities; /* 0x36 0x4 */
> + __u32 capabilities; /* 0x34 0x4 */
> - __u32 ext_lfb_base; /* 0x3a 0x4 */
> + __u32 ext_lfb_base; /* 0x38 0x4 */
> +
> + /* XXX 2 bytes hole, try to pack */
> +
> __u8 _reserved[2]; /* 0x3e 0x2 */
>
> /* size: 64, cachelines: 1, members: 36 */
> + /* sum members: 60, holes: 1, sum holes: 2 */
> +
> + /* BRAIN FART ALERT! 64 != 60 + 2(holes), diff = 2 */
> };
> [acme@quaco pahole]$
>
> Why the BTF encoder or loader is getting this in the wrong place...
> Debugging...
>
> /* size: 64, cachelines: 1, members: 36 */
> + /* sum members: 60, holes: 1, sum holes: 2 */
> +
> + /* BRAIN FART ALERT! 64 != 60 + 2(holes), diff = 2 */
> };
> struct apm_bios_info {
> __u16 version; /* 0 2 */
> @@ -630,7 +636,10 @@
> __u32 sectors_per_track; /* 12 4 */
> __u64 number_of_sectors; /* 16 8 */
> __u16 bytes_per_sector; /* 24 2 */
> - __u32 dpte_ptr; /* 26 4 */
> + __u32 dpte_ptr; /* 24 4 */
>
>
> Looks like the same problem as with screen_info, 'dpte_ptr' got
> "unionized" with 'bytes_per_sector'
>
> +
> + /* XXX 2 bytes hole, try to pack */
> +
> __u16 key; /* 30 2 */
> __u8 device_path_info_length; /* 32 1 */
> __u8 reserved2; /* 33 1 */
> @@ -683,7 +692,10 @@
> } atapi; /* 56 16 */
> struct {
> __u16 id; /* 56 2 */
> - __u64 lun; /* 58 8 */
> + __u64 lun; /* 56 8 */
>
> Ditto, here it could be trying to put 'lun' in its natural alignment,
> probably 'struct edd_device_params' is packed like screen_info? Yes...
> include/uapi/linux/edd.h line 72
>
> +
> + /* XXX 2 bytes hole, try to pack */
> +
> /* --- cacheline 1 boundary (64 bytes) was 2 bytes ago --- */
> __u16 reserved1; /* 66 2 */
> __u32 reserved2; /* 68 4 */
> @@ -733,7 +745,10 @@
> __u8 checksum; /* 73 1 */
>
> /* size: 74, cachelines: 2, members: 18 */
> + /* sum members: 70, holes: 1, sum holes: 2 */
> /* last cacheline: 10 bytes */
> +
> + /* BRAIN FART ALERT! 74 != 70 + 2(holes), diff = 2 */
> };
> struct edd_info {
> __u8 device; /* 0 1 */
> @@ -849,10 +864,13 @@
> };
> struct desc_ptr {
> short unsigned int size; /* 0 2 */
> - long unsigned int address; /* 2 8 */
> + long unsigned int address; /* 0 8 */
>
> One more packed struct:
>
> struct desc_ptr {
> unsigned short size;
> unsigned long address;
> } __attribute__((packed)) ;
>
> /* size: 10, cachelines: 1, members: 2 */
> + /* padding: 2 */
> /* last cacheline: 10 bytes */
> +
> + /* BRAIN FART ALERT! 10 != 2 + 0(holes), diff = 8 */
> };
> struct pgprot {
> pgprotval_t pgprot; /* 0 8 */
> @@ -1467,10 +1485,13 @@
> };
> struct x86_hw_tss {
>
> Also a packed struct
>
> u32 reserved1; /* 0 4 */
> - u64 sp0; /* 4 8 */
> - u64 sp1; /* 12 8 */
> - u64 sp2; /* 20 8 */
> - u64 reserved2; /* 28 8 */
> + u64 sp0; /* 0 8 */
> + u64 sp1; /* 8 8 */
> + u64 sp2; /* 16 8 */
> + u64 reserved2; /* 24 8 */
> +
> + /* XXX 4 bytes hole, try to pack */
> +
> u64 ist[7]; /* 36 56 */
> /* --- cacheline 1 boundary (64 bytes) was 28 bytes ago --- */
> u32 reserved3; /* 92 4 */
> @@ -1479,7 +1500,10 @@
> u16 io_bitmap_base; /* 102 2 */
>
> /* size: 104, cachelines: 2, members: 10 */
> + /* sum members: 96, holes: 1, sum holes: 4 */
> /* last cacheline: 40 bytes */
> +
> + /* BRAIN FART ALERT! 104 != 96 + 4(holes), diff = 4 */
> };
> struct seq_operations {
> void * (*start)(struct seq_file *, loff_t *); /* 0 8 */
>
>
> This last one is interesting, was this done on purpose? I think one
> level of indirection more and we would keep the original expressiveness.
>
> @@ -8473,7 +8497,7 @@
> struct lruvec_stat * lruvec_stat_cpu; /* 136 8 */
> atomic_long_t lruvec_stat[30]; /* 144 240 */
> /* --- cacheline 6 boundary (384 bytes) --- */
> - long unsigned int lru_zone_size[5][5]; /* 384 200 */
> + long unsigned int lru_zone_size[25]; /* 384 200 */
> /* --- cacheline 9 boundary (576 bytes) was 8 bytes ago --- */
> struct mem_cgroup_reclaim_iter iter[13]; /* 584 208 */
> /* --- cacheline 12 boundary (768 bytes) was 24 bytes ago --- */
>
> - Arnaldo
There appears to be problems with bitfields in both DWARF data and
pahole's conversion from DWARF to BTF (see test file source code and
pahole output at the bottom).
1. pahole believes unpacked.y1 has offset 7, which is incorrect
(should be 32?).
unpacked.y2 has offset 32, which also doesn't make sense.
DWARF info for unpacked.y1 and unpacked.y2 also seem wrong (how
do we get offset 18 for y1?).
<2><63>: Abbrev Number: 3 (DW_TAG_member)
<64> DW_AT_name : y1
<67> DW_AT_decl_file : 1
<68> DW_AT_decl_line : 5
<69> DW_AT_type : <0x87>
<6d> DW_AT_byte_size : 4
<6e> DW_AT_bit_size : 7
<6f> DW_AT_bit_offset : 18
<70> DW_AT_data_member_location: 0
<2><71>: Abbrev Number: 3 (DW_TAG_member)
<72> DW_AT_name : y2
<75> DW_AT_decl_file : 1
<76> DW_AT_decl_line : 6
<77> DW_AT_type : <0x87>
<7b> DW_AT_byte_size : 4
<7c> DW_AT_bit_size : 20
<7d> DW_AT_bit_offset : 12
<7e> DW_AT_data_member_location: 4
2. Similar problems with packed.y1 and packed.y2. packed.y2 offset
is negative value even in DWARF:
<2><d2>: Abbrev Number: 6 (DW_TAG_member)
<d3> DW_AT_name : y2
<d6> DW_AT_decl_file : 1
<d7> DW_AT_decl_line : 14
<d8> DW_AT_type : <0x87>
<dc> DW_AT_byte_size : 4
<dd> DW_AT_bit_size : 20
<de> DW_AT_bit_offset : -2
<df> DW_AT_data_member_location: 0
andriin@devvm$ cat bitfields.c
struct unpacked{
char x1: 1;
char x2: 3;
char x3: 3;
int y1: 7;
int y2: 20;
};
struct packed{
char x1: 1;
char x2: 3;
char x3: 3;
int y1: 7;
int y2: 20;
} __attribute__((packed));
union packed_union {
char x1;
int y2;
struct packed s1;
struct unpacked s2;
};
union unpacked_union {
char x1;
int y2;
struct unpacked s;
} __attribute__((packed));
int main() {
struct packed s1;
struct unpacked s2;
union packed_union u1;
union unpacked_union u2;
return 0;
}
andriin@devvm$ cc -g bitfields.c -o bitfields
andriin@devvm$ LLVM_OBJCOPY=objcopy ~/local/pahole/build/pahole -JV bitfields
File bitfields:
[1] STRUCT unpacked kind_flag=1 size=8 vlen=5
x1 type_id=2 bitfield_size=1 bits_offset=0
x2 type_id=2 bitfield_size=3 bits_offset=1
x3 type_id=2 bitfield_size=3 bits_offset=4
y1 type_id=3 bitfield_size=7 bits_offset=7
y2 type_id=3 bitfield_size=20 bits_offset=32
[2] INT char size=1 bit_offset=0 nr_bits=8 encoding=(none)
[3] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
[4] STRUCT packed kind_flag=1 size=5 vlen=5
x1 type_id=2 bitfield_size=1 bits_offset=0
x2 type_id=2 bitfield_size=3 bits_offset=1
x3 type_id=2 bitfield_size=3 bits_offset=4
y1 type_id=3 bitfield_size=7 bits_offset=7
y2 type_id=3 bitfield_size=255 bits_offset=16776974
[5] UNION packed_union kind_flag=0 size=8 vlen=4
x1 type_id=2 bits_offset=0
y2 type_id=3 bits_offset=0
s1 type_id=4 bits_offset=0
s2 type_id=1 bits_offset=0
[6] UNION unpacked_union kind_flag=0 size=8 vlen=3
x1 type_id=2 bits_offset=0
y2 type_id=3 bits_offset=0
s type_id=1 bits_offset=0
andriin@devvm$ readelf -wi bitfields
Contents of the .debug_info section:
...
<1><2d>: Abbrev Number: 2 (DW_TAG_structure_type)
<2e> DW_AT_name : (indirect string, offset: 0x4e): unpacked
<32> DW_AT_byte_size : 8
<33> DW_AT_decl_file : 1
<34> DW_AT_decl_line : 1
<35> DW_AT_sibling : <0x80>
<2><39>: Abbrev Number: 3 (DW_TAG_member)
<3a> DW_AT_name : x1
<3d> DW_AT_decl_file : 1
<3e> DW_AT_decl_line : 2
<3f> DW_AT_type : <0x80>
<43> DW_AT_byte_size : 1
<44> DW_AT_bit_size : 1
<45> DW_AT_bit_offset : 7
<46> DW_AT_data_member_location: 0
<2><47>: Abbrev Number: 3 (DW_TAG_member)
<48> DW_AT_name : x2
<4b> DW_AT_decl_file : 1
<4c> DW_AT_decl_line : 3
<4d> DW_AT_type : <0x80>
<51> DW_AT_byte_size : 1
<52> DW_AT_bit_size : 3
<53> DW_AT_bit_offset : 4
<54> DW_AT_data_member_location: 0
<2><55>: Abbrev Number: 3 (DW_TAG_member)
<56> DW_AT_name : x3
<59> DW_AT_decl_file : 1
<5a> DW_AT_decl_line : 4
<5b> DW_AT_type : <0x80>
<5f> DW_AT_byte_size : 1
<60> DW_AT_bit_size : 3
<61> DW_AT_bit_offset : 1
<62> DW_AT_data_member_location: 0
<2><63>: Abbrev Number: 3 (DW_TAG_member)
<64> DW_AT_name : y1
<67> DW_AT_decl_file : 1
<68> DW_AT_decl_line : 5
<69> DW_AT_type : <0x87>
<6d> DW_AT_byte_size : 4
<6e> DW_AT_bit_size : 7
<6f> DW_AT_bit_offset : 18
<70> DW_AT_data_member_location: 0
<2><71>: Abbrev Number: 3 (DW_TAG_member)
<72> DW_AT_name : y2
<75> DW_AT_decl_file : 1
<76> DW_AT_decl_line : 6
<77> DW_AT_type : <0x87>
<7b> DW_AT_byte_size : 4
<7c> DW_AT_bit_size : 20
<7d> DW_AT_bit_offset : 12
<7e> DW_AT_data_member_location: 4
<2><7f>: Abbrev Number: 0
<1><80>: Abbrev Number: 4 (DW_TAG_base_type)
<81> DW_AT_byte_size : 1
<82> DW_AT_encoding : 6 (signed char)
<83> DW_AT_name : (indirect string, offset: 0xb3): char
<1><87>: Abbrev Number: 5 (DW_TAG_base_type)
<88> DW_AT_byte_size : 4
<89> DW_AT_encoding : 5 (signed)
<8a> DW_AT_name : int
<1><8e>: Abbrev Number: 2 (DW_TAG_structure_type)
<8f> DW_AT_name : (indirect string, offset: 0x50): packed
<93> DW_AT_byte_size : 5
<94> DW_AT_decl_file : 1
<95> DW_AT_decl_line : 9
<96> DW_AT_sibling : <0xe1>
<2><9a>: Abbrev Number: 3 (DW_TAG_member)
<9b> DW_AT_name : x1
<9e> DW_AT_decl_file : 1
<9f> DW_AT_decl_line : 10
<a0> DW_AT_type : <0x80>
<a4> DW_AT_byte_size : 1
<a5> DW_AT_bit_size : 1
<a6> DW_AT_bit_offset : 7
<a7> DW_AT_data_member_location: 0
<2><a8>: Abbrev Number: 3 (DW_TAG_member)
<a9> DW_AT_name : x2
<ac> DW_AT_decl_file : 1
<ad> DW_AT_decl_line : 11
<ae> DW_AT_type : <0x80>
<b2> DW_AT_byte_size : 1
<b3> DW_AT_bit_size : 3
<b4> DW_AT_bit_offset : 4
<b5> DW_AT_data_member_location: 0
<2><b6>: Abbrev Number: 3 (DW_TAG_member)
<b7> DW_AT_name : x3
<ba> DW_AT_decl_file : 1
<bb> DW_AT_decl_line : 12
<bc> DW_AT_type : <0x80>
<c0> DW_AT_byte_size : 1
<c1> DW_AT_bit_size : 3
<c2> DW_AT_bit_offset : 1
<c3> DW_AT_data_member_location: 0
<2><c4>: Abbrev Number: 3 (DW_TAG_member)
<c5> DW_AT_name : y1
<c8> DW_AT_decl_file : 1
<c9> DW_AT_decl_line : 13
<ca> DW_AT_type : <0x87>
<ce> DW_AT_byte_size : 4
<cf> DW_AT_bit_size : 7
<d0> DW_AT_bit_offset : 18
<d1> DW_AT_data_member_location: 0
<2><d2>: Abbrev Number: 6 (DW_TAG_member)
<d3> DW_AT_name : y2
<d6> DW_AT_decl_file : 1
<d7> DW_AT_decl_line : 14
<d8> DW_AT_type : <0x87>
<dc> DW_AT_byte_size : 4
<dd> DW_AT_bit_size : 20
<de> DW_AT_bit_offset : -2
<df> DW_AT_data_member_location: 0
<2><e0>: Abbrev Number: 0
...
