Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 44 additions & 24 deletions lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,11 @@ ConstString ObjectFileMachO::GetSectionNameEHFrame() {
return g_section_name_eh_frame;
}

ConstString ObjectFileMachO::GetSectionNameLLDBNoNlist() {
static ConstString g_section_name_lldb_no_nlist("__lldb_no_nlist");
return g_section_name_lldb_no_nlist;
}

bool ObjectFileMachO::MagicBytesMatch(DataBufferSP data_sp,
lldb::addr_t data_offset,
lldb::addr_t data_length) {
Expand Down Expand Up @@ -2394,15 +2399,54 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
uint32_t memory_module_load_level = eMemoryModuleLoadLevelComplete;
bool is_shared_cache_image = IsSharedCacheBinary();
bool is_local_shared_cache_image = is_shared_cache_image && !IsInMemory();

ConstString g_segment_name_TEXT = GetSegmentNameTEXT();
ConstString g_segment_name_DATA = GetSegmentNameDATA();
ConstString g_segment_name_DATA_DIRTY = GetSegmentNameDATA_DIRTY();
ConstString g_segment_name_DATA_CONST = GetSegmentNameDATA_CONST();
ConstString g_segment_name_OBJC = GetSegmentNameOBJC();
ConstString g_section_name_eh_frame = GetSectionNameEHFrame();
ConstString g_section_name_lldb_no_nlist = GetSectionNameLLDBNoNlist();
SectionSP text_section_sp(
section_list->FindSectionByName(g_segment_name_TEXT));
SectionSP data_section_sp(
section_list->FindSectionByName(g_segment_name_DATA));
SectionSP linkedit_section_sp(
section_list->FindSectionByName(GetSegmentNameLINKEDIT()));
SectionSP data_dirty_section_sp(
section_list->FindSectionByName(g_segment_name_DATA_DIRTY));
SectionSP data_const_section_sp(
section_list->FindSectionByName(g_segment_name_DATA_CONST));
SectionSP objc_section_sp(
section_list->FindSectionByName(g_segment_name_OBJC));
SectionSP eh_frame_section_sp;
SectionSP lldb_no_nlist_section_sp;
if (text_section_sp.get()) {
eh_frame_section_sp = text_section_sp->GetChildren().FindSectionByName(
g_section_name_eh_frame);
lldb_no_nlist_section_sp = text_section_sp->GetChildren().FindSectionByName(
g_section_name_lldb_no_nlist);
} else {
eh_frame_section_sp =
section_list->FindSectionByName(g_section_name_eh_frame);
lldb_no_nlist_section_sp =
section_list->FindSectionByName(g_section_name_lldb_no_nlist);
}

if (process && m_header.filetype != llvm::MachO::MH_OBJECT &&
!is_local_shared_cache_image) {
Target &target = process->GetTarget();

memory_module_load_level = target.GetMemoryModuleLoadLevel();

// If __TEXT,__lldb_no_nlist section is present in this binary,
// and we're reading it out of memory, do not read any of the
// nlist entries. They are not needed in lldb and it may be
// expensive to load these. This is to handle a dylib consisting
// of only metadata, no code, but it has many nlist entries.
if (lldb_no_nlist_section_sp)
memory_module_load_level = eMemoryModuleLoadLevelMinimal;

// Reading mach file from memory in a process or core file...

if (linkedit_section_sp) {
Expand Down Expand Up @@ -2526,30 +2570,6 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {

const bool have_strtab_data = strtab_data.GetByteSize() > 0;

ConstString g_segment_name_TEXT = GetSegmentNameTEXT();
ConstString g_segment_name_DATA = GetSegmentNameDATA();
ConstString g_segment_name_DATA_DIRTY = GetSegmentNameDATA_DIRTY();
ConstString g_segment_name_DATA_CONST = GetSegmentNameDATA_CONST();
ConstString g_segment_name_OBJC = GetSegmentNameOBJC();
ConstString g_section_name_eh_frame = GetSectionNameEHFrame();
SectionSP text_section_sp(
section_list->FindSectionByName(g_segment_name_TEXT));
SectionSP data_section_sp(
section_list->FindSectionByName(g_segment_name_DATA));
SectionSP data_dirty_section_sp(
section_list->FindSectionByName(g_segment_name_DATA_DIRTY));
SectionSP data_const_section_sp(
section_list->FindSectionByName(g_segment_name_DATA_CONST));
SectionSP objc_section_sp(
section_list->FindSectionByName(g_segment_name_OBJC));
SectionSP eh_frame_section_sp;
if (text_section_sp.get())
eh_frame_section_sp = text_section_sp->GetChildren().FindSectionByName(
g_section_name_eh_frame);
else
eh_frame_section_sp =
section_list->FindSectionByName(g_section_name_eh_frame);

const bool is_arm = (m_header.cputype == llvm::MachO::CPU_TYPE_ARM);
const bool always_thumb = GetArchitecture().IsAlwaysThumbInstructions();

Expand Down
1 change: 1 addition & 0 deletions lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ class ObjectFileMachO : public lldb_private::ObjectFile {
static lldb_private::ConstString GetSegmentNameDWARF();
static lldb_private::ConstString GetSegmentNameLLVM_COV();
static lldb_private::ConstString GetSectionNameEHFrame();
static lldb_private::ConstString GetSectionNameLLDBNoNlist();

llvm::MachO::dysymtab_command m_dysymtab;
std::vector<llvm::MachO::section_64> m_mach_sections;
Expand Down
19 changes: 19 additions & 0 deletions lldb/test/API/macosx/no-nlist-memory-module/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
C_SOURCES := main.c
LD_EXTRAS = -Wl,-rpath "-Wl,$(shell pwd)" -L. -lno-nlists -lhas-nlists

.PHONY: build-libno-nlists build-libhas-nlists
all: build-libno-nlists build-libhas-nlists a.out

include Makefile.rules

build-libno-nlists: no-nlists.c no-nlist-sect.s
$(CC) $(CFLAGS) -c -o no-nlists.o $(<D)/no-nlists.c
$(CC) $(CFLAGS) -c -o no-nlist-sect.o $(<D)/no-nlist-sect.s
$(LD) -dynamiclib -o libno-nlists.dylib no-nlists.o no-nlist-sect.o -install_name "@executable_path/libno-nlists.dylib"

build-libhas-nlists: has-nlists.c
$(CC) $(CFLAGS) -c -o has-nlists.o $(<D)/has-nlists.c
$(LD) -dynamiclib -o libhas-nlists.dylib has-nlists.o -install_name "@executable_path/libhas-nlists.dylib"

clean::
rm -rf has-nlists.o no-nlists.o no-nlist-sect.o main.o main.s a.out a.out.dSYM libno-nlists.dylib libno-nlists.dylib.dSYM libhas-nlists.dylib libhas-nlists.dylib.dSYM
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be easier to rm -rf *.o *.dSYM *.dylib?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah tbh I don't think we ever even run the clean targets these days, I just stuck this in there out of habit. good suggestion.

Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
"""
Test that we read don't read the nlist symbols for a specially marked dylib
when read from memory.
"""

import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil
from time import sleep


class NoNlistsTestCase(TestBase):
NO_DEBUG_INFO_TESTCASE = True

@skipIfRemote
@skipUnlessDarwin
def test_no_nlist_symbols(self):
self.build()

exe = os.path.realpath(self.getBuildArtifact("a.out"))

popen = self.spawnSubprocess(exe)
pid = popen.pid

self.dbg.SetAsync(False)

m_no_nlist = lldb.SBModule()
m_has_nlist = lldb.SBModule()
target = lldb.SBTarget()
process = lldb.SBProcess()
reattach_count = 0

# Attach to the process, see if we have a memory module
# for libno-nlists.dylib and libhas-nlists.dylib.
# If not, detach, delete the Target, and flush the orphaned
# modules from the Debugger so we don't hold on to a reference
# of the on-disk binary.

# If we haven't succeeded after ten attemps of attaching and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand the need for this loop. Why would this fail the first time and succeed the second time? Is this trying to work around the debugger running in asynchronous mode or something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured out the answer when reading the test below: because the inferior is removing the dylib and there's no synchronization with the test.

But that leads to another question: why are you not doing this from Python in the test itself? Seems like that would simplify things a lot (the test could just sleep forever) and allow you to do the attach when you know the program is in the expected state?

Copy link
Collaborator Author

@jasonmolenda jasonmolenda Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried doing in python first -- but I ended up removing the dylib before it loaded in the process, and it crashed starting up. I'd have to add a sleep or some other sync to guarantee that the inferior had started and loaded the dylib before I remove it. (and sleeps are really unstable when a test gets run on a possibly-very-slow CI, of course. on any reasonable computer doing a sleep(1) in the python would let the process be launched and running, but I don't trust that kind of pattern with CI after years of seeing very unusual perf behavior)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I think I understand the challenge: you can't use lldb to synchronize (e.g. by waiting on a pause()) because by attaching, you're already going to be parsing the library. We have a few other tests that have this issue, and they use a file to synchronize through lldbutil.wait_for_file_on_target.

        # Use a file as a synchronization point between test and inferior.
        pid_file_path = lldbutil.append_to_process_working_directory(
            self, "pid_file_%d" % (int(time.time()))
        )
        self.addTearDownHook(
            lambda: self.run_platform_command("rm %s" % (pid_file_path))
        )

        popen = self.spawnSubprocess(exe, [pid_file_path])

        pid = lldbutil.wait_for_file_on_target(self, pid_file_path)

I think we could use the same approach here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

# detaching, fail the test.
while (not m_no_nlist.IsValid() or m_no_nlist.IsFileBacked()) and (
not m_has_nlist.IsValid() or m_has_nlist.IsFileBacked()
):
if process.IsValid():
process.Detach()
self.dbg.DeleteTarget(target)
self.dbg.MemoryPressureDetected()
time.sleep(2)

self.runCmd("process attach -p " + str(pid))
target = self.dbg.GetSelectedTarget()
process = target.GetProcess()
m_no_nlist = target.FindModule(lldb.SBFileSpec("libno-nlists.dylib"))
m_has_nlist = target.FindModule(lldb.SBFileSpec("libhas-nlists.dylib"))

reattach_count = reattach_count + 1
if reattach_count > 10:
break

self.assertTrue(process, PROCESS_IS_VALID)

# Test that we found libno-nlists.dylib, it is a memory
# module, and that it has no symbols.
self.assertTrue(m_no_nlist.IsValid())
self.assertFalse(m_no_nlist.IsFileBacked())
self.assertEqual(m_no_nlist.GetNumSymbols(), 0)

# Test that we found libhas-nlists.dylib, it is a memory
# module, and that it has more than zero symbols.
self.assertTrue(m_has_nlist.IsValid())
self.assertFalse(m_has_nlist.IsFileBacked())
self.assertGreater(m_has_nlist.GetNumSymbols(), 0)

# And as a sanity check, get the main binary's module,
# test that it is file backed and that it has more than
# zero symbols.
m_exe = target.FindModule(lldb.SBFileSpec("a.out"))
self.assertTrue(m_exe.IsValid())
self.assertTrue(m_exe.IsFileBacked())
self.assertGreater(m_exe.GetNumSymbols(), 0)
1 change: 1 addition & 0 deletions lldb/test/API/macosx/no-nlist-memory-module/has-nlists.c
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
int get_return_value2() { return 20; }
48 changes: 48 additions & 0 deletions lldb/test/API/macosx/no-nlist-memory-module/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#include <libgen.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/param.h>
#include <sys/stat.h>
#include <unistd.h>

int get_return_value();
int get_return_value2();

int main(int argc, char **argv) {

// Remove libno-nlists.dylib that we are linked against.
char executable_path[PATH_MAX];
realpath(argv[0], executable_path);
executable_path[PATH_MAX - 1] = '\0';

char *dir = dirname(executable_path);
char dylib_path[PATH_MAX];
snprintf(dylib_path, PATH_MAX, "%s/%s", dir, "libno-nlists.dylib");
dylib_path[PATH_MAX - 1] = '\0';
struct stat sb;
if (stat(dylib_path, &sb) == -1) {
printf("Could not find dylib %s to remove it\n", dylib_path);
exit(1);
}
if (unlink(dylib_path) == -1) {
printf("Could not remove dylib %s\n", dylib_path);
exit(2);
}
snprintf(dylib_path, PATH_MAX, "%s/%s", dir, "libhas-nlists.dylib");
dylib_path[PATH_MAX - 1] = '\0';
if (stat(dylib_path, &sb) == -1) {
printf("Could not find dylib %s to remove it\n", dylib_path);
exit(1);
}
if (unlink(dylib_path) == -1) {
printf("Could not remove dylib %s\n", dylib_path);
exit(2);
}

// This sleep will exit as soon as lldb attaches
// and interrupts it.
sleep(200);

int retval = get_return_value();
return retval + get_return_value2();
}
3 changes: 3 additions & 0 deletions lldb/test/API/macosx/no-nlist-memory-module/no-nlist-sect.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.section __TEXT,__lldb_no_nlist,regular,pure_instructions
.p2align 2
.byte 0x10
1 change: 1 addition & 0 deletions lldb/test/API/macosx/no-nlist-memory-module/no-nlists.c
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
int get_return_value() { return 10; }