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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.


================
Comment at: lldb/test/API/macosx/unregistered-macho/TestUnregisteredMacho.py:46
+        self.expect (both_gdb_packet, substrs=['response: {"images":[{"load_address":%d,' % macho_addr])
+        self.expect (both_gdb_packet, substrs=['response: {"load_address":%d,' % invalid_macho_addr], matching=False)
+
----------------
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?


================
Comment at: lldb/test/API/macosx/unregistered-macho/main.c:48
+  memcpy(p, &uuid, sizeof(uuid));
+  p += sizeof(uuid);
+
----------------
This is redundant (the test uses `macho_buf`).


================
Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:756
 
 bool MachProcess::GetMachOInformationFromMemory(
     uint32_t dyld_platform, nub_addr_t mach_o_header_addr, int wordsize,
----------------
Could this set `inf.valid` instead of having to loop later to set it?


================
Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:768
+    return true;
+  };
+
----------------
Personally I'd put this as a static function just before this one as it doesn't capture anything, but up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128956