[PATCH] D135583: [LLVM] Use DWARFv4 bitfields when tuning for GDB

[David Spickett via Phabricator via llvm-commits]

DavidSpickett added a comment.

> What's the difference in representation?

Test case:

  struct fields
  {
    unsigned a : 4;
    unsigned b : 4;
  } flags;
  
  int main() {
    flags.a = 1;
    flags.b = 2;
    return flags.a + flags.b;
  }

When tuning for GDB we emit `DW_AT_bit_offset` and `DW_AT_data_member_location` for each field:

  $ ./bin/clang /tmp/test.c -o /tmp/test.o -ggdb -gdwarf-4 && ./bin/llvm-dwarfdump /tmp/test.o
  0x00000047:     DW_TAG_member
                    DW_AT_name    ("a")
                    DW_AT_type    (0x00000066 "unsigned int")
                    DW_AT_decl_file       ("/tmp/test.c")
                    DW_AT_decl_line       (3)
                    DW_AT_byte_size       (0x04)
                    DW_AT_bit_size        (0x04)
                    DW_AT_bit_offset      (0x1c)
                    DW_AT_data_member_location    (0x00)
  
  0x00000056:     DW_TAG_member
                    DW_AT_name    ("b")
                    DW_AT_type    (0x00000066 "unsigned int")
                    DW_AT_decl_file       ("/tmp/test.c")
                    DW_AT_decl_line       (4)
                    DW_AT_byte_size       (0x04)
                    DW_AT_bit_size        (0x04)
                    DW_AT_bit_offset      (0x18)
                    DW_AT_data_member_location    (0x00)

When tuning for lldb we emit only `DW_AT_data_bit_offset`:

  $ ./bin/clang /tmp/test.c -o /tmp/test.o -glldb -gdwarf-4 && ./bin/llvm-dwarfdump /tmp/test.o
  0x0000004b:     DW_TAG_member
                    DW_AT_name    ("a")
                    DW_AT_type    (0x00000066 "unsigned int")
                    DW_AT_decl_file       ("/tmp/test.c")
                    DW_AT_decl_line       (3)
                    DW_AT_bit_size        (0x04)
                    DW_AT_data_bit_offset (0x00)
  
  0x00000058:     DW_TAG_member
                    DW_AT_name    ("b")
                    DW_AT_type    (0x00000066 "unsigned int")
                    DW_AT_decl_file       ("/tmp/test.c")
                    DW_AT_decl_line       (4)
                    DW_AT_bit_size        (0x04)
                    DW_AT_data_bit_offset (0x04)

As I understand it, `DW_AT_bit_offset` has to be emitted in a different ordering depending on endian (well, you offset from the other end of the storage). See https://github.com/llvm/llvm-project/blob/0577a9f8d06b97bf8b4d9aa171096e0797e4f5d1/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp#L1649. This has to be done by the producer of the DWARF.

`DW_AT_data_bit_offset` is meant to be endian agnostic and the tool interpreting the dwarf applies what it knows to be the endian to work out where the bit offset starts from (so in the link above, we just emit it without checking for any swapping).

>From the DWARF 4 spec:

  For a DW_AT_data_bit_offset attribute, the value is an integer constant (see Section 2.19) that
  specifies the number of bits from the beginning of the containing entity to the beginning of the
  data member. This value must be greater than or equal to zero, but is not limited to less than the
  number of bits per byte. 

  For big-endian architectures, bit offsets are counted from high-order to low-order bits
  within a byte (or larger storage unit); in this case, the bit offset identifies the high-order
  bit of the object.
  
  For little-endian architectures, bit offsets are counted from low-order to high-order bits
  within a byte (or larger storage unit); in this case, the bit offset identifies the low-order
  bit of the object. 

I'm not really arguing that one is inherently better, I just figured I'd make the code up to date with what gdb has been doing (I've been digging into bitfields for other reasons).

Alternatively I can just update the original comment with "GDB >= <version> does support DW_AT_data_bit_offset but we choose not to emit it to maintain compatibility.".

Once I can cite a specific GDB release, let's see how new it is and go from there?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135583/new/

https://reviews.llvm.org/D135583