[Lldb-commits] [lldb] Ensure that the executable module is ModuleList[0] (PR #78360)

via lldb-commits lldb-commits at lists.llvm.org
Tue Jan 16 15:18:16 PST 2024


https://github.com/jimingham updated https://github.com/llvm/llvm-project/pull/78360

>From 4d53ea53a9a5d9344d41195697b3b98359af1ede Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Tue, 16 Jan 2024 14:33:29 -0800
Subject: [PATCH 1/2] Ensure that the executable module is ModuleList[0]

We claim in a couple places that the zeroth element of the module
list for a target is the main executable, but we don't actually
enforce that in the ModuleList class.  As we saw, for instance, in

32dd5b20973bde1ef77fa3b84b9f85788a1a303a

it's not all that hard to get this to be off.  This patch ensures that
the first object file of type Executable added to it is moved to the front
of the ModuleList.  I also added a test for this.
---
 lldb/source/Core/ModuleList.cpp               | 24 +++++++++-
 .../functionalities/executable_first/Makefile |  4 ++
 .../executable_first/TestExecutableFirst.py   | 46 +++++++++++++++++++
 .../functionalities/executable_first/b.cpp    |  1 +
 .../functionalities/executable_first/main.cpp |  6 +++
 5 files changed, 80 insertions(+), 1 deletion(-)
 create mode 100644 lldb/test/API/functionalities/executable_first/Makefile
 create mode 100644 lldb/test/API/functionalities/executable_first/TestExecutableFirst.py
 create mode 100644 lldb/test/API/functionalities/executable_first/b.cpp
 create mode 100644 lldb/test/API/functionalities/executable_first/main.cpp

diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 2180f29f369427..80299f80a27869 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -211,7 +211,29 @@ ModuleList::~ModuleList() = default;
 void ModuleList::AppendImpl(const ModuleSP &module_sp, bool use_notifier) {
   if (module_sp) {
     std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
-    m_modules.push_back(module_sp);
+    // We are required to keep the first element of the Module List as the
+    // executable module.  So check here and if the first module is NOT an 
+    // but the new one is, we insert this module at the beginning, rather than 
+    // at the end.
+    // We don't need to do any of this if the list is empty:
+    if (m_modules.empty()) {
+      m_modules.push_back(module_sp);
+    } else {
+      // Since producing the ObjectFile may take some work, first check the 0th
+      // element, and only if that's NOT an executable look at the incoming
+      // ObjectFile.  That way in the normal case we only look at the element
+      // 0 ObjectFile. 
+      const bool elem_zero_is_executable 
+          = m_modules[0]->GetObjectFile()->GetType() 
+              == ObjectFile::Type::eTypeExecutable;
+      lldb_private::ObjectFile *obj = module_sp->GetObjectFile();
+      if (!elem_zero_is_executable && obj 
+          && obj->GetType() == ObjectFile::Type::eTypeExecutable) {
+        m_modules.insert(m_modules.begin(), module_sp);
+      } else {
+        m_modules.push_back(module_sp);
+      }
+    }
     if (use_notifier && m_notifier)
       m_notifier->NotifyModuleAdded(*this, module_sp);
   }
diff --git a/lldb/test/API/functionalities/executable_first/Makefile b/lldb/test/API/functionalities/executable_first/Makefile
new file mode 100644
index 00000000000000..74060ad03c6584
--- /dev/null
+++ b/lldb/test/API/functionalities/executable_first/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+DYLIB_CXX_SOURCES := b.cpp
+DYLIB_NAME := bar
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/executable_first/TestExecutableFirst.py b/lldb/test/API/functionalities/executable_first/TestExecutableFirst.py
new file mode 100644
index 00000000000000..bcf967a596deb8
--- /dev/null
+++ b/lldb/test/API/functionalities/executable_first/TestExecutableFirst.py
@@ -0,0 +1,46 @@
+# This test checks that we make the executable the first
+# element in the image list.
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestExecutableIsFirst(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_executable_is_first_before_run(self):
+        self.build()
+
+        ctx = self.platformContext
+        lib_name = ctx.shlib_prefix + "bar." + ctx.shlib_extension
+
+        exe = self.getBuildArtifact("a.out")
+        lib = self.getBuildArtifact(lib_name)
+
+        target = self.dbg.CreateTarget(None)
+        module = target.AddModule(lib, None, None)
+        self.assertTrue(module.IsValid(), "Added the module for the library")
+        
+        module = target.AddModule(exe, None, None)
+        self.assertTrue(module.IsValid(), "Added the executable module")
+
+       # This is the executable module so it should be the first in the list:
+        first_module = target.GetModuleAtIndex(0)
+        print("This is the first test, this one succeeds")
+        self.assertEqual(module, first_module, "This executable is the first module")
+
+        # The executable property is an SBFileSpec to the executable.  Make sure
+        # that is also right:
+        executable_module = target.executable
+        self.assertEqual(first_module.file, executable_module, "Python property is also our module")
+
+    def test_executable_is_first_during_run(self):
+        self.build()
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, "break after function call", lldb.SBFileSpec("main.cpp"))
+        
+        first_module = target.GetModuleAtIndex(0)
+        self.assertTrue(first_module.IsValid(), "We have at least one module")
+        executable_module = target.executable
+        self.assertEqual(first_module.file, executable_module, "They are the same")
diff --git a/lldb/test/API/functionalities/executable_first/b.cpp b/lldb/test/API/functionalities/executable_first/b.cpp
new file mode 100644
index 00000000000000..911447bee489ce
--- /dev/null
+++ b/lldb/test/API/functionalities/executable_first/b.cpp
@@ -0,0 +1 @@
+int LLDB_DYLIB_EXPORT b_function() { return 500; }
diff --git a/lldb/test/API/functionalities/executable_first/main.cpp b/lldb/test/API/functionalities/executable_first/main.cpp
new file mode 100644
index 00000000000000..ba61bf837b9651
--- /dev/null
+++ b/lldb/test/API/functionalities/executable_first/main.cpp
@@ -0,0 +1,6 @@
+extern int b_function();
+
+int main(int argc, char* argv[]) {
+  int ret_value = b_function();
+  return ret_value; // break after function call
+}

>From bd4f49d12d5ceecd559139fef0b4f2bfaa00164a Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Tue, 16 Jan 2024 15:17:45 -0800
Subject: [PATCH 2/2] Bow to the lord of darkness.

---
 .../executable_first/TestExecutableFirst.py        | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lldb/test/API/functionalities/executable_first/TestExecutableFirst.py b/lldb/test/API/functionalities/executable_first/TestExecutableFirst.py
index bcf967a596deb8..5c5573d5b38382 100644
--- a/lldb/test/API/functionalities/executable_first/TestExecutableFirst.py
+++ b/lldb/test/API/functionalities/executable_first/TestExecutableFirst.py
@@ -22,11 +22,11 @@ def test_executable_is_first_before_run(self):
         target = self.dbg.CreateTarget(None)
         module = target.AddModule(lib, None, None)
         self.assertTrue(module.IsValid(), "Added the module for the library")
-        
+
         module = target.AddModule(exe, None, None)
         self.assertTrue(module.IsValid(), "Added the executable module")
 
-       # This is the executable module so it should be the first in the list:
+        # This is the executable module so it should be the first in the list:
         first_module = target.GetModuleAtIndex(0)
         print("This is the first test, this one succeeds")
         self.assertEqual(module, first_module, "This executable is the first module")
@@ -34,12 +34,16 @@ def test_executable_is_first_before_run(self):
         # The executable property is an SBFileSpec to the executable.  Make sure
         # that is also right:
         executable_module = target.executable
-        self.assertEqual(first_module.file, executable_module, "Python property is also our module")
+        self.assertEqual(
+            first_module.file, executable_module, "Python property is also our module"
+        )
 
     def test_executable_is_first_during_run(self):
         self.build()
-        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, "break after function call", lldb.SBFileSpec("main.cpp"))
-        
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "break after function call", lldb.SBFileSpec("main.cpp")
+        )
+
         first_module = target.GetModuleAtIndex(0)
         self.assertTrue(first_module.IsValid(), "We have at least one module")
         executable_module = target.executable



More information about the lldb-commits mailing list