Re: [PATCH v3 2/2] Implement pahole-like 'ptype /o' option

[Simon Marchi <simon.marchi@polymtl.ca>]

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