jasonmolenda 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)
+
----------------
DavidSpickett wrote:
> 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?
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.
================
Comment at: lldb/test/API/macosx/unregistered-macho/main.c:48
+ memcpy(p, &uuid, sizeof(uuid));
+ p += sizeof(uuid);
+
----------------
DavidSpickett wrote:
> This is redundant (the test uses `macho_buf`).
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.
================
Comment at: lldb/test/API/macosx/unregistered-macho/main.c:33
+ seg.vmsize = 0x1000;
+ seg.fileoff = 0;
+ seg.filesize = 0;
----------------
DavidSpickett wrote:
> Does this need to be `sizeof (struct mach_header_64)`? Probably doesn't
> and/or it doesn't matter for this test.
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
```
================
Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:768
+ return true;
+ };
+
----------------
DavidSpickett wrote:
> Personally I'd put this as a static function just before this one as it
> doesn't capture anything, but up to you.
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128956/new/
https://reviews.llvm.org/D128956
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
