[Lldb-commits] [PATCH] D128956: make debugserver able to inspect mach-o binaries present in memory, but not yet registered with dyld

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM



================
Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:749
 
+static bool mach_header_validity_test (uint32_t magic, uint32_t cputype) {
+  if (magic != MH_MAGIC && magic != MH_CIGAM && magic != MH_MAGIC_64 &&
----------------
Seems like a slightly odd name. `is_valid_mach_header`?


================
Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:1021-1022
 
-    ////  First the image_infos array with (load addr, pathname, mod date)
-    ///tuples
-
-    for (size_t i = 0; i < image_count; i++) {
-      struct binary_image_information info;
-      nub_addr_t pathname_address;
-      if (pointer_size == 4) {
-        uint32_t load_address_32;
-        uint32_t pathname_address_32;
-        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) +
-                                   pointer_size + pointer_size,
-                 8);
-        info.load_address = load_address_64;
-        info.mod_date = mod_date_64;
-        pathname_address = pathname_address_64;
-      }
-      char strbuf[17];
-      info.filename = "";
-      uint64_t pathname_ptr = pathname_address;
-      bool still_reading = true;
-      while (still_reading &&
-             ReadMemory(pathname_ptr, sizeof(strbuf) - 1, strbuf) ==
-                 sizeof(strbuf) - 1) {
-        strbuf[sizeof(strbuf) - 1] = '\0';
-        info.filename += strbuf;
-        pathname_ptr += sizeof(strbuf) - 1;
-        // Stop if we found nul byte indicating the end of the string
-        for (size_t i = 0; i < sizeof(strbuf) - 1; i++) {
-          if (strbuf[i] == '\0') {
-            still_reading = false;
-            break;
-          }
+  ////  First the image_infos array with (load addr, pathname, mod date)
+  /// tuples
+
----------------
Nit: If you're touching this line this might be a good opportunity to fix up the weird quadruple/triple slash and additional space. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128956/new/

https://reviews.llvm.org/D128956