make debugserver able to inspect mach-o binaries present in memory, but not yet registered with dyld

[jasonmolenda]

Summary

lldb sends the jGetLoadedDynamicLibrariesInfos packet to debugserver with a list of addresses of mach-o binaries. Today, debugserver collects a list of all binaries that are loaded in the process, along with their filenames, and then debugserver parsers the mach-o binaries in memory and returns the important details in a JSON response so lldb doesn't need to read this data from the inferior itself. If lldb asks about a binary that isn't officially loaded in the process yet -- but is present in memory -- then this packet will return nothing about it.

This patch handles the case where dyld does not know about the binary. We cannot get a filepath for the binary, so "" is returned there, but debugserver can examine memory and if there's something that looks like a mach-o header + load commands, return that information to lldb.

In macOS Ventura, as part of the early startup, we launch with one dynamic linker (dyld) and that dyld hands off control to a dyld in the shared cache. During that transition, the new dyld is not officially loaded in the process yet. I'd like to switch to having lldb parse the not-yet-running dyld and put a breakpoint in it. This is needed first, to avoid a perf hit when lldb inspects the load commands of new-dyld.

I added a test case that constructs a tiny little mach-o binary in a memory buffer and have lldb send that buffer address to jGetLoadedDynamicLibrariesInfos. Current debugserver will return an empty list of binaries found; with this patch, debugserver will have an empty filepath but return everything from the load commands.

There's no real good reviewer in this case, this is all jason code, but if anyone has any questions or comments they'd like to make, I'd be interested in hearing them.

[JDevlieghere]

lldb/tools/debugserver/source/MacOSX/MachProcess.mm

1186–1191

What happens when it turns out that there's no Mach-O header at this load address? I expect it doesn't end up in the jGetLoadedDynamicLibrariesInfos response then? Where does this filtering take place?

[jasonmolenda]

lldb/tools/debugserver/source/MacOSX/MachProcess.mm

1186–1191

Yeah, it's a good question. MachProcess::GetMachOInformationFromMemory does the Mach-O parsing, and reviewing that code, I do some sanity checking but I don't check for the magic number in the header. At the very least, I should add that check. I should also check the return value from GetMachOInformationFromMemory and not add the entry to my list of binaries I'm going to respond to lldb with. So, a bit more work here would be a good idea, agreed.

[DavidSpickett]

Some random comments.

Also agree that a test for what happens when there is no header found would be good.

lldb/test/API/macosx/unregistered-macho/TestUnregisteredMacho.py

21

You can drop the () around the variable names.

lldb/test/API/macosx/unregistered-macho/main.c

21

It seems like this should be the size of the commands only, not including the header. Though I can't find documentation for it so maybe this is just an unfortunate name.

33

Does this need to be sizeof (struct mach_header_64)? Probably doesn't and/or it doesn't matter for this test.

52

No comment here other than I always like to see this kind of info in a test!

[jasonmolenda]

Thanks for the review David. I am still fiddling with handling the case where debugserver is handed the address of something that isn't actually a Mach-O binary in memory, but I'll update the patch soon with these.

lldb/test/API/macosx/unregistered-macho/TestUnregisteredMacho.py

21

Ah, yes thanks.

lldb/test/API/macosx/unregistered-macho/main.c

21

No, you're exactly correct. When I was writing the test case originally I had a little bug and switched sizeofcmds to be size of the mach header and the commands, and it's not correct. Most parsers limit their parsing to the number of load commands and the total size of load commands, so they wouldn't read past the actual load commands. Thanks for noticing, I fixed it after confirming I was wrong, hah.

[jasonmolenda]

Updated patch to address Jonas and David's feedback.

Updated the test C file so the mach header sizeofcmds is correct. Update test to test querying an invalid mach-o binary, and the combination of a valid & invalid mach-o binary addresses, and check that we get the correct replies.

Add simple checks to MachProcess::GetMachOInformationFromMemory() which parses inferior memory as a Mach-O binary, to detect things that are not mach-o binaries and return false in those case.

Update callers to GetMachOInformationFromMemory() to mark any entry that had a problem parsing it to mark that entry is is_valid=false.

Update MachProcess::FormatDynamicLibrariesIntoJSON() to check the is_valid flag for each entry, and don't add those entries to the StructuredData return array. Do return validly formatted mach-o's that were found during the scan, though.

MachProcess::GetLoadedDynamicLibrariesInfos() (not used on modern Darwin systems any more) - in the process of updating its handling of macho-parsing call, noticed that the indentation was all incorrect for this method. Fixed it, removed an extra return at the end. This looks like it's undergone some incorrect revision over the years -- probably by me. It's near time this could be removed altogether most likely, but I don't want to do that just yet.

I'll add an NDEBUG assert to DynamicLoaderMacOS later which checks that lldb gets back the same number of binary image entries that it asked for -- so we can flag any case where debugserver omits a binary in our testing. I can't imagine this would happen.

