[lldb/Plugins] Introduce Scripted Interface Factory

[mib]

Summary

This patch splits the previous ScriptedProcessPythonInterface into
multiple specific classes:

1. The ScriptedInterface abstract class that carries the interface instance object and its virtual pure abstract creation method.

2. The ScriptedPythonInterface that holds a generic Dispatch method that can be used by various interfaces to call python methods and also keeps a reference to the Python Script Interpreter instance.

3. The ScriptedProcessInterface that describes the base Scripted Process model with all the methods used in the underlying script.

All these components are used to refactor the ScriptedProcessPythonInterface
class, making it more modular.

This patch is also a requirement for the upcoming work on ScriptedThread.

Signed-off-by: Med Ismail Bennani <[e-mail]>

[JDevlieghere]

lldb/bindings/python/python-wrapper.swig

291

Unrelated whitespace change?

lldb/include/lldb/Interpreter/ScriptedInterface.h

26

We generally don't use const when passing by value.

31

lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp

28

Why is this needed?

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp

38–42

Why was this moved up? The lock should be as tight as possible.

99

102

Seems like you're always calling error_with_message with a constant string, so we can avoid allocating a std::string.

[jingham]

This is a good start, but it seems like there's still a lot of boilerplate once you get to the python side that could be trimmed down.

For instance, ScriptedProcessPythonInterface::GetThreadWithID is almost entirely generic. The only things that make it specific to this use are the method name that's called, and the fact that it passes an lldb::tid_t to and returns an SBStructuredData.

You've started to take care of the latter by building up a set of generic "call a method, return X" routines, and surely an SBStructuredData one will be pretty common.

Then you can add a way to pass arguments to the generic routines. Maybe an array of some little struct that has either the duple "python format string/value" or a PythonObject so the dispatcher can call the code appropriately?

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp

33–45

It seems like there is a whole lot of duplicated code here. The only difference is in what we do with the returned PythonObject.

Maybe we should have a centralized "dispatch" method and then these routines differ in the conversion.

Also, it seems a shame to throw away error info, but that's what the GetGenericInteger & GetGenericString do when the object instance isn't valid, etc. llvm::Optional is good for forcing validity checks, but has no information on why the operation didn't succeed. It would be better to have all these methods take a status object for errors. You can still return the result wrapped in an optional.

When you are developing one of these classes, it would be nice to be told "no such method" for instance so you can go check if you misspelled it, or something, rather than just getting llvm::None that eventually results in some more remote error.

[mib]

Address @JDevlieghere and implement the generic ScriptedPythonInterface::Dispatch method that @jingham suggested.

This method uses templated variadic arguments and a templated return type to be able to call any python method. This allows removing the type-specific helper functions.

[jingham]

I just started reading this but I have to go now. Had one comment about errors from Dispatch. I'll look more later.

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp

88–89

It seems like we also do this bit every time we do Dispatch. Maybe Dispatch could check the outgoing obj & obj->IsValid and if they aren't then it can put an appropriate message in the "error" object. Then you would just have to check error.Fail().

That would also simplify the logging. You could always dump the error's GetCString to your error_with_message. Right now, you don't log the error's GetCString in the case where the object is valid, but error.Fail is true, which might end up dropping some useful info.

[JDevlieghere]

Nobody will be surprised that I like the templated solution for the dispatch method :-) A small comment similar to Jim's but otherwise this looks good to me.

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp

38–42

This comment is marked as done, but the lock is still taken at the beginning of the function.

88–89

I had a similar observation but I was thinking that Dispatch should return an Expected<StructuredData::ObjectSP>. That way you can be more specific in your error message too and differentiate between the object being null, invalid or another error.

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h

40–46

This is a common pattern in llvm, but totally up to preference

112

FormatArgs has an overload for the no-args case so I don't think you need this?

[mib]

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp

88–89

TBH, I don't see the added value of using Expected<T> because I'll still need to check the expected object !expected which is the same as doing !obj.

Also, regarding @jingham's suggestion, since Dispatch's return type is templated, it is not guaranteed to have an operator bool() method (same with IsValid()) that's why the check is done after calling Dispatch, on a case by case basis.

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h

112

