Skip to content

Commit a4cd99e

Browse files
authored
Ensure that the executable module is ModuleList[0] (llvm#78360)
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.
1 parent 0266f41 commit a4cd99e

File tree

5 files changed

+84
-1
lines changed

5 files changed

+84
-1
lines changed

lldb/source/Core/ModuleList.cpp

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,29 @@ ModuleList::~ModuleList() = default;
211211
void ModuleList::AppendImpl(const ModuleSP &module_sp, bool use_notifier) {
212212
if (module_sp) {
213213
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
214-
m_modules.push_back(module_sp);
214+
// We are required to keep the first element of the Module List as the
215+
// executable module. So check here and if the first module is NOT an
216+
// but the new one is, we insert this module at the beginning, rather than
217+
// at the end.
218+
// We don't need to do any of this if the list is empty:
219+
if (m_modules.empty()) {
220+
m_modules.push_back(module_sp);
221+
} else {
222+
// Since producing the ObjectFile may take some work, first check the 0th
223+
// element, and only if that's NOT an executable look at the incoming
224+
// ObjectFile. That way in the normal case we only look at the element
225+
// 0 ObjectFile.
226+
const bool elem_zero_is_executable
227+
= m_modules[0]->GetObjectFile()->GetType()
228+
== ObjectFile::Type::eTypeExecutable;
229+
lldb_private::ObjectFile *obj = module_sp->GetObjectFile();
230+
if (!elem_zero_is_executable && obj
231+
&& obj->GetType() == ObjectFile::Type::eTypeExecutable) {
232+
m_modules.insert(m_modules.begin(), module_sp);
233+
} else {
234+
m_modules.push_back(module_sp);
235+
}
236+
}
215237
if (use_notifier && m_notifier)
216238
m_notifier->NotifyModuleAdded(*this, module_sp);
217239
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
CXX_SOURCES := main.cpp
2+
DYLIB_CXX_SOURCES := b.cpp
3+
DYLIB_NAME := bar
4+
include Makefile.rules
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# This test checks that we make the executable the first
2+
# element in the image list.
3+
4+
import lldb
5+
from lldbsuite.test.decorators import *
6+
from lldbsuite.test.lldbtest import *
7+
from lldbsuite.test import lldbutil
8+
9+
10+
class TestExecutableIsFirst(TestBase):
11+
NO_DEBUG_INFO_TESTCASE = True
12+
13+
def test_executable_is_first_before_run(self):
14+
self.build()
15+
16+
ctx = self.platformContext
17+
lib_name = ctx.shlib_prefix + "bar." + ctx.shlib_extension
18+
19+
exe = self.getBuildArtifact("a.out")
20+
lib = self.getBuildArtifact(lib_name)
21+
22+
target = self.dbg.CreateTarget(None)
23+
module = target.AddModule(lib, None, None)
24+
self.assertTrue(module.IsValid(), "Added the module for the library")
25+
26+
module = target.AddModule(exe, None, None)
27+
self.assertTrue(module.IsValid(), "Added the executable module")
28+
29+
# This is the executable module so it should be the first in the list:
30+
first_module = target.GetModuleAtIndex(0)
31+
print("This is the first test, this one succeeds")
32+
self.assertEqual(module, first_module, "This executable is the first module")
33+
34+
# The executable property is an SBFileSpec to the executable. Make sure
35+
# that is also right:
36+
executable_module = target.executable
37+
self.assertEqual(
38+
first_module.file, executable_module, "Python property is also our module"
39+
)
40+
41+
def test_executable_is_first_during_run(self):
42+
self.build()
43+
(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
44+
self, "break after function call", lldb.SBFileSpec("main.cpp")
45+
)
46+
47+
first_module = target.GetModuleAtIndex(0)
48+
self.assertTrue(first_module.IsValid(), "We have at least one module")
49+
executable_module = target.executable
50+
self.assertEqual(first_module.file, executable_module, "They are the same")
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
int LLDB_DYLIB_EXPORT b_function() { return 500; }
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
extern int b_function();
2+
3+
int main(int argc, char* argv[]) {
4+
int ret_value = b_function();
5+
return ret_value; // break after function call
6+
}

0 commit comments

Comments
 (0)