maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Support build-ids of other sizes than 16 in UUID::SetFromStringRef 

SBTarget::AddModule currently handles the UUID parameter in a very
weird way: UUIDs with more than 16 bytes are trimmed to 16 bytes. On
the other hand, shorter-than-16-bytes UUIDs are completely ignored. In
this patch, we change the parsing code to handle UUIDs of arbitrary
size.

To support arbitrary size UUIDs in SBTarget::AddModule, this patch
changes UUID::SetFromStringRef to parse UUIDs of arbitrary length. We
subtly change the semantics of SetFromStringRef - SetFromStringRef now
only succeeds if the entire input is consumed to prevent some
prefix-parsing confusion. This is up for discussion, but I believe
this is more consistent - we always return false for invalid UUIDs
rather than sometimes truncating to a valid prefix. Also, all the
call-sites except the API and interpreter seem to expect to consume
the entire input.

This also adds tests for adding existing modules 4-, 16-, and 20-byte
build-ids. Finally, we took the liberty of testing the minidump
scenario we care about - removing placeholder module from minidump and
replacing it with the real module.

Reviewed By: labath, friss

Differential Revision: https://reviews.llvm.org/D80755
This commit is contained in:
Jaroslav Sevcik 2020-06-01 20:43:18 +00:00
parent bd67d68ca1
commit 1beffc1888
12 changed files with 160 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
lldb/include/lldb/Utility/UUID.h
View File

@ -63,18 +63,13 @@ public:
std::string GetAsString(llvm::StringRef separator = "-") const;
size_t SetFromStringRef(llvm::StringRef str, uint32_t num_uuid_bytes = 16);
bool SetFromStringRef(llvm::StringRef str);
// Same as SetFromStringRef, but if the resultant UUID is all 0 bytes, set the
// UUID to invalid.
size_t SetFromOptionalStringRef(llvm::StringRef str,
uint32_t num_uuid_bytes = 16);
bool SetFromOptionalStringRef(llvm::StringRef str);
// Decode as many UUID bytes (up to 16) as possible from the C string "cstr"
// This is used for auto completion where a partial UUID might have been
// typed in. It
/// Decode as many UUID bytes (up to 16) as possible from the C
/// string \a cstr.
/// Decode as many UUID bytes as possible from the C string \a cstr.
///
/// \param[in] str
/// An llvm::StringRef that points at a UUID string value (no leading
@ -88,8 +83,7 @@ public:
/// The original string, with all decoded bytes removed.
static llvm::StringRef
DecodeUUIDBytesFromString(llvm::StringRef str,
llvm::SmallVectorImpl<uint8_t> &uuid_bytes,
uint32_t num_uuid_bytes = 16);
llvm::SmallVectorImpl<uint8_t> &uuid_bytes);
private:
UUID(llvm::ArrayRef<uint8_t> bytes) : m_bytes(bytes.begin(), bytes.end()) {}

2
lldb/source/Interpreter/OptionValueUUID.cpp
View File

