[v3,2/2] Implement pahole-like 'ptype /o' option
Commit Message
This commit implements the pahole-like '/o' option for 'ptype', which
prints the offsets and sizes of struct fields, reporting whenever
there is a hole found.
The output is heavily based on pahole(1), with a few modifications
here and there to adjust it to our reality. Here's an example:
(gdb) ptype /o stap_probe
/* offset | size */
struct stap_probe {
/* 0 | 40 */ struct probe {
/* 0 | 8 */ const probe_ops *pops;
/* 8 | 8 */ gdbarch *arch;
/* 16 | 8 */ const char *name;
/* 24 | 8 */ const char *provider;
/* 32 | 8 */ CORE_ADDR address;
} /* total size: 40 bytes */ p;
/* 40 | 8 */ CORE_ADDR sem_addr;
/* 48:31 | 4 */ unsigned int args_parsed : 1;
/* XXX 7-bit hole */
/* XXX 7-byte hole */
/* 56 | 8 */ union {
/* 8 */ const char *text;
/* 8 */ VEC_stap_probe_arg_s *vec;
} /* total size: 8 bytes */ args_u;
} /* total size: 64 bytes */
A big part of this patch handles the formatting logic of 'ptype',
which is a bit messy. I tried to be not very invasive, but I had to
do some cleanups here and there to make life easier.
This patch is the start of a long-term work I'll do to flush the local
patches we carry for Fedora GDB. In this specific case, I'm aiming at
upstreaming the feature implemented by the 'pahole.py' script that is
shipped with Fedora GDB:
<https://src.fedoraproject.org/rpms/gdb/blob/master/f/gdb-archer.patch#_311>
This has been regression-tested on the BuildBot. There's a new
testcase for it, along with an update to the documentation. I also
thought it was worth mentioning this feature in the NEWS file.
gdb/ChangeLog:
yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com>
PR cli/16224
* NEWS (Changes since GDB 8.0): Mention new '/o' flag.
* c-typeprint.c (OFFSET_SPC_LEN): New define.
(print_spaces_filtered_with_print_options): New function.
(output_access_specifier): Take new argument FLAGS. Modify
function to call 'print_spaces_filtered_with_print_options'.
(c_print_type_union_field_offset): New function.
(c_print_type_struct_field_offset): New function.
(c_type_print_base_struct_union): Print offsets and sizes for
struct/union/class fields.
* typeprint.c (const struct type_print_options
type_print_raw_options): Initialize 'print_offsets' and
'offset_bitpos'.
(static struct type_print_options default_ptype_flags):
Likewise.
(whatis_exp): Handle '/o' option.
(_initialize_typeprint): Add '/o' flag to ptype's help.
* typeprint.h (struct type_print_options) <print_offsets>: New
field.
<offset_bitpos>: Likewise.
gdb/testsuite/ChangeLog:
yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com>
PR cli/16224
* gdb.base/ptype-offsets.cc: New file.
* gdb.base/ptype-offsets.exp: New file.
gdb/doc/ChangeLog:
yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com>
PR cli/16224
* gdb.texinfo (ptype): Add new flag '/o'.
---
gdb/NEWS | 3 +
gdb/c-typeprint.c | 193 ++++++++++++++++++++++++++++---
gdb/doc/gdb.texinfo | 4 +
gdb/testsuite/gdb.base/ptype-offsets.cc | 138 ++++++++++++++++++++++
gdb/testsuite/gdb.base/ptype-offsets.exp | 158 +++++++++++++++++++++++++
gdb/typeprint.c | 15 ++-
gdb/typeprint.h | 9 ++
7 files changed, 502 insertions(+), 18 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/ptype-offsets.cc
create mode 100644 gdb/testsuite/gdb.base/ptype-offsets.exp
Comments
Hi Sergio,
LGTM, with some nits.
On 2017-12-11 02:57 PM, Sergio Durigan Junior wrote:
> @@ -867,14 +890,86 @@ output_access_specifier (struct ui_file *stream,
> if (last_access != s_public)
> {
> last_access = s_public;
> - fprintfi_filtered (level + 2, stream,
> - "public:\n");
> + print_spaces_filtered_with_print_options (level + 2, stream, flags);
> + fprintf_filtered (stream, "public:\n");
> }
> }
>
> return last_access;
> }
>
> +/* Print information about the offset of TYPE inside its union.
> + FIELD_IDX represents the index of this TYPE inside the union. We
> + just print the type size, and nothing more.
> +
> + The output is strongly based on pahole(1). */
> +
> +static void
> +c_print_type_union_field_offset (struct type *type, unsigned int field_idx,
> + struct ui_file *stream)
> +{
> + struct type *ftype = check_typedef (TYPE_FIELD_TYPE (type, field_idx));
> +
> + fprintf_filtered (stream, "/* %4u */", TYPE_LENGTH (ftype));
> +}
> +
> +/* Print information about the offset of TYPE inside its struct.
> + FIELD_IDX represents the index of this TYPE inside the struct, and
> + ENDPOS is the end position of the previous type (this is how we
> + calculate whether there are holes in the struct). At the end,
> + ENDPOS is updated.
The wording confuses me a bit. TYPE is not inside a struct; the struct
contains fields, not types. And TYPE is the struct type IIUC, not the
field type. So it should maybe be something like:
Print information about field at index FIELD_IDX of the struct type TYPE.
ENDPOS is the one-past-the-end bit position of the previous field (where
we expect this field to be if there is no hole). At the end, ENDPOS is
updated to the one-past-the-end bit position of the current field.
What does OFFSET_BITPOS do?
> +
> + The output is strongly based on pahole(1). */
> +
> +static void
> +c_print_type_struct_field_offset (struct type *type, unsigned int field_idx,
> + unsigned int *endpos, struct ui_file *stream,
> + unsigned int offset_bitpos)
> +{
> + struct type *ftype = check_typedef (TYPE_FIELD_TYPE (type, field_idx));
> + unsigned int bitpos = TYPE_FIELD_BITPOS (type, field_idx);
> + unsigned int fieldsize_byte = TYPE_LENGTH (ftype);
> + unsigned int fieldsize_bit;
> +
> + if (*endpos > 0 && *endpos < bitpos)
Why do you check for *endpos > 0? Did you see a case where *endpos is 0
and bitpos > 0? That would mean that there's a "hole" before the first field.
Would we want to show it as a hole anyway?
Simon
On Monday, December 11 2017, Simon Marchi wrote:
> Hi Sergio,
>
> LGTM, with some nits.
>
> On 2017-12-11 02:57 PM, Sergio Durigan Junior wrote:
>> @@ -867,14 +890,86 @@ output_access_specifier (struct ui_file *stream,
>> if (last_access != s_public)
>> {
>> last_access = s_public;
>> - fprintfi_filtered (level + 2, stream,
>> - "public:\n");
>> + print_spaces_filtered_with_print_options (level + 2, stream, flags);
>> + fprintf_filtered (stream, "public:\n");
>> }
>> }
>>
>> return last_access;
>> }
>>
>> +/* Print information about the offset of TYPE inside its union.
>> + FIELD_IDX represents the index of this TYPE inside the union. We
>> + just print the type size, and nothing more.
>> +
>> + The output is strongly based on pahole(1). */
>> +
>> +static void
>> +c_print_type_union_field_offset (struct type *type, unsigned int field_idx,
>> + struct ui_file *stream)
>> +{
>> + struct type *ftype = check_typedef (TYPE_FIELD_TYPE (type, field_idx));
>> +
>> + fprintf_filtered (stream, "/* %4u */", TYPE_LENGTH (ftype));
>> +}
>> +
>> +/* Print information about the offset of TYPE inside its struct.
>> + FIELD_IDX represents the index of this TYPE inside the struct, and
>> + ENDPOS is the end position of the previous type (this is how we
>> + calculate whether there are holes in the struct). At the end,
>> + ENDPOS is updated.
>
> The wording confuses me a bit. TYPE is not inside a struct; the struct
> contains fields, not types. And TYPE is the struct type IIUC, not the
> field type. So it should maybe be something like:
>
> Print information about field at index FIELD_IDX of the struct type TYPE.
> ENDPOS is the one-past-the-end bit position of the previous field (where
> we expect this field to be if there is no hole). At the end, ENDPOS is
> updated to the one-past-the-end bit position of the current field.
Thanks, I adopted your version.
> What does OFFSET_BITPOS do?
Ah, I forgot to include it in the comment. It is the offset value we
carry over when we are printing an inner struct. This is needed because
otherwise we'd print inner structs starting at position 0, which is
something that Tom didn't like and suggested changing.
>> +
>> + The output is strongly based on pahole(1). */
>> +
>> +static void
>> +c_print_type_struct_field_offset (struct type *type, unsigned int field_idx,
>> + unsigned int *endpos, struct ui_file *stream,
>> + unsigned int offset_bitpos)
>> +{
>> + struct type *ftype = check_typedef (TYPE_FIELD_TYPE (type, field_idx));
>> + unsigned int bitpos = TYPE_FIELD_BITPOS (type, field_idx);
>> + unsigned int fieldsize_byte = TYPE_LENGTH (ftype);
>> + unsigned int fieldsize_bit;
>> +
>> + if (*endpos > 0 && *endpos < bitpos)
>
> Why do you check for *endpos > 0? Did you see a case where *endpos is 0
> and bitpos > 0? That would mean that there's a "hole" before the first field.
> Would we want to show it as a hole anyway?
Yeah, this situation happens when we have a virtual method in a class.
Because of the vtable, the first field of the struct will start at
offset 8 (for 64-bit architectures), and in this case *endpos will be 0
because we won't have updated it, leading to a confusing message about a
8-byte hole in the beginning of the struct:
...
50 /* offset | size */
51 type = struct abc {
52 public:
53 /* XXX 8-byte hole */
54 /* 8 | 8 */ void *field1;
...
In order to suppress this first message, I check for *endpos > 0.
I will add a comment to the code explaining this scenario.
On 2017-12-11 18:24, Sergio Durigan Junior wrote:
>>> +
>>> + The output is strongly based on pahole(1). */
>>> +
>>> +static void
>>> +c_print_type_struct_field_offset (struct type *type, unsigned int
>>> field_idx,
>>> + unsigned int *endpos, struct ui_file *stream,
>>> + unsigned int offset_bitpos)
>>> +{
>>> + struct type *ftype = check_typedef (TYPE_FIELD_TYPE (type,
>>> field_idx));
>>> + unsigned int bitpos = TYPE_FIELD_BITPOS (type, field_idx);
>>> + unsigned int fieldsize_byte = TYPE_LENGTH (ftype);
>>> + unsigned int fieldsize_bit;
>>> +
>>> + if (*endpos > 0 && *endpos < bitpos)
>>
>> Why do you check for *endpos > 0? Did you see a case where *endpos is
>> 0
>> and bitpos > 0? That would mean that there's a "hole" before the
>> first field.
>> Would we want to show it as a hole anyway?
>
> Yeah, this situation happens when we have a virtual method in a class.
> Because of the vtable, the first field of the struct will start at
> offset 8 (for 64-bit architectures), and in this case *endpos will be 0
> because we won't have updated it, leading to a confusing message about
> a
> 8-byte hole in the beginning of the struct:
>
> ...
> 50 /* offset | size */
> 51 type = struct abc {
> 52 public:
> 53 /* XXX 8-byte hole */
> 54 /* 8 | 8 */ void *field1;
> ...
>
> In order to suppress this first message, I check for *endpos > 0.
>
> I will add a comment to the code explaining this scenario.
Ah ok that makes sense. Yeah, a comment would be nice. But now I'm
thinking that it would be nice if GDB showed the vtable. If I say the
first field at offset 8, it would probably take me some time to get why,
and would maybe think it's a bug. But if we showed a fake field, such
as:
/* 0 | 8 */ /* vtable */
It would be immediately obvious.
Simon
On Monday, December 11 2017, Simon Marchi wrote:
> On 2017-12-11 18:24, Sergio Durigan Junior wrote:
>>>> +
>>>> + The output is strongly based on pahole(1). */
>>>> +
>>>> +static void
>>>> +c_print_type_struct_field_offset (struct type *type, unsigned int
>>>> field_idx,
>>>> + unsigned int *endpos, struct ui_file *stream,
>>>> + unsigned int offset_bitpos)
>>>> +{
>>>> + struct type *ftype = check_typedef (TYPE_FIELD_TYPE (type,
>>>> field_idx));
>>>> + unsigned int bitpos = TYPE_FIELD_BITPOS (type, field_idx);
>>>> + unsigned int fieldsize_byte = TYPE_LENGTH (ftype);
>>>> + unsigned int fieldsize_bit;
>>>> +
>>>> + if (*endpos > 0 && *endpos < bitpos)
>>>
>>> Why do you check for *endpos > 0? Did you see a case where *endpos
>>> is 0
>>> and bitpos > 0? That would mean that there's a "hole" before the
>>> first field.
>>> Would we want to show it as a hole anyway?
>>
>> Yeah, this situation happens when we have a virtual method in a class.
>> Because of the vtable, the first field of the struct will start at
>> offset 8 (for 64-bit architectures), and in this case *endpos will be 0
>> because we won't have updated it, leading to a confusing message
>> about a
>> 8-byte hole in the beginning of the struct:
>>
>> ...
>> 50 /* offset | size */
>> 51 type = struct abc {
>> 52 public:
>> 53 /* XXX 8-byte hole */
>> 54 /* 8 | 8 */ void *field1;
>> ...
>>
>> In order to suppress this first message, I check for *endpos > 0.
>>
>> I will add a comment to the code explaining this scenario.
>
> Ah ok that makes sense. Yeah, a comment would be nice. But now I'm
> thinking that it would be nice if GDB showed the vtable. If I say the
> first field at offset 8, it would probably take me some time to get
> why, and would maybe think it's a bug. But if we showed a fake field,
> such as:
>
> /* 0 | 8 */ /* vtable */
>
> It would be immediately obvious.
OK, I did that.
BTW, a little status update.
Apparently the patch can't handle bitfields very well. I've found a few
cases where the bitfield handling gets confused, printing wrong
offsets/sizes/holes. Bitfields can be extremely complex to deal with
when it comes to offsets...
I spent hours trying to improve the patch, managed to make some
progress, but there are still corner cases to fix. For example, the
patch doesn't deal well with this case:
struct aa {
/* 0 | 1 */ char aa;
/* 1: 1 | 1 */ unsigned char a : 7;
/* 1:15 | 4 */ int b : 10;
} /* total size: 4 bytes */
In this case, the bitfield "b" would be combined with the previous
bitfield "a", like pahole reports:
struct aa {
char aa; /* 0 1 */
unsigned char a:7; /* 1: 1 1 */
/* Bitfield combined with previous fields */
int b:10; /* 0: 7 4 */
}
Confusing... I'm not sure why pahole reports b's offset to be 0.
Also, the patch doesn't understand cases like this:
struct tyu
{
unsigned int a1 : 1;
unsigned int a2 : 3;
unsigned int a9 : 23;
/* PROBLEM HAPPENS HERE */
unsigned char a0 : 2;
uint64_t a3;
unsigned int a4 : 5;
uint64_t a5 : 3;
};
In this case, we're switching types in the middle of the bitfield. The
bitfield "unsigned char a0" would be combined with the bitfields above
it, but the patch doesn't know that:
struct tyu {
/* 0:31 | 4 */ unsigned int a1 : 1;
/* 0:28 | 4 */ unsigned int a2 : 3;
/* 0: 5 | 4 */ unsigned int a9 : 23;
/* 3: 6 | 1 */ unsigned char a0 : 2;
/* XXX 3-bit hole */
/* XXX 4-byte hole */
/* 8 | 8 */ uint64_t a3;
/* 16:27 | 4 */ unsigned int a4 : 5;
/* 16:56 | 8 */ uint64_t a5 : 3;
} /* total size: 24 bytes */
Whereas pahole reports:
struct tyu {
unsigned int a1:1; /* 0:31 4 */
unsigned int a2:3; /* 0:28 4 */
unsigned int a9:23; /* 0: 5 4 */
/* XXX 253 bits hole, try to pack */
/* Bitfield combined with next fields */
unsigned char a0:2; /* 3: 3 1 */
/* XXX 6 bits hole, try to pack */
/* XXX 4 bytes hole, try to pack */
uint64_t a3; /* 8 8 */
unsigned int a4:5; /* 16:27 4 */
uint64_t a5:3; /* 16:56 8 */
};
Hm, TBH pahole itself seems a bit confused here, saying there is a
253-bit hole...
Anyway, long story short, this is much more complex that I thought it
would be (TM). I am extremely tired right now and can't continue, but I
intend to resume work tomorrow morning. But I'd like to leave a few
options on the table:
1) Remove the patch from the 8.1 wishlist, which will unblock the
branching.
2) Remove the bitfield handling from the patch, leaving it
feature-incomplete, but working.
3) Push the patch with these known limitations, document them, mark them
as KFAIL in the testcase, and open a bug to fix them (I personally
wouldn't like this).
4) Wait more time until these issues are resolved.
WDYT?
Thanks,
On 12/12/2017 06:19 AM, Sergio Durigan Junior wrote:
> BTW, a little status update.
>
> Apparently the patch can't handle bitfields very well. I've found a few
> cases where the bitfield handling gets confused, printing wrong
> offsets/sizes/holes. Bitfields can be extremely complex to deal with
> when it comes to offsets...
>
> I spent hours trying to improve the patch, managed to make some
> progress, but there are still corner cases to fix. For example, the
> patch doesn't deal well with this case:
>
> struct aa {
> /* 0 | 1 */ char aa;
> /* 1: 1 | 1 */ unsigned char a : 7;
> /* 1:15 | 4 */ int b : 10;
> } /* total size: 4 bytes */
>
> In this case, the bitfield "b" would be combined with the previous
> bitfield "a", like pahole reports:
>
> struct aa {
> char aa; /* 0 1 */
> unsigned char a:7; /* 1: 1 1 */
>
> /* Bitfield combined with previous fields */
>
> int b:10; /* 0: 7 4 */
> }
>
> Confusing... I'm not sure why pahole reports b's offset to be 0.
0 seems right to me. The bitfield's type is int, with size 4,
and it lives at byte offset 0:
<2><53>: Abbrev Number: 5 (DW_TAG_member)
<54> DW_AT_name : (indirect string, offset: 0x4ae): b
<58> DW_AT_decl_file : 1
<59> DW_AT_decl_line : 7
<5a> DW_AT_type : <0x71>
<5e> DW_AT_byte_size : 4
<5f> DW_AT_bit_size : 10
<60> DW_AT_bit_offset : 7
<61> DW_AT_data_member_location: 0 <<<
Replacing the 0 with 1, like:
int b:10; /* 1: 7 4 */
would look incorrect to me, because that'd make '(char*)&aa + 1 + 4'
(address of containing object + byte offset + byte size) overshoot the
size of the containing object.
What is that number after ":" in bitfields supposed to mean in
pahole's output (and I assume that that's what you're trying
to emulate)? We're missing documentation for that.
It seems like it's supposed to mean the number of bits left in the
containing anonymous object (i.e., in the 4 bytes of the declared
int)? Then "0:7" seems right, given:
sizeof (int) * 8 - bitsof(aa.a) - bitsof(aa.a) - bits(aa.b)
=> sizeof (int) * 8 - 7 - 10
=> 7
It took me a while to get to this conclusion (and writing a lot
of text that I ended up deleting... :-P), because I originally
assumed that this was meant to be the field's bit offset.
>
> Also, the patch doesn't understand cases like this:
>
> struct tyu
> {
> unsigned int a1 : 1;
> unsigned int a2 : 3;
> unsigned int a9 : 23;
> /* PROBLEM HAPPENS HERE */
> unsigned char a0 : 2;
> uint64_t a3;
> unsigned int a4 : 5;
> uint64_t a5 : 3;
> };
>
> In this case, we're switching types in the middle of the bitfield. The
> bitfield "unsigned char a0" would be combined with the bitfields above
> it, but the patch doesn't know that:
>
> struct tyu {
> /* 0:31 | 4 */ unsigned int a1 : 1;
> /* 0:28 | 4 */ unsigned int a2 : 3;
> /* 0: 5 | 4 */ unsigned int a9 : 23;
> /* 3: 6 | 1 */ unsigned char a0 : 2;
> /* XXX 3-bit hole */
> /* XXX 4-byte hole */
> /* 8 | 8 */ uint64_t a3;
> /* 16:27 | 4 */ unsigned int a4 : 5;
> /* 16:56 | 8 */ uint64_t a5 : 3;
> } /* total size: 24 bytes */
>
> Whereas pahole reports:
>
> struct tyu {
> unsigned int a1:1; /* 0:31 4 */
> unsigned int a2:3; /* 0:28 4 */
> unsigned int a9:23; /* 0: 5 4 */
>
> /* XXX 253 bits hole, try to pack */
> /* Bitfield combined with next fields */
>
> unsigned char a0:2; /* 3: 3 1 */
>
> /* XXX 6 bits hole, try to pack */
> /* XXX 4 bytes hole, try to pack */
>
> uint64_t a3; /* 8 8 */
> unsigned int a4:5; /* 16:27 4 */
> uint64_t a5:3; /* 16:56 8 */
> };
>
> Hm, TBH pahole itself seems a bit confused here, saying there is a
> 253-bit hole...
A 253-bit hole does look odd.
>
>
> Anyway, long story short, this is much more complex that I thought it
> would be (TM). I am extremely tired right now and can't continue, but I
> intend to resume work tomorrow morning. But I'd like to leave a few
> options on the table:
>
> 1) Remove the patch from the 8.1 wishlist, which will unblock the
> branching.
>
> 2) Remove the bitfield handling from the patch, leaving it
> feature-incomplete, but working.
>
> 3) Push the patch with these known limitations, document them, mark them
> as KFAIL in the testcase, and open a bug to fix them (I personally
> wouldn't like this).
>
> 4) Wait more time until these issues are resolved.
>
Joel said that we'd re-evaluate Wednesday, so there's still some time.
Thanks,
Pedro Alves
Thanks for looking into this.
On Tuesday, December 12 2017, Pedro Alves wrote:
> On 12/12/2017 06:19 AM, Sergio Durigan Junior wrote:
>
>> BTW, a little status update.
>>
>> Apparently the patch can't handle bitfields very well. I've found a few
>> cases where the bitfield handling gets confused, printing wrong
>> offsets/sizes/holes. Bitfields can be extremely complex to deal with
>> when it comes to offsets...
>>
>> I spent hours trying to improve the patch, managed to make some
>> progress, but there are still corner cases to fix. For example, the
>> patch doesn't deal well with this case:
>>
>> struct aa {
>> /* 0 | 1 */ char aa;
>> /* 1: 1 | 1 */ unsigned char a : 7;
>> /* 1:15 | 4 */ int b : 10;
>> } /* total size: 4 bytes */
>>
>> In this case, the bitfield "b" would be combined with the previous
>> bitfield "a", like pahole reports:
>>
>> struct aa {
>> char aa; /* 0 1 */
>> unsigned char a:7; /* 1: 1 1 */
>>
>> /* Bitfield combined with previous fields */
>>
>> int b:10; /* 0: 7 4 */
>> }
>>
>> Confusing... I'm not sure why pahole reports b's offset to be 0.
>
> 0 seems right to me. The bitfield's type is int, with size 4,
> and it lives at byte offset 0:
>
> <2><53>: Abbrev Number: 5 (DW_TAG_member)
> <54> DW_AT_name : (indirect string, offset: 0x4ae): b
> <58> DW_AT_decl_file : 1
> <59> DW_AT_decl_line : 7
> <5a> DW_AT_type : <0x71>
> <5e> DW_AT_byte_size : 4
> <5f> DW_AT_bit_size : 10
> <60> DW_AT_bit_offset : 7
> <61> DW_AT_data_member_location: 0 <<<
>
> Replacing the 0 with 1, like:
>
> int b:10; /* 1: 7 4 */
>
> would look incorrect to me, because that'd make '(char*)&aa + 1 + 4'
> (address of containing object + byte offset + byte size) overshoot the
> size of the containing object.
OK. The GDB patch is printing "1:15" because the offset is calculated
as:
(bitpos + offset_bitpos) / TARGET_CHAR_BIT
In this particular case, offset_bitpos is zero because we're not inside
an inner struct, so we don't care about it. bitpos is TYPE_FIELD_BITPOS
(type, field_idx), which is 15 in this case, because sizeof (aa.aa) +
bitsof (aa.a) = 15. When we divide it by TARGET_CHAR_BIT, we get 1. It
seems using TYPE_FIELD_BITPOS is not reliable when dealing with
bitfields.
So there is something I'm not accounting here.
> What is that number after ":" in bitfields supposed to mean in
> pahole's output (and I assume that that's what you're trying
> to emulate)? We're missing documentation for that.
Yeah. I tried to find documentation on the output, but couldn't.
Yesterday I was reading pahole's code. From what I understand, it is
the number of bits left in the object.
> It seems like it's supposed to mean the number of bits left in the
> containing anonymous object (i.e., in the 4 bytes of the declared
> int)? Then "0:7" seems right, given:
>
> sizeof (int) * 8 - bitsof(aa.a) - bitsof(aa.a) - bits(aa.b)
> => sizeof (int) * 8 - 7 - 10
> => 7
I guess you meant:
sizeof (int) * 8 - bitsof (aa.a) - bitsof(aa.b)
Right?
But yeah, it makes sense when you consider that the first bitfield got
"promoted" from "unsigned char" to "int". I haven't figured out a way
to keep track of these type changes in order to calculate the right
offsets, remaining space and holes.
> It took me a while to get to this conclusion (and writing a lot
> of text that I ended up deleting... :-P), because I originally
> assumed that this was meant to be the field's bit offset.
>
>>
>> Also, the patch doesn't understand cases like this:
>>
>> struct tyu
>> {
>> unsigned int a1 : 1;
>> unsigned int a2 : 3;
>> unsigned int a9 : 23;
>> /* PROBLEM HAPPENS HERE */
>> unsigned char a0 : 2;
>> uint64_t a3;
>> unsigned int a4 : 5;
>> uint64_t a5 : 3;
>> };
>>
>> In this case, we're switching types in the middle of the bitfield. The
>> bitfield "unsigned char a0" would be combined with the bitfields above
>> it, but the patch doesn't know that:
>>
>> struct tyu {
>> /* 0:31 | 4 */ unsigned int a1 : 1;
>> /* 0:28 | 4 */ unsigned int a2 : 3;
>> /* 0: 5 | 4 */ unsigned int a9 : 23;
>> /* 3: 6 | 1 */ unsigned char a0 : 2;
>> /* XXX 3-bit hole */
>> /* XXX 4-byte hole */
>> /* 8 | 8 */ uint64_t a3;
>> /* 16:27 | 4 */ unsigned int a4 : 5;
>> /* 16:56 | 8 */ uint64_t a5 : 3;
>> } /* total size: 24 bytes */
>>
>> Whereas pahole reports:
>>
>> struct tyu {
>> unsigned int a1:1; /* 0:31 4 */
>> unsigned int a2:3; /* 0:28 4 */
>> unsigned int a9:23; /* 0: 5 4 */
>>
>> /* XXX 253 bits hole, try to pack */
>> /* Bitfield combined with next fields */
>>
>> unsigned char a0:2; /* 3: 3 1 */
>>
>> /* XXX 6 bits hole, try to pack */
>> /* XXX 4 bytes hole, try to pack */
>>
>> uint64_t a3; /* 8 8 */
>> unsigned int a4:5; /* 16:27 4 */
>> uint64_t a5:3; /* 16:56 8 */
>> };
>>
>> Hm, TBH pahole itself seems a bit confused here, saying there is a
>> 253-bit hole...
>
> A 253-bit hole does look odd.
Yes. I haven't checked all the offsets too; there may be
inconsistencies.
>>
>>
>> Anyway, long story short, this is much more complex that I thought it
>> would be (TM). I am extremely tired right now and can't continue, but I
>> intend to resume work tomorrow morning. But I'd like to leave a few
>> options on the table:
>>
>> 1) Remove the patch from the 8.1 wishlist, which will unblock the
>> branching.
>>
>> 2) Remove the bitfield handling from the patch, leaving it
>> feature-incomplete, but working.
>>
>> 3) Push the patch with these known limitations, document them, mark them
>> as KFAIL in the testcase, and open a bug to fix them (I personally
>> wouldn't like this).
>>
>> 4) Wait more time until these issues are resolved.
>>
>
> Joel said that we'd re-evaluate Wednesday, so there's still some time.
OK, I'll keep trying here.
Thanks,
On 12/12/2017 06:40 PM, Sergio Durigan Junior wrote:
> On Tuesday, December 12 2017, Pedro Alves wrote:
>
>> On 12/12/2017 06:19 AM, Sergio Durigan Junior wrote:
>>
>>> Apparently the patch can't handle bitfields very well. I've found a few
>>> cases where the bitfield handling gets confused, printing wrong
>>> offsets/sizes/holes. Bitfields can be extremely complex to deal with
>>> when it comes to offsets...
>>>
>>> I spent hours trying to improve the patch, managed to make some
>>> progress, but there are still corner cases to fix. For example, the
>>> patch doesn't deal well with this case:
>>>
>>> struct aa {
>>> /* 0 | 1 */ char aa;
>>> /* 1: 1 | 1 */ unsigned char a : 7;
>>> /* 1:15 | 4 */ int b : 10;
>>> } /* total size: 4 bytes */
>>>
>>> In this case, the bitfield "b" would be combined with the previous
>>> bitfield "a", like pahole reports:
>>>
>>> struct aa {
>>> char aa; /* 0 1 */
>>> unsigned char a:7; /* 1: 1 1 */
>>>
>>> /* Bitfield combined with previous fields */
>>>
>>> int b:10; /* 0: 7 4 */
>>> }
>>>
>>> Confusing... I'm not sure why pahole reports b's offset to be 0.
>>
>> 0 seems right to me. The bitfield's type is int, with size 4,
>> and it lives at byte offset 0:
>>
>> <2><53>: Abbrev Number: 5 (DW_TAG_member)
>> <54> DW_AT_name : (indirect string, offset: 0x4ae): b
>> <58> DW_AT_decl_file : 1
>> <59> DW_AT_decl_line : 7
>> <5a> DW_AT_type : <0x71>
>> <5e> DW_AT_byte_size : 4
>> <5f> DW_AT_bit_size : 10
>> <60> DW_AT_bit_offset : 7
>> <61> DW_AT_data_member_location: 0 <<<
>>
>> Replacing the 0 with 1, like:
>>
>> int b:10; /* 1: 7 4 */
>>
>> would look incorrect to me, because that'd make '(char*)&aa + 1 + 4'
>> (address of containing object + byte offset + byte size) overshoot the
>> size of the containing object.
>
> OK. The GDB patch is printing "1:15" because the offset is calculated
> as:
>
> (bitpos + offset_bitpos) / TARGET_CHAR_BIT
>
> In this particular case, offset_bitpos is zero because we're not inside
> an inner struct, so we don't care about it. bitpos is TYPE_FIELD_BITPOS
> (type, field_idx), which is 15 in this case, because sizeof (aa.aa) +
> bitsof (aa.a) = 15.
Well, actually it's 15 because that's the bit number of the LSB of that
field, and x86 is a !gdbarch_bits_big_endian machine. Because we have:
<2><53>: Abbrev Number: 4 (DW_TAG_member)
<54> DW_AT_name : b
<56> DW_AT_decl_file : 1
<57> DW_AT_decl_line : 7
<58> DW_AT_type : <0x6f>
<5c> DW_AT_byte_size : 4
<5d> DW_AT_bit_size : 10
<5e> DW_AT_bit_offset : 7
<5f> DW_AT_data_member_location: 0
and with that, we reach here:
static void
dwarf2_add_field (struct field_info *fip, struct die_info *die,
struct dwarf2_cu *cu)
{
...
attr = dwarf2_attr (die, DW_AT_bit_offset, cu);
if (attr)
{
if (gdbarch_bits_big_endian (gdbarch))
{
...
}
else
{
...
attr = dwarf2_attr (die, DW_AT_byte_size, cu);
if (attr)
{
/* The size of the anonymous object containing
the bit field is explicit, so use the
indicated size (in bytes). */
anonymous_size = DW_UNSND (attr);
}
else
{
...
}
SET_FIELD_BITPOS (*fp,
(FIELD_BITPOS (*fp)
+ anonymous_size * bits_per_byte
- bit_offset - FIELD_BITSIZE (*fp)));
}
}
which gives us:
bitpos = DW_AT_byte_size * 8 - DW_AT_bit_offset - DW_AT_bit_size
= 4 * 8 - 7 - 10
= 15
and since x86 is a !gdbarch_bits_big_endian machine, 15 goes here:
byte: 0 1 2 3
bits: 7-0 15-8 23-16 31-24
^
which is what we see if we write a "1" to aa.b:
(gdb) p aa.b = 1
$1 = 1
(gdb) x /4xb &aa
0x60103c <aa>: 0x00 0x80 0x00 0x00
^^^^
> When we divide it by TARGET_CHAR_BIT, we get 1. It
> seems using TYPE_FIELD_BITPOS is not reliable when dealing with
> bitfields.
I noticed:
(gdb) p /x & aa.aa
$1 = 0x601040
(gdb) p /x & aa.a
$2 = 0x601041
(gdb) p /x & aa.b
$3 = 0x601040
(gdb)
Ignoring the fact that taking the address of bitfields is
not valid C/C++ [ :-) ], it seems like the addresses above
match what we'd expect the byte offset to be. At least for this
example.
So maybe what you need is to factor out or duplicate the logic
used by the related value printing code. Here in
value_primitive_field:
if (TYPE_FIELD_BITSIZE (arg_type, fieldno))
{
/* Handle packed fields.
...
LONGEST bitpos = TYPE_FIELD_BITPOS (arg_type, fieldno);
LONGEST container_bitsize = TYPE_LENGTH (type) * 8;
v = allocate_value_lazy (type);
v->bitsize = TYPE_FIELD_BITSIZE (arg_type, fieldno);
if ((bitpos % container_bitsize) + v->bitsize <= container_bitsize
&& TYPE_LENGTH (type) <= (int) sizeof (LONGEST))
v->bitpos = bitpos % container_bitsize;
else
v->bitpos = bitpos % 8;
v->offset = (value_embedded_offset (arg1)
+ offset
+ (bitpos - v->bitpos) / 8);
}
Above "v->offset" may be the byte offset that you're after.
>
> So there is something I'm not accounting here.
>
>> What is that number after ":" in bitfields supposed to mean in
>> pahole's output (and I assume that that's what you're trying
>> to emulate)? We're missing documentation for that.
>
> Yeah. I tried to find documentation on the output, but couldn't.
> Yesterday I was reading pahole's code. From what I understand, it is
> the number of bits left in the object.
>
>> It seems like it's supposed to mean the number of bits left in the
>> containing anonymous object (i.e., in the 4 bytes of the declared
>> int)? Then "0:7" seems right, given:
>>
>> sizeof (int) * 8 - bitsof(aa.a) - bitsof(aa.a) - bits(aa.b)
>> => sizeof (int) * 8 - 7 - 10
>> => 7
>
> I guess you meant:
>
> sizeof (int) * 8 - bitsof (aa.a) - bitsof(aa.b)
No, because that's be
=> 32 - 7 - 10
=> 15
I meant:
sizeof (int) * 8 - bitsof(aa.aa) - bitsof(aa.a) - bits(aa.b)
=> sizeof (int) * 8 - 8 - 7 - 10
=> 7
which gives us the 7 unused bits that pahole reports.
>
> Right?
>
> But yeah, it makes sense when you consider that the first bitfield got
> "promoted" from "unsigned char" to "int". I haven't figured out a way
> to keep track of these type changes in order to calculate the right
> offsets, remaining space and holes.
Do we really need to track type changes, or do we simply need to
detect that the byte offset moved in the negative direction?
I.e., here:
struct S {
char aa; /* 0 1 */
unsigned char a:7; /* 1: 1 1 */
/* Bitfield combined with previous fields */
int b:10; /* 0: 7 4 */
};
The byte offset went "0 -> 1 -> 0", and the "1 -> 0" would mean that the
bitfield was "combined"?
Thanks,
Pedro Alves
@@ -3,6 +3,9 @@
*** Changes since GDB 8.0
+* The 'ptype' command now accepts a '/o' flag, which prints the
+ offsets and sizes of fields in a struct, like the pahole(1) tool.
+
* New "--readnever" command line option instructs GDB to not read each
symbol file's symbolic debug information. This makes startup faster
but at the expense of not being able to perform symbolic debugging.
@@ -32,6 +32,14 @@
#include "cp-abi.h"
#include "cp-support.h"
+/* When printing the offsets of a struct and its fields (i.e., 'ptype
+ /o'; type_print_options::print_offsets), we use this many
+ characters when printing the offset information at the beginning of
+ the line. This is needed in order to generate the correct amount
+ of whitespaces when no offset info should be printed for a certain
+ field. */
+#define OFFSET_SPC_LEN 23
+
/* A list of access specifiers used for printing. */
enum access_specifier
@@ -836,21 +844,36 @@ c_type_print_template_args (const struct type_print_options *flags,
fputs_filtered (_("] "), stream);
}
+/* Use 'print_spaces_filtered', but take into consideration the
+ type_print_options FLAGS in order to determine how many whitespaces
+ will be printed. */
+
+static void
+print_spaces_filtered_with_print_options (int level, struct ui_file *stream,
+ const struct type_print_options *flags)
+{
+ if (!flags->print_offsets)
+ print_spaces_filtered (level, stream);
+ else
+ print_spaces_filtered (level + OFFSET_SPC_LEN, stream);
+}
+
/* Output an access specifier to STREAM, if needed. LAST_ACCESS is the
last access specifier output (typically returned by this function). */
static enum access_specifier
output_access_specifier (struct ui_file *stream,
enum access_specifier last_access,
- int level, bool is_protected, bool is_private)
+ int level, bool is_protected, bool is_private,
+ const struct type_print_options *flags)
{
if (is_protected)
{
if (last_access != s_protected)
{
last_access = s_protected;
- fprintfi_filtered (level + 2, stream,
- "protected:\n");
+ print_spaces_filtered_with_print_options (level + 2, stream, flags);
+ fprintf_filtered (stream, "protected:\n");
}
}
else if (is_private)
@@ -858,8 +881,8 @@ output_access_specifier (struct ui_file *stream,
if (last_access != s_private)
{
last_access = s_private;
- fprintfi_filtered (level + 2, stream,
- "private:\n");
+ print_spaces_filtered_with_print_options (level + 2, stream, flags);
+ fprintf_filtered (stream, "private:\n");
}
}
else
@@ -867,14 +890,86 @@ output_access_specifier (struct ui_file *stream,
if (last_access != s_public)
{
last_access = s_public;
- fprintfi_filtered (level + 2, stream,
- "public:\n");
+ print_spaces_filtered_with_print_options (level + 2, stream, flags);
+ fprintf_filtered (stream, "public:\n");
}
}
return last_access;
}
+/* Print information about the offset of TYPE inside its union.
+ FIELD_IDX represents the index of this TYPE inside the union. We
+ just print the type size, and nothing more.
+
+ The output is strongly based on pahole(1). */
+
+static void
+c_print_type_union_field_offset (struct type *type, unsigned int field_idx,
+ struct ui_file *stream)
+{
+ struct type *ftype = check_typedef (TYPE_FIELD_TYPE (type, field_idx));
+
+ fprintf_filtered (stream, "/* %4u */", TYPE_LENGTH (ftype));
+}
+
+/* Print information about the offset of TYPE inside its struct.
+ FIELD_IDX represents the index of this TYPE inside the struct, and
+ ENDPOS is the end position of the previous type (this is how we
+ calculate whether there are holes in the struct). At the end,
+ ENDPOS is updated.
+
+ The output is strongly based on pahole(1). */
+
+static void
+c_print_type_struct_field_offset (struct type *type, unsigned int field_idx,
+ unsigned int *endpos, struct ui_file *stream,
+ unsigned int offset_bitpos)
+{
+ struct type *ftype = check_typedef (TYPE_FIELD_TYPE (type, field_idx));
+ unsigned int bitpos = TYPE_FIELD_BITPOS (type, field_idx);
+ unsigned int fieldsize_byte = TYPE_LENGTH (ftype);
+ unsigned int fieldsize_bit;
+
+ if (*endpos > 0 && *endpos < bitpos)
+ {
+ /* If ENDPOS is smaller than the current type's bitpos, it means
+ there's a hole in the struct, so we report it here. */
+ unsigned int hole = bitpos - *endpos;
+ unsigned int hole_byte = hole / TARGET_CHAR_BIT;
+ unsigned int hole_bit = hole % TARGET_CHAR_BIT;
+
+ if (hole_bit > 0)
+ fprintf_filtered (stream, "/* XXX %2u-bit hole */\n", hole_bit);
+
+ if (hole_byte > 0)
+ fprintf_filtered (stream, "/* XXX %2u-byte hole */\n", hole_byte);
+ }
+
+ /* The position of the field, relative to the beginning of the
+ struct. Assume this number will have 4 digits. */
+ fprintf_filtered (stream, "/* %4u",
+ (bitpos + offset_bitpos) / TARGET_CHAR_BIT);
+
+ if (TYPE_FIELD_PACKED (type, field_idx))
+ {
+ /* We're dealing with a bitfield. Print how many bits are left
+ to be used. */
+ fieldsize_bit = TYPE_FIELD_BITSIZE (type, field_idx);
+ fprintf_filtered (stream, ":%u",
+ fieldsize_byte * TARGET_CHAR_BIT - fieldsize_bit);
+ }
+ else
+ {
+ fieldsize_bit = fieldsize_byte * TARGET_CHAR_BIT;
+ fprintf_filtered (stream, " ");
+ }
+
+ fprintf_filtered (stream, " | %4u */", fieldsize_byte);
+
+ *endpos = bitpos + fieldsize_bit;
+}
+
/* Return true is an access label (i.e., "public:", "private:",
"protected:") needs to be printed for TYPE. */
@@ -1083,6 +1178,8 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
int len = TYPE_NFIELDS (type);
vptr_fieldno = get_vptr_fieldno (type, &basetype);
+ unsigned int endpos = 0;
+
for (int i = TYPE_N_BASECLASSES (type); i < len; i++)
{
QUIT;
@@ -1099,18 +1196,51 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
section_type = output_access_specifier
(stream, section_type, level,
TYPE_FIELD_PROTECTED (type, i),
- TYPE_FIELD_PRIVATE (type, i));
+ TYPE_FIELD_PRIVATE (type, i), flags);
+ }
+
+ bool is_static = field_is_static (&TYPE_FIELD (type, i));
+
+ if (flags->print_offsets)
+ {
+ if (!is_static)
+ {
+ if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
+ c_print_type_struct_field_offset (type, i, &endpos, stream,
+ flags->offset_bitpos);
+ else if (TYPE_CODE (type) == TYPE_CODE_UNION)
+ c_print_type_union_field_offset (type, i, stream);
+ }
+ else
+ print_spaces_filtered (OFFSET_SPC_LEN, stream);
}
print_spaces_filtered (level + 4, stream);
- if (field_is_static (&TYPE_FIELD (type, i)))
+ if (is_static)
fprintf_filtered (stream, "static ");
+
+ int newshow = show - 1;
+
+ if (flags->print_offsets
+ && (TYPE_CODE (TYPE_FIELD_TYPE (type, i)) == TYPE_CODE_STRUCT
+ || TYPE_CODE (TYPE_FIELD_TYPE (type, i)) == TYPE_CODE_UNION))
+ {
+ /* If we're printing offsets and this field's type is
+ either a struct or an union, then we're interested in
+ expanding it. */
+ ++newshow;
+
+ /* Make sure we carry our offset when we expand the
+ struct. */
+ local_flags.offset_bitpos
+ = flags->offset_bitpos + TYPE_FIELD_BITPOS (type, i);
+ }
+
c_print_type (TYPE_FIELD_TYPE (type, i),
TYPE_FIELD_NAME (type, i),
- stream, show - 1, level + 4,
+ stream, newshow, level + 4,
&local_flags);
- if (!field_is_static (&TYPE_FIELD (type, i))
- && TYPE_FIELD_PACKED (type, i))
+ if (!is_static && TYPE_FIELD_PACKED (type, i))
{
/* It is a bitfield. This code does not attempt
to look at the bitpos and reconstruct filler,
@@ -1173,9 +1303,10 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
section_type = output_access_specifier
(stream, section_type, level,
TYPE_FN_FIELD_PROTECTED (f, j),
- TYPE_FN_FIELD_PRIVATE (f, j));
+ TYPE_FN_FIELD_PRIVATE (f, j), flags);
- print_spaces_filtered (level + 4, stream);
+ print_spaces_filtered_with_print_options (level + 4, stream,
+ flags);
if (TYPE_FN_FIELD_VIRTUAL_P (f, j))
fprintf_filtered (stream, "virtual ");
else if (TYPE_FN_FIELD_STATIC_P (f, j))
@@ -1194,9 +1325,16 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
&& !is_full_physname_constructor /* " " */
&& !is_type_conversion_operator (type, i, j))
{
+ unsigned int old_po = local_flags.print_offsets;
+
+ /* Temporarily disable print_offsets, because it
+ would mess with indentation. */
+ local_flags.print_offsets = 0;
c_print_type (TYPE_TARGET_TYPE (TYPE_FN_FIELD_TYPE (f, j)),
"", stream, -1, 0,
&local_flags);
+ local_flags.print_offsets = old_po;
+
fputs_filtered (" ", stream);
}
if (TYPE_FN_FIELD_STUB (f, j))
@@ -1277,9 +1415,16 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
for (int i = 0; i < TYPE_NESTED_TYPES_COUNT (type); ++i)
{
- print_spaces_filtered (level + 4, stream);
+ unsigned int old_po = semi_local_flags.print_offsets;
+
+ /* Temporarily disable print_offsets, because it
+ would mess with indentation. */
+ print_spaces_filtered_with_print_options (level + 4, stream,
+ flags);
+ semi_local_flags.print_offsets = 0;
c_print_type (TYPE_NESTED_TYPES_FIELD_TYPE (type, i),
"", stream, show, level + 4, &semi_local_flags);
+ semi_local_flags.print_offsets = old_po;
fprintf_filtered (stream, ";\n");
}
}
@@ -1305,11 +1450,17 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
section_type = output_access_specifier
(stream, section_type, level,
TYPE_TYPEDEF_FIELD_PROTECTED (type, i),
- TYPE_TYPEDEF_FIELD_PRIVATE (type, i));
+ TYPE_TYPEDEF_FIELD_PRIVATE (type, i), flags);
}
- print_spaces_filtered (level + 4, stream);
+ print_spaces_filtered_with_print_options (level + 4, stream,
+ flags);
fprintf_filtered (stream, "typedef ");
+ unsigned int old_po = semi_local_flags.print_offsets;
+ /* Temporarily disable print_offsets, because it
+ would mess with indentation. */
+ semi_local_flags.print_offsets = 0;
+
/* We want to print typedefs with substitutions
from the template parameters or globally-known
typedefs but not local typedefs. */
@@ -1317,13 +1468,21 @@ c_type_print_base_struct_union (struct type *type, struct ui_file *stream,
TYPE_TYPEDEF_FIELD_NAME (type, i),
stream, show - 1, level + 4,
&semi_local_flags);
+ semi_local_flags.print_offsets = old_po;
fprintf_filtered (stream, ";\n");
}
}
+ if (flags->print_offsets && level > 0)
+ print_spaces_filtered (OFFSET_SPC_LEN, stream);
+
fprintfi_filtered (level, stream, "}");
}
+ if (show > 0 && flags->print_offsets)
+ fprintf_filtered (stream, " /* total size: %4u bytes */",
+ TYPE_LENGTH (type));
+
do_cleanups (local_cleanups);
}
@@ -17211,6 +17211,10 @@ names are substituted when printing other types.
@item T
Print typedefs defined in the class. This is the default, but the flag
exists in case you change the default with @command{set print type typedefs}.
+
+@item o
+Print the offsets and sizes of fields in a struct, similar to what the
+@command{pahole} tool does.
@end table
@kindex ptype
new file mode 100644
@@ -0,0 +1,138 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2017 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* This file will be used to test 'ptype /o' on x86_64 only. */
+
+#include <stdint.h>
+
+/* A struct with many types of fields, in order to test 'ptype
+ /o'. */
+
+struct abc
+{
+ /* Virtual destructor. */
+ virtual ~abc ()
+ {}
+
+ /* 8-byte address. Because of the virtual destructor above, this
+ field's offset will be 8. */
+ void *field1;
+
+ /* No hole here. */
+
+ /* 4-byte int bitfield of 1-bit. */
+ unsigned int field2 : 1;
+
+ /* 31-bit hole here. */
+
+ /* 4-byte int. */
+ int field3;
+
+ /* No hole here. */
+
+ /* 1-byte char. */
+ char field4;
+
+ /* 7-byte hole here. */
+
+ /* 8-byte int. */
+ uint64_t field5;
+
+ /* We just print the offset and size of a union, ignoring its
+ fields. */
+ union
+ {
+ /* 8-byte address. */
+ void *field6;
+
+ /* 4-byte int. */
+ int field7;
+ } field8;
+
+ /* Empty constructor. */
+ abc ()
+ {}
+};
+
+/* This struct will be nested inside 'struct xyz'. */
+
+struct tuv
+{
+ int a1;
+
+ char *a2;
+
+ int a3;
+};
+
+/* This struct will be nested inside 'struct pqr'. */
+
+struct xyz
+{
+ int f1;
+
+ char f2;
+
+ void *f3;
+
+ struct tuv f4;
+};
+
+/* A struct with a nested struct. */
+
+struct pqr
+{
+ int ff1;
+
+ struct xyz ff2;
+
+ char ff3;
+};
+
+/* A union with two nested structs. */
+
+union qwe
+{
+ struct tuv fff1;
+
+ struct xyz fff2;
+};
+
+/* A struct with an union. */
+
+struct poi
+{
+ int f1;
+
+ union qwe f2;
+
+ uint16_t f3;
+
+ struct pqr f4;
+};
+
+int
+main (int argc, char *argv[])
+{
+ struct abc foo;
+ struct pqr bar;
+ union qwe c;
+ struct poi d;
+ uint8_t i;
+
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,158 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2017 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile .cc
+
+# Test only works on x86_64 LP64 targets. That's how we guarantee
+# that the expected holes will be present in the struct.
+if { !([istarget "x86_64-*-*"] && [is_lp64_target]) } {
+ untested "test work only on x86_64 lp64"
+ return 0
+}
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
+ { debug c++ optimize=-O0 }] } {
+ return -1
+}
+
+# Test general offset printing, ctor/dtor printing, union, formatting.
+gdb_test "ptype /o struct abc" \
+ [multi_line \
+"type = struct abc {" \
+"/\\\* offset | size \\\*/" \
+" public:" \
+"/\\\* 8 | 8 \\\*/ void \\\*field1;" \
+"/\\\* 16:31 | 4 \\\*/ unsigned int field2 : 1;" \
+"/\\\* XXX 7-bit hole \\\*/" \
+"/\\\* XXX 3-byte hole \\\*/" \
+"/\\\* 20 | 4 \\\*/ int field3;" \
+"/\\\* 24 | 1 \\\*/ char field4;" \
+"/\\\* XXX 7-byte hole \\\*/" \
+"/\\\* 32 | 8 \\\*/ uint64_t field5;" \
+"/\\\* 40 | 8 \\\*/ union {" \
+"/\\\* 8 \\\*/ void \\\*field6;" \
+"/\\\* 4 \\\*/ int field7;" \
+" } /\\\* total size: 8 bytes \\\*/ field8;" \
+"" \
+" abc\\(void\\);" \
+" ~abc\\(\\);" \
+"} /\\\* total size: 48 bytes \\\*/"] \
+ "ptype offset struct abc"
+
+# Test nested structs.
+gdb_test "ptype /o struct pqr" \
+ [multi_line \
+"type = struct pqr {" \
+"/\\\* offset | size \\\*/" \
+"/\\\* 0 | 4 \\\*/ int f1;" \
+"/\\\* XXX 4-byte hole \\\*/" \
+"/\\\* 8 | 16 \\\*/ struct xyz {" \
+"/\\\* 8 | 4 \\\*/ int f1;" \
+"/\\\* 12 | 1 \\\*/ char f2;" \
+"/\\\* XXX 3-byte hole \\\*/" \
+"/\\\* 16 | 8 \\\*/ void \\\*f3;" \
+"/\\\* 24 | 24 \\\*/ struct tuv {" \
+"/\\\* 24 | 4 \\\*/ int a1;" \
+"/\\\* XXX 4-byte hole \\\*/" \
+"/\\\* 32 | 8 \\\*/ char *a2;" \
+"/\\\* 40 | 4 \\\*/ int a3;" \
+" } /\\\* total size: 24 bytes \\\*/ f4;" \
+" } /\\\* total size: 40 bytes \\\*/ ff2;" \
+"/\\\* 48 | 1 \\\*/ char ff3;" \
+"} /\\\* total size: 56 bytes \\\*/"] \
+ "ptype offset struct pqr"
+
+# Test that the offset is properly reset when we are printing an union
+# and go inside two inner structs.
+# This also tests a struct inside a struct inside an union.
+gdb_test "ptype /o union qwe" \
+ [multi_line \
+"/\\\* offset | size \\\*/" \
+"/\\\* 24 \\\*/ struct tuv {" \
+"/\\\* 0 | 4 \\\*/ int a1;" \
+"/\\\* XXX 4-byte hole \\\*/" \
+"/\\\* 8 | 8 \\\*/ char \\\*a2;" \
+"/\\\* 16 | 4 \\\*/ int a3;" \
+" } /\\\* total size: 24 bytes \\\*/ fff1;" \
+"/\\\* 40 \\\*/ struct xyz {" \
+"/\\\* 0 | 4 \\\*/ int f1;" \
+"/\\\* 4 | 1 \\\*/ char f2;" \
+"/\\\* XXX 3-byte hole \\\*/" \
+"/\\\* 8 | 8 \\\*/ void \\\*f3;" \
+"/\\\* 16 | 24 \\\*/ struct tuv {" \
+"/\\\* 16 | 4 \\\*/ int a1;" \
+"/\\\* XXX 4-byte hole \\\*/" \
+"/\\\* 24 | 8 \\\*/ char \\\*a2;" \
+"/\\\* 32 | 4 \\\*/ int a3;" \
+" } /\\\* total size: 24 bytes \\\*/ f4;" \
+" } /\\\* total size: 40 bytes \\\*/ fff2;" \
+"} /\\\* total size: 40 bytes \\\*/"] \
+ "ptype offset union qwe"
+
+# Test printing a struct that contains an union, and that also
+# contains a struct.
+gdb_test "ptype /o struct poi" \
+ [multi_line \
+"/\\\* offset | size \\\*/" \
+"/\\\* 0 | 4 \\\*/ int f1;" \
+"/\\\* XXX 4-byte hole \\\*/" \
+"/\\\* 8 | 40 \\\*/ union qwe {" \
+"/\\\* 24 \\\*/ struct tuv {" \
+"/\\\* 8 | 4 \\\*/ int a1;" \
+"/\\\* XXX 4-byte hole \\\*/" \
+"/\\\* 16 | 8 \\\*/ char \\\*a2;" \
+"/\\\* 24 | 4 \\\*/ int a3;" \
+" } /\\\* total size: 24 bytes \\\*/ fff1;" \
+"/\\\* 40 \\\*/ struct xyz {" \
+"/\\\* 8 | 4 \\\*/ int f1;" \
+"/\\\* 12 | 1 \\\*/ char f2;" \
+"/\\\* XXX 3-byte hole \\\*/" \
+"/\\\* 16 | 8 \\\*/ void \\\*f3;" \
+"/\\\* 24 | 24 \\\*/ struct tuv {" \
+"/\\\* 24 | 4 \\\*/ int a1;" \
+"/\\\* XXX 4-byte hole \\\*/" \
+"/\\\* 32 | 8 \\\*/ char \\\*a2;" \
+"/\\\* 40 | 4 \\\*/ int a3;" \
+" } /\\\* total size: 24 bytes \\\*/ f4;" \
+" } /\\\* total size: 40 bytes \\\*/ fff2;" \
+" } /\\\* total size: 40 bytes \\\*/ f2;" \
+"/\\\* 48 | 2 \\\*/ uint16_t f3;" \
+"/\\\* XXX 6-byte hole */" \
+"/\\\* 56 | 56 \\\*/ struct pqr {" \
+"/\\\* 56 | 4 \\\*/ int ff1;" \
+"/\\\* XXX 4-byte hole \\\*/" \
+"/\\\* 64 | 40 \\\*/ struct xyz {" \
+"/\\\* 64 | 4 \\\*/ int f1;" \
+"/\\\* 68 | 1 \\\*/ char f2;" \
+"/\\\* XXX 3-byte hole \\\*/" \
+"/\\\* 72 | 8 \\\*/ void \\\*f3;" \
+"/\\\* 80 | 24 \\\*/ struct tuv {" \
+"/\\\* 80 | 4 \\\*/ int a1;" \
+"/\\\* XXX 4-byte hole \\\*/" \
+"/\\\* 88 | 8 \\\*/ char \\\*a2;" \
+"/\\\* 96 | 4 \\\*/ int a3;" \
+" } /\\\* total size: 24 bytes \\\*/ f4;" \
+" } /\\\* total size: 40 bytes \\\*/ ff2;" \
+"/\\\* 104 | 1 \\\*/ char ff3;" \
+" } /\\\* total size: 56 bytes \\\*/ f4;" \
+"} /\\\* total size: 112 bytes \\\*/"] \
+ "ptype offset struct poi"
+
+# Test that we don't print any header when issuing a "ptype /o" on a
+# non-struct, non-union, non-class type.
+gdb_test "ptype /o int" "int" "ptype offset int"
+gdb_test "ptype /o uint8_t" "char" "ptype offset uint8_t"
@@ -42,7 +42,9 @@ const struct type_print_options type_print_raw_options =
1, /* raw */
1, /* print_methods */
1, /* print_typedefs */
+ 0, /* print_offsets */
0, /* print_nested_type_limit */
+ 0, /* offset_bitpos */
NULL, /* local_typedefs */
NULL, /* global_table */
NULL /* global_printers */
@@ -55,7 +57,9 @@ static struct type_print_options default_ptype_flags =
0, /* raw */
1, /* print_methods */
1, /* print_typedefs */
+ 0, /* print_offsets */
0, /* print_nested_type_limit */
+ 0, /* offset_bitpos */
NULL, /* local_typedefs */
NULL, /* global_table */
NULL /* global_printers */
@@ -440,6 +444,9 @@ whatis_exp (const char *exp, int show)
case 'T':
flags.print_typedefs = 1;
break;
+ case 'o':
+ flags.print_offsets = 1;
+ break;
default:
error (_("unrecognized flag '%c'"), *exp);
}
@@ -499,6 +506,11 @@ whatis_exp (const char *exp, int show)
real_type = value_rtti_type (val, &full, &top, &using_enc);
}
+ if (flags.print_offsets &&
+ (TYPE_CODE (type) == TYPE_CODE_STRUCT
+ || TYPE_CODE (type) == TYPE_CODE_UNION))
+ fprintf_filtered (gdb_stdout, "/* offset | size */\n");
+
printf_filtered ("type = ");
if (!flags.raw)
@@ -759,7 +771,8 @@ Available FLAGS are:\n\
/m do not print methods defined in a class\n\
/M print methods defined in a class\n\
/t do not print typedefs defined in a class\n\
- /T print typedefs defined in a class"));
+ /T print typedefs defined in a class\n\
+ /o print offsets and sizes of fields in a struct (like pahole)\n"));
set_cmd_completer (c, expression_completer);
c = add_com ("whatis", class_vars, whatis_command,
@@ -35,9 +35,18 @@ struct type_print_options
/* True means print typedefs in a class. */
unsigned int print_typedefs : 1;
+ /* True means to print offsets, a la 'pahole'. */
+ unsigned int print_offsets : 1;
+
/* The number of nested type definitions to print. -1 == all. */
int print_nested_type_limit;
+ /* The offset to be applied to bitpos when PRINT_OFFSETS is true.
+ This is needed for when we are printing nested structs and want
+ to make sure that the printed offset for each field carries off
+ the offset of the outter struct. */
+ unsigned int offset_bitpos;
+
/* If not NULL, a local typedef hash table used when printing a
type. */
struct typedef_hash_table *local_typedefs;