[Lldb-commits] [lldb] r302327 - Be a little more permissive in DynamicLoaderMacOS::CanLoadImage

[Jim Ingham via lldb-commits]

Author: jingham
Date: Fri May  5 20:15:47 2017
New Revision: 302327

URL: http://llvm.org/viewvc/llvm-project?rev=302327&view=rev
Log:
Be a little more permissive in DynamicLoaderMacOS::CanLoadImage

If we can't find the "is dyld locked" symbol, assume it is safe
to load the image unless we only have 1 image loaded - in which case
we are in _dyld_start and it is definitely NOT safe.

Also add a little better errors to that function, and better logging
in SBProcess.cpp.

<rdar://problem/30174817>

Modified:
    lldb/trunk/source/API/SBProcess.cpp
    lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp

Modified: lldb/trunk/source/API/SBProcess.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBProcess.cpp?rev=302327&r1=302326&r2=302327&view=diff
==============================================================================
--- lldb/trunk/source/API/SBProcess.cpp (original)
+++ lldb/trunk/source/API/SBProcess.cpp Fri May  5 20:15:47 2017
@@ -1157,22 +1157,34 @@ uint32_t SBProcess::LoadImage(lldb::SBFi
 uint32_t SBProcess::LoadImage(const lldb::SBFileSpec &sb_local_image_spec,
                               const lldb::SBFileSpec &sb_remote_image_spec,
                               lldb::SBError &sb_error) {
+  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
   ProcessSP process_sp(GetSP());
   if (process_sp) {
     Process::StopLocker stop_locker;
     if (stop_locker.TryLock(&process_sp->GetRunLock())) {
+      if (log)
+        log->Printf("SBProcess(%p)::LoadImage() => calling Platform::LoadImage"
+                    "for: %s",
+                    static_cast<void *>(process_sp.get()),
+                    sb_local_image_spec.GetFilename());
+
       std::lock_guard<std::recursive_mutex> guard(
-          process_sp->GetTarget().GetAPIMutex());
+        process_sp->GetTarget().GetAPIMutex());
       PlatformSP platform_sp = process_sp->GetTarget().GetPlatform();
       return platform_sp->LoadImage(process_sp.get(), *sb_local_image_spec,
                                     *sb_remote_image_spec, sb_error.ref());
     } else {
-      Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
       if (log)
         log->Printf("SBProcess(%p)::LoadImage() => error: process is running",
                     static_cast<void *>(process_sp.get()));
       sb_error.SetErrorString("process is running");
     }
+  } else { 
+    if (log)
+      log->Printf("SBProcess(%p)::LoadImage() => error: called with invalid"
+                    " process",
+                    static_cast<void *>(process_sp.get()));
+    sb_error.SetErrorString("process is invalid");
   }
   return LLDB_INVALID_IMAGE_TOKEN;
 }

Modified: lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp?rev=302327&r1=302326&r2=302327&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp (original)
+++ lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp Fri May  5 20:15:47 2017
@@ -434,24 +434,25 @@ Error DynamicLoaderMacOS::CanLoadImage()
 
   // Default assumption is that it is OK to load images.
   // Only say that we cannot load images if we find the symbol in libdyld and it
-  // indicates that
-  // we cannot.
+  // indicates that we cannot.
 
   if (symbol_address != LLDB_INVALID_ADDRESS) {
     {
       int lock_held =
           m_process->ReadUnsignedIntegerFromMemory(symbol_address, 4, 0, error);
       if (lock_held != 0) {
-        error.SetErrorToGenericError();
+        error.SetErrorString("dyld lock held - unsafe to load images.");
       }
     }
   } else {
     // If we were unable to find _dyld_global_lock_held in any modules, or it is
-    // not loaded into
-    // memory yet, we may be at process startup (sitting at _dyld_start) - so we
-    // should not allow
-    // dlopen calls.
-    error.SetErrorToGenericError();
+    // not loaded into memory yet, we may be at process startup (sitting 
+    // at _dyld_start) - so we should not allow dlopen calls.
+    // But if we found more than one module then we are clearly past _dyld_start
+    // so in that case we'll default to "it's safe".
+    if (num_modules <= 1)
+        error.SetErrorString("could not find the dyld library or "
+                                       "the dyld lock symbol");
   }
   return error;
 }