[lldb/Plugins] Introduce Scripted Platform Plugin #99814
Conversation
|
@llvm/pr-subscribers-lldb Author: Med Ismail Bennani (medismailben) ChangesThis patch introduces Scripted Platform, a new platform plugin that can be customized with a python script. For now this can list processes described in the python script file but eventually, it will be used to spawn scripted processes and act as an interface between them. This patch is also a less intrusive implementation of 2d53527. It introduces a new PlatformMetadata held by every platform that contains various objects that might be used by a platform instance. In its current form, the PlatformMetadata holds a reference to the Debugger and a ScriptedMetadata pointer. These are necessary in other to instanciate the scripted object that the ScriptedPlatform interacts with. In order to make it less introsive with the rest of lldb's platform creation code, platform metadata are set after the platform creation, and requires to platform to reload them (using This approach has the tradeoff that the ScriptedPlaform instance is technically invalid and useless right after its creation. However, the user should never be in that situation, since we reload the platform metadata everytime with create or select the platform. This work was previously reviewed in: Patch is 46.91 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/99814.diff 35 Files Affected:
diff --git a/lldb/bindings/interface/SBPlatformDocstrings.i b/lldb/bindings/interface/SBPlatformDocstrings.i
index ef09d5bce13fd..b8cdf21e90f82 100644
--- a/lldb/bindings/interface/SBPlatformDocstrings.i
+++ b/lldb/bindings/interface/SBPlatformDocstrings.i
@@ -29,3 +29,15 @@ and executable type. If the architecture or executable type do not match,
a suitable platform will be found automatically."
) lldb::SBPlatform;
+
+%feature("docstring", "
+Create a platform instance using a specific platform plugin name, debugger,
+script name and user-provided dictionary.
+
+:param platform_name: name of the platform plugin to use to create the platform
+:param debugger: debugger instance owning the platform
+:param script_name: name of the script class and module to use to create the platform
+:param dict: user-provided dictionary that can be used when instanciating a platform
+") lldb::SBPlatform (const char *, const lldb::SBDebugger&,
+ const char *, const lldb::SBStructuredData&);
+
diff --git a/lldb/bindings/python/python-swigsafecast.swig b/lldb/bindings/python/python-swigsafecast.swig
index 34f8c6f0ff8d3..f006101c4ce2d 100644
--- a/lldb/bindings/python/python-swigsafecast.swig
+++ b/lldb/bindings/python/python-swigsafecast.swig
@@ -104,6 +104,16 @@ PythonObject SWIGBridge::ToSWIGWrapper(lldb::DataExtractorSP data_sp) {
SWIGTYPE_p_lldb__SBData);
}
+PythonObject ToSWIGWrapper(lldb::ProcessLaunchInfoSP launch_info_sp) {
+ return ToSWIGHelper(new lldb::ProcessLaunchInfoSP(std::move(launch_info_sp)),
+ SWIGTYPE_p_lldb__SBLaunchInfo);
+}
+
+PythonObject ToSWIGWrapper(lldb::ProcessAttachInfoSP attach_info_sp) {
+ return ToSWIGHelper(new lldb::ProcessAttachInfoSP(std::move(attach_info_sp)),
+ SWIGTYPE_p_lldb__SBAttachInfo);
+}
+
ScopedPythonObject<lldb::SBCommandReturnObject>
SWIGBridge::ToSWIGWrapper(CommandReturnObject &cmd_retobj) {
return ScopedPythonObject<lldb::SBCommandReturnObject>(
diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig
index 8f050643fa68b..24dafc7b3fcd8 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -675,6 +675,42 @@ void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBMemoryRegionInfo(PyOb
return sb_ptr;
}
+void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBDebugger(PyObject * data) {
+ lldb::SBDebugger *sb_ptr = nullptr;
+
+ int valid_cast =
+ SWIG_ConvertPtr(data, (void **)&sb_ptr, SWIGTYPE_p_lldb__SBDebugger, 0);
+
+ if (valid_cast == -1)
+ return NULL;
+
+ return sb_ptr;
+}
+
+void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBTarget(PyObject * data) {
+ lldb::SBTarget *sb_ptr = nullptr;
+
+ int valid_cast =
+ SWIG_ConvertPtr(data, (void **)&sb_ptr, SWIGTYPE_p_lldb__SBTarget, 0);
+
+ if (valid_cast == -1)
+ return NULL;
+
+ return sb_ptr;
+}
+
+void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBProcess(PyObject * data) {
+ lldb::SBProcess *sb_ptr = nullptr;
+
+ int valid_cast =
+ SWIG_ConvertPtr(data, (void **)&sb_ptr, SWIGTYPE_p_lldb__SBProcess, 0);
+
+ if (valid_cast == -1)
+ return NULL;
+
+ return sb_ptr;
+}
+
bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommand(
const char *python_function_name, const char *session_dictionary_name,
lldb::DebuggerSP debugger, const char *args,
diff --git a/lldb/examples/python/templates/scripted_platform.py b/lldb/examples/python/templates/scripted_platform.py
index 5805f99dea4ca..23073d4981ea5 100644
--- a/lldb/examples/python/templates/scripted_platform.py
+++ b/lldb/examples/python/templates/scripted_platform.py
@@ -60,14 +60,17 @@ def get_process_info(self, pid):
pass
@abstractmethod
- def attach_to_process(self, attach_info):
- """Attach to a process.
-
+ def attach_to_process(self, attach_info, target, debugger, error):
+ """ Attach to a process.
+
Args:
attach_info (lldb.SBAttachInfo): The information related to attach to a process.
-
+ target (lldb.SBTarget): The optional target that we are trying to attach to.
+ debugger (lldb.SBDebugger): The debugger instance.
+ error (lldb.SBError): A status object notifying if the attach succeeded.
+
Returns:
- lldb.SBError: A status object notifying if the attach succeeded.
+ lldb.SBProcess: The process that the platform attached to, or None.
"""
pass
diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 84ea9c0f772e1..6cf95698bdb52 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -16,6 +16,7 @@
namespace lldb_private {
class CommandPluginInterfaceImplementation;
+class ScriptInterpreter;
namespace python {
class SWIGBridge;
}
@@ -492,6 +493,7 @@ class LLDB_API SBDebugger {
protected:
friend class lldb_private::CommandPluginInterfaceImplementation;
+ friend class lldb_private::ScriptInterpreter;
friend class lldb_private::python::SWIGBridge;
SBDebugger(const lldb::DebuggerSP &debugger_sp);
diff --git a/lldb/include/lldb/API/SBPlatform.h b/lldb/include/lldb/API/SBPlatform.h
index d63d2ed1eaba6..e6e990b08894c 100644
--- a/lldb/include/lldb/API/SBPlatform.h
+++ b/lldb/include/lldb/API/SBPlatform.h
@@ -17,7 +17,6 @@
struct PlatformConnectOptions;
struct PlatformShellCommand;
-class ProcessInstanceInfoMatch;
namespace lldb {
@@ -100,6 +99,9 @@ class LLDB_API SBPlatform {
SBPlatform(const char *platform_name);
+ SBPlatform(const char *platform_name, const SBDebugger &debugger,
+ const char *script_name, const SBStructuredData &dict);
+
SBPlatform(const SBPlatform &rhs);
SBPlatform &operator=(const SBPlatform &rhs);
diff --git a/lldb/include/lldb/API/SBProcess.h b/lldb/include/lldb/API/SBProcess.h
index 778be79583990..dca16748714dd 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -17,6 +17,7 @@
#include <cstdio>
namespace lldb_private {
+class ScriptInterpreter;
namespace python {
class SWIGBridge;
}
@@ -597,6 +598,7 @@ class LLDB_API SBProcess {
friend class lldb_private::QueueImpl;
friend class lldb_private::python::SWIGBridge;
+ friend class lldb_private::ScriptInterpreter;
SBProcess(const lldb::ProcessSP &process_sp);
diff --git a/lldb/include/lldb/API/SBStructuredData.h b/lldb/include/lldb/API/SBStructuredData.h
index fc6e1ec95c7b8..94f41fdae3f42 100644
--- a/lldb/include/lldb/API/SBStructuredData.h
+++ b/lldb/include/lldb/API/SBStructuredData.h
@@ -111,6 +111,7 @@ class SBStructuredData {
protected:
friend class SBAttachInfo;
friend class SBLaunchInfo;
+ friend class SBPlatform;
friend class SBDebugger;
friend class SBTarget;
friend class SBProcess;
diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h
index 35c2ed9c20a23..038c7586dffe1 100644
--- a/lldb/include/lldb/API/SBTarget.h
+++ b/lldb/include/lldb/API/SBTarget.h
@@ -25,6 +25,7 @@
#include "lldb/API/SBWatchpointOptions.h"
namespace lldb_private {
+class ScriptInterpreter;
namespace python {
class SWIGBridge;
}
@@ -964,6 +965,7 @@ class LLDB_API SBTarget {
friend class SBVariablesOptions;
friend class lldb_private::python::SWIGBridge;
+ friend class lldb_private::ScriptInterpreter;
// Constructors are private, use static Target::Create function to create an
// instance of this class.
diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h
index 7feaa01fe89b8..4030196cc6f15 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h
@@ -30,8 +30,11 @@ class ScriptedPlatformInterface : virtual public ScriptedInterface {
return {};
}
- virtual Status AttachToProcess(lldb::ProcessAttachInfoSP attach_info) {
- return Status("ScriptedPlatformInterface cannot attach to a process");
+ virtual lldb::ProcessSP
+ AttachToProcess(lldb::ProcessAttachInfoSP attach_info_sp,
+ lldb::TargetSP target_sp, lldb::DebuggerSP debugger_sp,
+ Status &error) {
+ return {};
}
virtual Status LaunchProcess(lldb::ProcessLaunchInfoSP launch_info) {
diff --git a/lldb/include/lldb/Interpreter/OptionGroupPlatform.h b/lldb/include/lldb/Interpreter/OptionGroupPlatform.h
index 2ffd44f0035de..410b6bafe9af4 100644
--- a/lldb/include/lldb/Interpreter/OptionGroupPlatform.h
+++ b/lldb/include/lldb/Interpreter/OptionGroupPlatform.h
@@ -9,19 +9,20 @@
#ifndef LLDB_INTERPRETER_OPTIONGROUPPLATFORM_H
#define LLDB_INTERPRETER_OPTIONGROUPPLATFORM_H
+#include "lldb/Interpreter/OptionGroupPythonClassWithDict.h"
#include "lldb/Interpreter/Options.h"
#include "lldb/Utility/ConstString.h"
#include "llvm/Support/VersionTuple.h"
namespace lldb_private {
-
// PlatformOptionGroup
//
// Make platform options available to any commands that need the settings.
class OptionGroupPlatform : public OptionGroup {
public:
OptionGroupPlatform(bool include_platform_option)
- : m_include_platform_option(include_platform_option) {}
+ : m_include_platform_option(include_platform_option),
+ m_class_options("scripted platform", true, 'C', 'K', 'V', 0) {}
~OptionGroupPlatform() override = default;
@@ -61,12 +62,17 @@ class OptionGroupPlatform : public OptionGroup {
bool PlatformMatches(const lldb::PlatformSP &platform_sp) const;
+ OptionGroupPythonClassWithDict &GetScriptClassOptions() {
+ return m_class_options;
+ }
+
protected:
std::string m_platform_name;
std::string m_sdk_sysroot;
std::string m_sdk_build;
llvm::VersionTuple m_os_version;
bool m_include_platform_option;
+ OptionGroupPythonClassWithDict m_class_options;
};
} // namespace lldb_private
diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index 05f0d7f0955f3..f970cc2b97c81 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -12,11 +12,14 @@
#include "lldb/API/SBAttachInfo.h"
#include "lldb/API/SBBreakpoint.h"
#include "lldb/API/SBData.h"
+#include "lldb/API/SBDebugger.h"
#include "lldb/API/SBError.h"
#include "lldb/API/SBEvent.h"
#include "lldb/API/SBLaunchInfo.h"
#include "lldb/API/SBMemoryRegionInfo.h"
+#include "lldb/API/SBProcess.h"
#include "lldb/API/SBStream.h"
+#include "lldb/API/SBTarget.h"
#include "lldb/Breakpoint/BreakpointOptions.h"
#include "lldb/Core/PluginInterface.h"
#include "lldb/Core/SearchFilter.h"
@@ -548,6 +551,10 @@ class ScriptInterpreter : public PluginInterface {
lldb::ScriptLanguage GetLanguage() { return m_script_lang; }
+ virtual lldb::ScriptedPlatformInterfaceUP CreateScriptedPlatformInterface() {
+ return {};
+ }
+
virtual lldb::ScriptedProcessInterfaceUP CreateScriptedProcessInterface() {
return {};
}
@@ -565,10 +572,6 @@ class ScriptInterpreter : public PluginInterface {
return {};
}
- virtual lldb::ScriptedPlatformInterfaceUP GetScriptedPlatformInterface() {
- return {};
- }
-
virtual StructuredData::ObjectSP
CreateStructuredDataFromScriptObject(ScriptObject obj) {
return {};
@@ -586,6 +589,14 @@ class ScriptInterpreter : public PluginInterface {
lldb::BreakpointSP
GetOpaqueTypeFromSBBreakpoint(const lldb::SBBreakpoint &breakpoint) const;
+ lldb::DebuggerSP
+ GetOpaqueTypeFromSBDebugger(const lldb::SBDebugger &debugger) const;
+
+ lldb::TargetSP GetOpaqueTypeFromSBTarget(const lldb::SBTarget &target) const;
+
+ lldb::ProcessSP
+ GetOpaqueTypeFromSBProcess(const lldb::SBProcess &process) const;
+
lldb::ProcessAttachInfoSP
GetOpaqueTypeFromSBAttachInfo(const lldb::SBAttachInfo &attach_info) const;
diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index 5ed2fc33356d9..60252be9bcbcc 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -24,6 +24,7 @@
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/ScriptedMetadata.h"
#include "lldb/Utility/StructuredData.h"
#include "lldb/Utility/Timeout.h"
#include "lldb/Utility/UserIDResolver.h"
@@ -35,6 +36,7 @@
namespace lldb_private {
+class PlatformMetadata;
class ProcessInstanceInfo;
class ProcessInstanceInfoMatch;
typedef std::vector<ProcessInstanceInfo> ProcessInstanceInfoList;
@@ -632,6 +634,10 @@ class Platform : public PluginInterface {
virtual const char *GetLocalCacheDirectory();
+ void SetMetadata(std::unique_ptr<PlatformMetadata> metadata);
+
+ virtual bool ReloadMetadata() { return true; }
+
virtual std::string GetPlatformSpecificConnectionInformation() { return ""; }
virtual llvm::ErrorOr<llvm::MD5::MD5Result>
@@ -962,6 +968,7 @@ class Platform : public PluginInterface {
bool m_calculated_trap_handlers;
const std::unique_ptr<ModuleCache> m_module_cache;
LocateModuleCallback m_locate_module_callback;
+ std::unique_ptr<PlatformMetadata> m_metadata;
/// Ask the Platform subclass to fill in the list of trap handler names
///
@@ -1003,6 +1010,21 @@ class Platform : public PluginInterface {
FileSpec GetModuleCacheRoot();
};
+class PlatformMetadata {
+public:
+ PlatformMetadata(Debugger &debugger, const ScriptedMetadata metadata);
+ ~PlatformMetadata() = default;
+
+ Debugger &GetDebugger() const { return m_debugger; }
+ const ScriptedMetadata GetScriptedMetadata() const {
+ return m_scripted_metadata;
+ }
+
+protected:
+ Debugger &m_debugger;
+ const ScriptedMetadata m_scripted_metadata;
+};
+
class PlatformList {
public:
PlatformList() = default;
diff --git a/lldb/include/lldb/Utility/ScriptedMetadata.h b/lldb/include/lldb/Utility/ScriptedMetadata.h
index 69c83edce909a..3990309080e87 100644
--- a/lldb/include/lldb/Utility/ScriptedMetadata.h
+++ b/lldb/include/lldb/Utility/ScriptedMetadata.h
@@ -15,8 +15,8 @@
namespace lldb_private {
class ScriptedMetadata {
public:
- ScriptedMetadata(llvm::StringRef class_name,
- StructuredData::DictionarySP dict_sp)
+ ScriptedMetadata(const llvm::StringRef class_name,
+ const StructuredData::DictionarySP dict_sp)
: m_class_name(class_name.data()), m_args_sp(dict_sp) {}
ScriptedMetadata(const ProcessInfo &process_info) {
diff --git a/lldb/source/API/SBPlatform.cpp b/lldb/source/API/SBPlatform.cpp
index 3623fd35bcdf1..79cc0393fb548 100644
--- a/lldb/source/API/SBPlatform.cpp
+++ b/lldb/source/API/SBPlatform.cpp
@@ -15,6 +15,7 @@
#include "lldb/API/SBModuleSpec.h"
#include "lldb/API/SBPlatform.h"
#include "lldb/API/SBProcessInfoList.h"
+#include "lldb/API/SBStructuredData.h"
#include "lldb/API/SBTarget.h"
#include "lldb/API/SBUnixSignals.h"
#include "lldb/Host/File.h"
@@ -23,6 +24,7 @@
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/Instrumentation.h"
+#include "lldb/Utility/ScriptedMetadata.h"
#include "lldb/Utility/Status.h"
#include "llvm/Support/FileSystem.h"
@@ -299,6 +301,35 @@ SBPlatform::SBPlatform(const char *platform_name) {
m_opaque_sp = Platform::Create(platform_name);
}
+SBPlatform::SBPlatform(const char *platform_name, const SBDebugger &debugger,
+ const char *script_name, const SBStructuredData &dict)
+ : SBPlatform(platform_name) {
+ LLDB_INSTRUMENT_VA(this, platform_name, debugger, script_name, dict);
+
+ if (!m_opaque_sp)
+ return;
+
+ if (!script_name || !dict.IsValid() || !dict.m_impl_up)
+ return;
+
+ StructuredData::ObjectSP obj_sp = dict.m_impl_up->GetObjectSP();
+
+ if (!obj_sp)
+ return;
+
+ StructuredData::DictionarySP dict_sp =
+ std::make_shared<StructuredData::Dictionary>(obj_sp);
+ if (!dict_sp || dict_sp->GetType() == lldb::eStructuredDataTypeInvalid)
+ return;
+
+ const ScriptedMetadata scripted_metadata(script_name, dict_sp);
+
+ auto metadata =
+ std::make_unique<PlatformMetadata>(debugger.ref(), scripted_metadata);
+ m_opaque_sp->SetMetadata(std::move(metadata));
+ m_opaque_sp->ReloadMetadata();
+}
+
SBPlatform::SBPlatform(const SBPlatform &rhs) {
LLDB_INSTRUMENT_VA(this, rhs);
diff --git a/lldb/source/Commands/CommandObjectPlatform.cpp b/lldb/source/Commands/CommandObjectPlatform.cpp
index 5b18f2b60e92d..4d2e7f8382744 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -154,8 +154,11 @@ class CommandObjectPlatformSelect : public CommandObjectParsed {
false) // Don't include the "--platform" option by passing false
{
m_option_group.Append(&m_platform_options, LLDB_OPT_SET_ALL, 1);
+ m_option_group.Append(&m_platform_options.GetScriptClassOptions(),
+ LLDB_OPT_SET_1 | LLDB_OPT_SET_2, LLDB_OPT_SET_ALL);
m_option_group.Finalize();
- AddSimpleArgumentList(eArgTypePlatform);
+ CommandArgumentData platform_arg{eArgTypePlatform, eArgRepeatPlain};
+ m_arguments.push_back({platform_arg});
}
~CommandObjectPlatformSelect() override = default;
@@ -180,7 +183,17 @@ class CommandObjectPlatformSelect : public CommandObjectParsed {
m_interpreter, ArchSpec(), select, error, platform_arch));
if (platform_sp) {
GetDebugger().GetPlatformList().SetSelectedPlatform(platform_sp);
-
+ OptionGroupPythonClassWithDict &script_class_opts =
+ m_platform_options.GetScriptClassOptions();
+ const ScriptedMetadata scripted_metadata(
+ script_class_opts.GetName(),
+ script_class_opts.GetStructuredData());
+
+ auto metadata = std::make_unique<PlatformMetadata>(GetDebugger(),
+ scripted_metadata);
+ platform_sp->SetMetadata(std::move(metadata));
+ if (!platform_sp->ReloadMetadata())
+ result.AppendError("platform couldn't reload metadata\n");
platform_sp->GetStatus(result.GetOutputStream());
result.SetStatus(eReturnStatusSuccessFinishResult);
} else {
diff --git a/lldb/source/Commands/CommandObjectPlatform.h b/lldb/source/Commands/CommandObjectPlatform.h
index 86f55c7d9b085..a867be6a31e7a 100644
--- a/lldb/source/Commands/CommandObjectPlatform.h
+++ b/lldb/source/Commands/CommandObjectPlatform.h
@@ -10,6 +10,7 @@
#define LLDB_SOURCE_COMMANDS_COMMANDOBJECTPLATFORM_H
#include "lldb/Interpreter/CommandObjectMultiword.h"
+#include "lldb/Interpreter/OptionGroupPlatform.h"
namespace lldb_private {
diff --git a/lldb/source/Interpreter/ScriptInterpreter.cpp b/lldb/source/Interpreter/ScriptInterpreter.cpp
index fa23964a52ffe..ded146984f2b0 100644
--- a/lldb/source/Interpreter/ScriptInterpreter.cpp
+++ b/lldb/source/Interpreter/ScriptInterpreter.cpp
@@ -82,6 +82,21 @@ lldb::BreakpointSP ScriptInterpreter::GetOpaqueTypeFromSBBreakpoint(
return breakpoint.m_opaque_wp.lock();
}
+lldb::DebuggerSP ScriptInterpreter::GetOpaqueTypeFromSBDebugger(
+ const lldb::SBDebugger &debugger) const {
+ return debugger.m_opaque_sp;
+}
+
+lldb::TargetSP ScriptInterpreter::GetOpaqueTypeFromSBTarget(
+ const lldb::SBTarget &target) const {
+ return target.m_opaque_sp;
+}
+
+lldb::ProcessSP ScriptInterpreter::GetOpaqueTypeFromSBProcess(
+ const lldb::SBProcess &process) const {
+ return process.m_opaque_wp.lock();
+}
+
lldb::ProcessAttachInfoSP ScriptInterpreter::GetOpaqueTypeFromSBAttachInfo(
const lldb::SBAttachInfo &attach_info) const {
return attach_info.m_opaque_sp;
diff --git a/lldb/source/Plugins/Platform/CMakeLists.txt b/lldb/source/Plugins/Platform/CMakeLists.txt
index 6869587f917eb..094cc019a4821 100644
--- a/lldb/source/Plugins/Platform/CMakeLists.txt
+++ b/lldb/source/Plugins/Platform/CMakeLists.txt
@@ -7,4 +7,5 @@ add_subdirectory(NetBSD)
add_subdirectory(OpenBSD)
add_subdirectory(POSIX)
add_subdirectory(QemuUser)
+add_subdirectory(scripted)
add_subdirectory(Windows)
diff --git a/lldb/source/Plugins/Platform/scripted/CMakeLists.txt b/lldb/source/Plugins/Platform/scripted/CMakeLists.txt
new file mode 100644
index 0000000000000..96a38fdee14...
[truncated]
|
|
✅ With the latest revision this PR passed the Python code formatter. |
medismailben
force-pushed
the
scripted-platform
branch
2 times, most recently
from
8008a0a to
f236a76
Compare
medismailben
force-pushed
the
scripted-platform
branch
2 times, most recently
from
9b7d9a2 to
723b577
Compare
| os.environ["SKIP_SCRIPTED_PLATFORM_SELECT"] = "1" | ||
|
|
||
| def cleanup(): | ||
| del os.environ["SKIP_SCRIPTED_PLATFORM_SELECT"] | ||
|
|
||
| self.addTearDownHook(cleanup) |
There was a problem hiding this comment.
Is it important that the HandleCommand thingy happens in __lldb_init_module. Could we remove this environment business and move the call to the relevant test instead?
There was a problem hiding this comment.
I did the same for ScriptedProcess and ScriptedThreadPlan. I use that bit of logic to iterate quickly to import and instantiate the scripting extension in lldb (in interactive mode), to test things by hand. By setting the env variable, I don't have to change anything and it runs just fine in the test.
If you really feel strongly about it, I can get rid of it.
There was a problem hiding this comment.
I don't want to jump in the middle and have you change the way you do things, but I am somewhat allergic to using environment variables for passing information around, even if its just for test. Personally, I'd probably iterate on this by putting everything on an lldb command line (like, lldb -o "command script import ..." -o "platform select scripted-platform ...") and then running that over and over.
Up to you, I guess...
There was a problem hiding this comment.
I understand your point of view, I actually got comments from other people (@JDevlieghere) about this and I've been doing it in other scripted extension tests (i.e. scripted process).
Now that I'm not iterating on them anymore, I can remove these environment variables from all the tests but I prefer doing in a follow-up.
There was a problem hiding this comment.
Sounds good. (you could also do it before hand, as I think this patch is going to take a couple more iterations).
| } | ||
|
|
||
| protected: | ||
| Debugger &m_debugger; |
There was a problem hiding this comment.
I'm somewhat worried about how this makes a platform debugger-specific through a back door. I take it this is needed to access the script interpreter (?) What happens if this platform is later added to a different debugger object?
There was a problem hiding this comment.
I'm not sure how this could make it debugger specific. If you don't like it, I can try to store the debugger's script interpreter pointer instead. How does that sounds to you ?
There was a problem hiding this comment.
What I mean is that currently, when one creates a "normal" platform instance, it is not tied to any debugger. Later, one can add it to the platform list of any debugger (even multiple ones). This scripted platform is tied to a debugger (using this field), but it can still be inserted into the platform list of any debugger -- and it's not clear to me whether that's expected to work. In principle, I guess it can work, because there's a difference between the debugger whom you are serving, and the debugger which holds your script interpreter (and the two need not be the same in theory), but is that what you want to support?
If yes, then yeah, I think it'd be better to hold a scriptinterpreter here.
There was a problem hiding this comment.
I don't see how passing the debugger vs. the script interpreter would make a difference if the scripted platform was created with one debugger and copied to another debugger platform's list. Even if I held on the script interpreter from the first debugger, once copied to the other one, the scripted platform will still interact with the script interpreter from the first one, which is basically the same as holding to the debugger.
Am I understanding that correctly?
I'm not sure if this is a scenario we want to support, may be @jimingham would have some opinions about this.
@labath if you have some ideas how to support this scenario, I'd love to be convinced :)
There was a problem hiding this comment.
I don't see how passing the debugger vs. the script interpreter would make a difference if the scripted platform was created with one debugger and copied to another debugger platform's list. Even if I held on the script interpreter from the first debugger, once copied to the other one, the scripted platform will still interact with the script interpreter from the first one, which is basically the same as holding to the debugger.
It's basically the same, but it makes the intent clearer, I think.
I'm not sure if this is a scenario we want to support, may be @jimingham would have some opinions about this.
@labath if you have some ideas how to support this scenario, I'd love to be convinced :)
I am not saying we should support this scenario (I'm also not saying we should not). What I'm saying is that this (creating a platform with one debugger, but then adding it to another one) is something that can happen, so we should figure out what to do about it. Do we want to support it or not? Do we want to prevent it from happening or not?
I don't really have an answer to those, but I think we should have one..
There was a problem hiding this comment.
If there are platforms tied to a specific debugger, we should make sure they aren't look-upable by any other Debuggers. Maybe we need a Debugger specific list of Platforms where we put the scripted ones, and change the platform lookup so it consults the Debugger specific ones first, then the global list. That would have the added benefit (if you do the lookup right) of being able to wrap a known platform in a scripted one to slightly modify its behavior.
There was a problem hiding this comment.
Also discussed about that offline with @JDevlieghere and he suggested may be the scripted platforms should instantiate their own script interpreter so they're never dependent on a debugger. The issue with that is that now, the processes that will be create from the scripted platform will be restricted to use that script interpreter instead of using the target's debugger script interpreter. I'm not sure yet if this is desirable or even feasible but it's another lead we could take.
There was a problem hiding this comment.
I don't like that idea, a scripted platform is really really going to want to create scripted processes. That will be super awkward when the scripted platform and the scripted process can't share code because they are in different interpreters. I don't want to have to explain this to people trying to use the lldb extension points.
I think a much simpler solution is that the Debugger have a list of Platforms which is where dynamically created platforms are registered, and which it consults first when resolving "platform for name". Debuggers really should be independent of one another or things get hard to reason about. For instance, it would be very strange to have command script import in debugger A make a new Platform show up in debugger B.
So you want to keep platforms made by a debugger confined to that debugger, and having an "dynamic platforms" or whatever list in the debugger seems a pretty straightforward way of doing that.
medismailben
force-pushed
the
scripted-platform
branch
from
723b577 to
4d1b7aa
Compare
This patch loosen the parsing requirement to allow parsing not only JSON dictionaries but also JSON arrays. Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
medismailben
force-pushed
the
scripted-platform
branch
2 times, most recently
from
6f076dc to
69812dd
Compare
lldb/include/lldb/API/SBPlatform.h
Outdated
| SBPlatform(const char *platform_name, const SBDebugger &debugger, | ||
| const char *script_name, const SBStructuredData &dict); | ||
|
|
There was a problem hiding this comment.
is script_name the name of a class or a function? If it is a class that implements the platform API, then should we avoid passing in platform_name and have that name be discovered by a call to the platform class itself?
There was a problem hiding this comment.
That sounds like a good idea, I'll add a getter to the python base class.
EDIT: After looking at this more closely, this is not the name of the platform instance but rather the name of the platform plugin that we should instantiate. So IIUC, this is necessary to tell LLDB to which platform it should create. In the case of Scripted Platform, we could may be tell if that's the plugin we want to use by checking if script_name or dict are valid.
|
I haven't had a chance to review this, but I would appreciate if we can keep in mind how this might affect breaking up the platforms. Right now, the Platform abstraction covers two concepts:
We ran into this problem when adding the (downstream) |
|
Is part of the goal of this class to extend another platform by being able to override a built in platform? If so, we might want the script platform class to be able to specify the name of the platform it wants to extend with a method. Something like: Then the script platform python class would instantiate a |
|
The calling through to the extended platform would happen in C++ land, not in python... |
Very interesting idea. This is not was I was thinking of while implementing this: I think this is a pretty cool idea, but this PR is already pretty big & intrusive, so I think we should do it as a follow-up, once we've figured out how we could break up platforms like @JDevlieghere explained. I'd also love to hear other people opinion on extending platforms through ScriptedPlatform (cc @labath @jimingham @JDevlieghere ) |
This patch introduces Scripted Platform, a new platform plugin that can be customized with a python script. For now this can list processes described in the python script file but eventually, it will be used to spawn scripted processes and act as an interface between them. This patch is also a less intrusive implementation of 2d53527. It introduces a new PlatformMetadata held by every platform that contains various objects that might be used by a platform instance. In its current form, the PlatformMetadata holds a reference to the Debugger and a ScriptedMetadata pointer. These are necessary in other to instanciate the scripted object that the ScriptedPlatform interacts with. In order to make it less introsive with the rest of lldb's platform creation code, platform metadata are set after the platform creation, and requires to platform to reload them (using `Platform::ReloadMetadata`). This approach has the tradeoff that the ScriptedPlaform instance is technically invalid and useless right after its creation. However, the user should never be in that situation, since we reload the platform metadata everytime with create or select the platform. This work was previously reviewed in: - https://reviews.llvm.org/D139252 - https://reviews.llvm.org/D142266 Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
medismailben
force-pushed
the
scripted-platform
branch
from
69812dd to
d3ff793
Compare
I think that having the ability to forward operations to existing platform instances would be very useful. If you look at PlatformQemuUser, most of its functions just forward operations to be host platform. We also have another downstream plugin which looks very similar to that. I would love if both of these could be lightweight python wrappers. That said, I'm not exactly sure how Greg's idea would work, or if it's even needed. Some of this functionality without any special support, by just having an The main flaw in this idea is that the SBPlatform interface is pretty small compared to the internal Platform interface. I'm guessing the python class will be implementing something like the internal interface, while it will only have the external API to work with... |
For Greg's idea to be useful, you mostly don't want to override the platform you are extending, you want to do some stuff before the underlying platform call, then you want to do the platform call, then you potentially want to do some more work afterwards. So it would have to be something more like: And maybe: But particularly for the before & after wrapping, that seems like it would be awkward to read and write, since you have to partition the before & after into different methods. We should look at what methods the scripted platform has/will have. For the most part the ones there now look like things that it would be quite reasonable to ask the SBPlatform to do through the API. If that is true, then I think allowing the scripted platform to make an instance of the SBPlatform it wants to extend and just dispatch to that directly in Python would be a more straightforward design. |
|
To extend on Pavel's modification of my idea: Then every function would have 3 variants: So for "attach_to_process" this would mean the possible functions are: |
If we were to do it this way, I would not have "attach_to_process" be the override of the underlying platform's method, because you could also have a scripted platform that extends nothing, and it would use attach_to_process but override nothing. I think it would be clearer to have the "override" version explicitly state that it is that, so: But again, if most of the things that we want the scripted platform to do are methods that really ought to be available in the SBPlatform API, then I think it would be much more straightforward to be able to have the scripted version just make an instance of the platform it wants to wrap, and do the dispatch itself. I do think it would also be handy to be able to have a wrapping scripted platform override the name lookup of the platform it wraps as well. You might have something special you wanted to do for you installation with say the |
This patch introduces Scripted Platform, a new platform plugin that can be customized with a python script.
For now this can list processes described in the python script file but eventually, it will be used to spawn scripted processes and act as an interface between them.
This patch is also a less intrusive implementation of 2d53527.
It introduces a new PlatformMetadata held by every platform that contains various objects that might be used by a platform instance.
In its current form, the PlatformMetadata holds a reference to the Debugger and a ScriptedMetadata pointer. These are necessary in other to instanciate the scripted object that the ScriptedPlatform interacts with.
In order to make it less introsive with the rest of lldb's platform creation code, platform metadata are set after the platform creation, and requires to platform to reload them (using
Platform::ReloadMetadata).This approach has the tradeoff that the ScriptedPlaform instance is technically invalid and useless right after its creation. However, the user should never be in that situation, since we reload the platform metadata everytime with create or select the platform.
This work was previously reviewed in: