[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1beffc18886a: Support build-ids of other sizes than 16 in
UUID::SetFromStringRef (authored by jarin).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80755/new/
https://reviews.llvm.org/D80755
Files:
lldb/include/lldb/Utility/UUID.h
lldb/source/Interpreter/OptionValueUUID.cpp
lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Utility/UUID.cpp
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-4.yaml
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
lldb/unittests/Target/ModuleCacheTest.cpp
lldb/unittests/Utility/UUIDTest.cpp
Index: lldb/unittests/Utility/UUIDTest.cpp
===
--- lldb/unittests/Utility/UUIDTest.cpp
+++ lldb/unittests/Utility/UUIDTest.cpp
@@ -45,7 +45,7 @@
from_str.SetFromStringRef("----");
UUID opt_from_str;
opt_from_str.SetFromOptionalStringRef("----");
-
+
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("40x"));
- 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("40x"));
+ 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) {
Index: lldb/unittests/Target/ModuleCacheTest.cpp
===
--- lldb/unittests/Target/ModuleCacheTest.cpp
+++ 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;
Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ 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
+}
Index: lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
===
--- /dev/null
+++
[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef
jarin updated this revision to Diff 268497.
jarin added a comment.
Exclude UUID strings ending with "-".
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80755/new/
https://reviews.llvm.org/D80755
Files:
lldb/include/lldb/Utility/UUID.h
lldb/source/Interpreter/OptionValueUUID.cpp
lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Utility/UUID.cpp
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-4.yaml
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
lldb/unittests/Target/ModuleCacheTest.cpp
lldb/unittests/Utility/UUIDTest.cpp
Index: lldb/unittests/Utility/UUIDTest.cpp
===
--- lldb/unittests/Utility/UUIDTest.cpp
+++ lldb/unittests/Utility/UUIDTest.cpp
@@ -45,7 +45,7 @@
from_str.SetFromStringRef("----");
UUID opt_from_str;
opt_from_str.SetFromOptionalStringRef("----");
-
+
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("40x"));
- 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("40x"));
+ 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) {
Index: lldb/unittests/Target/ModuleCacheTest.cpp
===
--- lldb/unittests/Target/ModuleCacheTest.cpp
+++ 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;
Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ 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
+}
Index: lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
@@ -0,0 +1,15 @@
+--- !minidump
+Streams:
+ - Type:
[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef
friss added inline comments.
Comment at: lldb/source/Utility/UUID.cpp:66-67
uuid_bytes.clear();
while (!p.empty()) {
-if (isxdigit(p[0]) && isxdigit(p[1])) {
+if (p.size() >= 2 && isxdigit(p[0]) && isxdigit(p[1])) {
int hi_nibble = xdigit_to_int(p[0]);
labath wrote:
> I guess now obsoletes Fred's D80807.
>
> (Btw, I actually liked how Fred's solution rejects strings which end in a
> trailing dash.)
Yeah, I didn't have a strong opinion before, but given we want to reject a
buffer that isn't parsed completely, I think it's better to reject a buffer
ending with a `-`. As the code would test `p.size()` anyway, we might as well
use `p.size() >= 2 ` as the loop condition.
Otherwise this LGTM. Thanks!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80755/new/
https://reviews.llvm.org/D80755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Looks good. Let's just wait a while to see if Fred has any comments. If you
haven't already, you can use that time to get commit access. :)
Comment at: lldb/source/Utility/UUID.cpp:66-67
uuid_bytes.clear();
while (!p.empty()) {
-if (isxdigit(p[0]) && isxdigit(p[1])) {
+if (p.size() >= 2 && isxdigit(p[0]) && isxdigit(p[1])) {
int hi_nibble = xdigit_to_int(p[0]);
I guess now obsoletes Fred's D80807.
(Btw, I actually liked how Fred's solution rejects strings which end in a
trailing dash.)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80755/new/
https://reviews.llvm.org/D80755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef
jarin updated this revision to Diff 267754.
jarin added a comment.
Added a test for missing nibble in UUID.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80755/new/
https://reviews.llvm.org/D80755
Files:
lldb/include/lldb/Utility/UUID.h
lldb/source/Interpreter/OptionValueUUID.cpp
lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Utility/UUID.cpp
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-4.yaml
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
lldb/unittests/Target/ModuleCacheTest.cpp
lldb/unittests/Utility/UUIDTest.cpp
Index: lldb/unittests/Utility/UUIDTest.cpp
===
--- lldb/unittests/Utility/UUIDTest.cpp
+++ lldb/unittests/Utility/UUIDTest.cpp
@@ -45,7 +45,7 @@
from_str.SetFromStringRef("----");
UUID opt_from_str;
opt_from_str.SetFromOptionalStringRef("----");
-
+
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("40x"));
- 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("40x"));
+ 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) {
Index: lldb/unittests/Target/ModuleCacheTest.cpp
===
--- lldb/unittests/Target/ModuleCacheTest.cpp
+++ 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;
Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ 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
+}
Index: lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
@@ -0,0 +1,15 @@
+--- !minidump
+Streams:
+ - Type:
[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef
jarin updated this revision to Diff 267739.
jarin edited the summary of this revision.
jarin added a comment.
Removed size restrictions on UUIDs.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80755/new/
https://reviews.llvm.org/D80755
Files:
lldb/include/lldb/Utility/UUID.h
lldb/source/Interpreter/OptionValueUUID.cpp
lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Utility/UUID.cpp
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-4.yaml
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
lldb/unittests/Target/ModuleCacheTest.cpp
lldb/unittests/Utility/UUIDTest.cpp
Index: lldb/unittests/Utility/UUIDTest.cpp
===
--- lldb/unittests/Utility/UUIDTest.cpp
+++ lldb/unittests/Utility/UUIDTest.cpp
@@ -45,7 +45,7 @@
from_str.SetFromStringRef("----");
UUID opt_from_str;
opt_from_str.SetFromOptionalStringRef("----");
-
+
EXPECT_FALSE(empty);
EXPECT_TRUE(a16);
EXPECT_TRUE(a20);
@@ -57,25 +57,28 @@
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("40x"));
- 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("40x"));
+ 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);
}
TEST(UUIDTest, StringConverion) {
Index: lldb/unittests/Target/ModuleCacheTest.cpp
===
--- lldb/unittests/Target/ModuleCacheTest.cpp
+++ 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;
Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ 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
+}
Index: lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
@@ -0,0 +1,15 @@
+--- !minidump
+Streams:
+ - Type:
[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef
jarin marked an inline comment as done.
jarin added inline comments.
Comment at: lldb/source/Utility/UUID.cpp:109
*this = fromData(bytes);
-return str.size() - rest.size();
+return str.size();
}
jankratochvil wrote:
> Now the return type could be `bool`.
I was worried about having to touch even more call sites, but perhaps it is not
too bad.
Comment at: lldb/source/Utility/UUID.cpp:93-94
+bool UUID::SetFromStringRef(llvm::StringRef str) {
+ const size_t max_uuid_size = 20;
+ const size_t min_uuid_size = 4;
+
labath wrote:
> I don't think we should be restricting the size here in any way. It is
> possible to produce larger build-ids already
> (-Wl,--build-id=0xlonghexstring), and the rest of the UUID class does support
> arbitrary sizes. Some tools (e.g. llvm-readelf) will choke on them, but let's
> try to not make lldb one of those tools.
>
> Using super-short uuids is most likely a bad idea, and will result in a lot
> of collisions, but if someone really wants to use a 3-byte "uuid", I don't
> see a reason to stop him here.
Good idea, that makes the code even simpler.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80755/new/
https://reviews.llvm.org/D80755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef
labath added a reviewer: friss.
labath added inline comments.
Comment at: lldb/source/Utility/UUID.cpp:93-94
+bool UUID::SetFromStringRef(llvm::StringRef str) {
+ const size_t max_uuid_size = 20;
+ const size_t min_uuid_size = 4;
+
I don't think we should be restricting the size here in any way. It is possible
to produce larger build-ids already (-Wl,--build-id=0xlonghexstring), and the
rest of the UUID class does support arbitrary sizes. Some tools (e.g.
llvm-readelf) will choke on them, but let's try to not make lldb one of those
tools.
Using super-short uuids is most likely a bad idea, and will result in a lot of
collisions, but if someone really wants to use a 3-byte "uuid", I don't see a
reason to stop him here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80755/new/
https://reviews.llvm.org/D80755
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef
jarin updated this revision to Diff 267490.
jarin marked an inline comment as done.
jarin added a comment.
Change SetFromStringRef to return bool.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80755/new/
https://reviews.llvm.org/D80755
Files:
lldb/include/lldb/Utility/UUID.h
lldb/source/Interpreter/OptionValueUUID.cpp
lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Utility/UUID.cpp
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-4.yaml
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
lldb/unittests/Target/ModuleCacheTest.cpp
lldb/unittests/Utility/UUIDTest.cpp
Index: lldb/unittests/Utility/UUIDTest.cpp
===
--- lldb/unittests/Utility/UUIDTest.cpp
+++ lldb/unittests/Utility/UUIDTest.cpp
@@ -45,7 +45,7 @@
from_str.SetFromStringRef("----");
UUID opt_from_str;
opt_from_str.SetFromOptionalStringRef("----");
-
+
EXPECT_FALSE(empty);
EXPECT_TRUE(a16);
EXPECT_TRUE(a20);
@@ -57,25 +57,28 @@
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("40x"));
- 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("40x"));
+ 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);
}
TEST(UUIDTest, StringConverion) {
Index: lldb/unittests/Target/ModuleCacheTest.cpp
===
--- lldb/unittests/Target/ModuleCacheTest.cpp
+++ 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;
Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ 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
+}
Index: lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
@@ -0,0 +1,15 @@
+--- !minidump
+Streams:
+ - Type:
[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef
MaskRay added a comment. I haven't been paying close attention to this patch, but allowing 20 bytes makes sense. In GNU ld and gold, `--build-id` is `--build-id=sha1` (20 bytes). The 3 linkers (plus LLD) don't have a way to produce a build ID longer than 20 bytes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80755/new/ https://reviews.llvm.org/D80755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef
jankratochvil added inline comments. Comment at: lldb/source/Utility/UUID.cpp:109 *this = fromData(bytes); -return str.size() - rest.size(); +return str.size(); } Now the return type could be `bool`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80755/new/ https://reviews.llvm.org/D80755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef
jarin updated this revision to Diff 267241.
jarin retitled this revision from "Support build-ids of other sizes than 16 in
SBTarget::AddModule" to "Support build-ids of other sizes than 16 in
UUID::SetFromStringRef".
jarin edited the summary of this revision.
Herald added subscribers: MaskRay, emaste.
Herald added a reviewer: espindola.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80755/new/
https://reviews.llvm.org/D80755
Files:
lldb/include/lldb/Utility/UUID.h
lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Utility/UUID.cpp
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-4.yaml
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
lldb/unittests/Target/ModuleCacheTest.cpp
lldb/unittests/Utility/UUIDTest.cpp
Index: lldb/unittests/Utility/UUIDTest.cpp
===
--- lldb/unittests/Utility/UUIDTest.cpp
+++ lldb/unittests/Utility/UUIDTest.cpp
@@ -45,7 +45,7 @@
from_str.SetFromStringRef("----");
UUID opt_from_str;
opt_from_str.SetFromOptionalStringRef("----");
-
+
EXPECT_FALSE(empty);
EXPECT_TRUE(a16);
EXPECT_TRUE(a20);
@@ -63,19 +63,23 @@
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(
+ 45u, 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(36u, u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f"));
+
EXPECT_EQ(0u, u.SetFromStringRef("40x"));
EXPECT_EQ(0u, u.SetFromStringRef(""));
- EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNOPQRS", 20), u)
+ 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_EQ(41u,
+u.SetFromStringRef("404142434445464748494a4b4c4d4e4f-50515253"));
+ EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNOPQRS", 20), u);
+
+ EXPECT_EQ(8u, u.SetFromStringRef("40414243"));
+ EXPECT_EQ(UUID::fromData("@ABCD", 4), u);
}
TEST(UUIDTest, StringConverion) {
Index: lldb/unittests/Target/ModuleCacheTest.cpp
===
--- lldb/unittests/Target/ModuleCacheTest.cpp
+++ 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;
Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ 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
+}
Index: lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
===
--- /dev/null
+++ 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: 0x
+ - Type:ModuleList
+Modules:
+ - Base of Image: 0x1000
+Size of Image: 0x1000
+Module Name:
