[Lldb-commits] [PATCH] D139453: Switch to a different library-loaded notification function breakpoint in Darwin dyld

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp:307-308
+
+            int addr_size =
+                process->GetTarget().GetArchitecture().GetAddressByteSize();
             for (uint64_t i = 0; i < image_infos_count; i++) {
----------------
Nit: pretty sure `GetAddressByteSize()` returns an unsigned (`uint32_t` or `uint8_t`). 


================
Comment at: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp:314
+                  process->ReadPointerFromMemory(dyld_image_info, error);
+              if (error.Success()) {
                 image_load_addresses.push_back(addr);
----------------
jingham wrote:
> Is it expected that this memory read fail?  It seems weird to just not handle that case at all?  Maybe at least log something here, otherwise it looks like we just get nothing w/o knowing why.
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139453