[DavidSpickett]

lldb/test/API/macosx/unregistered-macho/TestUnregisteredMacho.py

47

For these would it be more reliable to just look for the load_address: bits?

If the packet contains multiple response, then fine but if it's one response then the second one wouldn't ever match even if debugserver did somehow find the one at the invalid address. Also if it did would it put it in a images section and therefore not match because of that too?

lldb/test/API/macosx/unregistered-macho/main.c

49

This is redundant (the test uses macho_buf).

lldb/tools/debugserver/source/MacOSX/MachProcess.mm

756

Could this set inf.valid instead of having to loop later to set it?

768

Personally I'd put this as a static function just before this one as it doesn't capture anything, but up to you.

[jasonmolenda]

lldb/test/API/macosx/unregistered-macho/TestUnregisteredMacho.py

47

Yeah, good suggestion. I wrote the two tests in the order they're in, and realized I couldn't match the images key in the second one; didn't go back and make the first one consistent.

lldb/test/API/macosx/unregistered-macho/main.c

33

I don't think it matters. I'm saying that this segment doesn't exist in the file. Quick check, I created a hello world program with a single BSS int, so it doesn't exist in the binary but is initialized to zero at runtime.

Load command 2
      cmd LC_SEGMENT_64
  cmdsize 152
  segname __DATA
   vmaddr 0x0000000100004000
   vmsize 0x0000000000004000
  fileoff 0
 filesize 0

49

I needed to increment p so it points to the end of the mach-o image if I want to write it to disk and try sending it through otool for testing. You're right though, this has no effect to the program itself.

lldb/tools/debugserver/source/MacOSX/MachProcess.mm

768

Yeah good point. It's operating on either a mach_header or mach_header_64 so there wasn't a single variable to capture and test.

[jasonmolenda]

One more patch update to address David's further feedback. This is ready to land now, will give it a day in case anyone has comments or suggestions.

[DavidSpickett]

Looks good from my perspective.

lldb/test/API/macosx/unregistered-macho/main.c

49

Got it, now I see it in the comments below. Fair enough.

[JDevlieghere]

LGTM

lldb/tools/debugserver/source/MacOSX/MachProcess.mm

749

Seems like a slightly odd name. is_valid_mach_header?

1021–1022

Nit: If you're touching this line this might be a good opportunity to fix up the weird quadruple/triple slash and additional space.

[jasonmolenda]

Final small patch revision.

lldb/test/API/macosx/unregistered-macho/Makefile

+C_SOURCES = main.c
+
+include Makefile.rules

lldb/test/API/macosx/unregistered-macho/TestUnregisteredMacho.py

+"""Test that debugserver will parse a mach-o in inferior memory even if it's not loaded."""
+
+import os
+import re
+import subprocess
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestUnregisteredMacho(TestBase):
+
+    # newer debugserver required for jGetLoadedDynamicLibrariesInfos 
+    # to support this
+    @skipIfOutOfTreeDebugserver  
+    @no_debug_info_test
+    @skipUnlessDarwin
+    def test(self):
+        self.build()
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "// break here", lldb.SBFileSpec("main.c"))
+
+        frame = thread.GetFrameAtIndex(0)
+        macho_buf = frame.GetValueForVariablePath("macho_buf")
+        gdb_packet = "process plugin packet send 'jGetLoadedDynamicLibrariesInfos:{\"solib_addresses\":[%d]}]'" % macho_buf.GetValueAsUnsigned() 
+
+        # Send the jGetLoadedDynamicLibrariesInfos packet
+        # to debugserver, asking it to parse the mach-o binary
+        # at this address and give us the UUID etc, even though
+        # dyld doesn't think there is a binary at that address.
+        # We won't get a pathname for the binary (from dyld), but
+        # we will get to the LC_UUID and include that.
+        self.expect (gdb_packet, substrs=['"pathname":""', '"uuid":"1B4E28BA-2FA1-11D2-883F-B9A761BDE3FB"'])

lldb/test/API/macosx/unregistered-macho/main.c

