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

Tue Sep 13 15:00:46 PDT 2022

jasonmolenda added inline comments.

================
Comment at: lldb/include/lldb/Target/DynamicLoader.h:231
+  ///     Name of the binary, if available.  If an empty StringRef is
+  ///     passed here, a name will be constructed for the module.
+  ///
----------------
DavidSpickett wrote:
> Can you list what it might be constructed from? My guess:
> * the filename if loading from a path
> * <format>@<address> if loading from something in memory
Thanks, this should have been clearer.  Yeah this name is only used when a file cannot be found on the debug host; it may construct a module out of live memory.  And in that case, it needs to make up a name for it.

================
Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp:420
         LLDB_LOGF(log,
-                  "ProcessMachCore::DoLoadCore: Using kernel corefile image "
+                  "ProcessMachCore::LoadBinariesAndSetDYLD: Using kernel "
+                  "corefile image "
----------------
DavidSpickett wrote:
> I think the various log messages could use `__FUNCTION__` instead but hey let's not make this diff any bigger :) .
> (the one time I know you need to write out the name is inside a lambda)
LOL I even updated the names of all the log messages to reflect their new methods, but of course you're right.  

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133680