diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h index e4816352daa5d..9753a916243b7 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h @@ -30,6 +30,8 @@ class ScriptedInterface { return m_object_instance_sp; } + virtual llvm::SmallVector GetAbstractMethods() const = 0; + template static Ret ErrorWithMessage(llvm::StringRef caller_name, llvm::StringRef error_msg, Status &error, diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h index 7feaa01fe89b8..167149cbf5b8f 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h @@ -24,6 +24,10 @@ class ScriptedPlatformInterface : virtual public ScriptedInterface { StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) = 0; + llvm::SmallVector GetAbstractMethods() const override { + return {}; + } + virtual StructuredData::DictionarySP ListProcesses() { return {}; } virtual StructuredData::DictionarySP GetProcessInfo(lldb::pid_t) { diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h index 10203b1f8baa7..e41f488d201e2 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h @@ -26,6 +26,10 @@ class ScriptedProcessInterface : virtual public ScriptedInterface { StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) = 0; + llvm::SmallVector GetAbstractMethods() const override { + return {}; + } + virtual StructuredData::DictionarySP GetCapabilities() { return {}; } virtual Status Attach(const ProcessAttachInfo &attach_info) { diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h index a7cfc690b67dc..0520b6905e250 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h @@ -25,6 +25,10 @@ class ScriptedThreadInterface : virtual public ScriptedInterface { StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) = 0; + llvm::SmallVector GetAbstractMethods() const override { + return {}; + } + virtual lldb::tid_t GetThreadID() { return LLDB_INVALID_THREAD_ID; } virtual std::optional GetName() { return std::nullopt; } diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h index ee24e0ee30f91..e27f4dd280106 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h @@ -29,6 +29,13 @@ class OperatingSystemPythonInterface StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) override; + llvm::SmallVector GetAbstractMethods() const override { + return llvm::SmallVector({"get_thread_info" + "get_register_data", + "get_stop_reason", + "get_register_context"}); + } + StructuredData::DictionarySP CreateThread(lldb::tid_t tid, lldb::addr_t context) override; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h index e04f2d02ab1d1..0842d3a003429 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h @@ -28,6 +28,12 @@ class ScriptedPlatformPythonInterface : public ScriptedPlatformInterface, StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) override; + llvm::SmallVector GetAbstractMethods() const override { + return llvm::SmallVector( + {"list_processes", "attach_to_process", "launch_process", + "kill_process"}); + } + StructuredData::DictionarySP ListProcesses() override; StructuredData::DictionarySP GetProcessInfo(lldb::pid_t) override; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h index f3cff619f6624..c75caa9340f25 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h @@ -29,6 +29,11 @@ class ScriptedProcessPythonInterface : public ScriptedProcessInterface, StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) override; + llvm::SmallVector GetAbstractMethods() const override { + return llvm::SmallVector( + {"read_memory_at_address", "is_alive", "get_scripted_thread_plugin"}); + } + StructuredData::DictionarySP GetCapabilities() override; Status Attach(const ProcessAttachInfo &attach_info) override; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h index 7af9816397099..163659234466d 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h @@ -32,6 +32,45 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { ScriptedPythonInterface(ScriptInterpreterPythonImpl &interpreter); ~ScriptedPythonInterface() override = default; + enum class AbstractMethodCheckerCases { + eNotImplemented, + eNotAllocated, + eNotCallable, + eValid + }; + + llvm::Expected> + CheckAbstractMethodImplementation( + const python::PythonDictionary &class_dict) const { + + using namespace python; + + std::map checker; +#define SET_ERROR_AND_CONTINUE(method_name, error) \ + { \ + checker[method_name] = error; \ + continue; \ + } + + for (const llvm::StringLiteral &method_name : GetAbstractMethods()) { + if (!class_dict.HasKey(method_name)) + SET_ERROR_AND_CONTINUE(method_name, + AbstractMethodCheckerCases::eNotImplemented) + auto callable_or_err = class_dict.GetItem(method_name); + if (!callable_or_err) + SET_ERROR_AND_CONTINUE(method_name, + AbstractMethodCheckerCases::eNotAllocated) + if (!PythonCallable::Check(callable_or_err.get().get())) + SET_ERROR_AND_CONTINUE(method_name, + AbstractMethodCheckerCases::eNotCallable) + checker[method_name] = AbstractMethodCheckerCases::eValid; + } + +#undef HANDLE_ERROR + + return checker; + } + template llvm::Expected CreatePluginObject(llvm::StringRef class_name, @@ -39,20 +78,20 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { using namespace python; using Locker = ScriptInterpreterPythonImpl::Locker; + auto create_error = [](std::string message) { + return llvm::createStringError(llvm::inconvertibleErrorCode(), message); + }; + bool has_class_name = !class_name.empty(); bool has_interpreter_dict = !(llvm::StringRef(m_interpreter.GetDictionaryName()).empty()); if (!has_class_name && !has_interpreter_dict && !script_obj) { if (!has_class_name) - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Missing script class name."); + return create_error("Missing script class name."); else if (!has_interpreter_dict) - return llvm::createStringError( - llvm::inconvertibleErrorCode(), - "Invalid script interpreter dictionary."); + return create_error("Invalid script interpreter dictionary."); else - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Missing scripting object."); + return create_error("Missing scripting object."); } Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN, @@ -67,26 +106,23 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { auto dict = PythonModule::MainModule().ResolveName( m_interpreter.GetDictionaryName()); - if (!dict.IsAllocated()) { - return llvm::createStringError( - llvm::inconvertibleErrorCode(), - "Could not find interpreter dictionary: %s", - m_interpreter.GetDictionaryName()); - } + if (!dict.IsAllocated()) + return create_error( + llvm::formatv("Could not find interpreter dictionary: %s", + m_interpreter.GetDictionaryName())); - auto method = + auto init = PythonObject::ResolveNameWithDictionary( class_name, dict); - if (!method.IsAllocated()) - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Could not find script class: %s", - class_name.data()); + if (!init.IsAllocated()) + return create_error(llvm::formatv("Could not find script class: %s", + class_name.data())); std::tuple original_args = std::forward_as_tuple(args...); auto transformed_args = TransformArgs(original_args); std::string error_string; - llvm::Expected arg_info = method.GetArgInfo(); + llvm::Expected arg_info = init.GetArgInfo(); if (!arg_info) { llvm::handleAllErrors( arg_info.takeError(), @@ -99,25 +135,87 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { } llvm::Expected expected_return_object = - llvm::createStringError(llvm::inconvertibleErrorCode(), - "Resulting object is not initialized."); + create_error("Resulting object is not initialized."); std::apply( - [&method, &expected_return_object](auto &&...args) { + [&init, &expected_return_object](auto &&...args) { llvm::consumeError(expected_return_object.takeError()); - expected_return_object = method(args...); + expected_return_object = init(args...); }, transformed_args); - if (llvm::Error e = expected_return_object.takeError()) - return std::move(e); - result = std::move(expected_return_object.get()); + if (!expected_return_object) + return expected_return_object.takeError(); + result = expected_return_object.get(); } if (!result.IsValid()) - return llvm::createStringError( - llvm::inconvertibleErrorCode(), - "Resulting object is not a valid Python Object."); + return create_error("Resulting object is not a valid Python Object."); + if (!result.HasAttribute("__class__")) + return create_error("Resulting object doesn't have '__class__' member."); + + PythonObject obj_class = result.GetAttributeValue("__class__"); + if (!obj_class.IsValid()) + return create_error("Resulting class object is not a valid."); + if (!obj_class.HasAttribute("__name__")) + return create_error( + "Resulting object class doesn't have '__name__' member."); + PythonString obj_class_name = + obj_class.GetAttributeValue("__name__").AsType(); + + PythonObject object_class_mapping_proxy = + obj_class.GetAttributeValue("__dict__"); + if (!obj_class.HasAttribute("__dict__")) + return create_error( + "Resulting object class doesn't have '__dict__' member."); + + PythonCallable dict_converter = PythonModule::BuiltinsModule() + .ResolveName("dict") + .AsType(); + if (!dict_converter.IsAllocated()) + return create_error( + "Python 'builtins' module doesn't have 'dict' class."); + + PythonDictionary object_class_dict = + dict_converter(object_class_mapping_proxy).AsType(); + if (!object_class_dict.IsAllocated()) + return create_error("Coudn't create dictionary from resulting object " + "class mapping proxy object."); + + auto checker_or_err = CheckAbstractMethodImplementation(object_class_dict); + if (!checker_or_err) + return checker_or_err.takeError(); + + for (const auto &method_checker : *checker_or_err) + switch (method_checker.second) { + case AbstractMethodCheckerCases::eNotImplemented: + LLDB_LOG(GetLog(LLDBLog::Script), + "Abstract method {0}.{1} not implemented.", + obj_class_name.GetString(), method_checker.first); + break; + case AbstractMethodCheckerCases::eNotAllocated: + LLDB_LOG(GetLog(LLDBLog::Script), + "Abstract method {0}.{1} not allocated.", + obj_class_name.GetString(), method_checker.first); + break; + case AbstractMethodCheckerCases::eNotCallable: + LLDB_LOG(GetLog(LLDBLog::Script), + "Abstract method {0}.{1} not callable.", + obj_class_name.GetString(), method_checker.first); + break; + case AbstractMethodCheckerCases::eValid: + LLDB_LOG(GetLog(LLDBLog::Script), + "Abstract method {0}.{1} implemented & valid.", + obj_class_name.GetString(), method_checker.first); + break; + } + + for (const auto &method_checker : *checker_or_err) + if (method_checker.second != AbstractMethodCheckerCases::eValid) + return create_error( + llvm::formatv("Abstract method {0}.{1} missing. Enable lldb " + "script log for more details.", + obj_class_name.GetString(), method_checker.first)); m_object_instance_sp = StructuredData::GenericSP( new StructuredPythonObject(std::move(result))); diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h index b7b7439461a03..5676f7f1d6752 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h @@ -28,6 +28,11 @@ class ScriptedThreadPythonInterface : public ScriptedThreadInterface, StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) override; + llvm::SmallVector GetAbstractMethods() const override { + return llvm::SmallVector( + {"get_stop_reason", "get_register_context"}); + } + lldb::tid_t GetThreadID() override; std::optional GetName() override; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp index fe3438c424715..ea0a1cdff40f1 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp @@ -663,6 +663,20 @@ bool PythonDictionary::Check(PyObject *py_obj) { return PyDict_Check(py_obj); } +bool PythonDictionary::HasKey(const llvm::Twine &key) const { + if (!IsValid()) + return false; + + PythonString key_object(key.isSingleStringRef() ? key.getSingleStringRef() + : key.str()); + + if (int res = PyDict_Contains(m_py_obj, key_object.get()) > 0) + return res; + + PyErr_Print(); + return false; +} + uint32_t PythonDictionary::GetSize() const { if (IsValid()) return PyDict_Size(m_py_obj); diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h index 012f16e95e770..82eee76e42b27 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h @@ -562,6 +562,8 @@ class PythonDictionary : public TypedPythonObject { static bool Check(PyObject *py_obj); + bool HasKey(const llvm::Twine &key) const; + uint32_t GetSize() const; PythonList GetKeys() const; diff --git a/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py b/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py index 1761122999f84..8c6b9c74bde47 100644 --- a/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py +++ b/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py @@ -60,6 +60,55 @@ def move_blueprint_to_dsym(self, blueprint_name): ) shutil.copy(blueprint_origin_path, blueprint_destination_path) + def test_missing_methods_scripted_register_context(self): + """Test that we only instanciate scripted processes if they implement + all the required abstract methods.""" + self.build() + + os.environ["SKIP_SCRIPTED_PROCESS_LAUNCH"] = "1" + + def cleanup(): + del os.environ["SKIP_SCRIPTED_PROCESS_LAUNCH"] + + self.addTearDownHook(cleanup) + + target = self.dbg.CreateTarget(self.getBuildArtifact("a.out")) + self.assertTrue(target, VALID_TARGET) + log_file = self.getBuildArtifact("script.log") + self.runCmd("log enable lldb script -f " + log_file) + self.assertTrue(os.path.isfile(log_file)) + script_path = os.path.join( + self.getSourceDir(), "missing_methods_scripted_process.py" + ) + self.runCmd("command script import " + script_path) + + launch_info = lldb.SBLaunchInfo(None) + launch_info.SetProcessPluginName("ScriptedProcess") + launch_info.SetScriptedProcessClassName( + "missing_methods_scripted_process.MissingMethodsScriptedProcess" + ) + error = lldb.SBError() + + process = target.Launch(launch_info, error) + + self.assertFailure(error) + + with open(log_file, "r") as f: + log = f.read() + + self.assertIn( + "Abstract method MissingMethodsScriptedProcess.read_memory_at_address not implemented", + log, + ) + self.assertIn( + "Abstract method MissingMethodsScriptedProcess.is_alive not implemented", + log, + ) + self.assertIn( + "Abstract method MissingMethodsScriptedProcess.get_scripted_thread_plugin not implemented", + log, + ) + @skipUnlessDarwin def test_invalid_scripted_register_context(self): """Test that we can launch an lldb scripted process with an invalid diff --git a/lldb/test/API/functionalities/scripted_process/missing_methods_scripted_process.py b/lldb/test/API/functionalities/scripted_process/missing_methods_scripted_process.py new file mode 100644 index 0000000000000..a1db0640033e0 --- /dev/null +++ b/lldb/test/API/functionalities/scripted_process/missing_methods_scripted_process.py @@ -0,0 +1,19 @@ +import os + + +class MissingMethodsScriptedProcess: + def __init__(self, exe_ctx, args): + pass + + +def __lldb_init_module(debugger, dict): + if not "SKIP_SCRIPTED_PROCESS_LAUNCH" in os.environ: + debugger.HandleCommand( + "process launch -C %s.%s" + % (__name__, MissingMethodsScriptedProcess.__name__) + ) + else: + print( + "Name of the class that will manage the scripted process: '%s.%s'" + % (__name__, MissingMethodsScriptedProcess.__name__) + ) diff --git a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp index 0b816a8ae0a58..efb8f725f6739 100644 --- a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp +++ b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp @@ -504,6 +504,9 @@ TEST_F(PythonDataObjectsTest, TestPythonDictionaryManipulation) { dict.SetItemForKey(keys[i], values[i]); EXPECT_EQ(dict_entries, dict.GetSize()); + EXPECT_FALSE(dict.HasKey("not_in_dict")); + EXPECT_TRUE(dict.HasKey(key_0)); + EXPECT_TRUE(dict.HasKey(key_1)); // Verify that the keys and values match PythonObject chk_value1 = dict.GetItemForKey(keys[0]);