Add a lock to PlatformPOSIX::DoLoadImage

Summary:
Multiple threads could be calling into DoLoadImage concurrently,
only one should be allowed to create the UtilityFunction.

Reviewers: jingham

Subscribers: emaste, lldb-commits

Differential Revision: https://reviews.llvm.org/D46733

llvm-svn: 332115

lldb/include/lldb/Target/Process.h

@@ -804,7 +804,7 @@ public:
   }
 
   //------------------------------------------------------------------
-  // FUTURE WORK: {Set,Get}LoadImageUtilityFunction are the first use we've
+  // FUTURE WORK: GetLoadImageUtilityFunction 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
@@ -819,35 +819,24 @@ public:
   //
   // 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.
-  /// 
+  /// @param[in] factory
+  ///     A function that will be called only once per-process in a
+  ///     thread-safe way to create the UtilityFunction if it has not
+  ///     been initialized yet.
+  ///
   /// @return
   ///     The cached utility function or null if the platform is not the
   ///     same as the target's platform.
   //------------------------------------------------------------------
-  UtilityFunction *GetLoadImageUtilityFunction(Platform *platform);
+  UtilityFunction *GetLoadImageUtilityFunction(
+      Platform *platform,
+      llvm::function_ref<std::unique_ptr<UtilityFunction>()> factory);
 
   //------------------------------------------------------------------
   /// Get the dynamic loader plug-in for this process.
@@ -3127,6 +3116,7 @@ protected:
   enum { eCanJITDontKnow = 0, eCanJITYes, eCanJITNo } m_can_jit;
   
   std::unique_ptr<UtilityFunction> m_dlopen_utility_func_up;
+  std::once_flag m_dlopen_utility_func_flag_once;
 
   size_t RemoveBreakpointOpcodesFromBuffer(lldb::addr_t addr, size_t size,
                                            uint8_t *buf) const;

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

@@ -929,10 +929,9 @@ Status PlatformPOSIX::EvaluateLibdlExpression(
   return Status();
 }
 
-UtilityFunction *
-PlatformPOSIX::MakeLoadImageUtilityFunction(ExecutionContext &exe_ctx, 
-                                            Status &error)
-{
+std::unique_ptr<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
@@ -982,7 +981,6 @@ PlatformPOSIX::MakeLoadImageUtilityFunction(ExecutionContext &exe_ctx,
   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();
@@ -1015,9 +1013,7 @@ PlatformPOSIX::MakeLoadImageUtilityFunction(ExecutionContext &exe_ctx,
   }
   
   // 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;
+  return dlopen_utility_func_up;
 }
 
 uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process *process,
@@ -1038,18 +1034,16 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process *process,
   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);
+  UtilityFunction *dlopen_utility_func;
   ValueList arguments;
   FunctionCaller *do_dlopen_function = nullptr;
-  
-  if (!dlopen_utility_func) {
-    // Make the UtilityFunction:
-    dlopen_utility_func = MakeLoadImageUtilityFunction(exe_ctx, 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.
+  dlopen_utility_func = process->GetLoadImageUtilityFunction(
+      this, [&]() -> std::unique_ptr<UtilityFunction> {
+        return 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;

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

@@ -199,10 +199,10 @@ protected:
   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); 
+
+  std::unique_ptr<lldb_private::UtilityFunction>
+  MakeLoadImageUtilityFunction(lldb_private::ExecutionContext &exe_ctx,
+                               lldb_private::Status &error);
 
   virtual
   llvm::StringRef GetLibdlFunctionDeclarations(lldb_private::Process *process);

lldb/source/Target/Process.cpp

@@ -6164,14 +6164,12 @@ Status Process::UpdateAutomaticSignalFiltering() {
   return Status();
 }
 
-UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+UtilityFunction *Process::GetLoadImageUtilityFunction(
+    Platform *platform,
+    llvm::function_ref<std::unique_ptr<UtilityFunction>()> factory) {
   if (platform != GetTarget().GetPlatform().get())
     return nullptr;
+  std::call_once(m_dlopen_utility_func_flag_once,
+                 [&] { m_dlopen_utility_func_up = factory(); });
   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);
-}
-