GCC fix: Make UUID POD

This eliminates all constructors, but nearly all points of use were
using the default constructor to initialize a UUID member variable as in
uuid_(). This syntax will still produce a zeroed-out UUID.

While compiling, for example, minidump_rva_list_writer.cc:

In file included from ../../minidump/minidump_rva_list_writer.h:25:0,
                 from ../../minidump/minidump_rva_list_writer.cc:15:
../../minidump/minidump_extensions.h:412:8: error: ignoring packed attribute because of unpacked non-POD field ‘crashpad::UUID crashpad::MinidumpCrashpadInfo::report_id’ [-Werror]
   UUID report_id;
        ^~~~~~~~~
../../minidump/minidump_extensions.h:424:8: error: ignoring packed attribute because of unpacked non-POD field ‘crashpad::UUID crashpad::MinidumpCrashpadInfo::client_id’ [-Werror]
   UUID client_id;
        ^~~~~~~~~

Tested with:
 - GCC 4.9 from NDK r13 targeting arm with SDK 16
 - GCC 4.9 from NDK r13 targeting arm64 with SDK 21
 - GCC 6.2 targeting x86_64

BUG=crashpad:30

Change-Id: Iec6b1557441b69d75246f2f75c59c4158fb7ca29
Reviewed-on: https://chromium-review.googlesource.com/409641
Reviewed-by: Scott Graham <scottmg@chromium.org>
This commit is contained in:
Mark Mentovai 2016-11-10 15:11:20 -05:00
parent 5b14b41992
commit 57b2210ed7
5 changed files with 26 additions and 38 deletions

View File

