Add support to lldb for reading a "load binary" LC_NOTE with Mach-O corefiles

[jasonmolenda]

Summary

For some of our non-userland / non-kernel environments, there can be multiple binary images involved in the environment, and each environment has a different way of discovering these binaries and would require a specialized DynamicLoader plugin in lldb to handle. This patch adds a new "load binary" LC_NOTE for corefiles which allows the corefile creator to list the binaries that lldb should attempt to load, and where to load them, for viewing the corefile.

The actual patch to read this new LC_NOTE is a dozen lines thanks to the "all image infos" support I added earlier this year; the mechanisms are similar. "all image infos" always gave us the segment load addresses for each binary, but "load binaries" allows for load address or a slide to be specified. So I added support to handle those cases in ObjectFileMachO, and added logging to help debug any problems.

I updated my TestCorefileDefaultPtrauth.py test from https://reviews.llvm.org/D115431 to add this new LC_NOTE to the corefile that it creates. Previously, TestCorefileDefaultPtrauth.py was manually loading the a.out binary and setting its load address; now the API test load the executable into a Target and deletes the Target, to get it registered in lldb's global module cache. Then we load the corefile and the binary is discovered by its UUID. No additional tests were needed to confirm the binary was loaded; the test looking at a global will fail if it is not.

[DavidSpickett]

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

6984

Seems like the cast wouldn't be needed:

const char *GetCStr(lldb::offset_t *offset_ptr) const;

7005

Maybe not for this change but if you made this early return you could dedent all the following by one level.

if (!image_infos.IsValid())
  return added_images;

7035

You can use the LLDB_LOGF to do the if (log) for you I think. Then do image.uuid.GetAsString().c_str() directly as one of the parameters.

7065

