[lldb][Mach-O] Don't read symbol table of specially marked binary (llvm#129967)

We have a binary image on Darwin that has no code, only metadata. It has
a large symbol table with many external symbol names that will not be
needed in the debugger. And it is possible to not have this binary on
the debugger system - so lldb must read all of the symbol names out of
memory, one at a time, which can be quite slow.

We're adding a section __TEXT,__lldb_no_nlist, to this binary to
indicate that lldb should not read the nlist symbols for it when we are
reading out of memory. If lldb is run with an on-disk version of the
binary, we will load the symbol table as we normally would, there's no
benefit to handling this binary differently.

I added a test where I create a dylib with this specially named section,
launch the process. The main binary deletes the dylib from the disk so
lldb is forced to read it out of memory. lldb attaches to the binary,
confirms that the dylib is present in the process and is a memory
Module. If the binary is not present, or lldb found the on-disk copy
because it hasn't been deleted yet, we delete the target, flush the
Debugger's module cache, sleep and retry, up to ten times. I create the
specially named section by compiling an assembly file that puts a byte
in the section which makes for a bit of a messy Makefile (the pre-canned
actions to build a dylib don't quite handle this case) but I don't think
it's much of a problem. This is a purely skipUnlessDarwin test case.

Relanding this change with a restructured Makefiles for the test case
that should pass on the CI bots.

rdar://146167816
(cherry picked from commit 1a31bb3)

Author: jasonmolenda

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

@@ -921,6 +921,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) {
@@ -2395,15 +2400,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) {
@@ -2528,30 +2572,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();
 

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h

@@ -285,6 +285,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;

lldb/test/API/macosx/no-nlist-memory-module/Makefile

@@ -0,0 +1,13 @@
+C_SOURCES := main.c
+LD_EXTRAS = -Wl,-rpath "-Wl,$(shell pwd)" -L. -lno-nlists -lhas-nlists
+
+include Makefile.rules
+
+a.out: dylib_HasNlists dylib_NoNlists
+
+dylib_HasNlists:
+	"$(MAKE)" -f $(MAKEFILE_RULES) \
+	DYLIB_ONLY=YES DYLIB_NAME=has-nlists DYLIB_C_SOURCES=has-nlists.c
+
+dylib_NoNlists:
+	"$(MAKE)" VPATH=$(SRCDIR) -I $(SRCDIR) -f $(SRCDIR)/NoNlists.mk

lldb/test/API/macosx/no-nlist-memory-module/NoNlists.mk

@@ -0,0 +1,12 @@
+DYLIB_ONLY := YES
+DYLIB_NAME := no-nlists
+DYLIB_C_SOURCES := no-nlists.c
+DYLIB_OBJECTS += no-nlist-sect.o
+
+no-nlist-sect.o:
+	$(CC) $(CFLAGS) -c -o no-nlist-sect.o $(SRCDIR)/no-nlist-sect.s
+
+include Makefile.rules
+
+clean::
+	rm -rf *.o *.dylib a.out *.dSYM

lldb/test/API/macosx/no-nlist-memory-module/TestNoNlistsDylib.py

@@ -0,0 +1,71 @@
+"""
+Test that we read don't read the nlist symbols for a specially marked dylib
+when read from memory.
+"""
+
+import os
+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"))
+
+        # 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))
+        )
+
+        # Spawn a new process
+        popen = self.spawnSubprocess(exe, [pid_file_path])
+
+        pid = lldbutil.wait_for_file_on_target(self, pid_file_path)
+
+        os.unlink(self.getBuildArtifact("libno-nlists.dylib"))
+        os.unlink(self.getBuildArtifact("libhas-nlists.dylib"))
+
+        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"))
+
+        self.assertTrue(process, PROCESS_IS_VALID)
+
+        if self.TraceOn():
+            self.runCmd("image list")
+            self.runCmd("target modules dump symtab libno-nlists.dylib")
+            self.runCmd("target modules dump symtab libhas-nlists.dylib")
+
+        # 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)

lldb/test/API/macosx/no-nlist-memory-module/has-nlists.c

@@ -0,0 +1 @@
+int get_return_value2() { return 20; }

lldb/test/API/macosx/no-nlist-memory-module/main.c

@@ -0,0 +1,62 @@
+#include <fcntl.h>
+#include <libgen.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/errno.h>
+#include <sys/param.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+int get_return_value();
+int get_return_value2();
+
+// Create \a file_name with the c-string of our
+// pid in it.  Initially open & write the contents
+// to a temporary file, then move it to the actual
+// filename once writing is completed.
+bool writePid(const char *file_name, const pid_t pid) {
+  char *tmp_file_name = (char *)malloc(strlen(file_name) + 16);
+  strcpy(tmp_file_name, file_name);
+  strcat(tmp_file_name, "_tmp");
+  int fd = open(tmp_file_name, O_CREAT | O_WRONLY | O_TRUNC, S_IRUSR | S_IWUSR);
+  if (fd == -1) {
+    fprintf(stderr, "open(%s) failed: %s\n", tmp_file_name, strerror(errno));
+    free(tmp_file_name);
+    return false;
+  }
+  char buffer[64];
+  snprintf(buffer, sizeof(buffer), "%ld", (long)pid);
+  bool res = true;
+  if (write(fd, buffer, strlen(buffer)) == -1) {
+    fprintf(stderr, "write(%s) failed: %s\n", buffer, strerror(errno));
+    res = false;
+  }
+  close(fd);
+
+  if (rename(tmp_file_name, file_name) == -1) {
+    fprintf(stderr, "rename(%s, %s) failed: %s\n", tmp_file_name, file_name,
+            strerror(errno));
+    res = false;
+  }
+  free(tmp_file_name);
+
+  return res;
+}
+
+int main(int argc, char **argv) {
+  if (writePid(argv[1], getpid())) {
+    // we've signaled lldb we are ready to be attached to,
+    // this sleep() call will be interrupted when lldb
+    // attaches.
+    sleep(200);
+  } else {
+    printf("Error writing pid to '%s', exiting.\n", argv[1]);
+    exit(3);
+  }
+
+  int retval = get_return_value();
+  return retval + get_return_value2();
+}

lldb/test/API/macosx/no-nlist-memory-module/no-nlist-sect.s

@@ -0,0 +1,3 @@
+   .section __TEXT,__lldb_no_nlist,regular,pure_instructions
+   .p2align 2
+   .byte 0x10

lldb/test/API/macosx/no-nlist-memory-module/no-nlists.c

@@ -0,0 +1 @@
+int get_return_value() { return 10; }