On Wed, 10 Nov 2021, Jakub Jelinek wrote: > Hi! > > For PCC_BITFIELD_TYPE_MATTERS field_byte_offset has quite large code > to deal with it since many years ago (see it e.g. in GCC 3.2, although it > used to be on HOST_WIDE_INTs, then on double_ints, now on offset_ints). > But that code apparently isn't able to cope with members with empty class > types with [[no_unique_address]] attribute, because the empty classes have > non-zero type size but zero decl size and so one can end up from the > computation with negative offset or offset 1 byte smaller than it should be. > For !PCC_BITFIELD_TYPE_MATTERS, we just use > tree_result = byte_position (decl); > which seems exactly right even for the empty classes or anything which is > not a bitfield (and for which we don't add DW_AT_bit_offset attribute). > So, instead of trying to handle those no_unique_address members in the > current already very complicated code, this limits it to bitfields. > > stor-layout.c PCC_BITFIELD_TYPE_MATTERS handling also affects only > bitfields, twice it checks DECL_BIT_FIELD and once DECL_BIT_FIELD_TYPE. > > The only thing I'm unsure about is whether the test should be > DECL_BIT_FIELD or DECL_BIT_FIELD_TYPE should be tested. I thought it > doesn't matter, but it seems stor-layout.c in some cases clears > DECL_BIT_FIELD if their TYPE_MODE can express the type exactly, and > dwarf2out.c (gen_field_die) uses > if (DECL_BIT_FIELD_TYPE (decl)) > to decide if DW_AT_bit_offset etc. attributes should be added. > So maybe I should go with && DECL_BIT_FIELD_TYPE (decl) instead.
You need DECL_BIT_FIELD_TYPE if you want to know whether it is a bitfield. DECL_BIT_FIELD can only be used to test whether the size is not a multiple of BITS_PER_UNIT. So the question is whether the code makes a difference if the bitfield is int a : 8; int b : 8; int c : 16; for example. If so then DECL_BIT_FIELD_TYPE is needed, otherwise what you test probably doesn't matter. > On > struct S { int e; int a : 1, b : 7, c : 8, d : 16; } s; > struct T { int a : 1, b : 7; long long c : 8; int d : 16; } t; > it doesn't make a difference though on x86_64, ppc64le nor ppc64... > > I think Ada has bitfields of aggregate types, so CCing Eric, though > I'd hope it doesn't have bitfields where type size is smaller than > field decl size like C++ has. > > Bootstrapped/regtested on x86_64-linux, i686-linux, powerpc64le-linux > and powerpc64-linux and Pedro has tested it on GDB testsuite. > > I can bootstrap/regtest the > + && DECL_BIT_FIELD_TYPE (decl) > version too. > > 2021-11-10 Jakub Jelinek <ja...@redhat.com> > > PR debug/101378 > * dwarf2out.c (field_byte_offset): Do the PCC_BITFIELD_TYPE_MATTERS > handling only for DECL_BIT_FIELD decls. > > * g++.dg/debug/dwarf2/pr101378.C: New test. > > --- gcc/dwarf2out.c.jj 2021-11-05 10:19:46.339457342 +0100 > +++ gcc/dwarf2out.c 2021-11-09 15:01:51.425437717 +0100 > @@ -19646,6 +19646,7 @@ field_byte_offset (const_tree decl, stru > properly dynamic byte offsets only when PCC bitfield type doesn't > matter. */ > if (PCC_BITFIELD_TYPE_MATTERS > + && DECL_BIT_FIELD (decl) > && TREE_CODE (DECL_FIELD_OFFSET (decl)) == INTEGER_CST) What's more interesting is the INTEGER_CST restriction - I'm sure that Ada allows bitfields to follow variable position other fields. Even C does: void foo (int n) { struct S { int a[n]; int b : 5; int c : 3; } s; } runs into the code above and ends up not honoring PCC_BITFIELD_TYPE_MATTERS ... Richard. > { > offset_int object_offset_in_bits; > --- gcc/testsuite/g++.dg/debug/dwarf2/pr101378.C.jj 2021-11-09 > 15:17:39.504975396 +0100 > +++ gcc/testsuite/g++.dg/debug/dwarf2/pr101378.C 2021-11-09 > 15:17:28.067137556 +0100 > @@ -0,0 +1,13 @@ > +// PR debug/101378 > +// { dg-do compile { target c++11 } } > +// { dg-options "-gdwarf-5 -dA" } > +// { dg-final { scan-assembler-times "0\[^0-9x\\r\\n\]* > DW_AT_data_member_location" 1 } } > +// { dg-final { scan-assembler-times "1\[^0-9x\\r\\n\]* > DW_AT_data_member_location" 1 } } > +// { dg-final { scan-assembler-times "2\[^0-9x\\r\\n\]* > DW_AT_data_member_location" 1 } } > +// { dg-final { scan-assembler-not "-1\[^0-9x\\r\\n\]* > DW_AT_data_member_location" } } > + > +struct E {}; > +struct S > +{ > + [[no_unique_address]] E e, f, g; > +} s; > > Jakub > > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)