Index: include/lldb/Utility/UUID.h =================================================================== --- include/lldb/Utility/UUID.h +++ include/lldb/Utility/UUID.h @@ -33,15 +33,13 @@ //------------------------------------------------------------------ // Constructors and Destructors //------------------------------------------------------------------ - UUID(); - UUID(const UUID &rhs); - UUID(const void *uuid_bytes, uint32_t num_uuid_bytes); - - ~UUID(); - - const UUID &operator=(const UUID &rhs); + UUID() = default; + UUID(const void *uuid_bytes, uint32_t num_uuid_bytes, bool accept_zeroes); + UUID(llvm::ArrayRef bytes, bool accept_zeroes) { + SetBytes(bytes, accept_zeroes); + } - void Clear(); + void Clear() { m_num_uuid_bytes = 0; } void Dump(Stream *s) const; @@ -49,9 +47,12 @@ return {m_uuid, m_num_uuid_bytes}; } - bool IsValid() const; + explicit operator bool() const { return IsValid(); } + bool IsValid() const { return m_num_uuid_bytes > 0; } - bool SetBytes(const void *uuid_bytes, uint32_t num_uuid_bytes = 16); + void SetBytes(const void *uuid_bytes, uint32_t num_uuid_bytes, + bool accept_zeroes); + void SetBytes(llvm::ArrayRef bytes, bool accept_zeroes); std::string GetAsString(const char *separator = nullptr) const; @@ -85,7 +86,7 @@ //------------------------------------------------------------------ // Classes that inherit from UUID can see and modify these //------------------------------------------------------------------ - uint32_t m_num_uuid_bytes; // Should be 16 or 20 + uint32_t m_num_uuid_bytes = 0; // Should be 0, 16 or 20 ValueType m_uuid; }; Index: source/API/SBModuleSpec.cpp =================================================================== --- source/API/SBModuleSpec.cpp +++ source/API/SBModuleSpec.cpp @@ -90,7 +90,8 @@ } bool SBModuleSpec::SetUUIDBytes(const uint8_t *uuid, size_t uuid_len) { - return m_opaque_ap->GetUUID().SetBytes(uuid, uuid_len); + m_opaque_ap->GetUUID().SetBytes(uuid, uuid_len, true); + return m_opaque_ap->GetUUID().IsValid(); } bool SBModuleSpec::GetDescription(lldb::SBStream &description) { Index: source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp =================================================================== --- source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp +++ source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp @@ -1368,7 +1368,7 @@ if (name_data == NULL) break; image_infos[i].SetName((const char *)name_data); - UUID uuid(extractor.GetData(&offset, 16), 16); + UUID uuid(extractor.GetData(&offset, 16), 16, false); image_infos[i].SetUUID(uuid); image_infos[i].SetLoadAddress(extractor.GetU64(&offset)); image_infos[i].SetSize(extractor.GetU64(&offset)); Index: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp =================================================================== --- source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp +++ source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp @@ -898,7 +898,7 @@ break; case llvm::MachO::LC_UUID: - dylib_info.uuid.SetBytes(data.GetData(&offset, 16)); + dylib_info.uuid.SetBytes(data.GetData(&offset, 16), 16, false); break; default: @@ -1110,7 +1110,7 @@ uuid_t shared_cache_uuid; if (m_process->ReadMemory(sharedCacheUUID_address, shared_cache_uuid, sizeof(uuid_t), err) == sizeof(uuid_t)) { - uuid.SetBytes(shared_cache_uuid); + uuid.SetBytes(shared_cache_uuid, 16, false); if (uuid.IsValid()) { using_shared_cache = eLazyBoolYes; } Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp =================================================================== --- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -733,13 +733,13 @@ if (gnu_debuglink_crc) { // Use 4 bytes of crc from the .gnu_debuglink section. uint32_t uuidt[4] = {gnu_debuglink_crc, 0, 0, 0}; - uuid.SetBytes(uuidt, sizeof(uuidt)); + uuid.SetBytes(uuidt, sizeof(uuidt), true); } else if (core_notes_crc) { // Use 8 bytes - first 4 bytes for *magic* prefix, mainly to make // it look different form .gnu_debuglink crc followed by 4 bytes // of note segments crc. uint32_t uuidt[4] = {g_core_uuid_magic, core_notes_crc, 0, 0}; - uuid.SetBytes(uuidt, sizeof(uuidt)); + uuid.SetBytes(uuidt, sizeof(uuidt), true); } } @@ -926,7 +926,7 @@ // different form .gnu_debuglink crc - followed by 4 bytes of note // segments crc. uint32_t uuidt[4] = {g_core_uuid_magic, core_notes_crc, 0, 0}; - m_uuid.SetBytes(uuidt, sizeof(uuidt)); + m_uuid.SetBytes(uuidt, sizeof(uuidt), true); } } else { if (!m_gnu_debuglink_crc) @@ -935,7 +935,7 @@ if (m_gnu_debuglink_crc) { // Use 4 bytes of crc from the .gnu_debuglink section. uint32_t uuidt[4] = {m_gnu_debuglink_crc, 0, 0, 0}; - m_uuid.SetBytes(uuidt, sizeof(uuidt)); + m_uuid.SetBytes(uuidt, sizeof(uuidt), true); } } Index: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp =================================================================== --- source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -2087,10 +2087,8 @@ version_str[6] = '\0'; if (strcmp(version_str, "dyld_v") == 0) { offset = offsetof(struct lldb_copy_dyld_cache_header_v1, uuid); - uint8_t uuid_bytes[sizeof(uuid_t)]; - memcpy(uuid_bytes, dsc_header_data.GetData(&offset, sizeof(uuid_t)), - sizeof(uuid_t)); - dsc_uuid.SetBytes(uuid_bytes); + dsc_uuid.SetBytes(dsc_header_data.GetData(&offset, sizeof(uuid_t)), + sizeof(uuid_t), false); } Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_SYMBOLS)); if (log && dsc_uuid.IsValid()) { @@ -4861,7 +4859,7 @@ if (!memcmp(uuid_bytes, opencl_uuid, 16)) return false; - uuid.SetBytes(uuid_bytes); + uuid.SetBytes(uuid_bytes, 16, false); return true; } return false; @@ -5392,12 +5390,11 @@ uuid_t raw_uuid; memset (raw_uuid, 0, sizeof (uuid_t)); - if (m_data.GetU32 (&offset, &type, 1) - && m_data.GetU64 (&offset, &address, 1) - && m_data.CopyData (offset, sizeof (uuid_t), raw_uuid) != 0 - && uuid.SetBytes (raw_uuid, sizeof (uuid_t))) - { - return true; + if (m_data.GetU32(&offset, &type, 1) && + m_data.GetU64(&offset, &address, 1) && + m_data.CopyData(offset, sizeof(uuid_t), raw_uuid) != 0) { + uuid.SetBytes(raw_uuid, sizeof(uuid_t), false); + return true; } } } Index: source/Plugins/Process/minidump/MinidumpParser.cpp =================================================================== --- source/Plugins/Process/minidump/MinidumpParser.cpp +++ source/Plugins/Process/minidump/MinidumpParser.cpp @@ -114,18 +114,9 @@ const CvRecordPdb70 *pdb70_uuid = nullptr; Status error = consumeObject(cv_record, pdb70_uuid); if (!error.Fail()) - return UUID(pdb70_uuid, sizeof(*pdb70_uuid)); - } else if (cv_signature == CvSignature::ElfBuildId) { - // ELF BuildID (found in Breakpad/Crashpad generated minidumps) - // - // This is variable-length, but usually 20 bytes - // as the binutils ld default is a SHA-1 hash. - // (We'll handle only 16 and 20 bytes signatures, - // matching LLDB support for UUIDs) - // - if (cv_record.size() == 16 || cv_record.size() == 20) - return UUID(cv_record.data(), cv_record.size()); - } + return UUID(pdb70_uuid, sizeof(*pdb70_uuid), true); + } else if (cv_signature == CvSignature::ElfBuildId) + return UUID(cv_record, true); return UUID(); } Index: source/Utility/DataExtractor.cpp =================================================================== --- source/Utility/DataExtractor.cpp +++ source/Utility/DataExtractor.cpp @@ -1100,11 +1100,9 @@ //---------------------------------------------------------------------- void DataExtractor::DumpUUID(Stream *s, offset_t offset) const { if (s) { - const uint8_t *uuid_data = PeekData(offset, 16); - if (uuid_data) { - lldb_private::UUID uuid(uuid_data, 16); + if (UUID uuid{PeekData(offset, 16), 16, true}) uuid.Dump(s); - } else { + else { s->Printf("", offset); } Index: source/Utility/UUID.cpp =================================================================== --- source/Utility/UUID.cpp +++ source/Utility/UUID.cpp @@ -21,29 +21,8 @@ using namespace lldb_private; -UUID::UUID() { Clear(); } - -UUID::UUID(const UUID &rhs) { - SetBytes(rhs.m_uuid, rhs.m_num_uuid_bytes); -} - -UUID::UUID(const void *uuid_bytes, uint32_t num_uuid_bytes) { - SetBytes(uuid_bytes, num_uuid_bytes); -} - -const UUID &UUID::operator=(const UUID &rhs) { - if (this != &rhs) { - m_num_uuid_bytes = rhs.m_num_uuid_bytes; - ::memcpy(m_uuid, rhs.m_uuid, sizeof(m_uuid)); - } - return *this; -} - -UUID::~UUID() {} - -void UUID::Clear() { - m_num_uuid_bytes = 16; - ::memset(m_uuid, 0, sizeof(m_uuid)); +UUID::UUID(const void *uuid_bytes, uint32_t num_uuid_bytes, bool accept_zeroes) { + SetBytes(uuid_bytes, num_uuid_bytes, accept_zeroes); } std::string UUID::GetAsString(const char *separator) const { @@ -74,36 +53,20 @@ s->PutCString(GetAsString().c_str()); } -bool UUID::SetBytes(const void *uuid_bytes, uint32_t num_uuid_bytes) { - if (uuid_bytes) { - switch (num_uuid_bytes) { - case 20: - m_num_uuid_bytes = 20; - break; - case 16: - m_num_uuid_bytes = 16; - m_uuid[16] = m_uuid[17] = m_uuid[18] = m_uuid[19] = 0; - break; - default: - // Unsupported UUID byte size - m_num_uuid_bytes = 0; - break; - } - - if (m_num_uuid_bytes > 0) { - ::memcpy(m_uuid, uuid_bytes, m_num_uuid_bytes); - return true; - } - } - ::memset(m_uuid, 0, sizeof(m_uuid)); - return false; +void UUID::SetBytes(const void *uuid_bytes, uint32_t num_uuid_bytes, bool accept_zeroes) { + if (uuid_bytes) + SetBytes({reinterpret_cast(uuid_bytes), num_uuid_bytes}, + accept_zeroes); } -bool UUID::IsValid() const { - return m_uuid[0] || m_uuid[1] || m_uuid[2] || m_uuid[3] || m_uuid[4] || - m_uuid[5] || m_uuid[6] || m_uuid[7] || m_uuid[8] || m_uuid[9] || - m_uuid[10] || m_uuid[11] || m_uuid[12] || m_uuid[13] || m_uuid[14] || - m_uuid[15] || m_uuid[16] || m_uuid[17] || m_uuid[18] || m_uuid[19]; +void UUID::SetBytes(llvm::ArrayRef bytes, bool accept_zeroes) { + if (bytes.size() != 20 && bytes.size() != 16) + bytes = {}; + if (!accept_zeroes && llvm::all_of(bytes, [](uint8_t b) { return b == 0; })) + bytes = {}; + + m_num_uuid_bytes = bytes.size(); + std::memcpy(m_uuid, bytes.data(), bytes.size()); } static inline int xdigit_to_int(char ch) { @@ -155,14 +118,15 @@ // Skip leading whitespace characters p = p.ltrim(); + ValueType bytes; uint32_t bytes_decoded = 0; llvm::StringRef rest = - UUID::DecodeUUIDBytesFromString(p, m_uuid, bytes_decoded, num_uuid_bytes); + UUID::DecodeUUIDBytesFromString(p, bytes, bytes_decoded, num_uuid_bytes); // If we successfully decoded a UUID, return the amount of characters that // were consumed if (bytes_decoded == num_uuid_bytes) { - m_num_uuid_bytes = num_uuid_bytes; + SetBytes(bytes, bytes_decoded, true); return str.size() - rest.size(); } Index: unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp =================================================================== --- unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp +++ unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp @@ -194,7 +194,7 @@ ASSERT_EQ(1u, result->size()); EXPECT_EQ("/foo/bar.so", result.getValue()[0].GetFileSpec().GetPath()); EXPECT_EQ(triple, result.getValue()[0].GetArchitecture().GetTriple()); - EXPECT_EQ(UUID("@ABCDEFGHIJKLMNO", 16), result.getValue()[0].GetUUID()); + EXPECT_EQ(UUID("@ABCDEFGHIJKLMNO", 16, true), result.getValue()[0].GetUUID()); EXPECT_EQ(0u, result.getValue()[0].GetObjectOffset()); EXPECT_EQ(1234u, result.getValue()[0].GetObjectSize()); } @@ -218,7 +218,8 @@ ASSERT_EQ(1u, result->size()); EXPECT_EQ("/foo/bar.so", result.getValue()[0].GetFileSpec().GetPath()); EXPECT_EQ(triple, result.getValue()[0].GetArchitecture().GetTriple()); - EXPECT_EQ(UUID("@ABCDEFGHIJKLMNOPQRS", 20), result.getValue()[0].GetUUID()); + EXPECT_EQ(UUID("@ABCDEFGHIJKLMNOPQRS", 20, true), + result.getValue()[0].GetUUID()); EXPECT_EQ(0u, result.getValue()[0].GetObjectOffset()); EXPECT_EQ(1234u, result.getValue()[0].GetObjectSize()); } Index: unittests/Utility/UUIDTest.cpp =================================================================== --- unittests/Utility/UUIDTest.cpp +++ unittests/Utility/UUIDTest.cpp @@ -15,10 +15,10 @@ TEST(UUIDTest, RelationalOperators) { UUID empty; - UUID a16("1234567890123456", 16); - UUID b16("1234567890123457", 16); - UUID a20("12345678901234567890", 20); - UUID b20("12345678900987654321", 20); + UUID a16("1234567890123456", 16, true); + UUID b16("1234567890123457", 16, true); + UUID a20("12345678901234567890", 20, true); + UUID b20("12345678900987654321", 20, true); EXPECT_EQ(empty, empty); EXPECT_EQ(a16, a16); @@ -34,3 +34,40 @@ EXPECT_LT(a16, b16); EXPECT_GT(a20, b20); } + +TEST(UUIDTest, Validity) { + UUID empty; + std::vector zeroes(20, 0); + UUID a16(zeroes.data(), 16, true); + UUID a20(zeroes.data(), 20, true); + UUID a16_0(zeroes.data(), 16, false); + UUID a20_0(zeroes.data(), 20, false); + EXPECT_FALSE(empty); + EXPECT_TRUE(a16); + EXPECT_TRUE(a20); + EXPECT_FALSE(a16_0); + EXPECT_FALSE(a20_0); +} + +TEST(UUIDTest, SetFromStringRef) { + UUID u; + EXPECT_EQ(32, u.SetFromStringRef("404142434445464748494a4b4c4d4e4f")); + EXPECT_EQ(UUID("@ABCDEFGHIJKLMNO", 16, true), u); + + EXPECT_EQ(36, u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f")); + EXPECT_EQ(UUID("@ABCDEFGHIJKLMNO", 16, true), u); + + EXPECT_EQ(45, u.SetFromStringRef( + "40-41-42-43-4445464748494a4b4c4d4e4f-50515253", 20)); + EXPECT_EQ(UUID("@ABCDEFGHIJKLMNOPQRS", 20, true), u); + + EXPECT_EQ(0, u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f", 20)); + EXPECT_EQ(0, u.SetFromStringRef("40xxxxx")); + EXPECT_EQ(0, u.SetFromStringRef("")); + EXPECT_EQ(UUID("@ABCDEFGHIJKLMNOPQRS", 20, true), u) + << "uuid was changed by failed parse calls"; + + EXPECT_EQ( + 32, u.SetFromStringRef("404142434445464748494a4b4c4d4e4f-50515253", 16)); + EXPECT_EQ(UUID("@ABCDEFGHIJKLMNO", 16, true), u); +}