Is the slide address the amount you slide the load address *by* or is it an address all to itself? My assumption was the former.
(which makes the log message a bit odd since it's a distance not an amount)

7077

unslid sounds a bit like you removed a slide from it, I'd go with "without a slide" but that's just me.

[jasonmolenda]

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

7005

Oh yes, nice.

7035

Oh that's clever, but I prefer the Printf method so I'll do it this way.

7065

Yeah, I was banging out these log messages past bedtime trying to wrap it up, and wasn't as clear as I meant to be. Cleaned these up, thanks.

[jasonmolenda]

Update patch to address the points David made. Thanks!

[jasonmolenda]

Clean up the way the newly loaded Module is added to the Target, and we notify for breakpoint etc evaluation in ObjectFileMachO::LoadCoreFileImages.

[DavidSpickett]

LGTM assuming the change in TestFunctionStarts.py wasn't meant to be included.

lldb/test/API/macosx/function-starts/TestFunctionStarts.py

68 ↗

(On Diff #393668)

Stray change?

[jasonmolenda]

lldb/test/API/macosx/function-starts/TestFunctionStarts.py

68 ↗

(On Diff #393668)

*cough* lol I was looking into a testsuite failure and forgot ;)

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

protected:
   /// A corefile may include metadata about all of the binaries that were
   /// present in the process when the corefile was taken.  This is only
   /// implemented for Mach-O files for now; we'll generalize it when we
   /// have other systems that can include the same.
   struct MachOCorefileImageEntry {
     std::string filename;
     lldb_private::UUID uuid;
     lldb::addr_t load_address = LLDB_INVALID_ADDRESS;
-    bool currently_executing;
+    lldb::addr_t slide = 0;
+    bool currently_executing = false;
     std::vector<std::tuple<lldb_private::ConstString, lldb::addr_t>>
         segment_load_addresses;
   };
 
   struct LCNoteEntry {
     LCNoteEntry(uint32_t addr_byte_size, lldb::ByteOrder byte_order)
         : payload(lldb_private::Stream::eBinary, addr_byte_size, byte_order) {}
 

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

if (lc.cmd == LC_NOTE) {
             seg_vmaddrs_offset += 8; /* unused */
 
             std::tuple<ConstString, addr_t> new_seg{ConstString(segname),
                                                     vmaddr};
             image_entry.segment_load_addresses.push_back(new_seg);
           }
           image_infos.all_image_infos.push_back(image_entry);
         }
+      } else if (strcmp("load binary", data_owner) == 0) {
+        uint32_t version = m_data.GetU32(&fileoff);
+        if (version == 1) {
+          uuid_t uuid;
+          memcpy(&uuid, m_data.GetData(&fileoff, sizeof(uuid_t)),
+                 sizeof(uuid_t));
+          uint64_t load_address = m_data.GetU64(&fileoff);
+          uint64_t slide = m_data.GetU64(&fileoff);
+          std::string filename = m_data.GetCStr(&fileoff);
+
+          MachOCorefileImageEntry image_entry;
+          image_entry.filename = filename;
+          image_entry.uuid = UUID::fromData(uuid, sizeof(uuid_t));
+          image_entry.load_address = load_address;
+          image_entry.slide = slide;
+          image_infos.all_image_infos.push_back(image_entry);
+        }
       }
     }
     offset = cmd_offset + lc.cmdsize;
   }
 
   return image_infos;
 }
 
 bool ObjectFileMachO::LoadCoreFileImages(lldb_private::Process &process) {
   MachOCorefileAllImageInfos image_infos = GetCorefileAllImageInfos();
-  bool added_images = false;
-  if (image_infos.IsValid()) {
-    for (const MachOCorefileImageEntry &image : image_infos.all_image_infos) {
-      ModuleSpec module_spec;
-      module_spec.GetUUID() = image.uuid;
-      module_spec.GetFileSpec() = FileSpec(image.filename.c_str());
-      if (image.currently_executing) {
-        Symbols::DownloadObjectAndSymbolFile(module_spec, true);
-        if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
-          process.GetTarget().GetOrCreateModule(module_spec, false);
-        }
-      }
-      Status error;
-      ModuleSP module_sp =
-          process.GetTarget().GetOrCreateModule(module_spec, false, &error);
-      if (!module_sp.get() || !module_sp->GetObjectFile()) {
-        if (image.load_address != LLDB_INVALID_ADDRESS) {
-          module_sp = process.ReadModuleFromMemory(module_spec.GetFileSpec(),
-                                                   image.load_address);
-        }
-      }
-      if (module_sp.get()) {
-        added_images = true;
+  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+
+  ModuleList added_modules;
+      // Will call ModulesDidLoad with all modules once they've all
+      // been added to the Target with load addresses.  Don't notify
+      // here, before the load address is set.
+      const bool notify = false;
+      process.GetTarget().GetImages().AppendIfNeeded(module_sp, notify);
+      added_modules.Append(module_sp, notify);
+      if (image.segment_load_addresses.size() > 0) {
+        if (log) {
+          std::string uuidstr = image.uuid.GetAsString();
+          log->Printf("ObjectFileMachO::LoadCoreFileImages adding binary '%s' "
+                      "UUID %s with section load addresses",
+                      image.filename.c_str(), uuidstr.c_str());
+        }
         for (auto name_vmaddr_tuple : image.segment_load_addresses) {
           SectionList *sectlist = module_sp->GetObjectFile()->GetSectionList();
           if (sectlist) {
             SectionSP sect_sp =
                 sectlist->FindSectionByName(std::get<0>(name_vmaddr_tuple));
             if (sect_sp) {
               process.GetTarget().SetSectionLoadAddress(
                   sect_sp, std::get<1>(name_vmaddr_tuple));
             }
           }
         }
+      } else if (image.load_address != LLDB_INVALID_ADDRESS) {
+        if (log) {
+          std::string uuidstr = image.uuid.GetAsString();
+          log->Printf("ObjectFileMachO::LoadCoreFileImages adding binary '%s' "
+                      "UUID %s with load address 0x%" PRIx64,
+                      image.filename.c_str(), uuidstr.c_str(),
+                      image.load_address);
         }
+        const bool address_is_slide = false;
+        bool changed = false;
+        module_sp->SetLoadAddress(process.GetTarget(), image.load_address,
+                                  address_is_slide, changed);
+      } else if (image.slide != 0) {
+        if (log) {
+          std::string uuidstr = image.uuid.GetAsString();
+          log->Printf("ObjectFileMachO::LoadCoreFileImages adding binary '%s' "
+                      "UUID %s with slide amount 0x%" PRIx64,
+                      image.filename.c_str(), uuidstr.c_str(), image.slide);
+        }
+        const bool address_is_slide = true;
+        bool changed = false;
+        module_sp->SetLoadAddress(process.GetTarget(), image.slide,
+                                  address_is_slide, changed);
+      } else {
+        if (log) {
+          std::string uuidstr = image.uuid.GetAsString();
+          log->Printf("ObjectFileMachO::LoadCoreFileImages adding binary '%s' "
+                      "UUID %s at its file address, no slide applied",
+                      image.filename.c_str(), uuidstr.c_str());
         }
+        const bool address_is_slide = true;
+        bool changed = false;
+        module_sp->SetLoadAddress(process.GetTarget(), 0, address_is_slide,
+                                  changed);
       }
-  return added_images;
+    }
+  }
+  if (added_modules.GetSize() > 0) {
+    process.GetTarget().ModulesDidLoad(added_modules);
+    process.Flush();
+    return true;
+  }
+  return false;
 }

