On Wed, Nov 10, 2021 at 10:52:42AM +0100, Richard Biener wrote:
> > 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.
Yeah, so I think DECL_BIT_FIELD_TYPE is the right test and I'll retest with
that.
> 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.
Apparently I made a mistake of trying those tests with -g -dA.
The problem is that for bitfields we then emit DW_AT_data_bit_offset
(which is already in DWARF4, but we chose to emit it only for DWARF5
as many consumers didn't handle it).
So, 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;
int
main ()
{
s.c = 0x55;
s.d = 0xaaaa;
t.c = 0x55;
t.d = 0xaaaa;
s.e++;
}
the difference with the patch as posted and -gdwarf-4 -dA is:
.uleb128 0x4 # (DIE (0x5f) DW_TAG_member)
.ascii "c\0" # DW_AT_name
.byte 0x1 # DW_AT_decl_file (hoho.C)
.byte 0x1 # DW_AT_decl_line
.byte 0x25 # DW_AT_decl_column
.long 0x7c # DW_AT_type
.byte 0x4 # DW_AT_byte_size
.byte 0x8 # DW_AT_bit_size
- .byte 0x10 # DW_AT_bit_offset
- .byte 0x4 # DW_AT_data_member_location
+ .byte 0x18 # DW_AT_bit_offset
+ .byte 0x5 # DW_AT_data_member_location
.uleb128 0x4 # (DIE (0x6d) DW_TAG_member)
.ascii "d\0" # DW_AT_name
.byte 0x1 # DW_AT_decl_file (hoho.C)
.byte 0x1 # DW_AT_decl_line
.byte 0x2c # DW_AT_decl_column
.long 0x7c # DW_AT_type
.byte 0x4 # DW_AT_byte_size
.byte 0x10 # DW_AT_bit_size
- .byte 0 # DW_AT_bit_offset
- .byte 0x4 # DW_AT_data_member_location
+ .byte 0x10 # DW_AT_bit_offset
+ .byte 0x6 # DW_AT_data_member_location
.byte 0 # end of children of DIE 0x2d
and
.uleb128 0x4 # (DIE (0xbe) DW_TAG_member)
.ascii "c\0" # DW_AT_name
.byte 0x1 # DW_AT_decl_file (hoho.C)
.byte 0x2 # DW_AT_decl_line
.byte 0x28 # DW_AT_decl_column
.long 0xdb # DW_AT_type
.byte 0x8 # DW_AT_byte_size
.byte 0x8 # DW_AT_bit_size
- .byte 0x30 # DW_AT_bit_offset
- .byte 0 # DW_AT_data_member_location
+ .byte 0x38 # DW_AT_bit_offset
+ .byte 0x1 # DW_AT_data_member_location
.uleb128 0x4 # (DIE (0xcc) DW_TAG_member)
.ascii "d\0" # DW_AT_name
.byte 0x1 # DW_AT_decl_file (hoho.C)
.byte 0x2 # DW_AT_decl_line
.byte 0x33 # DW_AT_decl_column
.long 0x7c # DW_AT_type
.byte 0x4 # DW_AT_byte_size
.byte 0x10 # DW_AT_bit_size
- .byte 0 # DW_AT_bit_offset
- .byte 0 # DW_AT_data_member_location
+ .byte 0x10 # DW_AT_bit_offset
+ .byte 0x2 # DW_AT_data_member_location
.byte 0 # end of children of DIE 0x97
but GDB handles both fine.
> > 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 ...
True. And, apparently we've been mishandling this forever.
The function actually doesn't and has never had any way to signal to caller
that it gave up and was unable to handle it correctly, before the addition
of dw_loc_descr_ref return the function was just returning HOST_WIDE_INT
and returned 0 both in cases where the field offset was really 0 and where
it gave up (e.g. because bit_position wasn't INTEGER_CST representable
in hwi).
I'm afraid I forgot the DECL_FIELD_OFFSET vs. DECL_FIELD_BIT_OFFSET stuff
enough that I'm not sure what is the right fix for that case, maybe it would
work if we dropped the && TREE_CODE (DECL_FIELD_OFFSET (decl)) == INTEGER_CST
check and instead used:
- bitpos_int = wi::to_offset (DECL_FIELD_BIT_OFFSET (decl));
+ if (TREE_CODE (DECL_FIELD_OFFSET (decl)) == INTEGER_CST)
+ bitpos_int = wi::to_offset (bit_position (decl));
+ else
+ bitpos_int = wi::to_offset (DECL_FIELD_BIT_OFFSET (decl));
at the start and
- if (ctx->variant_part_offset == NULL_TREE)
+ if (ctx->variant_part_offset == NULL_TREE
+ && TREE_CODE (DECL_FIELD_OFFSET (decl)) == INTEGER_CST)
{
*cst_offset = object_offset_in_bytes.to_shwi ();
return NULL;
}
tree_result = wide_int_to_tree (sizetype, object_offset_in_bytes);
+ if (TREE_CODE (DECL_FIELD_OFFSET (decl)) == INTEGER_CST)
+ tree_result = size_binop (PLUS_EXPR, DECL_FIELD_OFFSET (decl),
+ tree_result);
or so.
But, is it ok to defer that to stage3 and incremental patch?
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_TYPE 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_TYPE (decl)
&& TREE_CODE (DECL_FIELD_OFFSET (decl)) == INTEGER_CST)
{
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
