Re: [PATCH 1/1] DWARF reader: fix bitfield offset calculations

[David Marchand <david.marchand@redhat.com>]

On Sat, Jul 10, 2021 at 6:52 PM Giuliano Procida <gprocida@google.com> wrote:
> >
> > But now, I face an issue trying to waive this.
> > The addition of the "dev_configured" field in this bitfield is right
> > after a 1 bit wide "lro" field and before a "rx_queue_state" field.
> > https://git.dpdk.org/dpdk/tree/lib/ethdev/rte_ethdev_core.h?id=f0b97fc4fe4cd16dd492cf30ece4296451e10e10#n165
> >
> > The rule below does not work:
> > [suppress_type]
> >         name = rte_eth_dev_data
> >         has_data_member_inserted_between = {offset_after(lro),
> > offset_of(rx_queue_state)}
> >
> > While on the other hand, this one works:
> > [suppress_type]
> >         name = rte_eth_dev_data
> >         has_data_member_inserted_between = {34005, offset_of(rx_queue_state)}
> >
> > Not sure what the issue is, but supposing the offsets are now correct,
> > then my bet is on the "lro" field size (here, 1 bit).
> >
>
> I'd open a separate bug for that.

Yep, I hesitated at first, but now I think we need one, see below.


>
> >
> > Back to my reproducer, that I updated:
> >
> > struct bigstruct {
> >     char name[128];
> >     uint8_t bitfield0:1
> > #ifndef BEFORE
> >             ,bitfield1:1
> > #endif
> >     ;
> >     uint8_t other;
> > };
> >
> > I wrote a suppression rule:
> > [suppress_type]
> >     name = bigstruct
> >     has_data_member_inserted_between = {offset_after(bitfield0),
> > offset_of(other)}
> >
> > And then I tried to debug, but I am lost in libabigail internals.
> > I attached a gdb to catch all get_size_in_bits() calls, I see either
> > 1024 (sizeof name), 8 (sizeof other?), 0 (this is suspicious) or 64
> > (?).
> >
>
> I would advise the following: separate ABI extraction (abidw) from ABI
> comparison (abidiff), if you haven't already.

That's what we do in dpdk.
Just, for testing here, I only used abidiff with object files.


>
> You can at least carefully check the XML before comparing XML files.

Yeah, good idea, and so, looking at xml:

<abi-corpus version='2.0' path='after_plop.o' architecture='elf-amd-x86_64'>
  <elf-function-symbols>
    <elf-symbol name='access_bigstruct' type='func-type'
binding='global-binding' visibility='default-visibility'
is-defined='yes'/>
  </elf-function-symbols>
  <abi-instr address-size='64' path='plop.c'
comp-dir-path='/home/dmarchan/libabigail_bitfields'
language='LANG_C99'>
    <type-decl name='char' size-in-bits='8' id='type-id-1'/>
    <array-type-def dimensions='1' type-id='type-id-1'
size-in-bits='1024' id='type-id-2'>
      <subrange length='128' type-id='type-id-3' id='type-id-4'/>
    </array-type-def>
    <type-decl name='unsigned char' size-in-bits='8' id='type-id-5'/>
    <type-decl name='unsigned long int' size-in-bits='64' id='type-id-3'/>
    <type-decl name='void' id='type-id-6'/>
    <class-decl name='bigstruct' size-in-bits='1040' is-struct='yes'
visibility='default'
filepath='/home/dmarchan/libabigail_bitfields/plop.c' line='3'
column='1' id='type-id-7'>
      <data-member access='public' layout-offset-in-bits='0'>
        <var-decl name='name' type-id='type-id-2' visibility='default'
filepath='/home/dmarchan/libabigail_bitfields/plop.c' line='4'
column='1'/>
      </data-member>
      <data-member access='public' layout-offset-in-bits='1024'>
        <var-decl name='bitfield0' type-id='type-id-8'
visibility='default'
filepath='/home/dmarchan/libabigail_bitfields/plop.c' line='5'
column='1'/>
                                            ^^^^^^^^^
      </data-member>
      <data-member access='public' layout-offset-in-bits='1025'>
        <var-decl name='bitfield1' type-id='type-id-8'
visibility='default'
filepath='/home/dmarchan/libabigail_bitfields/plop.c' line='7'
column='1'/>
      </data-member>
      <data-member access='public' layout-offset-in-bits='1032'>
        <var-decl name='other' type-id='type-id-8'
visibility='default'
filepath='/home/dmarchan/libabigail_bitfields/plop.c' line='10'
column='1'/>
      </data-member>
    </class-decl>
    <typedef-decl name='uint8_t' type-id='type-id-9'
filepath='/usr/include/bits/stdint-uintn.h' line='24' column='1'
id='type-id-8'/>
    <typedef-decl name='__uint8_t' type-id='type-id-5'
filepath='/usr/include/bits/types.h' line='38' column='1'
id='type-id-9'/>
    <pointer-type-def type-id='type-id-7' size-in-bits='64' id='type-id-10'/>
    <function-decl name='access_bigstruct'
mangled-name='access_bigstruct'
filepath='/home/dmarchan/libabigail_bitfields/plop.c' line='13'
column='1' visibility='default' binding='global' size-in-bits='64'
elf-symbol-id='access_bigstruct'>
      <parameter type-id='type-id-10' name='st'
filepath='/home/dmarchan/libabigail_bitfields/plop.c' line='13'
column='1'/>
      <return type-id='type-id-6'/>
    </function-decl>
  </abi-instr>
</abi-corpus>


My two 1bit fields are marked with type-id-8 which I understand points
at uint8_t.
I'll open a bz.


-- 
David Marchand