Switch the ScriptedBreakpointResolver over to the ScriptedInterface form (#150720)

This is NFC, I'm modernizing the interface before I add to it in a
subsequent commit.

Author: jimingham

lldb/bindings/python/python-wrapper.swig

@@ -229,78 +229,6 @@ PythonObject lldb_private::python::SWIGBridge::LLDBSwigPythonCreateCommandObject
   return pfunc(SWIGBridge::ToSWIGWrapper(std::move(debugger_sp)), dict);
 }
 
-PythonObject lldb_private::python::SWIGBridge::LLDBSwigPythonCreateScriptedBreakpointResolver(
-    const char *python_class_name, const char *session_dictionary_name,
-    const StructuredDataImpl &args_impl,
-    const lldb::BreakpointSP &breakpoint_sp) {
-
-  if (python_class_name == NULL || python_class_name[0] == '\0' ||
-      !session_dictionary_name)
-    return PythonObject();
-
-  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())
-    return PythonObject();
-
-  PythonObject result =
-      pfunc(SWIGBridge::ToSWIGWrapper(breakpoint_sp), SWIGBridge::ToSWIGWrapper(args_impl), dict);
-  // FIXME: At this point we should check that the class we found supports all
-  // the methods that we need.
-
-  if (result.IsAllocated()) {
-    // Check that __callback__ is defined:
-    auto callback_func = result.ResolveName<PythonCallable>("__callback__");
-    if (callback_func.IsAllocated())
-      return result;
-  }
-  return PythonObject();
-}
-
-unsigned int lldb_private::python::SWIGBridge::LLDBSwigPythonCallBreakpointResolver(
-    void *implementor, const char *method_name,
-    lldb_private::SymbolContext *sym_ctx) {
-  PyErr_Cleaner py_err_cleaner(false);
-  PythonObject self(PyRefType::Borrowed, static_cast<PyObject *>(implementor));
-  auto pfunc = self.ResolveName<PythonCallable>(method_name);
-
-  if (!pfunc.IsAllocated())
-    return 0;
-
-  PythonObject result = sym_ctx ? pfunc(SWIGBridge::ToSWIGWrapper(*sym_ctx)) : pfunc();
-
-  if (PyErr_Occurred()) {
-    PyErr_Print();
-    PyErr_Clear();
-    return 0;
-  }
-
-  // The callback will return a bool, but we're need to also return ints
-  // so we're squirrelling the bool through as an int...  And if you return
-  // nothing, we'll continue.
-  if (strcmp(method_name, "__callback__") == 0) {
-    if (result.get() == Py_False)
-      return 0;
-    else
-      return 1;
-  }
-
-  long long ret_val = unwrapOrSetPythonException(As<long long>(result));
-
-  if (PyErr_Occurred()) {
-    PyErr_Print();
-    PyErr_Clear();
-    return 0;
-  }
-
-  return ret_val;
-}
-
 // wrapper that calls an optional instance member of an object taking no
 // arguments
 static PyObject *LLDBSwigPython_CallOptionalMember(
@@ -554,6 +482,18 @@ void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBStream(PyObject * dat
   return sb_ptr;
 }
 
+void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBSymbolContext(PyObject * data) {
+  lldb::SBSymbolContext *sb_ptr = nullptr;
+
+  int valid_cast =
+      SWIG_ConvertPtr(data, (void **)&sb_ptr, SWIGTYPE_p_lldb__SBSymbolContext, 0);
+
+  if (valid_cast == -1)
+    return NULL;
+
+  return sb_ptr;
+}
+
 void *lldb_private::python::LLDBSWIGPython_CastPyObjectToSBValue(PyObject * data) {
   lldb::SBValue *sb_ptr = NULL;
 

lldb/include/lldb/API/SBSymbolContext.h

@@ -80,6 +80,8 @@ class LLDB_API SBSymbolContext {
 
   lldb_private::SymbolContext *get() const;
 
+  friend class lldb_private::ScriptInterpreter;
+
 private:
   std::unique_ptr<lldb_private::SymbolContext> m_opaque_up;
 };

lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h

@@ -12,6 +12,7 @@
 #include "lldb/Breakpoint/BreakpointResolver.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/StructuredDataImpl.h"
+#include "lldb/Interpreter/Interfaces/ScriptedBreakpointInterface.h"
 #include "lldb/lldb-forward.h"
 
 namespace lldb_private {
@@ -64,7 +65,8 @@ class BreakpointResolverScripted : public BreakpointResolver {
   std::string m_class_name;
   lldb::SearchDepth m_depth;
   StructuredDataImpl m_args;
-  StructuredData::GenericSP m_implementation_sp;
+  Status m_error;
+  lldb::ScriptedBreakpointInterfaceSP m_interface_sp;
 
   BreakpointResolverScripted(const BreakpointResolverScripted &) = delete;
   const BreakpointResolverScripted &

lldb/include/lldb/Interpreter/Interfaces/ScriptedBreakpointInterface.h

@@ -0,0 +1,32 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_INTERFACES_SCRIPTEDBREAKPOINTINTERFACE_H
+#define LLDB_INTERPRETER_INTERFACES_SCRIPTEDBREAKPOINTINTERFACE_H
+
+#include "ScriptedInterface.h"
+#include "lldb/Symbol/SymbolContext.h"
+#include "lldb/lldb-private.h"
+
+namespace lldb_private {
+class ScriptedBreakpointInterface : public ScriptedInterface {
+public:
+  virtual llvm::Expected<StructuredData::GenericSP>
+  CreatePluginObject(llvm::StringRef class_name, lldb::BreakpointSP break_sp,
+                     const StructuredDataImpl &args_sp) = 0;
+
+  /// "ResolverCallback" will get called when a new module is loaded.  The
+  /// new module information is passed in sym_ctx.  The Resolver will add
+  /// any breakpoint locations it found in that module.
+  virtual bool ResolverCallback(SymbolContext sym_ctx) { return true; }
+  virtual lldb::SearchDepth GetDepth() { return lldb::eSearchDepthModule; }
+  virtual std::optional<std::string> GetShortHelp() { return nullptr; }
+};
+} // namespace lldb_private
+
+#endif // LLDB_INTERPRETER_INTERFACES_SCRIPTEDSTOPHOOKINTERFACE_H

lldb/include/lldb/Interpreter/ScriptInterpreter.h

@@ -18,6 +18,7 @@
 #include "lldb/API/SBLaunchInfo.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/API/SBSymbolContext.h"
 #include "lldb/Breakpoint/BreakpointOptions.h"
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Core/SearchFilter.h"
@@ -29,6 +30,7 @@
 #include "lldb/Interpreter/Interfaces/ScriptedProcessInterface.h"
 #include "lldb/Interpreter/Interfaces/ScriptedThreadInterface.h"
 #include "lldb/Interpreter/ScriptObject.h"
+#include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Utility/Broadcaster.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StructuredData.h"
@@ -257,26 +259,6 @@ class ScriptInterpreter : public PluginInterface {
     return false;
   }
 
-  virtual StructuredData::GenericSP
-  CreateScriptedBreakpointResolver(const char *class_name,
-                                   const StructuredDataImpl &args_data,
-                                   lldb::BreakpointSP &bkpt_sp) {
-    return StructuredData::GenericSP();
-  }
-
-  virtual bool
-  ScriptedBreakpointResolverSearchCallback(StructuredData::GenericSP implementor_sp,
-                                           SymbolContext *sym_ctx)
-  {
-    return false;
-  }
-
-  virtual lldb::SearchDepth
-  ScriptedBreakpointResolverSearchDepth(StructuredData::GenericSP implementor_sp)
-  {
-    return lldb::eSearchDepthModule;
-  }
-
   virtual StructuredData::ObjectSP
   LoadPluginModule(const FileSpec &file_spec, lldb_private::Status &error) {
     return StructuredData::ObjectSP();
@@ -566,6 +548,11 @@ class ScriptInterpreter : public PluginInterface {
     return {};
   }
 
+  virtual lldb::ScriptedBreakpointInterfaceSP
+  CreateScriptedBreakpointInterface() {
+    return {};
+  }
+
   virtual StructuredData::ObjectSP
   CreateStructuredDataFromScriptObject(ScriptObject obj) {
     return {};
@@ -580,6 +567,9 @@ class ScriptInterpreter : public PluginInterface {
 
   lldb::StreamSP GetOpaqueTypeFromSBStream(const lldb::SBStream &stream) const;
 
+  SymbolContext
+  GetOpaqueTypeFromSBSymbolContext(const lldb::SBSymbolContext &sym_ctx) const;
+
   lldb::BreakpointSP
   GetOpaqueTypeFromSBBreakpoint(const lldb::SBBreakpoint &breakpoint) const;
 

lldb/include/lldb/lldb-forward.h

@@ -188,6 +188,7 @@ class Scalar;
 class ScriptInterpreter;
 class ScriptInterpreterLocker;
 class ScriptedMetadata;
+class ScriptedBreakpointInterface;
 class ScriptedPlatformInterface;
 class ScriptedProcessInterface;
 class ScriptedStopHookInterface;
@@ -418,6 +419,8 @@ typedef std::shared_ptr<lldb_private::ScriptedThreadInterface>
     ScriptedThreadInterfaceSP;
 typedef std::shared_ptr<lldb_private::ScriptedThreadPlanInterface>
     ScriptedThreadPlanInterfaceSP;
+typedef std::shared_ptr<lldb_private::ScriptedBreakpointInterface>
+    ScriptedBreakpointInterfaceSP;
 typedef std::shared_ptr<lldb_private::Section> SectionSP;
 typedef std::unique_ptr<lldb_private::SectionList> SectionListUP;
 typedef std::weak_ptr<lldb_private::Section> SectionWP;

lldb/source/Breakpoint/BreakpointResolverScripted.cpp

@@ -35,7 +35,7 @@ BreakpointResolverScripted::BreakpointResolverScripted(
 
 void BreakpointResolverScripted::CreateImplementationIfNeeded(
     BreakpointSP breakpoint_sp) {
-  if (m_implementation_sp)
+  if (m_interface_sp)
     return;
 
   if (m_class_name.empty())
@@ -50,8 +50,27 @@ void BreakpointResolverScripted::CreateImplementationIfNeeded(
   if (!script_interp)
     return;
 
-  m_implementation_sp = script_interp->CreateScriptedBreakpointResolver(
-      m_class_name.c_str(), m_args, breakpoint_sp);
+  m_interface_sp = script_interp->CreateScriptedBreakpointInterface();
+  if (!m_interface_sp) {
+    m_error = Status::FromErrorStringWithFormat(
+        "BreakpointResolverScripted::%s () - ERROR: %s", __FUNCTION__,
+        "Script interpreter couldn't create Scripted Breakpoint Interface");
+    return;
+  }
+
+  auto obj_or_err =
+      m_interface_sp->CreatePluginObject(m_class_name, breakpoint_sp, m_args);
+  if (!obj_or_err) {
+    m_error = Status::FromError(obj_or_err.takeError());
+    return;
+  }
+
+  StructuredData::ObjectSP object_sp = *obj_or_err;
+  if (!object_sp || !object_sp->IsValid()) {
+    m_error = Status::FromErrorStringWithFormat(
+        "ScriptedBreakpoint::%s () - ERROR: %s", __FUNCTION__,
+        "Failed to create valid script object");
+  }
 }
 
 void BreakpointResolverScripted::NotifyBreakpointSet() {
@@ -104,13 +123,10 @@ ScriptInterpreter *BreakpointResolverScripted::GetScriptInterpreter() {
 Searcher::CallbackReturn BreakpointResolverScripted::SearchCallback(
     SearchFilter &filter, SymbolContext &context, Address *addr) {
   bool should_continue = true;
-  if (!m_implementation_sp)
+  if (!m_interface_sp)
     return Searcher::eCallbackReturnStop;
 
-  ScriptInterpreter *interp = GetScriptInterpreter();
-  should_continue = interp->ScriptedBreakpointResolverSearchCallback(
-      m_implementation_sp,
-      &context);
+  should_continue = m_interface_sp->ResolverCallback(context);
   if (should_continue)
     return Searcher::eCallbackReturnContinue;
 
@@ -120,25 +136,21 @@ Searcher::CallbackReturn BreakpointResolverScripted::SearchCallback(
 lldb::SearchDepth
 BreakpointResolverScripted::GetDepth() {
   lldb::SearchDepth depth = lldb::eSearchDepthModule;
-  if (m_implementation_sp) {
-    ScriptInterpreter *interp = GetScriptInterpreter();
-    depth = interp->ScriptedBreakpointResolverSearchDepth(
-        m_implementation_sp);
-  }
+  if (m_interface_sp)
+    depth = m_interface_sp->GetDepth();
+
   return depth;
 }
 
 void BreakpointResolverScripted::GetDescription(Stream *s) {
   StructuredData::GenericSP generic_sp;
-  std::string short_help;
+  std::optional<std::string> short_help;
 
-  if (m_implementation_sp) {
-    ScriptInterpreter *interp = GetScriptInterpreter();
-    interp->GetShortHelpForCommandObject(m_implementation_sp,
-                                         short_help);
+  if (m_interface_sp) {
+    short_help = m_interface_sp->GetShortHelp();
   }
-  if (!short_help.empty())
-    s->PutCString(short_help.c_str());
+  if (short_help && !short_help->empty())
+    s->PutCString(short_help->c_str());
   else
     s->Printf("python class = %s", m_class_name.c_str());
 }

lldb/source/Interpreter/ScriptInterpreter.cpp

@@ -116,6 +116,13 @@ lldb::StreamSP ScriptInterpreter::GetOpaqueTypeFromSBStream(
   return nullptr;
 }
 
+SymbolContext ScriptInterpreter::GetOpaqueTypeFromSBSymbolContext(
+    const lldb::SBSymbolContext &sb_sym_ctx) const {
+  if (sb_sym_ctx.m_opaque_up)
+    return *sb_sym_ctx.m_opaque_up.get();
+  return {};
+}
+
 std::optional<MemoryRegionInfo>
 ScriptInterpreter::GetOpaqueTypeFromSBMemoryRegionInfo(
     const lldb::SBMemoryRegionInfo &mem_region) const {

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

@@ -26,6 +26,7 @@ add_lldb_library(lldbPluginScriptInterpreterPythonInterfaces PLUGIN
   ScriptedProcessPythonInterface.cpp
   ScriptedPythonInterface.cpp
   ScriptedStopHookPythonInterface.cpp
+  ScriptedBreakpointPythonInterface.cpp
   ScriptedThreadPlanPythonInterface.cpp
   ScriptedThreadPythonInterface.cpp
 

lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptInterpreterPythonInterfaces.cpp

@@ -29,6 +29,7 @@ void ScriptInterpreterPythonInterfaces::Initialize() {
   ScriptedPlatformPythonInterface::Initialize();
   ScriptedProcessPythonInterface::Initialize();
   ScriptedStopHookPythonInterface::Initialize();
+  ScriptedBreakpointPythonInterface::Initialize();
   ScriptedThreadPlanPythonInterface::Initialize();
 }
 
@@ -37,6 +38,7 @@ void ScriptInterpreterPythonInterfaces::Terminate() {
   ScriptedPlatformPythonInterface::Terminate();
   ScriptedProcessPythonInterface::Terminate();
   ScriptedStopHookPythonInterface::Terminate();
+  ScriptedBreakpointPythonInterface::Terminate();
   ScriptedThreadPlanPythonInterface::Terminate();
 }