Introduce struct packed template
authorPedro Alves <pedro@palves.net>
Tue, 12 Jul 2022 10:20:13 +0000 (12:20 +0200)
committerTom de Vries <tdevries@suse.de>
Tue, 12 Jul 2022 10:20:13 +0000 (12:20 +0200)
commit68c0faca76a9b179af1a098946d6020091f43e06
tree0311cd8a1fa47cd871455b513777964d91c2d8dd
parenta14413ddff3009418711b9f78a7d3dc7ac10dab0
Introduce struct packed template

When building gdb with -fsanitize=thread and gcc 12, and running test-case
gdb.dwarf2/dwz.exp, we run into a few data races.  For example, between:

 ...
   Write of size 1 at 0x7b200000300e by thread T4:
     #0 process_psymtab_comp_unit gdb/dwarf2/read.c:6789 (gdb+0x830720)
 ...

and:

 ...
   Previous read of size 1 at 0x7b200000300e by main thread:
     #0 cutu_reader::cutu_reader(dwarf2_per_cu_data*, dwarf2_per_objfile*, \
     abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) gdb/dwarf2/read.c:6164 \
     (gdb+0x82edab)
 ...

In other words, between:

 ...
       this_cu->unit_type = DW_UT_partial;
 ...

and:

 ...
   if (this_cu->reading_dwo_directly)
 ...

The problem is that the written fields are part of the same memory
location as the read fields, so executing a read and write in
different threads is undefined behavour.

Making the written fields separate memory locations, like this:

...
  struct {
    ENUM_BITFIELD (dwarf_unit_type) unit_type : 8;
  };
...

fixes it, however that also increases the size of struct
dwarf2_per_cu_data, because it introduces padding due to alignment of
these new structs, which align on the natural alignment of the
specified type of their fields.  We can fix that with
__attribute__((packed)), like so:

  struct {
    ENUM_BITFIELD (dwarf_unit_type) unit_type : 8 __attribute__((packed));
  };

but to avoid having to write that in several places and add suitable
comments explaining how that concoction works, introduce a new struct
packed template that wraps/hides this.  Instead of the above, we'll be
able to write:

    packed<dwarf_unit_type, 1> unit_type;

Note that we can't change the type of dwarf_unit_type, as that is
defined in include/, and shared with other projects, some of those
written in C.

This patch just adds the struct packed type.  Following patches will
make use of it.  One of those patches will want to wrap a struct
packed in an std::atomic, like:

  std::atomic<std::packed<language, 1>> m_lang;

so the new gdbsupport/packed.h header adds some operators to make
comparisions between that std::atomic and the type that the wrapped
struct packed wraps work, like in:

   if (m_lang == language_c)

It would be possible to implement struct packed without using
__attribute__((packed)), by having it store an array of bytes of the
appropriate size instead, however that would make it less convenient
to debug GDB.  The way it's implemented, printing a struct packed
variable just prints its field using its natural type, which is
particularly useful if the type is an enum.  I believe that
__attribute__((packed)) is supported by all compilers that are able to
build GDB.  Even a few BFD headers use on ATTRIBUTE_PACKED on external
types:

 include/coff/external.h:  } ATTRIBUTE_PACKED
 include/coff/external.h:} ATTRIBUTE_PACKED ;
 include/coff/external.h:} ATTRIBUTE_PACKED ;
 include/coff/pe.h:} ATTRIBUTE_PACKED ;
 include/coff/pe.h:} ATTRIBUTE_PACKED;
 include/elf/external.h:} ATTRIBUTE_PACKED  Elf_External_Versym;

It is not possible to build GDB with MSVC today, but if it could, that
would be one compiler that doesn't support this attribute.  However,
it supports packing via pragmas, so there's a way to cross that bridge
if we ever get to it.  I believe any compiler worth its salt supports
some way of packing.

In any case, the worse that happens without the attribute is that some
types become larger than ideal.  Regardless, I've added a couple
static assertions to catch such compilers in action:

    /* Ensure size and aligment are what we expect.  */
    gdb_static_assert (sizeof (packed) == Bytes);
    gdb_static_assert (alignof (packed) == 1);

Change-Id: Ifa94f0a2cebfae5e8f6ddc73265f05e7fd9e1532
gdbsupport/packed.h [new file with mode: 0644]