This revision was automatically updated to reflect the committed changes.
Closed by commit rL335612: Represent invalid UUIDs as UUIDs with length zero
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D48479?vs=152699&id=152893#toc
Repository:
rL LLVM
https://reviews.llvm.org/D48479
Files:
lldb/trunk/include/lldb/Utility/DataExtractor.h
lldb/trunk/include/lldb/Utility/UUID.h
lldb/trunk/source/API/SBModuleSpec.cpp
lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/source/Utility/DataExtractor.cpp
lldb/trunk/source/Utility/UUID.cpp
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
lldb/trunk/unittests/Utility/UUIDTest.cpp
Index: lldb/trunk/unittests/Utility/UUIDTest.cpp
===================================================================
--- lldb/trunk/unittests/Utility/UUIDTest.cpp
+++ lldb/trunk/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 = UUID::fromData("1234567890123456", 16);
+ UUID b16 = UUID::fromData("1234567890123457", 16);
+ UUID a20 = UUID::fromData("12345678901234567890", 20);
+ UUID b20 = UUID::fromData("12345678900987654321", 20);
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<uint8_t> zeroes(20, 0);
+ UUID a16 = UUID::fromData(zeroes.data(), 16);
+ UUID a20 = UUID::fromData(zeroes.data(), 20);
+ UUID a16_0 = UUID::fromOptionalData(zeroes.data(), 16);
+ UUID a20_0 = UUID::fromOptionalData(zeroes.data(), 20);
+ EXPECT_FALSE(empty);
+ EXPECT_TRUE(a16);
+ EXPECT_TRUE(a20);
+ EXPECT_FALSE(a16_0);
+ EXPECT_FALSE(a20_0);
+}
+
+TEST(UUIDTest, SetFromStringRef) {
+ UUID u;
+ EXPECT_EQ(32u, u.SetFromStringRef("404142434445464748494a4b4c4d4e4f"));
+ EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16), u);
+
+ EXPECT_EQ(36u, u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f"));
+ EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16), u);
+
+ EXPECT_EQ(45u, u.SetFromStringRef(
+ "40-41-42-43-4445464748494a4b4c4d4e4f-50515253", 20));
+ EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNOPQRS", 20), u);
+
+ EXPECT_EQ(0u, u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f", 20));
+ EXPECT_EQ(0u, u.SetFromStringRef("40xxxxx"));
+ EXPECT_EQ(0u, u.SetFromStringRef(""));
+ EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNOPQRS", 20), u)
+ << "uuid was changed by failed parse calls";
+
+ EXPECT_EQ(
+ 32u, u.SetFromStringRef("404142434445464748494a4b4c4d4e4f-50515253", 16));
+ EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16), u);
+}
Index: lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===================================================================
--- lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -194,7 +194,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("@ABCDEFGHIJKLMNO", 16), result.getValue()[0].GetUUID());
+ EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16),
+ result.getValue()[0].GetUUID());
EXPECT_EQ(0u, result.getValue()[0].GetObjectOffset());
EXPECT_EQ(1234u, result.getValue()[0].GetObjectSize());
}
@@ -218,7 +219,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::fromData("@ABCDEFGHIJKLMNOPQRS", 20),
+ result.getValue()[0].GetUUID());
EXPECT_EQ(0u, result.getValue()[0].GetObjectOffset());
EXPECT_EQ(1234u, result.getValue()[0].GetObjectSize());
}
Index: lldb/trunk/source/Utility/UUID.cpp
===================================================================
--- lldb/trunk/source/Utility/UUID.cpp
+++ lldb/trunk/source/Utility/UUID.cpp
@@ -21,29 +21,12 @@
using namespace lldb_private;
-UUID::UUID() { Clear(); }
+UUID::UUID(llvm::ArrayRef<uint8_t> bytes) {
+ if (bytes.size() != 20 && bytes.size() != 16)
+ bytes = {};
-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));
+ m_num_uuid_bytes = bytes.size();
+ std::memcpy(m_uuid, bytes.data(), bytes.size());
}
std::string UUID::GetAsString(const char *separator) const {
@@ -74,38 +57,6 @@
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;
-}
-
-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];
-}
-
static inline int xdigit_to_int(char ch) {
ch = tolower(ch);
if (ch >= 'a' && ch <= 'f')
@@ -155,14 +106,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;
+ *this = fromData(bytes, bytes_decoded);
return str.size() - rest.size();
}
Index: lldb/trunk/source/Utility/DataExtractor.cpp
===================================================================
--- lldb/trunk/source/Utility/DataExtractor.cpp
+++ lldb/trunk/source/Utility/DataExtractor.cpp
@@ -1093,24 +1093,6 @@
return offset; // Return the offset at which we ended up
}
-//----------------------------------------------------------------------
-// DumpUUID
-//
-// Dump out a UUID starting at 'offset' bytes into the buffer
-//----------------------------------------------------------------------
-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);
- uuid.Dump(s);
- } else {
- s->Printf("<not enough data for UUID at offset 0x%8.8" PRIx64 ">",
- offset);
- }
- }
-}
-
size_t DataExtractor::Copy(DataExtractor &dest_data) const {
if (m_data_sp) {
// we can pass along the SP to the data
Index: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
===================================================================
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ lldb/trunk/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::fromData(pdb70_uuid, sizeof(*pdb70_uuid));
+ } else if (cv_signature == CvSignature::ElfBuildId)
+ return UUID::fromData(cv_record);
return UUID();
}
Index: lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
===================================================================
--- lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
+++ lldb/trunk/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 = UUID::fromOptionalData(data.GetData(&offset, 16), 16);
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 = UUID::fromOptionalData(shared_cache_uuid, 16);
if (uuid.IsValid()) {
using_shared_cache = eLazyBoolYes;
}
Index: lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
===================================================================
--- lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ lldb/trunk/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 = UUID::fromOptionalData(extractor.GetData(&offset, 16), 16);
image_infos[i].SetUUID(uuid);
image_infos[i].SetLoadAddress(extractor.GetU64(&offset));
image_infos[i].SetSize(extractor.GetU64(&offset));
Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/trunk/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 = UUID::fromData(uuidt, sizeof(uuidt));
} 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 = UUID::fromData(uuidt, sizeof(uuidt));
}
}
@@ -926,16 +926,16 @@
// 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 = UUID::fromData(uuidt, sizeof(uuidt));
}
} else {
if (!m_gnu_debuglink_crc)
m_gnu_debuglink_crc =
calc_gnu_debuglink_crc32(m_data.GetDataStart(), m_data.GetByteSize());
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 = UUID::fromData(uuidt, sizeof(uuidt));
}
}
@@ -1284,7 +1284,7 @@
}
// Save the build id as the UUID for the module.
- uuid.SetBytes(uuidbuf, note.n_descsz);
+ uuid = UUID::fromData(uuidbuf, note.n_descsz);
}
}
break;
Index: lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/trunk/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 = UUID::fromOptionalData(
+ dsc_header_data.GetData(&offset, sizeof(uuid_t)), sizeof(uuid_t));
}
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 = UUID::fromOptionalData(uuid_bytes, 16);
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 = UUID::fromOptionalData(raw_uuid, sizeof(uuid_t));
+ return true;
}
}
}
@@ -5660,7 +5657,7 @@
+ 100); // sharedCacheBaseAddress <mach-o/dyld_images.h>
}
}
- uuid.SetBytes(sharedCacheUUID_address);
+ uuid = UUID::fromOptionalData(sharedCacheUUID_address, sizeof(uuid_t));
}
}
} else {
@@ -5685,7 +5682,7 @@
dyld_process_info_get_cache (process_info, &sc_info);
if (sc_info.cacheBaseAddress != 0) {
base_addr = sc_info.cacheBaseAddress;
- uuid.SetBytes (sc_info.cacheUUID);
+ uuid = UUID::fromOptionalData(sc_info.cacheUUID, sizeof(uuid_t));
}
dyld_process_info_release (process_info);
}
Index: lldb/trunk/source/API/SBModuleSpec.cpp
===================================================================
--- lldb/trunk/source/API/SBModuleSpec.cpp
+++ lldb/trunk/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() = UUID::fromOptionalData(uuid, uuid_len);
+ return m_opaque_ap->GetUUID().IsValid();
}
bool SBModuleSpec::GetDescription(lldb::SBStream &description) {
Index: lldb/trunk/include/lldb/Utility/UUID.h
===================================================================
--- lldb/trunk/include/lldb/Utility/UUID.h
+++ lldb/trunk/include/lldb/Utility/UUID.h
@@ -33,25 +33,47 @@
//------------------------------------------------------------------
// Constructors and Destructors
//------------------------------------------------------------------
- UUID();
- UUID(const UUID &rhs);
- UUID(const void *uuid_bytes, uint32_t num_uuid_bytes);
+ UUID() = default;
- ~UUID();
+ /// Creates a UUID from the data pointed to by the bytes argument. No special
+ /// significance is attached to any of the values.
+ static UUID fromData(const void *bytes, uint32_t num_bytes) {
+ if (bytes)
+ return fromData({reinterpret_cast<const uint8_t *>(bytes), num_bytes});
+ return UUID();
+ }
+
+ /// Creates a uuid from the data pointed to by the bytes argument. No special
+ /// significance is attached to any of the values.
+ static UUID fromData(llvm::ArrayRef<uint8_t> bytes) { return UUID(bytes); }
+
+ /// Creates a UUID from the data pointed to by the bytes argument. Data
+ /// consisting purely of zero bytes is treated as an invalid UUID.
+ static UUID fromOptionalData(const void *bytes, uint32_t num_bytes) {
+ if (bytes)
+ return fromOptionalData(
+ {reinterpret_cast<const uint8_t *>(bytes), num_bytes});
+ return UUID();
+ }
- const UUID &operator=(const UUID &rhs);
+ /// Creates a UUID from the data pointed to by the bytes argument. Data
+ /// consisting purely of zero bytes is treated as an invalid UUID.
+ static UUID fromOptionalData(llvm::ArrayRef<uint8_t> bytes) {
+ if (llvm::all_of(bytes, [](uint8_t b) { return b == 0; }))
+ return UUID();
+ return UUID(bytes);
+ }
- void Clear();
+ void Clear() { m_num_uuid_bytes = 0; }
void Dump(Stream *s) const;
llvm::ArrayRef<uint8_t> GetBytes() const {
return {m_uuid, m_num_uuid_bytes};
}
- bool IsValid() const;
-
- bool SetBytes(const void *uuid_bytes, uint32_t num_uuid_bytes = 16);
+ explicit operator bool() const { return IsValid(); }
+ bool IsValid() const { return m_num_uuid_bytes > 0; }
std::string GetAsString(const char *separator = nullptr) const;
@@ -81,11 +103,10 @@
uint32_t &bytes_decoded,
uint32_t num_uuid_bytes = 16);
-protected:
- //------------------------------------------------------------------
- // Classes that inherit from UUID can see and modify these
- //------------------------------------------------------------------
- uint32_t m_num_uuid_bytes; // Should be 16 or 20
+private:
+ UUID(llvm::ArrayRef<uint8_t> bytes);
+
+ uint32_t m_num_uuid_bytes = 0; // Should be 0, 16 or 20
ValueType m_uuid;
};
Index: lldb/trunk/include/lldb/Utility/DataExtractor.h
===================================================================
--- lldb/trunk/include/lldb/Utility/DataExtractor.h
+++ lldb/trunk/include/lldb/Utility/DataExtractor.h
@@ -223,22 +223,6 @@
const char *type_format = nullptr) const;
//------------------------------------------------------------------
- /// Dump a UUID value at \a offset.
- ///
- /// Dump a UUID starting at \a offset bytes into this object's data. If the
- /// stream \a s is nullptr, the output will be sent to Log().
- ///
- /// @param[in] s
- /// The stream to dump the output to. If nullptr the output will
- /// be dumped to Log().
- ///
- /// @param[in] offset
- /// The offset into the data at which to extract and dump a
- /// UUID value.
- //------------------------------------------------------------------
- void DumpUUID(Stream *s, lldb::offset_t offset) const;
-
- //------------------------------------------------------------------
/// Extract an arbitrary number of bytes in the specified byte order.
///
/// Attemps to extract \a length bytes starting at \a offset bytes into this
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