lldb/test/API/macosx/corefile-default-ptrauth/TestCorefileDefaultPtrauth.py

 
 class TestCorefileDefaultPtrauth(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @skipIf(debug_info=no_match(["dsym"]), bugnumber="This test is looking explicitly for a dSYM")
     @skipIf(archs=no_match(['arm64','arm64e']))
     @skipUnlessDarwin
+    @skipIfRemote
     def test_lc_note(self):
         self.build()
         self.test_exe = self.getBuildArtifact("a.out")
         self.create_corefile = self.getBuildArtifact("create-corefile")
         self.corefile = self.getBuildArtifact("core")
 
         ### Create our corefile
         retcode = call(self.create_corefile + " " +  self.test_exe + " " + self.corefile, shell=True)
 
         ## This corefile has no metadata telling us how many bits are
         ## used for ptrauth signed function pointers.  We will need lldb
         ## to fall back on its old default value for Darwin arm64 ABIs
         ## to correctly strip the bits.
 
+        # Create a Target with our main executable binary to get it
+        # seeded in lldb's global module cache.  Then delete the Target.
+        # This way when the corefile searches for a binary with its UUID,
+        # it'll be found by that search.
+        initial_target = self.dbg.CreateTarget(self.test_exe)
+        self.dbg.DeleteTarget(initial_target)
+
         self.target = self.dbg.CreateTarget('')
         err = lldb.SBError()
         self.process = self.target.LoadCore(self.corefile)
         self.assertEqual(self.process.IsValid(), True)
-        modspec = lldb.SBModuleSpec()
-        modspec.SetFileSpec(lldb.SBFileSpec(self.test_exe, True))
-        m = self.target.AddModule(modspec)
-        self.assertTrue(m.IsValid())
-        self.target.SetModuleLoadAddress (m, 0)
-        
         # target variable should show us both the actual function
         # pointer with ptrauth bits and the symbol it resolves to,
         # with the ptrauth bits stripped, e.g.
         #  (int (*)(...)) fmain = 0xe46bff0100003f90 (actual=0x0000000100003f90 a.out`main at main.c:3)
 
         self.expect("target variable fmain", substrs=['fmain = 0x', '(actual=0x', 'main at main.c'])

lldb/test/API/macosx/corefile-default-ptrauth/create-corefile.c

 #include <mach-o/loader.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <mach/machine.h>
 #include <string.h>
 #include <mach/machine/thread_state.h>
 #include <inttypes.h>
 #include <sys/syslimits.h>
+#include <uuid/uuid.h>
 
 // Given an executable binary with 
 //   "fmain" (a function pointer to main)
 //   "main"
 // symbols, create a fake arm64e corefile that
 // contains a memory segment for the fmain 
 // function pointer, with the value of the 
 // address of main() with ptrauth bits masked on.
if (strstr (buf, "_fmain")) {
       fmain_addr = strtoul (buf, NULL, 16);
     }
     if (strstr (buf, "_main")) {
       main_addr = strtoul (buf, NULL, 16);
     }
   }
   pclose (nm);
 
+  sprintf (buf, "dwarfdump -u '%s'", argv[1]);
+  FILE *dwarfdump = popen(buf, "r");
+  if (!dwarfdump) {
+    fprintf (stderr, "Unable to run dwarfdump -u on '%s'\n", argv[1]);
+    exit (1);
+  }
+  uuid_t uuid;
+  uuid_clear (uuid);
+  while (fgets (buf, sizeof(buf), dwarfdump)) {
+    if (strncmp (buf, "UUID: ", 6) == 0) {
+      buf[6 + 36] = '\0';
+      if (uuid_parse (buf + 6, uuid) != 0) {
+        fprintf (stderr, "Unable to parse UUID in '%s'\n", buf);
+        exit (1);
+      }
+    }
+  }
+  if (uuid_is_null(uuid)) {
+    fprintf (stderr, "Got a null uuid for the binary\n");
+    exit (1);
+  }
+
   if (main_addr == 0 || fmain_addr == 0) {
     fprintf(stderr, "Unable to find address of main or fmain in %s.\n",
         argv[1]);
     exit (1);
   }
 
   // Write out a corefile with contents in this order:
   //    1. mach header
   //    2. LC_THREAD load command
   //    3. LC_SEGMENT_64 load command
