[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in SBTarget::AddModule

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin created this revision.
jarin added a reviewer: labath.
jarin added a project: LLDB.
Herald added a subscriber: lldb-commits.

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 sizes up to 20 bytes, which is currently the maximum we can get (with --build-id=sha1). If the UUID cannot parse the input string completely, we simply ignore the UUID argument (we never trim too long UUIDs to shorter ones).

This also adds tests for three different kind of build-ids (fast, sha1, uuid), two of which were failing without this patch (fast failed because its 4 byte UUID is ignored, sha1 failed because its 20 bytes were trimmed 16).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80755

Files:
  lldb/source/API/SBTarget.cpp
  lldb/test/API/python_api/addmodule-uuid/Makefile
  lldb/test/API/python_api/addmodule-uuid/TestAddModuleUUID.py
  lldb/test/API/python_api/addmodule-uuid/main.cpp


Index: lldb/test/API/python_api/addmodule-uuid/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/python_api/addmodule-uuid/main.cpp
@@ -0,0 +1,3 @@
+int main() {
+  return 0; // break here
+}
Index: lldb/test/API/python_api/addmodule-uuid/TestAddModuleUUID.py
===================================================================
--- /dev/null
+++ lldb/test/API/python_api/addmodule-uuid/TestAddModuleUUID.py
@@ -0,0 +1,45 @@
+"""Test Python APIs for adding modules"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestAddModuleUUID(TestBase):
+    mydir = TestBase.compute_mydir(__file__)
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @skipIf(oslist=no_match(['linux']))
+    def test_add_existing_module_with_buildid_fast(self):
+      self.run_test_add_existing_module_with_uuid('-Wl,--build-id=fast')
+
+    @skipIf(oslist=no_match(['linux']))
+    def test_add_existing_module_with_buildid_sha1(self):
+      self.run_test_add_existing_module_with_uuid('-Wl,--build-id=sha1')
+
+    @skipIf(oslist=no_match(['linux']))
+    def test_add_existing_module_with_buildid_uuid(self):
+      self.run_test_add_existing_module_with_uuid('-Wl,--build-id=uuid')
+
+    def run_test_add_existing_module_with_uuid(self, ldflags):
+        d = self.getBuildFlags()
+        d['LD_EXTRAS'] = ldflags
+        self.build(dictionary=d)
+        (target, _, _, _) = lldbutil.run_to_source_breakpoint(
+            self, "// break here", lldb.SBFileSpec("main.cpp"))
+        module = target.FindModule(target.GetExecutable())
+
+        # Check that adding the existing module gives that module back.
+        path = str(module.GetFileSpec())
+        uuid = module.GetUUIDString();
+        add_result = target.AddModule(path, "", uuid)
+        self.assertEqual(module, add_result)
+
+        # Change the UUID and check that adding with wrong UUID fails.
+        if len(uuid) != 0:
+          if uuid[0] == "0":
+            different_uuid = "1" + uuid[1:]
+          else:
+            different_uuid = "0" + uuid[1:]
+          add_result = target.AddModule(path, "", different_uuid)
+          self.assertTrue(not add_result.IsValid())
Index: lldb/test/API/python_api/addmodule-uuid/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/python_api/addmodule-uuid/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/API/SBTarget.cpp
===================================================================
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -1586,8 +1586,16 @@
     if (path)
       module_spec.GetFileSpec().SetFile(path, FileSpec::Style::native);
 
-    if (uuid_cstr)
-      module_spec.GetUUID().SetFromStringRef(uuid_cstr);
+    if (uuid_cstr) {
+      llvm::StringRef uuid_str = llvm::StringRef(uuid_cstr).trim();
+      llvm::SmallVector<uint8_t, 20> bytes;
+      llvm::StringRef rest =
+          UUID::DecodeUUIDBytesFromString(uuid_str, bytes, 20);
+      // If the UUID string was consumed and it is not empty, let us use it for
+      // the lookup.
+      if (rest.empty() && bytes.size() > 0)
+        module_spec.GetUUID() = UUID::fromData(bytes);
+    }
 
     if (triple)
       module_spec.GetArchitecture() = Platform::GetAugmentedArchSpec(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80755.267004.patch
Type: text/x-patch
Size: 3430 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200528/21b57a80/attachment.bin>