@ -31,7 +31,8 @@ namespace test {
namespace { namespace {
void TestImageReaderChild(const base::string16& directory_modification) { void TestImageReaderChild(const base::string16& directory_modification) {
UUID done_uuid(UUID::InitializeWithNewTag{}); UUID done_uuid;
done_uuid.InitializeWithNew();
ScopedKernelHANDLE done( ScopedKernelHANDLE done(
CreateEvent(nullptr, true, false, done_uuid.ToString16().c_str())); CreateEvent(nullptr, true, false, done_uuid.ToString16().c_str()));
ASSERT_TRUE(done.get()); ASSERT_TRUE(done.get());

View File

@ -42,23 +42,15 @@ namespace crashpad {
static_assert(sizeof(UUID) == 16, "UUID must be 16 bytes"); static_assert(sizeof(UUID) == 16, "UUID must be 16 bytes");
#if CXX_LIBRARY_VERSION >= 2011 #if CXX_LIBRARY_VERSION >= 2011
static_assert(std::is_standard_layout<UUID>::value, static_assert(std::is_pod<UUID>::value, "UUID must be POD");
"UUID must be standard layout");
#endif #endif
UUID::UUID() : data_1(0), data_2(0), data_3(0), data_4(), data_5() {
}
UUID::UUID(InitializeWithNewTag) {
CHECK(InitializeWithNew());
}
UUID::UUID(const uint8_t* bytes) {
InitializeFromBytes(bytes);
}
bool UUID::operator==(const UUID& that) const { bool UUID::operator==(const UUID& that) const {
return memcmp(this, &that, sizeof(UUID)) == 0; return memcmp(this, &that, sizeof(*this)) == 0;
}
void UUID::InitializeToZero() {
memset(this, 0, sizeof(*this));
} }
void UUID::InitializeFromBytes(const uint8_t* bytes) { void UUID::InitializeFromBytes(const uint8_t* bytes) {
@ -132,7 +124,7 @@ void UUID::InitializeFromSystemUUID(const ::UUID* system_uuid) {
"unexpected system uuid size"); "unexpected system uuid size");
static_assert(offsetof(::UUID, Data1) == offsetof(UUID, data_1), static_assert(offsetof(::UUID, Data1) == offsetof(UUID, data_1),
"unexpected system uuid layout"); "unexpected system uuid layout");
memcpy(this, system_uuid, sizeof(::UUID)); memcpy(this, system_uuid, sizeof(*this));
} }
#endif // OS_WIN #endif // OS_WIN

View File

@ -36,27 +36,14 @@ namespace crashpad {
//! //!
//! A %UUID is a unique 128-bit number specified by RFC 4122. //! A %UUID is a unique 128-bit number specified by RFC 4122.
//! //!
//! This is a standard-layout structure. //! This is a POD structure.
struct UUID { struct UUID {
//! \brief Initializes the %UUID to zero.
UUID();
//! \brief Tag to pass to constructor to indicate it should initialize with
//! generated data.
struct InitializeWithNewTag {};
//! \brief Initializes the %UUID using a standard system facility to generate
//! the value.
//!
//! CHECKs on failure with a message logged.
explicit UUID(InitializeWithNewTag);
//! \copydoc InitializeFromBytes()
explicit UUID(const uint8_t* bytes);
bool operator==(const UUID& that) const; bool operator==(const UUID& that) const;
bool operator!=(const UUID& that) const { return !operator==(that); } bool operator!=(const UUID& that) const { return !operator==(that); }
//! \brief Initializes the %UUID to zero.
void InitializeToZero();
//! \brief Initializes the %UUID from a sequence of bytes. //! \brief Initializes the %UUID from a sequence of bytes.
//! //!
//! \a bytes is taken as a %UUID laid out in big-endian format in memory. On //! \a bytes is taken as a %UUID laid out in big-endian format in memory. On

View File

@ -31,6 +31,7 @@ namespace {
TEST(UUID, UUID) { TEST(UUID, UUID) {
UUID uuid_zero; UUID uuid_zero;
uuid_zero.InitializeToZero();
EXPECT_EQ(0u, uuid_zero.data_1); EXPECT_EQ(0u, uuid_zero.data_1);
EXPECT_EQ(0u, uuid_zero.data_2); EXPECT_EQ(0u, uuid_zero.data_2);
EXPECT_EQ(0u, uuid_zero.data_3); EXPECT_EQ(0u, uuid_zero.data_3);
@ -60,7 +61,8 @@ TEST(UUID, UUID) {
0x0d, 0x0d,
0x0e, 0x0e,
0x0f}; 0x0f};
UUID uuid(kBytes); UUID uuid;
uuid.InitializeFromBytes(kBytes);
EXPECT_EQ(0x00010203u, uuid.data_1); EXPECT_EQ(0x00010203u, uuid.data_1);
EXPECT_EQ(0x0405u, uuid.data_2); EXPECT_EQ(0x0405u, uuid.data_2);
EXPECT_EQ(0x0607u, uuid.data_3); EXPECT_EQ(0x0607u, uuid.data_3);
@ -78,7 +80,8 @@ TEST(UUID, UUID) {
EXPECT_FALSE(uuid == uuid_zero); EXPECT_FALSE(uuid == uuid_zero);
EXPECT_NE(uuid, uuid_zero); EXPECT_NE(uuid, uuid_zero);
UUID uuid_2(kBytes); UUID uuid_2;
uuid_2.InitializeFromBytes(kBytes);
EXPECT_EQ(uuid, uuid_2); EXPECT_EQ(uuid, uuid_2);
EXPECT_FALSE(uuid != uuid_2); EXPECT_FALSE(uuid != uuid_2);
@ -155,7 +158,8 @@ TEST(UUID, UUID) {
EXPECT_EQ(0x45u, uuid.data_5[5]); EXPECT_EQ(0x45u, uuid.data_5[5]);
EXPECT_EQ("45454545-4545-4545-4545-454545454545", uuid.ToString()); EXPECT_EQ("45454545-4545-4545-4545-454545454545", uuid.ToString());
UUID initialized_generated(UUID::InitializeWithNewTag{}); UUID initialized_generated;
initialized_generated.InitializeWithNew();
EXPECT_NE(initialized_generated, uuid_zero); EXPECT_NE(initialized_generated, uuid_zero);
} }
@ -182,7 +186,9 @@ TEST(UUID, FromString) {
{"6d247a34-53d5-40ec-a90d-d8dea9e94cc01", false} {"6d247a34-53d5-40ec-a90d-d8dea9e94cc01", false}
}; };
const std::string empty_uuid = UUID().ToString(); UUID uuid_zero;
uuid_zero.InitializeToZero();
const std::string empty_uuid = uuid_zero.ToString();
for (size_t index = 0; index < arraysize(kCases); ++index) { for (size_t index = 0; index < arraysize(kCases); ++index) {
const TestCase& test_case = kCases[index]; const TestCase& test_case = kCases[index];
@ -190,6 +196,7 @@ TEST(UUID, FromString) {
"index %" PRIuS ": %s", index, test_case.uuid_string)); "index %" PRIuS ": %s", index, test_case.uuid_string));
UUID uuid; UUID uuid;
uuid.InitializeToZero();
EXPECT_EQ(test_case.success, EXPECT_EQ(test_case.success,
uuid.InitializeFromString(test_case.uuid_string)); uuid.InitializeFromString(test_case.uuid_string));
if (test_case.success) { if (test_case.success) {

View File

@ -132,7 +132,8 @@ TEST(ProcessInfo, Self) {
void TestOtherProcess(const base::string16& directory_modification) { void TestOtherProcess(const base::string16& directory_modification) {
ProcessInfo process_info; ProcessInfo process_info;
UUID done_uuid(UUID::InitializeWithNewTag{}); UUID done_uuid;
done_uuid.InitializeWithNew();
ScopedKernelHANDLE done( ScopedKernelHANDLE done(
CreateEvent(nullptr, true, false, done_uuid.ToString16().c_str())); CreateEvent(nullptr, true, false, done_uuid.ToString16().c_str()));