friss updated this revision to Diff 146268.
friss added a comment.
I heted the idea of hosting the once_flag in Process but using
it in PlatformPOSIX. I did a bigger refactoring where platforms
pass a factory lambda to the accessor and the thread-safe init
is done in the Process class itself using this lambda.
https://reviews.llvm.org/D46733
Files:
include/lldb/Target/Process.h
source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
source/Plugins/Platform/POSIX/PlatformPOSIX.h
source/Target/Process.cpp
Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -6160,14 +6160,12 @@
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);
-}
-
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.h
===================================================================
--- source/Plugins/Platform/POSIX/PlatformPOSIX.h
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.h
@@ -200,10 +200,10 @@
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);
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===================================================================
--- source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -927,10 +927,9 @@
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
@@ -980,7 +979,6 @@
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();
@@ -1013,9 +1011,7 @@
}
// 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,
@@ -1036,18 +1032,16 @@
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;
Index: include/lldb/Target/Process.h
===================================================================
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -804,7 +804,7 @@
}
//------------------------------------------------------------------
- // 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 @@
//
// 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 @@
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-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