+#include <mach/machine.h>
+#include <mach-o/loader.h>
+#include <stdlib.h>
+#include <string.h>
+#include <uuid/uuid.h>
+
+int main ()
+{
+  int size_of_mh_and_cmds = sizeof (struct mach_header_64) + 
+                          sizeof (struct segment_command_64) +
+                          sizeof (struct uuid_command);
+  uint8_t *macho_buf = (uint8_t *) malloc 
+                         (size_of_mh_and_cmds);
+  uint8_t *p = macho_buf;
+  struct mach_header_64 mh;
+  mh.magic = MH_MAGIC_64;
+  mh.cputype = CPU_TYPE_ARM64;
+  mh.cpusubtype = 0;
+  mh.filetype = MH_EXECUTE;
+  mh.ncmds = 2;
+  mh.sizeofcmds = size_of_mh_and_cmds;
+  mh.flags = MH_NOUNDEFS | MH_DYLDLINK | MH_TWOLEVEL | MH_PIE;
+
+  memcpy (p, &mh, sizeof (mh));
+  p += sizeof (mh);
+
+  struct segment_command_64 seg;
+  seg.cmd = LC_SEGMENT_64;
+  seg.cmdsize = sizeof (seg);
+  strcpy (seg.segname, "__TEXT");
+  seg.vmaddr = 0x5000;
+  seg.vmsize = 0x1000;
+  seg.fileoff = 0;
+  seg.filesize = 0;
+  seg.maxprot = 0;
+  seg.initprot = 0;
+  seg.nsects = 0;
+  seg.flags = 0;
+
+  memcpy (p, &seg, sizeof (seg));
+  p += sizeof (seg);
+
+  struct uuid_command uuid;
+  uuid.cmd = LC_UUID;
+  uuid.cmdsize = sizeof (uuid);
+  uuid_clear (uuid.uuid);
+  uuid_parse ("1b4e28ba-2fa1-11d2-883f-b9a761bde3fb", uuid.uuid);
+
+  memcpy (p, &uuid, sizeof (uuid));
+  p += sizeof (uuid);
+
+  // If this needs to be debugged, the memory buffer can be written
+  // to a file with
+  // (lldb) mem rea -b -o /tmp/t -c `p - macho_buf` macho_buf
+  // (lldb) platform shell otool -hlv /tmp/t
+  // to verify that it is well formed.
+
+  // And inside lldb, it should be inspectable via
+  // (lldb) script print(lldb.frame.locals["macho_buf"][0].GetValueAsUnsigned())
+  // 105553162403968
+  // (lldb) process plugin packet send 'jGetLoadedDynamicLibrariesInfos:{"solib_addresses":[105553162403968]}]'
+
+  return 0;  // break here
+}

lldb/tools/debugserver/source/MacOSX/MachProcess.mm

         uint32_t mod_date_32;
         ::memcpy(&load_address_32, image_info_buf + (i * 3 * pointer_size), 4);
         ::memcpy(&pathname_address_32,
                  image_info_buf + (i * 3 * pointer_size) + pointer_size, 4);
         ::memcpy(&mod_date_32, image_info_buf + (i * 3 * pointer_size) +
                                    pointer_size + pointer_size,
                  4);
         info.load_address = load_address_32;
         info.mod_date = mod_date_32;
         pathname_address = pathname_address_32;
       } else {
         uint64_t load_address_64;
         uint64_t pathname_address_64;
         uint64_t mod_date_64;
         ::memcpy(&load_address_64, image_info_buf + (i * 3 * pointer_size), 8);
         ::memcpy(&pathname_address_64,
                  image_info_buf + (i * 3 * pointer_size) + pointer_size, 8);
         ::memcpy(&mod_date_64, image_info_buf + (i * 3 * pointer_size) +

 // using the
 // dyld SPIs that exist in macOS 10.12, iOS 10, tvOS 10, watchOS 3 and newer.
 JSONGenerator::ObjectSP MachProcess::GetLibrariesInfoForAddresses(
     nub_process_t pid, std::vector<uint64_t> &macho_addresses) {
   JSONGenerator::DictionarySP reply_sp;
 
   int pointer_size = GetInferiorAddrSize(pid);
 
+  // Collect the list of all binaries that dyld knows about in
+  // the inferior process.
   std::vector<struct binary_image_information> all_image_infos;
   GetAllLoadedBinariesViaDYLDSPI(all_image_infos);
   uint32_t platform = GetPlatform();
 
   std::vector<struct binary_image_information> image_infos;
   const size_t macho_addresses_count = macho_addresses.size();
   const size_t all_image_infos_count = all_image_infos.size();
+
   for (size_t i = 0; i < macho_addresses_count; i++) {
+    bool found_matching_entry = false;
     for (size_t j = 0; j < all_image_infos_count; j++) {
       if (all_image_infos[j].load_address == macho_addresses[i]) {
         image_infos.push_back(all_image_infos[j]);
+        found_matching_entry = true;
       }
     }
+    if (!found_matching_entry) {
+      // dyld doesn't think there is a binary at this address,
+      // but maybe there isn't a binary YET - let's look in memory
+      // for a proper mach-o header etc and return what we can.
+      // We will have an empty filename for the binary (because dyld
+      // doesn't know about it yet) but we can read all of the mach-o
+      // load commands from memory directly.
+      struct binary_image_information entry;
+      entry.load_address = macho_addresses[i];
+      image_infos.push_back(entry);
+    }
   }
 
     const size_t image_infos_count = image_infos.size();
     for (size_t i = 0; i < image_infos_count; i++) {
       GetMachOInformationFromMemory(platform,
                                     image_infos[i].load_address, pointer_size,
                                     image_infos[i].macho_info);
     }