-  //    4. memory segment contents
+  //    4. LC_NOTE load command
+  //    5. memory segment contents
+  //    6. "load binary" note contents
 
   // struct thread_command {
   //       uint32_t        cmd;    
   //       uint32_t        cmdsize;
   //       uint32_t flavor      
   //       uint32_t count       
   //       struct XXX_thread_state state
   int size_of_thread_cmd = 4 + 4 + 4 + 4 + sizeof (arm_thread_state64_t);
 
   struct mach_header_64 mh;
   mh.magic = 0xfeedfacf;
   mh.cputype = CPU_TYPE_ARM64;
   mh.cpusubtype = CPU_SUBTYPE_ARM64E;
   mh.filetype = MH_CORE;
-  mh.ncmds = 2; // LC_THREAD, LC_SEGMENT_64
-  mh.sizeofcmds = size_of_thread_cmd + sizeof(struct segment_command_64);
+  mh.ncmds = 3; // LC_THREAD, LC_SEGMENT_64, LC_NOTE
+  mh.sizeofcmds = size_of_thread_cmd + sizeof(struct segment_command_64) + sizeof(struct note_command);
   mh.flags = 0;
   mh.reserved = 0;
 
   fwrite(&mh, sizeof (mh), 1, out);
 
+  struct note_command lcnote;
   struct segment_command_64 seg;
   seg.cmd = LC_SEGMENT_64;
   seg.cmdsize = sizeof(seg);
   memset (&seg.segname, 0, 16);
   seg.vmaddr = fmain_addr;
   seg.vmsize = 8;
   // Offset to segment contents
-  seg.fileoff = sizeof (mh) + size_of_thread_cmd + sizeof(seg);
+  seg.fileoff = sizeof (mh) + size_of_thread_cmd + sizeof(seg) + sizeof(lcnote);
   seg.filesize = 8;
   seg.maxprot = 3;
   seg.initprot = 3;
   seg.nsects = 0;
   seg.flags = 0;
 
   fwrite (&seg, sizeof (seg), 1, out);
 
   uint32_t cmd = LC_THREAD;
   fwrite (&cmd, sizeof (cmd), 1, out);
   uint32_t cmdsize = size_of_thread_cmd;
   fwrite (&cmdsize, sizeof (cmdsize), 1, out);
   uint32_t flavor = ARM_THREAD_STATE64;
   fwrite (&flavor, sizeof (flavor), 1, out);
   // count is number of uint32_t's of the register context
   uint32_t count = sizeof (arm_thread_state64_t) / 4;
   fwrite (&count, sizeof (count), 1, out);
   arm_thread_state64_t regstate;
   memset (&regstate, 0, sizeof (regstate));
   fwrite (&regstate, sizeof (regstate), 1, out);
 
+  lcnote.cmd = LC_NOTE;
+  lcnote.cmdsize = sizeof (lcnote);
+  strcpy (lcnote.data_owner, "load binary");
+
+  // 8 is the size of the LC_SEGMENT contents
+  lcnote.offset = sizeof (mh) + size_of_thread_cmd + sizeof(seg) + sizeof(lcnote) + 8;
+
+  // struct load_binary
+  // {
+  // uint32_t version;        // currently 1
+  // uuid_t   uuid;           // all zeroes if uuid not specified
+  // uint64_t load_address;   // virtual address where the macho is loaded, UINT64_MAX if unavail
+  // uint64_t slide;          // slide to be applied to file address to get load address, 0 if unavail
+  // char     name_cstring[]; // must be nul-byte terminated c-string, '\0' alone if name unavail
+  // } __attribute__((packed));
+  lcnote.size = 4 + 16 + 8 + 8 + sizeof("a.out");
+
+  fwrite (&lcnote, sizeof(lcnote), 1, out);
+
+  // Write the contents of the memory segment
 
   // Or together a random PAC value from a system using 39 bits 
   // of addressing with the address of main().  lldb will need
   // to correctly strip off the high bits to find the address of
   // main.
   uint64_t segment_contents = 0xe46bff0000000000 | main_addr;
-
   fwrite (&segment_contents, sizeof (segment_contents), 1, out);
 
+  // Now write the contents of the "load binary" LC_NOTE.
+  {
+    uint32_t version = 1;
+    fwrite (&version, sizeof (version), 1, out);
+    fwrite (&uuid, sizeof (uuid), 1, out);
+    uint64_t load_address = UINT64_MAX;
+    fwrite (&load_address, sizeof (load_address), 1, out);
+    uint64_t slide = 0;
+    fwrite (&slide, sizeof (slide), 1, out);
+    strcpy (buf, "a.out");
+    fwrite (buf, 6, 1, out);
+  }
+
   fclose (out);
 
   exit (0);
 }