2023-01-01 20:49:04 +08:00
|
|
|
/* Copyright (C) 2022-2023 Free Software Foundation, Inc.
|
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
2022-07-12 18:20:13 +08:00
|
|
|
|
|
|
|
This file is part of GDB.
|
|
|
|
|
|
|
|
This program is free software; you can redistribute it and/or modify
|
|
|
|
it under the terms of the GNU General Public License as published by
|
|
|
|
the Free Software Foundation; either version 3 of the License, or
|
|
|
|
(at your option) any later version.
|
|
|
|
|
|
|
|
This program is distributed in the hope that it will be useful,
|
|
|
|
but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
|
|
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
|
|
GNU General Public License for more details.
|
|
|
|
|
|
|
|
You should have received a copy of the GNU General Public License
|
|
|
|
along with this program. If not, see <http://www.gnu.org/licenses/>. */
|
|
|
|
|
|
|
|
#ifndef PACKED_H
|
|
|
|
#define PACKED_H
|
|
|
|
|
2022-07-12 19:36:57 +08:00
|
|
|
#include "traits.h"
|
2022-07-19 07:26:33 +08:00
|
|
|
#include <atomic>
|
2022-07-12 19:36:57 +08:00
|
|
|
|
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
2022-07-12 18:20:13 +08:00
|
|
|
/* Each instantiation and full specialization of the packed template
|
|
|
|
defines a type that behaves like a given scalar type, but that has
|
|
|
|
byte alignment, and, may optionally have a smaller size than the
|
|
|
|
given scalar type. This is typically used as alternative to
|
|
|
|
bit-fields (and ENUM_BITFIELD), when the fields must have separate
|
|
|
|
memory locations to avoid data races. */
|
|
|
|
|
2022-07-19 07:26:33 +08:00
|
|
|
/* There are two implementations here -- one standard compliant, using
|
|
|
|
a byte array for internal representation, and another that relies
|
|
|
|
on bitfields and attribute packed (and attribute gcc_struct on
|
|
|
|
Windows). The latter is preferable, as it is more convenient when
|
|
|
|
debugging GDB -- printing a struct packed variable prints its field
|
|
|
|
using its natural type, which is particularly useful if the type is
|
|
|
|
an enum -- but may not work on all compilers. */
|
|
|
|
|
|
|
|
/* Clang targeting Windows does not support attribute gcc_struct, so
|
2023-06-04 04:43:57 +08:00
|
|
|
we use the alternative byte array implementation there. */
|
2022-07-19 07:26:33 +08:00
|
|
|
#if defined _WIN32 && defined __clang__
|
|
|
|
# define PACKED_USE_ARRAY 1
|
|
|
|
#else
|
|
|
|
# define PACKED_USE_ARRAY 0
|
|
|
|
#endif
|
|
|
|
|
|
|
|
/* For the preferred implementation, we need gcc_struct on Windows, as
|
|
|
|
otherwise the size of e.g., "packed<int, 1>" will be larger than
|
|
|
|
what we want. Clang targeting Windows does not support attribute
|
|
|
|
gcc_struct. */
|
|
|
|
#if !PACKED_USE_ARRAY && defined _WIN32 && !defined __clang__
|
2022-07-19 07:26:33 +08:00
|
|
|
# define ATTRIBUTE_GCC_STRUCT __attribute__((__gcc_struct__))
|
|
|
|
#else
|
|
|
|
# define ATTRIBUTE_GCC_STRUCT
|
|
|
|
#endif
|
|
|
|
|
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
2022-07-12 18:20:13 +08:00
|
|
|
template<typename T, size_t Bytes = sizeof (T)>
|
2022-07-19 07:26:33 +08:00
|
|
|
struct ATTRIBUTE_GCC_STRUCT packed
|
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
2022-07-12 18:20:13 +08:00
|
|
|
{
|
|
|
|
public:
|
2022-07-14 16:20:16 +08:00
|
|
|
packed () noexcept = default;
|
|
|
|
|
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
2022-07-12 18:20:13 +08:00
|
|
|
packed (T val)
|
|
|
|
{
|
2023-10-19 10:44:11 +08:00
|
|
|
static_assert (sizeof (ULONGEST) >= sizeof (T));
|
2022-07-19 07:26:33 +08:00
|
|
|
|
|
|
|
#if PACKED_USE_ARRAY
|
|
|
|
ULONGEST tmp = val;
|
|
|
|
for (int i = (Bytes - 1); i >= 0; --i)
|
|
|
|
{
|
|
|
|
m_bytes[i] = (gdb_byte) tmp;
|
|
|
|
tmp >>= HOST_CHAR_BIT;
|
|
|
|
}
|
|
|
|
#else
|
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
2022-07-12 18:20:13 +08:00
|
|
|
m_val = val;
|
2022-07-19 07:26:33 +08:00
|
|
|
#endif
|
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
2022-07-12 18:20:13 +08:00
|
|
|
|
|
|
|
/* Ensure size and aligment are what we expect. */
|
2023-10-19 10:44:11 +08:00
|
|
|
static_assert (sizeof (packed) == Bytes);
|
|
|
|
static_assert (alignof (packed) == 1);
|
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
2022-07-12 18:20:13 +08:00
|
|
|
|
|
|
|
/* Make sure packed can be wrapped with std::atomic. */
|
2022-07-12 19:36:57 +08:00
|
|
|
#if HAVE_IS_TRIVIALLY_COPYABLE
|
2023-10-19 10:44:11 +08:00
|
|
|
static_assert (std::is_trivially_copyable<packed>::value);
|
2022-07-12 19:36:57 +08:00
|
|
|
#endif
|
2023-10-19 10:44:11 +08:00
|
|
|
static_assert (std::is_copy_constructible<packed>::value);
|
|
|
|
static_assert (std::is_move_constructible<packed>::value);
|
|
|
|
static_assert (std::is_copy_assignable<packed>::value);
|
|
|
|
static_assert (std::is_move_assignable<packed>::value);
|
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
2022-07-12 18:20:13 +08:00
|
|
|
}
|
|
|
|
|
|
|
|
operator T () const noexcept
|
|
|
|
{
|
2022-07-19 07:26:33 +08:00
|
|
|
#if PACKED_USE_ARRAY
|
|
|
|
ULONGEST tmp = 0;
|
|
|
|
for (int i = 0;;)
|
|
|
|
{
|
|
|
|
tmp |= m_bytes[i];
|
|
|
|
if (++i == Bytes)
|
|
|
|
break;
|
|
|
|
tmp <<= HOST_CHAR_BIT;
|
|
|
|
}
|
|
|
|
return (T) tmp;
|
|
|
|
#else
|
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
2022-07-12 18:20:13 +08:00
|
|
|
return m_val;
|
2022-07-19 07:26:33 +08:00
|
|
|
#endif
|
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
2022-07-12 18:20:13 +08:00
|
|
|
}
|
|
|
|
|
|
|
|
private:
|
2022-07-19 07:26:33 +08:00
|
|
|
#if PACKED_USE_ARRAY
|
|
|
|
gdb_byte m_bytes[Bytes];
|
|
|
|
#else
|
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
2022-07-12 18:20:13 +08:00
|
|
|
T m_val : (Bytes * HOST_CHAR_BIT) ATTRIBUTE_PACKED;
|
2022-07-19 07:26:33 +08:00
|
|
|
#endif
|
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
2022-07-12 18:20:13 +08:00
|
|
|
};
|
|
|
|
|
2022-07-19 07:26:33 +08:00
|
|
|
/* Add some comparisons between std::atomic<packed<T>> and packed<T>
|
|
|
|
and T. We need this because even though std::atomic<T> doesn't
|
|
|
|
define these operators, the relational expressions still work via
|
|
|
|
implicit conversions. Those wouldn't work when wrapped in packed
|
|
|
|
without these operators, because they'd require two implicit
|
|
|
|
conversions to go from T to packed<T> to std::atomic<packed<T>>
|
|
|
|
(and back), and C++ only does one. */
|
|
|
|
|
|
|
|
#define PACKED_ATOMIC_OP(OP) \
|
|
|
|
template<typename T, size_t Bytes> \
|
|
|
|
bool operator OP (const std::atomic<packed<T, Bytes>> &lhs, \
|
|
|
|
const std::atomic<packed<T, Bytes>> &rhs) \
|
|
|
|
{ \
|
|
|
|
return lhs.load () OP rhs.load (); \
|
|
|
|
} \
|
|
|
|
\
|
|
|
|
template<typename T, size_t Bytes> \
|
|
|
|
bool operator OP (T lhs, const std::atomic<packed<T, Bytes>> &rhs) \
|
|
|
|
{ \
|
|
|
|
return lhs OP rhs.load (); \
|
|
|
|
} \
|
|
|
|
\
|
|
|
|
template<typename T, size_t Bytes> \
|
|
|
|
bool operator OP (const std::atomic<packed<T, Bytes>> &lhs, T rhs) \
|
|
|
|
{ \
|
|
|
|
return lhs.load () OP rhs; \
|
|
|
|
} \
|
|
|
|
\
|
|
|
|
template<typename T, size_t Bytes> \
|
|
|
|
bool operator OP (const std::atomic<packed<T, Bytes>> &lhs, \
|
|
|
|
packed<T, Bytes> rhs) \
|
|
|
|
{ \
|
|
|
|
return lhs.load () OP rhs; \
|
|
|
|
} \
|
|
|
|
\
|
|
|
|
template<typename T, size_t Bytes> \
|
|
|
|
bool operator OP (packed<T, Bytes> lhs, \
|
|
|
|
const std::atomic<packed<T, Bytes>> &rhs) \
|
|
|
|
{ \
|
|
|
|
return lhs OP rhs.load (); \
|
|
|
|
}
|
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
2022-07-12 18:20:13 +08:00
|
|
|
|
2022-07-19 07:26:33 +08:00
|
|
|
PACKED_ATOMIC_OP (==)
|
|
|
|
PACKED_ATOMIC_OP (!=)
|
|
|
|
PACKED_ATOMIC_OP (>)
|
|
|
|
PACKED_ATOMIC_OP (<)
|
|
|
|
PACKED_ATOMIC_OP (>=)
|
|
|
|
PACKED_ATOMIC_OP (<=)
|
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
2022-07-12 18:20:13 +08:00
|
|
|
|
2022-07-19 07:26:33 +08:00
|
|
|
#undef PACKED_ATOMIC_OP
|
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
2022-07-12 18:20:13 +08:00
|
|
|
|
|
|
|
#endif
|