[Lldb-commits] [lldb] r332115 - Add a lock to PlatformPOSIX::DoLoadImage
Frederic Riss via lldb-commits
lldb-commits at lists.llvm.org
Fri May 11 11:21:11 PDT 2018
Author: friss
Date: Fri May 11 11:21:11 2018
New Revision: 332115
URL: http://llvm.org/viewvc/llvm-project?rev=332115&view=rev
Log:
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
Modified:
lldb/trunk/include/lldb/Target/Process.h
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h
lldb/trunk/source/Target/Process.cpp
Modified: lldb/trunk/include/lldb/Target/Process.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=332115&r1=332114&r2=332115&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Fri May 11 11:21:11 2018
@@ -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
@@ -820,34 +820,23 @@ 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;
Modified: lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp?rev=332115&r1=332114&r2=332115&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp Fri May 11 11:21:11 2018
@@ -929,10 +929,9 @@ Status PlatformPOSIX::EvaluateLibdlExpre
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::MakeLoadImageUtilityFunct
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::MakeLoadImageUtilityFunct
}
// 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
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;
Modified: lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h?rev=332115&r1=332114&r2=332115&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h (original)
+++ lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h Fri May 11 11:21:11 2018
@@ -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);
Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=332115&r1=332114&r2=332115&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Fri May 11 11:21:11 2018
@@ -6164,14 +6164,12 @@ Status Process::UpdateAutomaticSignalFil
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);
-}
-
More information about the lldb-commits
mailing list