Revert "[lldb] Check for abstract methods implementation in Scripted Plugin Objects (#71260)"

medismailben · medismailben · commit c2ad9f8b6079 · 2023-11-07T13:04:01.000-08:00
This reverts commit cc9ad72 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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -32,66 +32,27 @@ 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,
                      StructuredData::Generic *script_obj, Args... args) {
     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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Original file line number	Diff line number	Diff line change
`@@ -30,8 +30,6 @@ class ScriptedInterface {`
`30`	`30`	`return m_object_instance_sp;`
`31`	`31`	`}`
`32`	`32`
`33`		`- virtual llvm::SmallVector<llvm::StringLiteral> GetAbstractMethods() const = 0;`
`34`		`-`
`35`	`33`	`template <typename Ret>`
`36`	`34`	`static Ret ErrorWithMessage(llvm::StringRef caller_name,`
`37`	`35`	`llvm::StringRef error_msg, Status &error,`