I need it since PyObject_CallMethod expects a nullptr for the format string when the param pack is empty, not an empty string.

[mib]

Address @JDevlieghere feedbacks:

• Tighten to PythonInterpreter Lock.
• Use llvm's common pattern when checking SBError*

[JDevlieghere]

LGTM

[jankratochvil]

It broke Fedora 34 x86_64 buildbot (using gcc) lldb-x86_64-fedora = https://lab.llvm.org/staging/#/builders/16 - the build fails:
https://lab.llvm.org/staging/#/builders/16/builds/10450

[mib]

In D107521#2983069, @jankratochvil wrote:

It broke Fedora 34 x86_64 buildbot (using gcc) lldb-x86_64-fedora = https://lab.llvm.org/staging/#/builders/16 - the build fails:
https://lab.llvm.org/staging/#/builders/16/builds/10450

Looking! I'll try to send a quick fix ASAP!

[mib]

I have a fix. I'm testing it locally before landing it.

[mib]

In D107521#2983109, @mib wrote:

In D107521#2983069, @jankratochvil wrote:

It broke Fedora 34 x86_64 buildbot (using gcc) lldb-x86_64-fedora = https://lab.llvm.org/staging/#/builders/16 - the build fails:
https://lab.llvm.org/staging/#/builders/16/builds/10450

Looking! I'll try to send a quick fix ASAP!

This should get fixed with rG5f6f33da9ee6cbaef5f10b4a7be34a91d5185b2b

lldb/bindings/python/python-wrapper.swig

