[Lldb-commits] [PATCH] D133680: Add support for mach-o corefile loading of a platform binary

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added reviewers: JDevlieghere, jingham.
jasonmolenda added a project: LLDB.
Herald added a reviewer: ributzka.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

This builds on the work in https://reviews.llvm.org/D133534 where support for a platform binary in live process debugging was added, and adds it to the Mach-O corefile loading as well.  It has five changes:

1. Add a name field to `DynamicLoader::LoadBinaryWithUUIDAndAddress` when we have a name, but may end up reading the binary out of memory, so we can use the name instead of a generic placeholder.

2. In `ObjectFileMachO::LoadCoreFileImages`, check if the image address is a platform binary, load it and set the Platform/DynamicLoader as appropriate.

3. Clean up the different cases in `LoadCoreFileImages` that handle corefile binary loading so it is clearer to read.

4. Separate `ProcessMachCore::DoLoadCore` into half a dozen smaller helper methods.  This method had grown quite large and it was difficult to trace the code flow through the different parts, and the complexity was unnecessary.

5. In the new `ProcessMachCore::LoadBinariesViaMetadata` method which handles LC_NOTE type hints about binaries, when we have a `main bin spec` LC_NOTE that is explicitly a kernel, don't load the binary here -- just save the address and note that this will be a `DynamicLoaderDarwinKernel` dynamic loader, and leave the loading up to DynamicLoaderDarwinKernel to do correctly.

The minor cleanups in `ObjectFileMachO::LoadCoreFileImages` are easy to read in the phabracator if you only read the new version of the code.  The (seriously needed) separation of `ObjectFileMachO::LoadCoreFileImages` into separate methods has made the diff very difficult to read.  In my own reviewing of the patch, I've settled on simply reading through the new version of the file.  Most of it is simple code movement, but I did make some cleanups to how sections of code were working -- for instance in the exhaustive search for a dyld or kernel binary, when multiple images that appear to be a kernel are found, I handle this better now by allowing DynamicLoaderDarwinKernel to pick the kernel image.  It's common that we get fragments in a corefile that look similar to a kernel and can confuse the exhaustive search algorithm.

Similar to the problem of testing this with a live process, to test this I'd need to construct a corefile with a Mach-O fileset with a kernel image in it.  It would not be easy to construct a test case without a corefile writer that synthesized all of this together, a not small bit of code, and it would also need to modify a test binary's mach header to look enough like a kernel that lldb would be tricked into using it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133680

Files:
  lldb/include/lldb/Target/DynamicLoader.h
  lldb/source/Core/DynamicLoader.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133680.459398.patch
Type: text/x-patch
Size: 29876 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220912/13227a0e/attachment-0001.bin>