Merge pull request #10199 from jasonmolenda/r146167816-dont-read-nlist-records-from-memory-for-specially-marked-binary-6.1

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

Author: JDevlieghere

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; }