Use a utility function to speed up "process load image" for POSIX Platforms

[jingham]

Summary

The original implementation of PlatformPosix::DoLoadImage compiled the expression to load the image & check the error a fresh every time. That was causing performance problems in some cases where lldb had to load a bunch of images into the process.

This patch replaces that mechanism with an lldb UtilityFunction, so we can JIT the required code once per process and then reuse it on subsequent calls. The work to set up a UtilityFunction is simpler than to compile an expression, so even in the case where you only do one load this will be faster. But then if you are doing many it will be much faster.

The only tricky bit in this is that the Platform holds the DoLoadImage code, but the UtilityFunction has to be per process. Rather than trying to get the Platform to track the Targets & Processes that use it (which it does not do at present) I put the Utility function in the Process, and hand it out from there.

I think I got the android dlopen remapping header in there correctly, but I don't have a way to test that, so if somebody on the Android side could check that out, that would be helpful.

[davide]

First round of comments. Thanks for this!

source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

1075–1078 ↗

(On Diff #142702)

Where the 2*addr_size is coming from? Probably we should add a comment.

1117 ↗

(On Diff #142702)

why are we ignoring breakpoints?

1121 ↗

(On Diff #142702)

Shouldd we try setting up a slightly more generous timeout?

[jingham]

Commented inline.

source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

1075–1078 ↗

(On Diff #142702)

I'll add a comment. I describe the return structure a bit above where I fill in the argument vector - it's two pointers. But I'll make it clear that's what this is.

1117 ↗

(On Diff #142702)

This wasn't really a breakpoint that was hit by code initiated by the app you are debugging, so it makes more sense to treat it as an internal matter than to report breakpoint hits to the user.

Suppose the user had put a breakpoint on strcmp. Then they did some operation that turned on an instrumentation function, which in turn required the instrumentation code to load a library. You wouldn't want that operation to be interrupted mid-course, and return to the user with a "Stopped at breakpoint 1.1" UI. That would be disconcerting - especially since from the user's perspective the target isn't even running - and the user would have to figure out what she should do to fix the situation.

There really isn' t much benefit to stopping in whatever breakpoints dlopen hit while doing this load operation.

1121 ↗

(On Diff #142702)

I didn't change the timeout: 2 seconds is what most of the utility functions use. We haven't had any reports of this being too short.

[labath]

Thanks for the heads up. I can test the android part on Wednesday, but right now I don't see a reason why that shouldn't work there.

Overall, I like the idea of using the process class for caching some platform resources. If we end up needing to do that more often, we can think of making using some more generic container for that instead of making a pair of getters/setters for each value.

Apart from the low-level inline comments, the one high-level comment I have is that the DoLoadImage function is getting a bit big. I think it would help readability to split it up a bit. At least the get-cached-function-or-build-one-if-it-is-not-present part seems like it could be easily separated out into a helper function.

source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

1036–1037 ↗

(On Diff #142702)

If SetLoadImageUtilityFunction takes a unique_ptr then you can do a SetLoadImageUtilityFunction(std::move(dlopen_utility_func_up)) here.

source/Target/Process.cpp

6251 ↗

(On Diff #142702)

Will this ever be true? I would not expect one to be able to change the platform of a process mid-session.

6256 ↗

(On Diff #142702)

I think the argument of this function should be a unique_ptr as well.

[clayborg]

include/lldb/Target/Process.h

811 ↗

(On Diff #142702)

Fix comment to say "Set" instead of "Get"

837 ↗

(On Diff #142702)

I would think that saving utility functions can be used by other plug-ins. It might be nice to change these two functions to take a name + utility function for the setter. So these functions would look like:

void SetCachedUtilityFunction(ConstString name, UtilityFunction *utility_func);
UtilityFunction *GetCachedUtilityFunction(ConstString name);

Then other plug-ins could cache utility function. I would rather not have each new plug-in that needs this to have to add specific accessor functions.

3165 ↗

(On Diff #142702)

Make this a map of ConstString -> std::unique_ptr<UtilityFunction> if we end up using SetCachedUtilityFunction as described above

[labath]

I don't think doing this is necessary when we have only one customer, but if we are going to be designing an general purpose storage facility then I don't think we should be using strings (and particularly not ConstStrings) as the lookup keys.

I would propose going for the the token based approach where each customer has a "token" (it can be just a char) and then you use the address of that token as the key.

Besides being faster, it is more auditable as only the thing that has access to the token can manipulate the data. Also the system could be extended in the future to store arbitrary objects in a type-safe way by making a Token<T> template.

[clayborg]

In D45703#1069921, @labath wrote:

I don't think doing this is necessary when we have only one customer, but if we are going to be designing an general purpose storage facility then I don't think we should be using strings (and particularly not ConstStrings) as the lookup keys.

I would propose going for the the token based approach where each customer has a "token" (it can be just a char) and then you use the address of that token as the key.

a "const void *" is fine as the key. In this case it could be the "Platform *" or the pointer to the ConstString name of the platform plug-in.

Besides being faster, it is more auditable as only the thing that has access to the token can manipulate the data. Also the system could be extended in the future to store arbitrary objects in a type-safe way by making a Token<T> template.

That is fine. For the helper functions though, I would rather us implement this now even if we have only one customer as the next person will see the pattern and say "I need to add my own custom functions to add accessors for my utility function", so I would rather do this right in this patch if possible.

[jingham]

I'm not sure what facility we are designing here? If it is only utility functions, I think that is a very narrow use case, and I'm not sure it needs a special purpose facility. I would prefer to wait till I see a second instance for that. After all, this is not SB API, we can change it as we go along. I would prefer to just leave a comment "if you do this again make it more general" to avoid copying the pattern if somebody's ever going to do that.

If it is more general "I'm going to stash something in the process" then we need to know who owns the thing? Will it be enough to just have the process destroy it when it goes away? Do we want to notify the registree? For this particular use case, I know the answer, but this is specifically a cache, which can always be rebuilt if we get nothing from the cache. Is that what we're modeling in general...

I feel like it is premature to design this store till we have some more prospective uses.

source/Target/Process.cpp

6251 ↗

(On Diff #142702)

Reading through the code I could not convince myself that it could NOT happen. Target::SetPlatform is a function anybody can call at any time. I agree that it would be odd to do so, and we might think about prohibiting it (Target::SetPlatform could return an error, and forbid changing the Platform, if the Target has a live process.)

If everybody agrees that that is a good idea, I can do that and remove this check.

6256 ↗

(On Diff #142702)

Okay, I'll try that out.

[zturner]

source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

957–958 ↗

(On Diff #142702)

Can we use the version that returns a std::string? I consider this version unsafe as it might not always return the full path

[jingham]

Address review comments.

I changed over to passing a up to get stored in the process.
I separated the maker code into a separate function.
I changed GetPath versions (the buffer one was in the original, but I agree the std::string one is better.)
I didn't address the "changing platforms mid-stream" comment. If we want to make that a policy, I think it's better to do that separately.
I didn't add a general purpose facility to cache in the Process, since I don't have enough examples to know what to do there.

[clayborg]

I am fine if we don't want to do the general solution now. LGTM if all is well in the test suite.

[labath]

I like the idea of leaving some comment to have a record that we discussed making a more generic storage facility, but I don't think we need to do anything else right now. I doubt we will see many new clients for this very soon.

source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

1166 ↗

(On Diff #142800)

Use LLDB_INVALID_IMAGE_TOKEN here?

source/Target/Process.cpp

6251 ↗

(On Diff #142702)

I see three call in the codebase to SetPlatform. All of them seem to be in the initialization code, though some of them seem to happen after the process is launched (DynamicLoaderDarwinKernel constructor).

So it may not be possible to completely forbid setting the platform this way, but I think we can at least ban changing the platform once it has already been set in a pretty hard way (lldb_assert or even assert). I think a lot of things would currently break if someone started changing the platforms of a running process all of a sudden.

[jingham]

I added a comment, I'll upload the diff with that in a sec.

source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

1166 ↗

(On Diff #142800)

That wouldn't be right. LLDB_INVALID_IMAGE_TOKEN is the invalid token in lldb's namespace for image loading tokens, which has no relationship to any given platform's invalid token specifier. In fact, LLDB_INVALID_IMAGE_TOKEN is UINT32_MAX, so it is actually different from the POSIX invalid image token.

source/Target/Process.cpp

6251 ↗

(On Diff #142702)

Not sure. It seems reasonable to make a target, run it against one platform, then switch the platform and run it again against the new platform. I'm not sure I'm comfortable saying that a target can only ever use one platform.

[jingham]

Added a comment describing our discussions for a general "stash this in the process" facility.

[jingham]

The only outstanding question is whether we should enforce "Targets can't change their platforms" in which case we could remove the check in GetLoadImageUtilityFunction. The check does no harm either way, and this doesn't seem the right forum to decide the larger policy question. The testsuite is clean, so I'm going to check this in.

[labath]

source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

1166 ↗

(On Diff #142800)

I see. Thanks for the explanation.

source/Target/Process.cpp

6251 ↗

(On Diff #142702)

Yeah, you're right. I suppose that makes sense. In my mind a target was associated with a given platform from the moment it got created (that is how I always do things: platform select, platform connect, and only then target create). I never tried what would happen if I reversed that. So I guess the platform can change during the lifetime of a Target, but I hope that is not the case for the lifetime of a Process.

lldb/trunk/include/lldb/Target/Process.h

virtual Status DoLoadCore() {
     Status error;
     error.SetErrorStringWithFormat(
         "error: %s does not support loading core files.",
         GetPluginName().GetCString());
     return error;
   }
 
   //------------------------------------------------------------------
+  // FUTURE WORK: {Set,Get}LoadImageUtilityFunction are the first use we've
+  // had of having other plugins cache data in the Process.  This is handy for
+  // long-living plugins - like the Platform - which manage interactions whose 
+  // lifetime is governed by the Process lifetime.  If we find we need to do 
+  // this more often, we should construct a general solution to the problem.
+  // The consensus suggestion was that we have a token based registry in the
+  // Process.
+  // Some undecided questions are 
+  // (1) who manages the tokens.  It's probably best that you add the element 
+  // and get back a token that represents it.  That will avoid collisions.  But
+  // there may be some utility in the registerer controlling the token?
+  // (2) whether the thing added should be simply owned by Process, and
+  // just go away when it does
+  // (3) whether the registree should be notified of the Process' demise.
+  //
+  // We are postponing designing this till we have at least a second use case.
+  //------------------------------------------------------------------
+  //------------------------------------------------------------------
+  /// Set the cached UtilityFunction that assists in loading binary
+  /// images into the process.
+  ///
+  /// This UtilityFunction is maintained in the Process since the Platforms
+  /// don't track the lifespan of the Targets/Processes that use them.  
+  /// But it is not intended to be comprehended by the Process, it's up to the
+  /// Platform that set it to do it right.
+  ///
+  /// @param[in] utility_func_up
+  ///     The incoming utility_function.  The process will manage the function's
+  ///     lifetime.
+  ///
+  //------------------------------------------------------------------
+  void SetLoadImageUtilityFunction(std::unique_ptr<UtilityFunction> 
+                                   utility_func_up);
+  
+  //------------------------------------------------------------------
+  /// Get the cached UtilityFunction that assists in loading binary
+  /// images into the process.
+  ///
+  /// @param[in] platform
+  ///     The platform fetching the UtilityFunction.
+  /// 
+  /// @return
+  ///     The cached utility function or null if the platform is not the
+  ///     same as the target's platform.
+  //------------------------------------------------------------------
+  UtilityFunction *GetLoadImageUtilityFunction(Platform *platform);
+
+  //------------------------------------------------------------------
   /// Get the dynamic loader plug-in for this process.
   ///
   /// The default action is to let the DynamicLoader plug-ins check
   /// the main executable and the DynamicLoader will select itself
   /// automatically. Subclasses can override this if inspecting the
   /// executable is not desired, or if Process subclasses can only
   /// use a specific DynamicLoader plug-in.
   //------------------------------------------------------------------
bool m_can_interpret_function_calls;  // Some targets, e.g the OSX kernel,
                                         // don't support the ability to modify
                                         // the stack.
   WarningsCollection m_warnings_issued; // A set of object pointers which have
                                         // already had warnings printed
   std::mutex m_run_thread_plan_lock;
   StructuredDataPluginMap m_structured_data_plugin_map;
 
   enum { eCanJITDontKnow = 0, eCanJITYes, eCanJITNo } m_can_jit;
   
+  std::unique_ptr<UtilityFunction> m_dlopen_utility_func_up;
+
   size_t RemoveBreakpointOpcodesFromBuffer(lldb::addr_t addr, size_t size,
                                            uint8_t *buf) const;
 
   void SynchronouslyNotifyStateChanged(lldb::StateType state);
 
   void SetPublicState(lldb::StateType new_state, bool restarted);
 
   void SetPrivateState(lldb::StateType state);

lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h

std::map<lldb_private::CommandInterpreter *,
       m_options;
   lldb::PlatformSP m_remote_platform_sp; // Allow multiple ways to connect to a
                                          // remote POSIX-compliant OS
 
   lldb_private::Status
   EvaluateLibdlExpression(lldb_private::Process *process, const char *expr_cstr,
                           llvm::StringRef expr_prefix,
                           lldb::ValueObjectSP &result_valobj_sp);
   
+  lldb_private::UtilityFunction *                        
+  MakeLoadImageUtilityFunction(lldb_private::ExecutionContext &exe_ctx, 
+                               lldb_private::Status &error); 
+
   virtual
   llvm::StringRef GetLibdlFunctionDeclarations(lldb_private::Process *process);
 
 private:
   DISALLOW_COPY_AND_ASSIGN(PlatformPOSIX);
 };
 
 #endif // liblldb_PlatformPOSIX_h_

lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

 // C++ Includes
 // Other libraries and framework includes
 // Project includes
 
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/ValueObject.h"
+#include "lldb/Expression/DiagnosticManager.h"
+#include "lldb/Expression/FunctionCaller.h"
 #include "lldb/Expression/UserExpression.h"
+#include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileCache.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/ProcessLaunchInfo.h"
 #include "lldb/Target/Thread.h"
+#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
Status PlatformPOSIX::EvaluateLibdlExpression(
   if (result != eExpressionCompleted)
     return expr_error;
 
   if (result_valobj_sp->GetError().Fail())
     return result_valobj_sp->GetError();
   return Status();
 }
 
+UtilityFunction *
+PlatformPOSIX::MakeLoadImageUtilityFunction(ExecutionContext &exe_ctx, 
+                                            Status &error)
+{
+  // Remember to prepend this with the prefix from GetLibdlFunctionDeclarations.
+  // The returned values are all in __lldb_dlopen_result for consistency.
+  // The wrapper returns a void * but doesn't use it because
+  // UtilityFunctions don't work with void returns at present.
+  static const char *dlopen_wrapper_code = R"(
+  struct __lldb_dlopen_result {
+    void *image_ptr;
+    const char *error_str;
+  };
+
+  void * __lldb_dlopen_wrapper (const char *path, 
+                                __lldb_dlopen_result *result_ptr)
+  {
+    result_ptr->image_ptr = dlopen(path, 2);
+    if (result_ptr->image_ptr == (void *) 0x0)
+      result_ptr->error_str = dlerror();
+    return nullptr;
+  }
+  )";
+
+  static const char *dlopen_wrapper_name = "__lldb_dlopen_wrapper";
+  Process *process = exe_ctx.GetProcessSP().get();
+  // Insert the dlopen shim defines into our generic expression:
+  std::string expr(GetLibdlFunctionDeclarations(process));
+  expr.append(dlopen_wrapper_code);
+  Status utility_error;
+  DiagnosticManager diagnostics;
+  
+  std::unique_ptr<UtilityFunction> dlopen_utility_func_up(process
+      ->GetTarget().GetUtilityFunctionForLanguage(expr.c_str(),
+                                                  eLanguageTypeObjC,
+                                                  dlopen_wrapper_name,
+                                                  utility_error));
+  if (utility_error.Fail()) {
+    error.SetErrorStringWithFormat("dlopen error: could not make utility"
+                                   "function: %s", utility_error.AsCString());
+    return nullptr;
+  }
+  if (!dlopen_utility_func_up->Install(diagnostics, exe_ctx)) {
+    error.SetErrorStringWithFormat("dlopen error: could not install utility"
+                                   "function: %s", 
+                                   diagnostics.GetString().c_str());
+    return nullptr;
+  }
+
+  Value value;
+  ValueList arguments;
+  FunctionCaller *do_dlopen_function = nullptr;
+  UtilityFunction *dlopen_utility_func = nullptr;
+
+  // Fetch the clang types we will need:
+  ClangASTContext *ast = process->GetTarget().GetScratchClangASTContext();
+
+  CompilerType clang_void_pointer_type
+      = ast->GetBasicType(eBasicTypeVoid).GetPointerType();
+  CompilerType clang_char_pointer_type
+        = ast->GetBasicType(eBasicTypeChar).GetPointerType();
+
+  // We are passing two arguments, the path to dlopen, and a pointer to the
+  // storage we've made for the result:
+  value.SetValueType(Value::eValueTypeScalar);
+  value.SetCompilerType(clang_void_pointer_type);
+  arguments.PushValue(value);
+  value.SetCompilerType(clang_char_pointer_type);
+  arguments.PushValue(value);
+  
+  do_dlopen_function = dlopen_utility_func_up->MakeFunctionCaller(
+      clang_void_pointer_type, arguments, exe_ctx.GetThreadSP(), utility_error);
+  if (utility_error.Fail()) {
+    error.SetErrorStringWithFormat("dlopen error: could not make function"
+                                   "caller: %s", utility_error.AsCString());
+    return nullptr;
+  }
+  
+  do_dlopen_function = dlopen_utility_func_up->GetFunctionCaller();
+  if (!do_dlopen_function) {
+    error.SetErrorString("dlopen error: could not get function caller.");
+    return nullptr;
+  }
+  
+  // We made a good utility function, so cache it in the process:
+  dlopen_utility_func = dlopen_utility_func_up.get();
+  process->SetLoadImageUtilityFunction(std::move(dlopen_utility_func_up));
+  return dlopen_utility_func;
+}
+
 uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process *process,
                                     const lldb_private::FileSpec &remote_file,
                                     lldb_private::Status &error) {
-  char path[PATH_MAX];
-  remote_file.GetPath(path, sizeof(path));
+  std::string path;
+  path = remote_file.GetPath();
   
-  StreamString expr;
-  expr.Printf(R"(
-                   struct __lldb_dlopen_result { void *image_ptr; const char *error_str; } the_result;
-                   the_result.image_ptr = dlopen ("%s", 2);
-                   if (the_result.image_ptr == (void *) 0x0)
-                   {
-                       the_result.error_str = dlerror();
-                   }
-                   else
-                   {
-                       the_result.error_str = (const char *) 0x0;
-                   }
-                   the_result;
-                  )",
-              path);
-  llvm::StringRef prefix = GetLibdlFunctionDeclarations(process);
-  lldb::ValueObjectSP result_valobj_sp;
-  error = EvaluateLibdlExpression(process, expr.GetData(), prefix,
-                                  result_valobj_sp);
-  if (error.Fail())
+  ThreadSP thread_sp = process->GetThreadList().GetExpressionExecutionThread();
+  if (!thread_sp) {
+    error.SetErrorString("dlopen error: no thread available to call dlopen.");
+    return LLDB_INVALID_IMAGE_TOKEN;
+  
+  DiagnosticManager diagnostics;
+  
+  ExecutionContext exe_ctx;
+  thread_sp->CalculateExecutionContext(exe_ctx);
+
+  Status utility_error;
+  
+  // The UtilityFunction is held in the Process.  Platforms don't track the 
+  // lifespan of the Targets that use them, we can't put this in the Platform.
+  UtilityFunction *dlopen_utility_func 
+      = process->GetLoadImageUtilityFunction(this);
+  ValueList arguments;
+  FunctionCaller *do_dlopen_function = nullptr;
+  
+  if (!dlopen_utility_func) {
+    // Make the UtilityFunction:
+    dlopen_utility_func = MakeLoadImageUtilityFunction(exe_ctx, error);
+  // If we couldn't make it, the error will be in error, so we can exit here.
+  if (!dlopen_utility_func)
     return LLDB_INVALID_IMAGE_TOKEN;
     
-  error = result_valobj_sp->GetError();
-  if (error.Fail())
+  do_dlopen_function = dlopen_utility_func->GetFunctionCaller();
+  if (!do_dlopen_function) {
+    error.SetErrorString("dlopen error: could not get function caller.");
     return LLDB_INVALID_IMAGE_TOKEN;
+  }
+  arguments = do_dlopen_function->GetArgumentValues();
   
-  Scalar scalar;
-  ValueObjectSP image_ptr_sp = result_valobj_sp->GetChildAtIndex(0, true);
-  if (!image_ptr_sp || !image_ptr_sp->ResolveValue(scalar)) {
-    error.SetErrorStringWithFormat("unable to load '%s'", path);
+  // Now insert the path we are searching for and the result structure into
+  // the target.
+  uint32_t permissions = ePermissionsReadable|ePermissionsWritable;
+  size_t path_len = path.size() + 1;
+  lldb::addr_t path_addr = process->AllocateMemory(path_len, 
+                                                   permissions,
+                                                   utility_error);
+  if (path_addr == LLDB_INVALID_ADDRESS) {
+    error.SetErrorStringWithFormat("dlopen error: could not allocate memory"
+                                    "for path: %s", utility_error.AsCString());
     return LLDB_INVALID_IMAGE_TOKEN;
   }
   
-  addr_t image_ptr = scalar.ULongLong(LLDB_INVALID_ADDRESS);
-  if (image_ptr != 0 && image_ptr != LLDB_INVALID_ADDRESS)
-    return process->AddImageToken(image_ptr);
-
-  if (image_ptr == 0) {
-    ValueObjectSP error_str_sp = result_valobj_sp->GetChildAtIndex(1, true);
-    if (error_str_sp) {
-      DataBufferSP buffer_sp(new DataBufferHeap(10240, 0));
-      size_t num_chars =
-          error_str_sp->ReadPointedString(buffer_sp, error, 10240).first;
-      if (error.Success() && num_chars > 0)
-        error.SetErrorStringWithFormat("dlopen error: %s",
-                                       buffer_sp->GetBytes());
-      else
-        error.SetErrorStringWithFormat("dlopen failed for unknown reasons.");
-      return LLDB_INVALID_IMAGE_TOKEN;
-    }
+  // Make sure we deallocate the input string memory:
+  CleanUp path_cleanup([process, path_addr] { 
+      process->DeallocateMemory(path_addr); 
+  });
+  
+  process->WriteMemory(path_addr, path.c_str(), path_len, utility_error);
+  if (utility_error.Fail()) {
+    error.SetErrorStringWithFormat("dlopen error: could not write path string:"
+                                    " %s", utility_error.AsCString());
+    return LLDB_INVALID_IMAGE_TOKEN;
+  }
+  
+  // Make space for our return structure.  It is two pointers big: the token and
+  // the error string.
+  const uint32_t addr_size = process->GetAddressByteSize();
+  lldb::addr_t return_addr = process->CallocateMemory(2*addr_size,
+                                                      permissions,
+                                                      utility_error);
+  if (utility_error.Fail()) {
+    error.SetErrorStringWithFormat("dlopen error: could not allocate memory"
+                                    "for path: %s", utility_error.AsCString());
+    return LLDB_INVALID_IMAGE_TOKEN;
+  }
+  
+  // Make sure we deallocate the result structure memory
+  CleanUp return_cleanup([process, return_addr] {
+      process->DeallocateMemory(return_addr);
+  });
+  
+  // Set the values into our args and write them to the target:
+  arguments.GetValueAtIndex(0)->GetScalar() = path_addr;
+  arguments.GetValueAtIndex(1)->GetScalar() = return_addr;
+  
+  lldb::addr_t func_args_addr = LLDB_INVALID_ADDRESS;
+  
+  diagnostics.Clear();
+  if (!do_dlopen_function->WriteFunctionArguments(exe_ctx, 
+                                                 func_args_addr,
+                                                 arguments,
+                                                 diagnostics)) {
+    error.SetErrorStringWithFormat("dlopen error: could not write function "
+                                   "arguments: %s", 
+                                   diagnostics.GetString().c_str());
+    return LLDB_INVALID_IMAGE_TOKEN;
+  }
+  
+  // Make sure we clean up the args structure.  We can't reuse it because the
+  // Platform lives longer than the process and the Platforms don't get a
+  // signal to clean up cached data when a process goes away.
+  CleanUp args_cleanup([do_dlopen_function, &exe_ctx, func_args_addr] {
+    do_dlopen_function->DeallocateFunctionResults(exe_ctx, func_args_addr);
+  });
+  
+  // Now run the caller:
+  EvaluateExpressionOptions options;
+  options.SetExecutionPolicy(eExecutionPolicyAlways);
+  options.SetLanguage(eLanguageTypeC_plus_plus);
+  options.SetIgnoreBreakpoints(true);
+  options.SetUnwindOnError(true);
+  options.SetTrapExceptions(false); // dlopen can't throw exceptions, so
+                                    // don't do the work to trap them.
+  options.SetTimeout(std::chrono::seconds(2));
+  
+  Value return_value;
+  // Fetch the clang types we will need:
+  ClangASTContext *ast = process->GetTarget().GetScratchClangASTContext();
+
+  CompilerType clang_void_pointer_type
+      = ast->GetBasicType(eBasicTypeVoid).GetPointerType();
+
+  return_value.SetCompilerType(clang_void_pointer_type);
+  
+  ExpressionResults results = do_dlopen_function->ExecuteFunction(
+      exe_ctx, &func_args_addr, options, diagnostics, return_value);
+  if (results != eExpressionCompleted) {
+    error.SetErrorStringWithFormat("dlopen error: could write execute "
+                                   "dlopen wrapper function: %s", 
+                                   diagnostics.GetString().c_str());
+    return LLDB_INVALID_IMAGE_TOKEN;
+  }
+  
+  // Read the dlopen token from the return area:
+  lldb::addr_t token = process->ReadPointerFromMemory(return_addr, 
+                                                      utility_error);
+  if (utility_error.Fail()) {
+    error.SetErrorStringWithFormat("dlopen error: could not read the return "
+                                    "struct: %s", utility_error.AsCString());
+  
+  // The dlopen succeeded!
+  if (token != 0x0)
+    return process->AddImageToken(token);
+    
+  // We got an error, lets read in the error string:
+  std::string dlopen_error_str;
+  lldb::addr_t error_addr 
+    = process->ReadPointerFromMemory(return_addr + addr_size, utility_error);
+  if (utility_error.Fail()) {
+    error.SetErrorStringWithFormat("dlopen error: could not read error string: "
+                                    "%s", utility_error.AsCString());
+    return LLDB_INVALID_IMAGE_TOKEN;
   }
-  error.SetErrorStringWithFormat("unable to load '%s'", path);
+  
+  size_t num_chars = process->ReadCStringFromMemory(error_addr + addr_size, 
+                                                    dlopen_error_str, 
+                                                    utility_error);
+  if (utility_error.Success() && num_chars > 0)
+    error.SetErrorStringWithFormat("dlopen error: %s",
+                                   dlopen_error_str.c_str());
+  else
+    error.SetErrorStringWithFormat("dlopen failed for unknown reasons.");
+
   return LLDB_INVALID_IMAGE_TOKEN;
 }
 
 Status PlatformPOSIX::UnloadImage(lldb_private::Process *process,
                                   uint32_t image_token) {
   const addr_t image_addr = process->GetImagePtrFromToken(image_token);
   if (image_addr == LLDB_INVALID_ADDRESS)
     return Status("Invalid image token");

lldb/trunk/source/Target/Process.cpp

 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/State.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/IRDynamicChecks.h"
 #include "lldb/Expression/UserExpression.h"
+#include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Host/Pipe.h"
 #include "lldb/Host/Terminal.h"
 #include "lldb/Host/ThreadLauncher.h"
bool Process::RouteAsyncStructuredData(
   return true;
 }
 
 Status Process::UpdateAutomaticSignalFiltering() {
   // Default implementation does nothign.
   // No automatic signal filtering to speak of.
   return Status();
 }
+
+UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+  if (platform != GetTarget().GetPlatform().get())
+    return nullptr;
+  return m_dlopen_utility_func_up.get();
+}
+
+void Process::SetLoadImageUtilityFunction(std::unique_ptr<UtilityFunction> 
+                                          utility_func_up) {
+  m_dlopen_utility_func_up.swap(utility_func_up);
+}
+