[DWARVES]: Properly find holes in bitfields
Make class__find_holes understand that when a byte offset goes backward it is
because the compiler is "combining" small bitfields with previous fields, and
using from the end of the combined bitfield + small fields. This made your
head hurt, huh? One example:
struct usb_bus {
struct device * controller; /* 0 8 */
int busnum; /* 8 4 */
/* XXX 4 bytes hole, try to pack */
char * bus_name; /* 16 8 */
u8 uses_dma; /* 24 1 */
u8 otg_port; /* 25 1 */
/* Bitfield combined with previous fields */
unsigned int is_b_host:1; /* 24:15 4 */
unsigned int b_hnp_enable:1; /* 24:14 4 */
/* XXX 14 bits hole, try to pack */
int devnum_next; /* 28 4 */
struct usb_devmap devmap; /* 32 16 */
<SNIP>
};
See? is_b_host:1 .. b_hnp_enable:1 makes a bitfield of just two bits.
The programmer decided to make this a 'unsigned int' bitfield, so taking 4 bytes.
And placed this "4" bytes bitfield just after two fields of one byte.
The compiler put the "4" bytes bitfield "in the same place" as the "uses_dma"
field, but its really not clobbering it neither "otg_port", as it allocates it
from (offset 24 + sizeof(unsigned int) - 1), backwards.
So at the end there is a, now correctly calculated, 14 bits hole, and that
matches the bit offset used for the last field, that is "14", as offsets for
bits and bytes starts at zero, all is explained now.
One last thing is that since we actually have 14 bits we in fact have a one
byte hole + a 6 bits hole, but that should be clear (haha) for those looking
for holes :-)
Nah, just run pahole reorganize on this beast and you'll have (for the complete
structure, with the <SNIP> part back in:
struct usb_bus {
struct device * controller; /* 0 8 */
int busnum; /* 8 4 */
unsigned char is_b_host:1; /* 12:247 1 */
unsigned char b_hnp_enable:1; /* 12:246 1 */
/* XXX 6 bits hole, try to pack */
u8 otg_port; /* 13 1 */
u8 uses_dma; /* 14 1 */
/* XXX 1 byte hole, try to pack */
char * bus_name; /* 16 8 */
int bandwidth_isoc_reqs; /* 24 4 */
int devnum_next; /* 28 4 */
struct usb_devmap devmap; /* 32 16 */
struct usb_device * root_hub; /* 48 8 */
struct list_head bus_list; /* 56 16 */
/* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
int bandwidth_allocated; /* 72 4 */
int bandwidth_int_reqs; /* 76 4 */
struct dentry * usbfs_dentry; /* 80 8 */
struct class_device * class_dev; /* 88 8 */
struct mon_bus * mon_bus; /* 96 8 */
int monitored; /* 104 4 */
/* size: 112, cachelines: 2 */
/* sum members: 107, holes: 1, sum holes: 1 */
/* bit holes: 1, sum bit holes: 6 bits */
/* padding: 4 */
/* last cacheline: 48 bytes */
}; /* saved 8 bytes! */
And we save 8 bytes and reduce the previous complexity. Hey, but look at those
bit offsets at is_b_host and b_hnp_enable... damn, exposing the bit offsets I
just exposed another bug, that is: the reorganization code is not fixing up the
bit offsets, one more for the TODO list, nah, just compile it and pass the
results back to the dwarves and we get:
struct usb_bus {
struct device * controller; /* 0 8 */
int busnum; /* 8 4 */
unsigned char is_b_host:1; /* 12: 7 1 */
unsigned char b_hnp_enable:1; /* 12: 6 1 */
/* XXX 6 bits hole, try to pack */
u8 otg_port; /* 13 1 */
u8 uses_dma; /* 14 1 */
/* XXX 1 byte hole, try to pack */
char * bus_name; /* 16 8 */
int bandwidth_isoc_reqs; /* 24 4 */
int devnum_next; /* 28 4 */
struct usb_devmap devmap; /* 32 16 */
struct usb_device * root_hub; /* 48 8 */
struct list_head bus_list; /* 56 16 */
/* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
int bandwidth_allocated; /* 72 4 */
int bandwidth_int_reqs; /* 76 4 */
struct dentry * usbfs_dentry; /* 80 8 */
struct class_device * class_dev; /* 88 8 */
struct mon_bus * mon_bus; /* 96 8 */
int monitored; /* 104 4 */
/* size: 112, cachelines: 2 */
/* sum members: 107, holes: 1, sum holes: 1 */
/* bit holes: 1, sum bit holes: 6 bits */
/* padding: 4 */
/* last cacheline: 48 bytes */
};
See? this time gcc fixed up things for us and even agreed on the reorganization
the dwarves did! 8-)
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This commit is contained in:
Arnaldo Carvalho de Melo
parent
31d20380cc
commit
68be38bddd
23
dwarves.c
23
dwarves.c
|
|
@ -1830,6 +1830,7 @@ void class__find_holes(struct class *self, const struct cu *cu)
|
|||
struct class_member *pos, *last = NULL;
|
||||
size_t last_size = 0, size;
|
||||
uint32_t bit_sum = 0;
|
||||
uint32_t bitfield_real_offset = 0;
|
||||
|
||||
self->nr_holes = 0;
|
||||
self->nr_bit_holes = 0;
|
||||
|
|
@ -1844,9 +1845,8 @@ void class__find_holes(struct class *self, const struct cu *cu)
|
|||
/*
|
||||
* We have to cast both offsets to int64_t because
|
||||
* the current offset can be before the last offset
|
||||
* in some bitfield cases at least with gcc as
|
||||
* was the case with struct usb_bus field is_b_host in
|
||||
* the linux kernel circa 2.6.24-rc3.
|
||||
* when we are starting a bitfield that combines with
|
||||
* the previous, small size fields.
|
||||
*/
|
||||
const ssize_t cc_last_size = ((int64_t)pos->offset -
|
||||
(int64_t)last->offset);
|
||||
|
|
@ -1871,6 +1871,11 @@ void class__find_holes(struct class *self, const struct cu *cu)
|
|||
++self->nr_holes;
|
||||
|
||||
if (bit_sum != 0) {
|
||||
if (bitfield_real_offset != 0) {
|
||||
last_size = bitfield_real_offset - last->offset;
|
||||
bitfield_real_offset = 0;
|
||||
}
|
||||
|
||||
last->bit_hole = (last_size * 8) -
|
||||
bit_sum;
|
||||
if (last->bit_hole != 0)
|
||||
|
|
@ -1878,7 +1883,8 @@ void class__find_holes(struct class *self, const struct cu *cu)
|
|||
|
||||
bit_sum = 0;
|
||||
}
|
||||
}
|
||||
} else if (cc_last_size < 0 && bit_sum == 0)
|
||||
bitfield_real_offset = last->offset + last_size;
|
||||
}
|
||||
|
||||
bit_sum += pos->bit_size;
|
||||
|
|
@ -2387,12 +2393,9 @@ size_t class__fprintf(struct class *self, const struct cu *cu,
|
|||
fputc('\n', fp);
|
||||
++printed;
|
||||
}
|
||||
printed += fprintf(fp, "%.*s/* WARNING: DWARF"
|
||||
" offset=%zd, real offset="
|
||||
"%zd */\n",
|
||||
cconf.indent, tabs,
|
||||
pos->offset,
|
||||
last_offset + last_size);
|
||||
printed += fprintf(fp, "%.*s/* Bitfield combined"
|
||||
" with previous fields */\n",
|
||||
cconf.indent, tabs);
|
||||
} else if (cc_last_size > 0 &&
|
||||
(size_t)cc_last_size < last_size) {
|
||||
if (!newline++) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue