[PATCH] dwarf2out: Fix up field_byte_offset [PR101378]

Jakub Jelinek jakub@redhat.com
Wed Nov 10 11:03:53 GMT 2021


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  <jakub@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



More information about the Gcc-patches mailing list