Switch to a different library-loaded notification function breakpoint in Darwin dyld

[jasonmolenda]

Summary

Darwin's dynamic linker, dyld, calls a no-op function whenever libraries are loaded or removed from a process. There are two of these functions - one for older compatibility, which lldb is using, and actual one it calls first.

In a future patch, I'll change how lldb discovers the address of this function. lldb is currently not using the "canonical" one that I'll discover that way, and their arguments are slightly different, so this patch lays the groundwork by switching to this canonical notification function with the same discovery mechanism.

The important difference between these two is that the old function (lldb currently uses) is passed an array of uint64_t addresses of binary mach-o headers. In the new, canonical function, the headers are passed as an array of ptrsize -- so 32-bits, on a target like arm64_32 which is ILP32 (we don't have any genuine 32-bit targets that we still support). Beyond that, it's only a different function name.

[jasonmolenda]

oops, slight misstatement from not reading the patch when I wrote the description. The function we currently use has an array of uint64_t's, the one I am switching to is given an array of dyld_image_info objects, the first element has the ptrsize address of the mach-o header.

[jingham]

This makes sense. I think you should at least log if you can't read the memory dyld told you was where the image info should be. Seems like this is the sort of thing that shouldn't happen, but if it does we probably want to say something. Other than that minor grip, LGTM.

lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp

343

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.

[JDevlieghere]

lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp

336–337

Nit: pretty sure GetAddressByteSize() returns an unsigned (uint32_t or uint8_t).

343

+1

[jasonmolenda]

Update patch to address Jim and Jonas' feedback, most importantly report a warning if we can't read one of the mach-o binary addresses that dyld is telling us are being added/removed.

[jasonmolenda]

After reverting to avoid CI bot failures on macos intel, this is an updated patch that (1) corrects the binary image count size, (2) allows for a fallback if it can't find the notification function lldb_image_notifier, by name.

[jasonmolenda]

Update the patch to address the failures we were hitting on the Intel macOS 10.15 CI bot. In addition to lldb_notifier_function, look for "gdb_notifier_function" which is what it was called back in macOS 10.15. Read the notification function address out of the dyld_all_image_infos as a last resort, if dyld_all_image_infos has been initialized.

lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp

 // class DynamicLoader class decide if we should stop or not (based on global
 // preference).
 bool DynamicLoaderMacOS::NotifyBreakpointHit(void *baton,
                                              StoppointCallbackContext *context,
                                              lldb::user_id_t break_id,
                                              lldb::user_id_t break_loc_id) {
   // Let the event know that the images have changed
   // DYLD passes three arguments to the notification breakpoint.
-  // Arg1: enum dyld_notify_mode mode - 0 = adding, 1 = removing, 2 = remove
-  // all Arg2: unsigned long icount        - Number of shared libraries
-  // added/removed Arg3: uint64_t mach_headers[]     - Array of load addresses
-  // of binaries added/removed
+  //
+  // Arg1: enum dyld_notify_mode mode
+  // 0 = adding, 1 = removing, 2 = remove all, 3 = dyld moved
+  //
+  // Arg2: unsigned long count
+  // Number of shared libraries added/removed
+  //
+  // Arg3: struct dyld_image_info mach_headers[]
+  // Array of load addresses of binaries added/removed
 
   DynamicLoaderMacOS *dyld_instance = (DynamicLoaderMacOS *)baton;
 
   ExecutionContext exe_ctx(context->exe_ctx_ref);
   Process *process = exe_ctx.GetProcessPtr();
 
   // This is a sanity check just in case this dyld_instance is an old dyld
   // plugin's breakpoint still lying around.
if (abi) {
     TypeSystemClangSP scratch_ts_sp =
         ScratchTypeSystemClang::GetForTarget(process->GetTarget());
     if (!scratch_ts_sp)
       return false;
 
     ValueList argument_values;
 
     Value mode_value;    // enum dyld_notify_mode { dyld_notify_adding=0,
-                         // dyld_notify_removing=1, dyld_notify_remove_all=2 };
+                         // dyld_notify_removing=1, dyld_notify_remove_all=2,
+                         // dyld_notify_dyld_moved=3 };
     Value count_value;   // unsigned long count
-    Value headers_value; // uint64_t machHeaders[] (aka void*)
+    Value headers_value; // struct dyld_image_info machHeaders[]
 
     CompilerType clang_void_ptr_type =
         scratch_ts_sp->GetBasicType(eBasicTypeVoid).GetPointerType();
     CompilerType clang_uint32_type =
         scratch_ts_sp->GetBuiltinTypeForEncodingAndBitSize(lldb::eEncodingUint,
                                                            32);
     CompilerType clang_uint64_type =
         scratch_ts_sp->GetBuiltinTypeForEncodingAndBitSize(lldb::eEncodingUint,
if (abi) {
 
     headers_value.SetValueType(Value::ValueType::Scalar);
     headers_value.SetCompilerType(clang_void_ptr_type);
 
     argument_values.PushValue(mode_value);
     argument_values.PushValue(count_value);
     argument_values.PushValue(headers_value);
 
+    // void lldb_image_notifier(enum dyld_image_mode mode, uint32_t infoCount,
+    // const dyld_image_info info[])
+
     if (abi->GetArgumentValues(exe_ctx.GetThreadRef(), argument_values)) {
       uint32_t dyld_mode =
           argument_values.GetValueAtIndex(0)->GetScalar().UInt(-1);
       if (dyld_mode != static_cast<uint32_t>(-1)) {
         // Okay the mode was right, now get the number of elements, and the
         // array of new elements...
         uint32_t image_infos_count =
             argument_values.GetValueAtIndex(1)->GetScalar().UInt(-1);
         if (image_infos_count != static_cast<uint32_t>(-1)) {
           addr_t header_array =
               argument_values.GetValueAtIndex(2)->GetScalar().ULongLong(-1);
           if (header_array != static_cast<uint64_t>(-1)) {
             std::vector<addr_t> image_load_addresses;
+
+            // struct dyld_image_info_32 {
+            //     uint32_t                    imageLoadAddress;
+            //     uint32_t                    imageFilePath;
+            //     uint32_t                    imageFileModDate;
+            // };
+            // struct dyld_image_info_64 {
+            //     uint64_t                    imageLoadAddress;
+            //     uint64_t                    imageFilePath;
+            //     uint64_t                    imageFileModDate;
+            // };
+
+            uint32_t addr_size =
+                process->GetTarget().GetArchitecture().GetAddressByteSize();
             for (uint64_t i = 0; i < image_infos_count; i++) {
               Status error;
-              addr_t addr = process->ReadUnsignedIntegerFromMemory(
-                  header_array + (8 * i), 8, LLDB_INVALID_ADDRESS, error);
-              if (addr != LLDB_INVALID_ADDRESS) {
+              addr_t dyld_image_info = header_array + (3 * addr_size * i);
+              addr_t addr =
+                  process->ReadPointerFromMemory(dyld_image_info, error);
+              if (error.Success()) {
                 image_load_addresses.push_back(addr);
+              } else {
+                Debugger::ReportWarning(
+                    "DynamicLoaderMacOS::NotifyBreakpointHit unable "
+                    "to read binary mach-o load address at 0x%" PRIx64,
+                    addr);
               }
             }
             if (dyld_mode == 0) {
               // dyld_notify_adding
               if (process->GetTarget().GetImages().GetSize() == 0) {
                 // When all images have been removed, we're doing the
                 // dyld handover from a launch-dyld to a shared-cache-dyld,
                 // and we've just hit our one-shot address breakpoint in
if (abi->GetArgumentValues(exe_ctx.GetThreadRef(), argument_values)) {
               int addr_size =
                   process->GetTarget().GetArchitecture().GetAddressByteSize();
               addr_t notification_location = all_image_infos + 4 + // version
                                              4 +        // infoArrayCount
                                              addr_size; // infoArray
               Status error;
               addr_t notification_addr =
                   process->ReadPointerFromMemory(notification_location, error);
+              if (!error.Success()) {
+                Debugger::ReportWarning(
+                    "DynamicLoaderMacOS::NotifyBreakpointHit unable "
+                    "to read address of dyld-handover notification function at "
+                    "0x%" PRIx64,
+                    notification_location);
+              } else {
                 if (ABISP abi_sp = process->GetABI())
                   notification_addr = abi_sp->FixCodeAddress(notification_addr);
 
                 dyld_instance->SetDYLDHandoverBreakpoint(notification_addr);
               }
             }
           }
         }
       }
+    }
   } else {
     Target &target = process->GetTarget();
     Debugger::ReportWarning(
         "no ABI plugin located for triple " +
             target.GetArchitecture().GetTriple().getTriple() +
             ": shared libraries will not be registered",
         target.GetDebugger().GetID());
   }
if (dyld_sp) {
       LazyBool skip_prologue = eLazyBoolNo;
       FileSpecList *source_files = nullptr;
       FileSpecList dyld_filelist;
       dyld_filelist.Append(dyld_sp->GetFileSpec());
 
       Breakpoint *breakpoint =
           m_process->GetTarget()
               .CreateBreakpoint(&dyld_filelist, source_files,
-                                "_dyld_debugger_notification",
-                                eFunctionNameTypeFull, eLanguageTypeC, 0,
-                                skip_prologue, internal, hardware)
+                                "lldb_image_notifier", eFunctionNameTypeFull,
+                                eLanguageTypeC, 0, skip_prologue, internal,
+                                hardware)
               .get();
       breakpoint->SetCallback(DynamicLoaderMacOS::NotifyBreakpointHit, this,
                               true);
       breakpoint->SetBreakpointKind("shared-library-event");
       m_break_id = breakpoint->GetID();
     }
   }
   return m_break_id != LLDB_INVALID_BREAK_ID;