jingham created this revision.
jingham added reviewers: JDevlieghere, clayborg, labath.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
At present, there are two ways to get data into a UUID object, an unchecked
version and what's called the "Optional" version. So for instance, there's
setFromData and setFromOptionalData. The difference is that the Optional
version looks at the UUID bytes and if they are all 0's it clears the data
store for the UUID, rendering it an Invalid UUID.
However, there was no apparatus for determining which of these two variants to
use. The PDB version always uses the Optional version, the Darwin support code
used both somewhat inconsistently. The ELF object file code uses the unchecked
version. And the generic parts of the code sometimes use one and sometimes
another. So the current approach is not very coherent.
On Darwin, treating all-zero's UUID's as significant is actually wrong behavior
- so that needs to be changed. But that still leaves the system seeming
arbitrary. For instance, SBModuleSpec::SetUUIDBytes uses the checked method.
So even if some ObjectFile format treats UUID's of 0 as significant, you
wouldn't be able to find it with SBModuleSpec.
My guess is this is all just historical and it would be fine to treat UUID's of
0 as meaning "not valid for matching", which is what this patch does.
If this is not the case, we'll have to add something like
ObjectFile::AllZerosUUIDIsValid, and then check it everywhere outside the
object file reader that we build UUID's. But I'd rather not do that if there
isn't a compelling reason to do so.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D132191
Files:
lldb/include/lldb/Utility/UUID.h
lldb/source/API/SBModuleSpec.cpp
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/source/Utility/UUID.cpp
lldb/unittests/Utility/UUIDTest.cpp
Index: lldb/unittests/Utility/UUIDTest.cpp
===================================================================
--- lldb/unittests/Utility/UUIDTest.cpp
+++ lldb/unittests/Utility/UUIDTest.cpp
@@ -39,20 +39,13 @@
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);
UUID from_str;
from_str.SetFromStringRef("00000000-0000-0000-0000-000000000000");
- UUID opt_from_str;
- opt_from_str.SetFromOptionalStringRef("00000000-0000-0000-0000-000000000000");
EXPECT_FALSE(empty);
- EXPECT_TRUE(a16);
- EXPECT_TRUE(a20);
- EXPECT_TRUE(from_str);
- EXPECT_FALSE(a16_0);
- EXPECT_FALSE(a20_0);
- EXPECT_FALSE(opt_from_str);
+ EXPECT_FALSE(a16);
+ EXPECT_FALSE(a20);
+ EXPECT_FALSE(from_str);
}
TEST(UUIDTest, SetFromStringRef) {
Index: lldb/source/Utility/UUID.cpp
===================================================================
--- lldb/source/Utility/UUID.cpp
+++ lldb/source/Utility/UUID.cpp
@@ -41,8 +41,8 @@
llvm::sys::swapByteOrder(debug_info.Uuid.Data3);
llvm::sys::swapByteOrder(debug_info.Age);
if (debug_info.Age)
- return UUID::fromOptionalData(&debug_info, sizeof(debug_info));
- return UUID::fromOptionalData(&debug_info.Uuid, sizeof(debug_info.Uuid));
+ return UUID::fromData(&debug_info, sizeof(debug_info));
+ return UUID::fromData(&debug_info.Uuid, sizeof(debug_info.Uuid));
}
std::string UUID::GetAsString(llvm::StringRef separator) const {
@@ -110,13 +110,3 @@
*this = fromData(bytes);
return true;
}
-
-bool UUID::SetFromOptionalStringRef(llvm::StringRef str) {
- bool result = SetFromStringRef(str);
- if (result) {
- if (llvm::all_of(m_bytes, [](uint8_t b) { return b == 0; }))
- Clear();
- }
-
- return result;
-}
Index: lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
===================================================================
--- lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -68,13 +68,13 @@
return UUID();
if (GetArchitecture().GetTriple().isOSBinFormatELF()) {
if (pdb70_uuid->Age != 0)
- return UUID::fromOptionalData(pdb70_uuid, sizeof(*pdb70_uuid));
- return UUID::fromOptionalData(&pdb70_uuid->Uuid,
+ return UUID::fromData(pdb70_uuid, sizeof(*pdb70_uuid));
+ return UUID::fromData(&pdb70_uuid->Uuid,
sizeof(pdb70_uuid->Uuid));
}
return UUID::fromCvRecord(*pdb70_uuid);
} else if (cv_signature == CvSignature::ElfBuildId)
- return UUID::fromOptionalData(cv_record);
+ return UUID::fromData(cv_record);
return UUID();
}
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2177,7 +2177,7 @@
version_str[6] = '\0';
if (strcmp(version_str, "dyld_v") == 0) {
offset = offsetof(struct lldb_copy_dyld_cache_header_v1, uuid);
- dsc_uuid = UUID::fromOptionalData(
+ dsc_uuid = UUID::fromData(
dsc_header_data.GetData(&offset, sizeof(uuid_t)), sizeof(uuid_t));
}
Log *log = GetLog(LLDBLog::Symbols);
@@ -2318,7 +2318,7 @@
const uint8_t *uuid_bytes = m_data.PeekData(offset, 16);
if (uuid_bytes)
- image_uuid = UUID::fromOptionalData(uuid_bytes, 16);
+ image_uuid = UUID::fromData(uuid_bytes, 16);
break;
}
@@ -2715,7 +2715,7 @@
return;
if (process_shared_cache_uuid.IsValid() &&
- process_shared_cache_uuid != UUID::fromOptionalData(&cache_uuid, 16))
+ process_shared_cache_uuid != UUID::fromData(&cache_uuid, 16))
return;
dyld_shared_cache_for_each_image(shared_cache, ^(dyld_image_t image) {
@@ -2724,7 +2724,7 @@
return;
dyld_image_copy_uuid(image, &dsc_image_uuid);
- if (image_uuid != UUID::fromOptionalData(dsc_image_uuid, 16))
+ if (image_uuid != UUID::fromData(dsc_image_uuid, 16))
return;
found_image = true;
@@ -4842,7 +4842,7 @@
if (!memcmp(uuid_bytes, opencl_uuid, 16))
return UUID();
- return UUID::fromOptionalData(uuid_bytes, 16);
+ return UUID::fromData(uuid_bytes, 16);
}
return UUID();
}
@@ -5605,8 +5605,7 @@
}
if (m_data.CopyData(offset, sizeof(uuid_t), raw_uuid) != 0) {
- if (!uuid_is_null(raw_uuid))
- uuid = UUID::fromOptionalData(raw_uuid, sizeof(uuid_t));
+ uuid = UUID::fromData(raw_uuid, sizeof(uuid_t));
// convert the "main bin spec" type into our
// ObjectFile::BinaryType enum
switch (binspec_type) {
@@ -5901,7 +5900,7 @@
100); // sharedCacheBaseAddress <mach-o/dyld_images.h>
}
}
- uuid = UUID::fromOptionalData(sharedCacheUUID_address, sizeof(uuid_t));
+ uuid = UUID::fromData(sharedCacheUUID_address, sizeof(uuid_t));
}
}
} else {
@@ -5930,7 +5929,7 @@
dyld_process_info_get_cache(process_info, &sc_info);
if (sc_info.cacheBaseAddress != 0) {
base_addr = sc_info.cacheBaseAddress;
- uuid = UUID::fromOptionalData(sc_info.cacheUUID, sizeof(uuid_t));
+ uuid = UUID::fromData(sc_info.cacheUUID, sizeof(uuid_t));
}
dyld_process_info_release(process_info);
}
@@ -6917,8 +6916,7 @@
MachOCorefileImageEntry image_entry;
image_entry.filename = (const char *)m_data.GetCStr(&filepath_offset);
- if (!uuid_is_null(uuid))
- image_entry.uuid = UUID::fromData(uuid, sizeof(uuid_t));
+ image_entry.uuid = UUID::fromData(uuid, sizeof(uuid_t));
image_entry.load_address = load_address;
image_entry.currently_executing = currently_executing;
@@ -6949,8 +6947,7 @@
MachOCorefileImageEntry image_entry;
image_entry.filename = filename;
- if (!uuid_is_null(uuid))
- image_entry.uuid = UUID::fromData(uuid, sizeof(uuid_t));
+ image_entry.uuid = UUID::fromData(uuid, sizeof(uuid_t));
image_entry.load_address = load_address;
image_entry.slide = slide;
image_entry.currently_executing = true;
Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
===================================================================
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
@@ -885,7 +885,7 @@
break;
case llvm::MachO::LC_UUID:
- dylib_info.uuid = UUID::fromOptionalData(data.GetData(&offset, 16), 16);
+ dylib_info.uuid = UUID::fromData(data.GetData(&offset, 16), 16);
break;
default:
@@ -1093,7 +1093,7 @@
uuid_t shared_cache_uuid;
if (m_process->ReadMemory(sharedCacheUUID_address, shared_cache_uuid,
sizeof(uuid_t), err) == sizeof(uuid_t)) {
- uuid = UUID::fromOptionalData(shared_cache_uuid, 16);
+ uuid = UUID::fromData(shared_cache_uuid, 16);
if (uuid.IsValid()) {
using_shared_cache = eLazyBoolYes;
}
Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
===================================================================
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -501,7 +501,7 @@
image_infos[i].segments.push_back(segment);
}
- image_infos[i].uuid.SetFromOptionalStringRef(
+ image_infos[i].uuid.SetFromStringRef(
image->GetValueForKey("uuid")->GetAsString()->GetValue());
// All sections listed in the dyld image info structure will all either be
Index: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
===================================================================
--- lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -1389,7 +1389,7 @@
if (name_data == nullptr)
break;
image_infos[i].SetName((const char *)name_data);
- UUID uuid = UUID::fromOptionalData(extractor.GetData(&offset, 16), 16);
+ UUID uuid = UUID::fromData(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/source/API/SBModuleSpec.cpp
===================================================================
--- lldb/source/API/SBModuleSpec.cpp
+++ lldb/source/API/SBModuleSpec.cpp
@@ -134,7 +134,7 @@
bool SBModuleSpec::SetUUIDBytes(const uint8_t *uuid, size_t uuid_len) {
LLDB_INSTRUMENT_VA(this, uuid, uuid_len)
- m_opaque_up->GetUUID() = UUID::fromOptionalData(uuid, uuid_len);
+ m_opaque_up->GetUUID() = UUID::fromData(uuid, uuid_len);
return m_opaque_up->GetUUID().IsValid();
}
Index: lldb/include/lldb/Utility/UUID.h
===================================================================
--- lldb/include/lldb/Utility/UUID.h
+++ lldb/include/lldb/Utility/UUID.h
@@ -21,6 +21,9 @@
class Stream;
class UUID {
+ // Represents UUID's of various sizes. In all cases, a uuid of all zeros is
+ // treated as an "Invalid UUID" marker, and the UUID created from such data
+ // will return false for IsValid.
public:
UUID() = default;
@@ -40,35 +43,16 @@
/// Create a UUID from CvRecordPdb70.
static UUID fromCvRecord(CvRecordPdb70 debug_info);
- /// Creates a UUID from the data pointed to by the bytes argument. No special
- /// significance is attached to any of the values.
+ /// Creates a UUID from the data pointed to by the bytes argument.
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.
+ /// Creates a uuid from the data pointed to by the bytes argument.
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();
- }
-
- /// 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() { m_bytes.clear(); }
void Dump(Stream *s) const;
@@ -77,15 +61,11 @@
explicit operator bool() const { return IsValid(); }
bool IsValid() const { return !m_bytes.empty(); }
-
+
std::string GetAsString(llvm::StringRef separator = "-") const;
bool SetFromStringRef(llvm::StringRef str);
- // Same as SetFromStringRef, but if the resultant UUID is all 0 bytes, set the
- // UUID to invalid.
- bool SetFromOptionalStringRef(llvm::StringRef str);
-
/// Decode as many UUID bytes as possible from the C string \a cstr.
///
/// \param[in] str
@@ -103,7 +83,11 @@
llvm::SmallVectorImpl<uint8_t> &uuid_bytes);
private:
- UUID(llvm::ArrayRef<uint8_t> bytes) : m_bytes(bytes.begin(), bytes.end()) {}
+ UUID(llvm::ArrayRef<uint8_t> bytes) : m_bytes(bytes.begin(), bytes.end()) {
+ if (llvm::all_of(m_bytes, [](uint8_t b) { return b == 0; })) {
+ Clear();
+ }
+ }
// GNU ld generates 20-byte build-ids. Size chosen to avoid heap allocations
// for this case.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
