Skip to content
This repository has been archived by the owner on Jan 20, 2024. It is now read-only.

Commit

Permalink
Ensure that the executable module is ModuleList[0] (#78360)
Browse files Browse the repository at this point in the history
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

32dd5b2

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.

In the normal course of operation, where the executable is added first,
this only adds a check for whether the first element in the module list
is an executable. If that's true, we just append as normal.

Note, the code in Target::GetExecutableModule doesn't actually agree
that the zeroth element must be the executable, it instead returns the
first Module of type Executable. But I can't tell whether that was a
change in intention or just working around the bug that we don't always
maintain this ordering. But given we've said this in scripting as well
as internally, I think we shouldn't change our minds about this.
  • Loading branch information
jimingham authored Jan 17, 2024
1 parent 0266f41 commit a4cd99e
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 1 deletion.
24 changes: 23 additions & 1 deletion lldb/source/Core/ModuleList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 4 additions & 0 deletions lldb/test/API/functionalities/executable_first/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
CXX_SOURCES := main.cpp
DYLIB_CXX_SOURCES := b.cpp
DYLIB_NAME := bar
include Makefile.rules
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# 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")
1 change: 1 addition & 0 deletions lldb/test/API/functionalities/executable_first/b.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
int LLDB_DYLIB_EXPORT b_function() { return 500; }
6 changes: 6 additions & 0 deletions lldb/test/API/functionalities/executable_first/main.cpp
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit a4cd99e

Please sign in to comment.