diff --git a/lldb/include/lldb/Utility/UUID.h b/lldb/include/lldb/Utility/UUID.h --- a/lldb/include/lldb/Utility/UUID.h +++ b/lldb/include/lldb/Utility/UUID.h @@ -63,18 +63,13 @@ 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); - - // 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. + bool SetFromOptionalStringRef(llvm::StringRef str); + + /// 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 @@ /// The original string, with all decoded bytes removed. static llvm::StringRef DecodeUUIDBytesFromString(llvm::StringRef str, - llvm::SmallVectorImpl &uuid_bytes, - uint32_t num_uuid_bytes = 16); + llvm::SmallVectorImpl &uuid_bytes); private: UUID(llvm::ArrayRef bytes) : m_bytes(bytes.begin(), bytes.end()) {} diff --git a/lldb/source/Interpreter/OptionValueUUID.cpp b/lldb/source/Interpreter/OptionValueUUID.cpp --- a/lldb/source/Interpreter/OptionValueUUID.cpp +++ b/lldb/source/Interpreter/OptionValueUUID.cpp @@ -38,7 +38,7 @@ 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 { diff --git a/lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp b/lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp --- a/lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp +++ b/lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp @@ -205,7 +205,7 @@ // 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)); diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp b/lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp --- a/lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp +++ b/lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp @@ -445,7 +445,7 @@ if (uuid_str.size() < 32) return uuid; - if (uuid.SetFromStringRef(uuid_str) == 0) { + if (!uuid.SetFromStringRef(uuid_str)) { UUID invalid_uuid; return invalid_uuid; } diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -3623,7 +3623,7 @@ 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 @@ 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)) diff --git a/lldb/source/Utility/UUID.cpp b/lldb/source/Utility/UUID.cpp --- a/lldb/source/Utility/UUID.cpp +++ b/lldb/source/Utility/UUID.cpp @@ -61,10 +61,9 @@ llvm::StringRef UUID::DecodeUUIDBytesFromString(llvm::StringRef p, - llvm::SmallVectorImpl &uuid_bytes, - uint32_t num_uuid_bytes) { + llvm::SmallVectorImpl &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 @@ // 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 @@ 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 bytes; - llvm::StringRef rest = - UUID::DecodeUUIDBytesFromString(p, bytes, num_uuid_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(); - } + llvm::StringRef rest = UUID::DecodeUUIDBytesFromString(p, bytes); + + // 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; +} diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py --- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py +++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py @@ -185,3 +185,85 @@ 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)) diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-4.yaml b/lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-4.yaml new file mode 100644 --- /dev/null +++ b/lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-4.yaml @@ -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 +... diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml b/lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml new file mode 100644 --- /dev/null +++ b/lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml @@ -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 +... diff --git a/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp b/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp --- a/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp +++ b/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp @@ -156,7 +156,7 @@ 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 @@ auto entry_point_addr = module_sp->GetObjectFile()->GetEntryPointAddress(); ASSERT_EQ(entry_point_addr.GetAddressClass(), AddressClass::eCode); -} \ No newline at end of file +} diff --git a/lldb/unittests/Target/ModuleCacheTest.cpp b/lldb/unittests/Target/ModuleCacheTest.cpp --- a/lldb/unittests/Target/ModuleCacheTest.cpp +++ b/lldb/unittests/Target/ModuleCacheTest.cpp @@ -41,7 +41,6 @@ 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 @@ 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; diff --git a/lldb/unittests/Utility/UUIDTest.cpp b/lldb/unittests/Utility/UUIDTest.cpp --- a/lldb/unittests/Utility/UUIDTest.cpp +++ b/lldb/unittests/Utility/UUIDTest.cpp @@ -45,7 +45,7 @@ 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, 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) {