Author: Med Ismail Bennani Date: 2023-11-07T13:04:01-08:00 New Revision: c2ad9f8b607931430e86da7420493599c48e62a0
URL: https://github.com/llvm/llvm-project/commit/c2ad9f8b607931430e86da7420493599c48e62a0 DIFF: https://github.com/llvm/llvm-project/commit/c2ad9f8b607931430e86da7420493599c48e62a0.diff LOG: Revert "[lldb] Check for abstract methods implementation in Scripted Plugin Objects (#71260)" This reverts commit cc9ad72713405ef8f2468c7a714a137b4a3343ba since it breaks some tests upstream: https://lab.llvm.org/buildbot/#/builders/68/builds/63112 ******************** Failed Tests (4): lldb-api :: functionalities/gdb_remote_client/TestThreadSelectionBug.py lldb-api :: functionalities/plugins/python_os_plugin/TestPythonOSPlugin.py lldb-api :: functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py lldb-api :: functionalities/postmortem/mach-core/TestMachCore.py Added: Modified: lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp Removed: lldb/test/API/functionalities/scripted_process/missing_methods_scripted_process.py ################################################################################ diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h index 9753a916243b7be..e4816352daa5db6 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h @@ -30,8 +30,6 @@ class ScriptedInterface { return m_object_instance_sp; } - virtual llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const = 0; - template <typename Ret> 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 167149cbf5b8f89..7feaa01fe89b860 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedPlatformInterface.h @@ -24,10 +24,6 @@ class ScriptedPlatformInterface : virtual public ScriptedInterface { StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) = 0; - llvm::SmallVector<llvm::StringLiteral> 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 e41f488d201e2cb..10203b1f8baa7aa 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h @@ -26,10 +26,6 @@ class ScriptedProcessInterface : virtual public ScriptedInterface { StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) = 0; - llvm::SmallVector<llvm::StringLiteral> 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 0520b6905e2501f..a7cfc690b67dc74 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadInterface.h @@ -25,10 +25,6 @@ class ScriptedThreadInterface : virtual public ScriptedInterface { StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) = 0; - llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override { - return {}; - } - virtual lldb::tid_t GetThreadID() { return LLDB_INVALID_THREAD_ID; } virtual std::optional<std::string> 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 e27f4dd28010662..ee24e0ee30f91b3 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/OperatingSystemPythonInterface.h @@ -29,13 +29,6 @@ class OperatingSystemPythonInterface StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) override; - llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override { - return llvm::SmallVector<llvm::StringLiteral>({"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 0842d3a00342914..e04f2d02ab1d120 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPlatformPythonInterface.h @@ -28,12 +28,6 @@ class ScriptedPlatformPythonInterface : public ScriptedPlatformInterface, StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) override; - llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override { - return llvm::SmallVector<llvm::StringLiteral>( - {"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 c75caa9340f2504..f3cff619f6624f0 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h @@ -29,11 +29,6 @@ class ScriptedProcessPythonInterface : public ScriptedProcessInterface, StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) override; - llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override { - return llvm::SmallVector<llvm::StringLiteral>( - {"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 163659234466d38..7af981639709997 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h @@ -32,45 +32,6 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { ScriptedPythonInterface(ScriptInterpreterPythonImpl &interpreter); ~ScriptedPythonInterface() override = default; - enum class AbstractMethodCheckerCases { - eNotImplemented, - eNotAllocated, - eNotCallable, - eValid - }; - - llvm::Expected<std::map<llvm::StringLiteral, AbstractMethodCheckerCases>> - CheckAbstractMethodImplementation( - const python::PythonDictionary &class_dict) const { - - using namespace python; - - std::map<llvm::StringLiteral, AbstractMethodCheckerCases> 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 <typename... Args> llvm::Expected<StructuredData::GenericSP> CreatePluginObject(llvm::StringRef class_name, @@ -78,20 +39,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 create_error("Missing script class name."); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Missing script class name."); else if (!has_interpreter_dict) - return create_error("Invalid script interpreter dictionary."); + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + "Invalid script interpreter dictionary."); else - return create_error("Missing scripting object."); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Missing scripting object."); } Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN, @@ -106,23 +67,26 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { auto dict = PythonModule::MainModule().ResolveName<python::PythonDictionary>( m_interpreter.GetDictionaryName()); - if (!dict.IsAllocated()) - return create_error( - llvm::formatv("Could not find interpreter dictionary: %s", - m_interpreter.GetDictionaryName())); + if (!dict.IsAllocated()) { + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + "Could not find interpreter dictionary: %s", + m_interpreter.GetDictionaryName()); + } - auto init = + auto method = PythonObject::ResolveNameWithDictionary<python::PythonCallable>( class_name, dict); - if (!init.IsAllocated()) - return create_error(llvm::formatv("Could not find script class: %s", - class_name.data())); + if (!method.IsAllocated()) + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Could not find script class: %s", + class_name.data()); std::tuple<Args...> original_args = std::forward_as_tuple(args...); auto transformed_args = TransformArgs(original_args); std::string error_string; - llvm::Expected<PythonCallable::ArgInfo> arg_info = init.GetArgInfo(); + llvm::Expected<PythonCallable::ArgInfo> arg_info = method.GetArgInfo(); if (!arg_info) { llvm::handleAllErrors( arg_info.takeError(), @@ -135,87 +99,25 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { } llvm::Expected<PythonObject> expected_return_object = - create_error("Resulting object is not initialized."); + llvm::createStringError(llvm::inconvertibleErrorCode(), + "Resulting object is not initialized."); std::apply( - [&init, &expected_return_object](auto &&...args) { + [&method, &expected_return_object](auto &&...args) { llvm::consumeError(expected_return_object.takeError()); - expected_return_object = init(args...); + expected_return_object = method(args...); }, transformed_args); - if (!expected_return_object) - return expected_return_object.takeError(); - result = expected_return_object.get(); + if (llvm::Error e = expected_return_object.takeError()) + return std::move(e); + result = std::move(expected_return_object.get()); } if (!result.IsValid()) - 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<PythonString>(); - - 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<PythonCallable>(); - 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<PythonDictionary>(); - 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)); + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + "Resulting object is not a valid Python Object."); 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 5676f7f1d67528f..b7b7439461a03d6 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPythonInterface.h @@ -28,11 +28,6 @@ class ScriptedThreadPythonInterface : public ScriptedThreadInterface, StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj = nullptr) override; - llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const override { - return llvm::SmallVector<llvm::StringLiteral>( - {"get_stop_reason", "get_register_context"}); - } - lldb::tid_t GetThreadID() override; std::optional<std::string> GetName() override; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp index ea0a1cdff40f1e9..fe3438c42471543 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp @@ -663,20 +663,6 @@ 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 82eee76e42b27a2..012f16e95e770df 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h @@ -562,8 +562,6 @@ class PythonDictionary : public TypedPythonObject<PythonDictionary> { 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 8c6b9c74bde4703..1761122999f848e 100644 --- a/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py +++ b/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py @@ -60,55 +60,6 @@ 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 deleted file mode 100644 index a1db0640033e0ed..000000000000000 --- a/lldb/test/API/functionalities/scripted_process/missing_methods_scripted_process.py +++ /dev/null @@ -1,19 +0,0 @@ -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 efb8f725f6739ae..0b816a8ae0a587e 100644 --- a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp +++ b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp @@ -504,9 +504,6 @@ 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]); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