(
     const lldb::TargetSP& target_sp,
     lldb_private::StructuredDataImpl *args_impl,
     std::string &error_string
 )
 {
     if (python_class_name == NULL || python_class_name[0] == '\0' || !session_dictionary_name)
         Py_RETURN_NONE;
 
-
     PyErr_Cleaner py_err_cleaner(true);
 
     auto dict = PythonModule::MainModule().ResolveName<PythonDictionary>(session_dictionary_name);
     auto pfunc = PythonObject::ResolveNameWithDictionary<PythonCallable>(python_class_name, dict);
 
     if (!pfunc.IsAllocated()) {
         error_string.append("could not find script class: ");
         error_string.append(python_class_name);

lldb/include/lldb/Interpreter/ScriptedInterface.h

+//===-- ScriptedInterface.h -------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_INTERPRETER_SCRIPTEDINTERFACE_H
+#define LLDB_INTERPRETER_SCRIPTEDINTERFACE_H
+
+#include "lldb/Core/StructuredDataImpl.h"
+#include "lldb/Target/ExecutionContext.h"
+#include "lldb/lldb-private.h"
+
+#include <string>
+
+namespace lldb_private {
+class ScriptedInterface {
+public:
+  ScriptedInterface() = default;
+  virtual ~ScriptedInterface() = default;
+
+  virtual StructuredData::GenericSP
+  CreatePluginObject(const llvm::StringRef class_name,
+                     ExecutionContext &exe_ctx,
+                     StructuredData::DictionarySP args_sp) = 0;
+
+protected:
+  StructuredData::GenericSP m_object_instance_sp = nullptr;
+};
+} // namespace lldb_private
+#endif // LLDB_INTERPRETER_SCRIPTEDINTERFACE_H

lldb/include/lldb/Interpreter/ScriptedProcessInterface.h

 //===-- ScriptedProcessInterface.h ------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INTERPRETER_SCRIPTEDPROCESSINTERFACE_H
 #define LLDB_INTERPRETER_SCRIPTEDPROCESSINTERFACE_H
 
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Interpreter/ScriptedInterface.h"
+
 #include "lldb/lldb-private.h"
 
 #include <string>
 
 namespace lldb_private {
-class ScriptedProcessInterface {
+class ScriptedProcessInterface : virtual public ScriptedInterface {
 public:
-  ScriptedProcessInterface() : m_object_instance_sp(nullptr) {}
-
-  virtual ~ScriptedProcessInterface() = default;
-
-  virtual StructuredData::GenericSP
-  CreatePluginObject(const llvm::StringRef class_name, lldb::TargetSP target_sp,
-                     StructuredData::DictionarySP args_sp) {
+  StructuredData::GenericSP
+  CreatePluginObject(const llvm::StringRef class_name,
+                     ExecutionContext &exe_ctx,
+                     StructuredData::DictionarySP args_sp) override {
     return nullptr;
   }
 
   virtual Status Launch() { return Status("ScriptedProcess did not launch"); }
 
   virtual Status Resume() { return Status("ScriptedProcess did not resume"); }
 
   virtual bool ShouldStop() { return true; }
ReadMemoryAtAddress(lldb::addr_t address, size_t size, Status &error) {
     return nullptr;
   }
 
   virtual StructuredData::DictionarySP GetLoadedImages() { return nullptr; }
 
   virtual lldb::pid_t GetProcessID() { return LLDB_INVALID_PROCESS_ID; }
 
   virtual bool IsAlive() { return true; }
-
-private:
-  StructuredData::ObjectSP m_object_instance_sp;
 };
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_SCRIPTEDPROCESSINTERFACE_H

lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp

 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/RegisterContext.h"
 
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Logging.h"
 #include "lldb/Utility/State.h"
 
+#include "llvm/ADT/Twine.h"
+
 #include <mutex>
 
 LLDB_PLUGIN_DEFINE(ScriptedProcess)
 
 using namespace lldb;
 using namespace lldb_private;
 
 ConstString ScriptedProcess::GetPluginNameStatic() {
ScriptedProcess::ScriptedProcess(
 
   if (!m_interpreter) {
     error.SetErrorStringWithFormat("ScriptedProcess::%s () - ERROR: %s",
                                    __FUNCTION__,
                                    "Debugger has no Script Interpreter");
     return;
   }
 
-  StructuredData::ObjectSP object_sp = GetInterface().CreatePluginObject(
-      m_scripted_process_info.GetClassName().c_str(), target_sp,
+  ExecutionContext exe_ctx(target_sp, /*get_process=*/false);
+
+  StructuredData::GenericSP object_sp = GetInterface().CreatePluginObject(
+      m_scripted_process_info.GetClassName().c_str(), exe_ctx,
       m_scripted_process_info.GetDictionarySP());
 
   if (!object_sp || !object_sp->IsValid()) {
     error.SetErrorStringWithFormat("ScriptedProcess::%s () - ERROR: %s",
                                    __FUNCTION__,
                                    "Failed to create valid script object");
     return;
   }

lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt

 if(NOT LLDB_PYTHON_RELATIVE_PATH)
   message(FATAL_ERROR "LLDB_PYTHON_RELATIVE_PATH is not set.")
 endif()
 add_definitions(-DLLDB_PYTHON_RELATIVE_LIBDIR="${LLDB_PYTHON_RELATIVE_PATH}")
 
 if (LLDB_ENABLE_LIBEDIT)
   list(APPEND LLDB_LIBEDIT_LIBS ${LibEdit_LIBRARIES})
 endif()
 
 add_lldb_library(lldbPluginScriptInterpreterPython PLUGIN
   PythonDataObjects.cpp
   PythonReadline.cpp
   ScriptInterpreterPython.cpp
+  ScriptedPythonInterface.cpp
   ScriptedProcessPythonInterface.cpp
   SWIGPythonBridge.cpp
 
   LINK_LIBS
     lldbBreakpoint
     lldbCore
     lldbDataFormatters
     lldbHost

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h

 //===-- ScriptedProcessPythonInterface.h ------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPROCESSPYTHONINTERFACE_H
 #define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPROCESSPYTHONINTERFACE_H
 
 #include "lldb/Host/Config.h"
 
 #if LLDB_ENABLE_PYTHON
 
+#include "ScriptedPythonInterface.h"
 #include "lldb/Interpreter/ScriptedProcessInterface.h"
 
 namespace lldb_private {
-class ScriptInterpreterPythonImpl;
-class ScriptedProcessPythonInterface : public ScriptedProcessInterface {
+class ScriptedProcessPythonInterface : public ScriptedProcessInterface,
+                                       public ScriptedPythonInterface {
 public:
-  ScriptedProcessPythonInterface(ScriptInterpreterPythonImpl &interpreter)
-      : ScriptedProcessInterface(), m_interpreter(interpreter) {}
+  ScriptedProcessPythonInterface(ScriptInterpreterPythonImpl &interpreter);
 
   StructuredData::GenericSP
-  CreatePluginObject(const llvm::StringRef class_name, lldb::TargetSP target_sp,
+  CreatePluginObject(const llvm::StringRef class_name,
+                     ExecutionContext &exe_ctx,
                      StructuredData::DictionarySP args_sp) override;
 
   Status Launch() override;
 
   Status Resume() override;
 
   bool ShouldStop() override;
 
   Status Stop() override;
 
-  lldb::MemoryRegionInfoSP
-  GetMemoryRegionContainingAddress(lldb::addr_t address) override;
+  //  lldb::MemoryRegionInfoSP
+  //  GetMemoryRegionContainingAddress(lldb::addr_t address) override;
 
   StructuredData::DictionarySP GetThreadWithID(lldb::tid_t tid) override;
 
   StructuredData::DictionarySP GetRegistersForThread(lldb::tid_t tid) override;
 
   lldb::DataExtractorSP ReadMemoryAtAddress(lldb::addr_t address, size_t size,
                                             Status &error) override;
 
   StructuredData::DictionarySP GetLoadedImages() override;
 
   lldb::pid_t GetProcessID() override;
 
   bool IsAlive() override;
 
-protected:
-  llvm::Optional<unsigned long long>
-  GetGenericInteger(llvm::StringRef method_name);
-  Status GetStatusFromMethod(llvm::StringRef method_name);
 
 private:
-  // The lifetime is managed by the ScriptInterpreter
-  ScriptInterpreterPythonImpl &m_interpreter;
-  StructuredData::GenericSP m_object_instance_sp;
 };
 } // namespace lldb_private
 
 #endif // LLDB_ENABLE_PYTHON
 #endif // LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPROCESSPYTHONINTERFACE_H

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp

 //===-- ScriptedProcessPythonInterface.cpp --------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/Config.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/Logging.h"
 #include "lldb/lldb-enumerations.h"
 
 #if LLDB_ENABLE_PYTHON
 
 // LLDB Python header must be included first
 #include "lldb-python.h"
 
 #include "SWIGPythonBridge.h"
 #include "ScriptInterpreterPythonImpl.h"
 #include "ScriptedProcessPythonInterface.h"
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::python;
 using Locker = ScriptInterpreterPythonImpl::Locker;
 
+ScriptedProcessPythonInterface::ScriptedProcessPythonInterface(
+    ScriptInterpreterPythonImpl &interpreter)
+    : ScriptedProcessInterface(), ScriptedPythonInterface(interpreter) {}
+
 StructuredData::GenericSP ScriptedProcessPythonInterface::CreatePluginObject(
-    const llvm::StringRef class_name, lldb::TargetSP target_sp,
+    const llvm::StringRef class_name, ExecutionContext &exe_ctx,
     StructuredData::DictionarySP args_sp) {
   if (class_name.empty())
     return {};
 
-  std::string error_string;
+  Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
+                 Locker::FreeLock);
+
+  TargetSP target_sp = exe_ctx.GetTargetSP();
   StructuredDataImpl *args_impl = nullptr;
   if (args_sp) {
     args_impl = new StructuredDataImpl();
     args_impl->SetObjectSP(args_sp);
   }
+  std::string error_string;
 
-  void *ret_val;
-
-  {
-
-    Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
-                   Locker::FreeLock);
-
-    ret_val = LLDBSwigPythonCreateScriptedProcess(
-        class_name.str().c_str(), m_interpreter.GetDictionaryName(), target_sp,
-        args_impl, error_string);
-  }
+  void *ret_val = LLDBSwigPythonCreateScriptedProcess(
+
+  if (!ret_val)
+    return {};
 
   m_object_instance_sp =
       StructuredData::GenericSP(new StructuredPythonObject(ret_val));
 
   return m_object_instance_sp;
 }
 
 Status ScriptedProcessPythonInterface::Launch() {
bool ScriptedProcessPythonInterface::ShouldStop() {
 
   return static_cast<bool>(*should_stop);
 }
 
 Status ScriptedProcessPythonInterface::Stop() {
   return GetStatusFromMethod("stop");
 }
 
-Status ScriptedProcessPythonInterface::GetStatusFromMethod(
-    llvm::StringRef method_name) {
+StructuredData::DictionarySP
+ScriptedProcessPythonInterface::GetThreadWithID(lldb::tid_t tid) {
   Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
                  Locker::FreeLock);
 
-  if (!m_object_instance_sp)
-    return Status("Python object ill-formed.");
-
-  if (!m_object_instance_sp)
-    return Status("Cannot convert Python object to StructuredData::Generic.");
-  PythonObject implementor(PyRefType::Borrowed,
-                           (PyObject *)m_object_instance_sp->GetValue());
-
-  if (!implementor.IsAllocated())
-    return Status("Python implementor not allocated.");
-
-  PythonObject pmeth(
-      PyRefType::Owned,
-      PyObject_GetAttrString(implementor.get(), method_name.str().c_str()));
-
-  if (PyErr_Occurred())
-    PyErr_Clear();
-
-  if (!pmeth.IsAllocated())
-    return Status("Python method not allocated.");
-
-  if (PyCallable_Check(pmeth.get()) == 0) {
-    if (PyErr_Occurred())
-      PyErr_Clear();
-    return Status("Python method not callable.");
-  }
-
-  if (PyErr_Occurred())
-    PyErr_Clear();
-
-  PythonObject py_return(PyRefType::Owned,
-                         PyObject_CallMethod(implementor.get(),
-                                             method_name.str().c_str(),
-                                             nullptr));
-
-  if (PyErr_Occurred()) {
-    PyErr_Print();
-    PyErr_Clear();
-    return Status("Python method could not be called.");
-  }
-
-  if (PyObject *py_ret_ptr = py_return.get()) {
-    lldb::SBError *sb_error =
-        (lldb::SBError *)LLDBSWIGPython_CastPyObjectToSBError(py_ret_ptr);
-
-    if (!sb_error)
-      return Status("Couldn't cast lldb::SBError to lldb::Status.");
-
-    Status status = m_interpreter.GetStatusFromSBError(*sb_error);
-
-    if (status.Fail())
-      return Status("error: %s", status.AsCString());
-
-    return status;
-  }
-
-  return Status("Returned object is null.");
-}
+  auto error_with_message = [](llvm::StringRef message) {
+    LLDB_LOGF(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS),
+              "ScriptedProcess::%s ERROR = %s", __FUNCTION__,
+              message.str().c_str());
+    return StructuredData::DictionarySP();
+  };
 
-llvm::Optional<unsigned long long>
-ScriptedProcessPythonInterface::GetGenericInteger(llvm::StringRef method_name) {
-  Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
-                 Locker::FreeLock);
+  static char callee_name[] = "get_thread_with_id";
+  std::string param_format = GetPythonValueFormatString(tid);
 
   if (!m_object_instance_sp)
-    return llvm::None;
+    return error_with_message("Python object ill-formed.");
 
   if (!m_object_instance_sp)
-    return llvm::None;
+    return error_with_message("Python method not callable.");
+
   PythonObject implementor(PyRefType::Borrowed,
                            (PyObject *)m_object_instance_sp->GetValue());
 
   if (!implementor.IsAllocated())
-    return llvm::None;
+    return error_with_message("Python implementor not allocated.");
 
-  PythonObject pmeth(
-      PyRefType::Owned,
-      PyObject_GetAttrString(implementor.get(), method_name.str().c_str()));
+  PythonObject pmeth(PyRefType::Owned,
+                     PyObject_GetAttrString(implementor.get(), callee_name));
 
   if (PyErr_Occurred())
     PyErr_Clear();
 
   if (!pmeth.IsAllocated())
-    return llvm::None;
+    return error_with_message("Python method not allocated.");
 
   if (PyCallable_Check(pmeth.get()) == 0) {
     if (PyErr_Occurred())
       PyErr_Clear();
-    return llvm::None;
+    return error_with_message("Python method not callable.");
   }
 
   if (PyErr_Occurred())
     PyErr_Clear();
 
   PythonObject py_return(PyRefType::Owned,
-                         PyObject_CallMethod(implementor.get(),
-                                             method_name.str().c_str(),
-                                             nullptr));
+                         PyObject_CallMethod(implementor.get(), callee_name,
+                                             param_format.c_str(), tid));
 
   if (PyErr_Occurred()) {
     PyErr_Print();
     PyErr_Clear();
+    return error_with_message("Python method could not be called.");
   }
 
   if (!py_return.get())
-    return llvm::None;
-
-  llvm::Expected<unsigned long long> size = py_return.AsUnsignedLongLong();
-  // FIXME: Handle error.
-  if (!size)
-    return llvm::None;
-
-  return *size;
-}
-
-lldb::MemoryRegionInfoSP
-ScriptedProcessPythonInterface::GetMemoryRegionContainingAddress(
-    lldb::addr_t address) {
-  // TODO: Implement
-  return nullptr;
-}
+    return error_with_message("Returned object is null.");
 
-StructuredData::DictionarySP
-ScriptedProcessPythonInterface::GetThreadWithID(lldb::tid_t tid) {
-  // TODO: Implement
-  return nullptr;
+  PythonDictionary result_dict(PyRefType::Borrowed, py_return.get());
+  return result_dict.CreateStructuredDictionary();
 }
 
 StructuredData::DictionarySP
 ScriptedProcessPythonInterface::GetRegistersForThread(lldb::tid_t tid) {
   // TODO: Implement
   return nullptr;
 }
 

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h

+//===-- ScriptedPythonInterface.h -------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPYTHONINTERFACE_H
+#define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPYTHONINTERFACE_H
+
+#include "lldb/Host/Config.h"
+
+#if LLDB_ENABLE_PYTHON
+
+#include "lldb/Interpreter/ScriptedInterface.h"
+
+namespace lldb_private {
+class ScriptInterpreterPythonImpl;
+class ScriptedPythonInterface : virtual public ScriptedInterface {
+public:
+  ScriptedPythonInterface(ScriptInterpreterPythonImpl &interpreter);
+  virtual ~ScriptedPythonInterface() = default;
+
+protected:
+  llvm::Optional<unsigned long long>
+  GetGenericInteger(llvm::StringRef method_name);
+  llvm::Optional<llvm::StringRef> GetGenericString(llvm::StringRef method_name);
+  Status GetStatusFromMethod(llvm::StringRef method_name);
+
+protected:
+  // The lifetime is managed by the ScriptInterpreter
+  ScriptInterpreterPythonImpl &m_interpreter;
+};
+} // namespace lldb_private
+
+#endif // LLDB_ENABLE_PYTHON
+#endif // LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPYTHONINTERFACE_H

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp

+//===-- ScriptedPythonInterface.cpp ---------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/Config.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/Logging.h"
+#include "lldb/lldb-enumerations.h"
+
+#if LLDB_ENABLE_PYTHON
+
+// LLDB Python header must be included first
+#include "lldb-python.h"
+
+#include "SWIGPythonBridge.h"
+#include "ScriptInterpreterPythonImpl.h"
+#include "ScriptedPythonInterface.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::python;
+using Locker = ScriptInterpreterPythonImpl::Locker;
+
+ScriptedPythonInterface::ScriptedPythonInterface(
+    ScriptInterpreterPythonImpl &interpreter)
+    : ScriptedInterface(), m_interpreter(interpreter) {}
+
+llvm::Optional<unsigned long long>
+ScriptedPythonInterface::GetGenericInteger(llvm::StringRef method_name) {
+  Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
+                 Locker::FreeLock);
+
+  if (!m_object_instance_sp)
+    return llvm::None;
+
+  if (!m_object_instance_sp)
+    return llvm::None;
+  PythonObject implementor(PyRefType::Borrowed,
+                           (PyObject *)m_object_instance_sp->GetValue());
+
+  if (!implementor.IsAllocated())
+    return llvm::None;
+
+  PythonObject pmeth(
+      PyRefType::Owned,
+      PyObject_GetAttrString(implementor.get(), method_name.str().c_str()));
+
+  if (PyErr_Occurred())
+    PyErr_Clear();
+
+  if (!pmeth.IsAllocated())
+    return llvm::None;
+
+  if (PyCallable_Check(pmeth.get()) == 0) {
+    if (PyErr_Occurred())
+      PyErr_Clear();
+    return llvm::None;
+  }
+
+  if (PyErr_Occurred())
+    PyErr_Clear();
+
+  PythonObject py_return(PyRefType::Owned,
+                         PyObject_CallMethod(implementor.get(),
+                                             method_name.str().c_str(),
+                                             nullptr));
+
+  if (PyErr_Occurred()) {
+    PyErr_Print();
+    PyErr_Clear();
+  }
+
+  if (!py_return.get())
+    return llvm::None;
+
+  llvm::Expected<unsigned long long> size = py_return.AsUnsignedLongLong();
+  // FIXME: Handle error.
+  if (!size)
+    return llvm::None;
+
+  return *size;
+}
+
+llvm::Optional<llvm::StringRef>
+ScriptedPythonInterface::GetGenericString(llvm::StringRef method_name) {
+  Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
+                 Locker::FreeLock);
+
+  if (!m_object_instance_sp)
+    return llvm::None;
+
+  if (!m_object_instance_sp)
+    return llvm::None;
+  PythonObject implementor(PyRefType::Borrowed,
+                           (PyObject *)m_object_instance_sp->GetValue());
+
+  if (!implementor.IsAllocated())
+    return llvm::None;
+
+  PythonObject pmeth(
+      PyRefType::Owned,
+      PyObject_GetAttrString(implementor.get(), method_name.str().c_str()));
+
+  if (PyErr_Occurred())
+    PyErr_Clear();
+
+  if (!pmeth.IsAllocated())
+    return llvm::None;
+
+  if (PyCallable_Check(pmeth.get()) == 0) {
+    if (PyErr_Occurred())
+      PyErr_Clear();
+    return llvm::None;
+  }
+
+  if (PyErr_Occurred())
+    PyErr_Clear();
+
+  PythonObject py_return(PyRefType::Owned,
+                         PyObject_CallMethod(implementor.get(),
+                                             method_name.str().c_str(),
+                                             nullptr));
+
+  if (PyErr_Occurred()) {
+    PyErr_Print();
+    PyErr_Clear();
+  }
+
+  if (!py_return.get())
+    return llvm::None;
+
+  PythonString result_string(PyRefType::Borrowed, py_return.get());
+  return result_string.GetString();
+}
+
+Status
+ScriptedPythonInterface::GetStatusFromMethod(llvm::StringRef method_name) {
+  Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
+                 Locker::FreeLock);
+
+  if (!m_object_instance_sp)
+    return Status("Python object ill-formed.");
+
+  if (!m_object_instance_sp)
+    return Status("Cannot convert Python object to StructuredData::Generic.");
+  PythonObject implementor(PyRefType::Borrowed,
+                           (PyObject *)m_object_instance_sp->GetValue());
+
+  if (!implementor.IsAllocated())
+    return Status("Python implementor not allocated.");
+
+  PythonObject pmeth(
+      PyRefType::Owned,
+      PyObject_GetAttrString(implementor.get(), method_name.str().c_str()));
+
+  if (PyErr_Occurred())
+    PyErr_Clear();
+
+  if (!pmeth.IsAllocated())
+    return Status("Python method not allocated.");
+
+  if (PyCallable_Check(pmeth.get()) == 0) {
+    if (PyErr_Occurred())
+      PyErr_Clear();
+    return Status("Python method not callable.");
+  }
+
+  if (PyErr_Occurred())
+    PyErr_Clear();
+
+  PythonObject py_return(PyRefType::Owned,
+                         PyObject_CallMethod(implementor.get(),
+                                             method_name.str().c_str(),
+                                             nullptr));
+
+  if (PyErr_Occurred()) {
+    PyErr_Print();
+    PyErr_Clear();
+    return Status("Python method could not be called.");
+  }
+
+  if (PyObject *py_ret_ptr = py_return.get()) {
+    lldb::SBError *sb_error =
+        (lldb::SBError *)LLDBSWIGPython_CastPyObjectToSBError(py_ret_ptr);
+
+    if (!sb_error)
+      return Status("Couldn't cast lldb::SBError to lldb::Status.");
+
+    Status status = m_interpreter.GetStatusFromSBError(*sb_error);
+
+    if (status.Fail())
+      return Status("error: %s", status.AsCString());
+
+    return status;
+  }
+
+  return Status("Returned object is null.");
+}
+
+#endif