@ -38,7 +38,7 @@ Status OptionValueUUID::SetValueFromString(llvm::StringRef value,
case eVarSetOperationReplace:
case eVarSetOperationAssign: {
if (m_uuid.SetFromStringRef(value) == 0)
if (!m_uuid.SetFromStringRef(value))
error.SetErrorStringWithFormat("invalid uuid string value '%s'",
value.str().c_str());
else {

2
lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
View File

@ -205,7 +205,7 @@ llvm::Optional<InfoRecord> InfoRecord::parse(llvm::StringRef Line) {
// use this as the UUID. Otherwise, we should revert back to the module ID.
UUID ID;
if (Line.trim().empty()) {
if (Str.empty() || ID.SetFromStringRef(Str, Str.size() / 2) != Str.size())
if (Str.empty() || !ID.SetFromStringRef(Str))
return llvm::None;
}
return InfoRecord(std::move(ID));

2
lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
View File

@ -445,7 +445,7 @@ lldb_private::UUID CommunicationKDP::GetUUID() {
if (uuid_str.size() < 32)
return uuid;
if (uuid.SetFromStringRef(uuid_str) == 0) {
if (!uuid.SetFromStringRef(uuid_str)) {
UUID invalid_uuid;
return invalid_uuid;
}

5
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
View File

@ -3623,7 +3623,7 @@ bool GDBRemoteCommunicationClient::GetModuleInfo(
StringExtractor extractor(value);
std::string uuid;
extractor.GetHexByteString(uuid);
module_spec.GetUUID().SetFromStringRef(uuid, uuid.size() / 2);
module_spec.GetUUID().SetFromStringRef(uuid);
} else if (name == "triple") {
StringExtractor extractor(value);
std::string triple;
@ -3659,8 +3659,7 @@ ParseModuleSpec(StructuredData::Dictionary *dict) {
if (!dict->GetValueForKeyAsString("uuid", string))
return llvm::None;
if (result.GetUUID().SetFromStringRef(string, string.size() / 2) !=
string.size())
if (!result.GetUUID().SetFromStringRef(string))
return llvm::None;
if (!dict->GetValueForKeyAsInteger("file_offset", integer))

41
lldb/source/Utility/UUID.cpp
View File

@ -61,10 +61,9 @@ static inline int xdigit_to_int(char ch) {
llvm::StringRef
UUID::DecodeUUIDBytesFromString(llvm::StringRef p,
llvm::SmallVectorImpl<uint8_t> &uuid_bytes,
uint32_t num_uuid_bytes) {
llvm::SmallVectorImpl<uint8_t> &uuid_bytes) {
uuid_bytes.clear();
while (!p.empty()) {
while (p.size() >= 2) {
if (isxdigit(p[0]) && isxdigit(p[1])) {
int hi_nibble = xdigit_to_int(p[0]);
int lo_nibble = xdigit_to_int(p[1]);
@ -73,11 +72,6 @@ UUID::DecodeUUIDBytesFromString(llvm::StringRef p,
// Skip both hex digits
p = p.drop_front(2);
// Increment the byte that we are decoding within the UUID value and
// break out if we are done
if (uuid_bytes.size() == num_uuid_bytes)
break;
} else if (p.front() == '-') {
// Skip dashes
p = p.drop_front();
@ -89,35 +83,30 @@ UUID::DecodeUUIDBytesFromString(llvm::StringRef p,
return p;
}
size_t UUID::SetFromStringRef(llvm::StringRef str, uint32_t num_uuid_bytes) {
bool UUID::SetFromStringRef(llvm::StringRef str) {
llvm::StringRef p = str;
// Skip leading whitespace characters
p = p.ltrim();
llvm::SmallVector<uint8_t, 20> bytes;
llvm::StringRef rest =
UUID::DecodeUUIDBytesFromString(p, bytes, num_uuid_bytes);
llvm::StringRef rest = UUID::DecodeUUIDBytesFromString(p, bytes);
// If we successfully decoded a UUID, return the amount of characters that
// were consumed
if (bytes.size() == num_uuid_bytes) {
*this = fromData(bytes);
return str.size() - rest.size();
}
// Return false if we could not consume the entire string or if the parsed
// UUID is empty.
if (!rest.empty() || bytes.empty())
return false;
// Else return zero to indicate we were not able to parse a UUID value
return 0;
*this = fromData(bytes);
return true;
}
size_t UUID::SetFromOptionalStringRef(llvm::StringRef str,
uint32_t num_uuid_bytes) {
size_t num_chars_consumed = SetFromStringRef(str, num_uuid_bytes);
if (num_chars_consumed) {
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 num_chars_consumed;
}
return result;
}

82
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
View File

@ -185,3 +185,85 @@ class MiniDumpUUIDTestCase(TestBase):
self.getSourcePath("relative_module_name.yaml"))
self.assertEqual(1, len(modules))
self.verify_module(modules[0], name, None)
def test_add_module_build_id_16(self):
"""
Test that adding module with 16 byte UUID returns the existing
module or fails.
"""
modules = self.get_minidump_modules("linux-arm-uuids-elf-build-id-16.yaml")
self.assertEqual(2, len(modules))
# Add the existing modules.
self.assertEqual(modules[0], self.target.AddModule(
"/some/local/a", "", "01020304-0506-0708-090A-0B0C0D0E0F10"))
self.assertEqual(modules[1], self.target.AddModule(
"/some/local/b", "", "0A141E28-323C-4650-5A64-6E78828C96A0"))
# Adding modules with non-existing UUID should fail.
self.assertFalse(
self.target.AddModule(
"a", "", "12345678-1234-1234-1234-123456789ABC").IsValid())
self.assertFalse(
self.target.AddModule(
"a", "", "01020304-0506-0708-090A-0B0C0D0E0F10-12345678").IsValid())
def test_add_module_build_id_20(self):
"""
Test that adding module with 20 byte UUID returns the existing
module or fails.
"""
modules = self.get_minidump_modules("linux-arm-uuids-elf-build-id-20.yaml")
# Add the existing modules.
self.assertEqual(modules[0], self.target.AddModule(
"/some/local/a", "", "01020304-0506-0708-090A-0B0C0D0E0F10-11121314"))
self.assertEqual(modules[1], self.target.AddModule(
"/some/local/b", "", "0A141E28-323C-4650-5A64-6E78828C96A0-AAB4BEC8"))
# Adding modules with non-existing UUID should fail.
self.assertFalse(
self.target.AddModule(
"a", "", "01020304-0506-0708-090A-0B0C0D0E0F10").IsValid())
self.assertFalse(
self.target.AddModule(
"a", "", "01020304-0506-0708-090A-0B0C0D0E0F10-12345678").IsValid())
def test_add_module_build_id_4(self):
"""
Test that adding module with 4 byte UUID returns the existing
module or fails.
"""
modules = self.get_minidump_modules("linux-arm-uuids-elf-build-id-4.yaml")
# Add the existing modules.
self.assertEqual(modules[0], self.target.AddModule(
"/some/local/a.so", "", "01020304"))
self.assertEqual(modules[1], self.target.AddModule(
"/some/local/b.so", "", "0A141E28"))
# Adding modules with non-existing UUID should fail.
self.assertFalse(
self.target.AddModule(
"a", "", "01020304-0506-0708-090A-0B0C0D0E0F10").IsValid())
self.assertFalse(self.target.AddModule("a", "", "01020305").IsValid())
def test_remove_placeholder_add_real_module(self):
"""
Test that removing a placeholder module and adding back the real
module succeeds.
"""
so_path = self.getBuildArtifact("libuuidmatch.so")
self.yaml2obj("libuuidmatch.yaml", so_path)
modules = self.get_minidump_modules("linux-arm-uuids-match.yaml")
uuid = "7295E17C-6668-9E05-CBB5-DEE5003865D5-5267C116";
self.assertEqual(1, len(modules))
self.verify_module(modules[0], "/target/path/libuuidmatch.so",uuid)
self.target.RemoveModule(modules[0])
new_module = self.target.AddModule(so_path, "", uuid)
self.verify_module(new_module, so_path, uuid)
self.assertEqual(new_module, self.target.modules[0])
self.assertEqual(1, len(self.target.modules))

19
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-4.yaml Normal file
View File

@ -0,0 +1,19 @@
--- !minidump
Streams:
- Type: SystemInfo
Processor Arch: ARM
Platform ID: Linux
CSD Version: '15E216'
CPU:
CPUID: 0x00000000
- Type: ModuleList
Modules:
- Base of Image: 0x0000000000001000
Size of Image: 0x00001000
Module Name: '/tmp/a.so'
CodeView Record: 4C45704201020304
- Base of Image: 0x0000000000001000
Size of Image: 0x00001000
Module Name: '/tmp/b.so'
CodeView Record: 4C4570420A141E28
...

15
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml Normal file
View File

@ -0,0 +1,15 @@
--- !minidump
Streams:
- Type: SystemInfo
Processor Arch: ARM
Platform ID: Linux
CSD Version: '15E216'
CPU:
CPUID: 0x00000000
- Type: ModuleList
Modules:
- Base of Image: 0x0000000000001000
Size of Image: 0x00001000
Module Name: '/target/path/libuuidmatch.so'
CodeView Record: 525344537295E17C66689E05CBB5DEE5003865D55267C116
...

4
lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
View File

@ -156,7 +156,7 @@ TEST_F(ObjectFileELFTest, GetModuleSpecifications_EarlySectionHeaders) {
ModuleSpec Spec;
ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
UUID Uuid;
Uuid.SetFromStringRef("1b8a73ac238390e32a7ff4ac8ebe4d6a41ecf5c9", 20);
Uuid.SetFromStringRef("1b8a73ac238390e32a7ff4ac8ebe4d6a41ecf5c9");
EXPECT_EQ(Spec.GetUUID(), Uuid);
}
@ -284,4 +284,4 @@ Sections:
auto entry_point_addr = module_sp->GetObjectFile()->GetEntryPointAddress();
ASSERT_EQ(entry_point_addr.GetAddressClass(), AddressClass::eCode);
}
}

3
lldb/unittests/Target/ModuleCacheTest.cpp
View File

@ -41,7 +41,6 @@ static const char dummy_remote_dir[] = "bin";
static const char module_name[] = "TestModule.so";
static const char module_uuid[] =
"F4E7E991-9B61-6AD4-0073-561AC3D9FA10-C043A476";
static const uint32_t uuid_bytes = 20;
static const size_t module_size = 5602;
static FileSpec GetDummyRemotePath() {
@ -87,7 +86,7 @@ void ModuleCacheTest::TryGetAndPut(const FileSpec &cache_dir,
ModuleCache mc;
ModuleSpec module_spec;
module_spec.GetFileSpec() = GetDummyRemotePath();
module_spec.GetUUID().SetFromStringRef(module_uuid, uuid_bytes);
module_spec.GetUUID().SetFromStringRef(module_uuid);
module_spec.SetObjectSize(module_size);
ModuleSP module_sp;
bool did_create;

29
lldb/unittests/Utility/UUIDTest.cpp
View File

@ -45,7 +45,7 @@ TEST(UUIDTest, Validity) {
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);
@ -57,25 +57,30 @@ TEST(UUIDTest, Validity) {
TEST(UUIDTest, SetFromStringRef) {
UUID u;
EXPECT_EQ(32u, u.SetFromStringRef("404142434445464748494a4b4c4d4e4f"));
EXPECT_TRUE(u.SetFromStringRef("404142434445464748494a4b4c4d4e4f"));
EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16), u);
EXPECT_EQ(36u, u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f"));
EXPECT_TRUE(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_TRUE(
u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f-50515253"));
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)
EXPECT_TRUE(u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f"));
EXPECT_FALSE(u.SetFromStringRef("40xxxxx"));
EXPECT_FALSE(u.SetFromStringRef(""));
EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16), u)
<< "uuid was changed by failed parse calls";
EXPECT_EQ(
32u, u.SetFromStringRef("404142434445464748494a4b4c4d4e4f-50515253", 16));
EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16), u);
EXPECT_TRUE(u.SetFromStringRef("404142434445464748494a4b4c4d4e4f-50515253"));
EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNOPQRS", 20), u);
EXPECT_TRUE(u.SetFromStringRef("40414243"));
EXPECT_EQ(UUID::fromData("@ABCD", 4), u);
EXPECT_FALSE(u.SetFromStringRef("4"));
}
TEST(UUIDTest, StringConverion) {