[Lldb-commits] [lldb] [lldb/Interpreter] Discard ScriptedThreadPlan::GetStopDescription return value (PR #96985)

Med Ismail Bennani via lldb-commits lldb-commits at lists.llvm.org
Fri Jun 28 01:31:13 PDT 2024


https://github.com/medismailben updated https://github.com/llvm/llvm-project/pull/96985

>From 5636039087cccad8cddcb2ea48d048a669c80ed7 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani <ismail at bennani.ma>
Date: Thu, 27 Jun 2024 16:30:58 -0700
Subject: [PATCH 1/2] [lldb/Interpreter] Discard
 ScriptedThreadPlan::GetStopDescription return value

This patch changes `ScriptedThreadPlan::GetStopDescription` behaviour by
discarding its return value since it is optional in the first place
(i.e. the user doesn't need to provide a return value in their implementation).

This patch also re-enables the tests that were XFAIL'd previously.

Signed-off-by: Med Ismail Bennani <ismail at bennani.ma>
---
 .../Interpreter/Interfaces/ScriptedThreadPlanInterface.h    | 4 ++--
 .../Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp | 4 ++--
 .../Python/Interfaces/ScriptedThreadPlanPythonInterface.h   | 2 +-
 lldb/source/Target/ThreadPlanPython.cpp                     | 6 +++---
 .../API/functionalities/step_scripted/TestStepScripted.py   | 5 +----
 5 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h
index 9130f9412cb0b..0f832b3b2029f 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h
@@ -29,8 +29,8 @@ class ScriptedThreadPlanInterface : public ScriptedInterface {
 
   virtual lldb::StateType GetRunState() { return lldb::eStateStepping; }
 
-  virtual llvm::Expected<bool> GetStopDescription(lldb_private::Stream *s) {
-    return true;
+  virtual llvm::Error GetStopDescription(lldb_private::Stream *s) {
+    return llvm::Error::success();
   }
 };
 } // namespace lldb_private
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp
index b7e475812f22b..8148e138ae564 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp
@@ -91,7 +91,7 @@ lldb::StateType ScriptedThreadPlanPythonInterface::GetRunState() {
       static_cast<uint32_t>(lldb::eStateStepping)));
 }
 
-llvm::Expected<bool>
+llvm::Error
 ScriptedThreadPlanPythonInterface::GetStopDescription(lldb_private::Stream *s) {
   Status error;
   Dispatch("stop_description", error, s);
@@ -99,7 +99,7 @@ ScriptedThreadPlanPythonInterface::GetStopDescription(lldb_private::Stream *s) {
   if (error.Fail())
     return error.ToError();
 
-  return true;
+  return llvm::Error::success();
 }
 
 #endif
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h
index 33f086786c47b..563874a590794 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h
@@ -40,7 +40,7 @@ class ScriptedThreadPlanPythonInterface : public ScriptedThreadPlanInterface,
 
   lldb::StateType GetRunState() override;
 
-  llvm::Expected<bool> GetStopDescription(lldb_private::Stream *s) override;
+  llvm::Error GetStopDescription(lldb_private::Stream *s) override;
 };
 } // namespace lldb_private
 
diff --git a/lldb/source/Target/ThreadPlanPython.cpp b/lldb/source/Target/ThreadPlanPython.cpp
index 373555324ba6e..5c0beb6409b90 100644
--- a/lldb/source/Target/ThreadPlanPython.cpp
+++ b/lldb/source/Target/ThreadPlanPython.cpp
@@ -182,9 +182,9 @@ void ThreadPlanPython::GetDescription(Stream *s, lldb::DescriptionLevel level) {
   if (m_implementation_sp) {
     ScriptInterpreter *script_interp = GetScriptInterpreter();
     if (script_interp) {
-      auto desc_or_err = m_interface->GetStopDescription(s);
-      if (!desc_or_err || !*desc_or_err) {
-        LLDB_LOG_ERROR(GetLog(LLDBLog::Thread), desc_or_err.takeError(),
+      llvm::Error err = m_interface->GetStopDescription(s);
+      if (err) {
+        LLDB_LOG_ERROR(GetLog(LLDBLog::Thread), std::move(err),
                        "Can't call ScriptedThreadPlan::GetStopDescription.");
         s->Printf("Python thread plan implemented by class %s.",
             m_class_name.c_str());
diff --git a/lldb/test/API/functionalities/step_scripted/TestStepScripted.py b/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
index bb7479414dbbb..53901718019f9 100644
--- a/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
+++ b/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
@@ -7,6 +7,7 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
+
 class StepScriptedTestCase(TestBase):
     NO_DEBUG_INFO_TESTCASE = True
 
@@ -15,14 +16,12 @@ def setUp(self):
         self.main_source_file = lldb.SBFileSpec("main.c")
         self.runCmd("command script import Steps.py")
 
-    @expectedFailureAll()
     def test_standard_step_out(self):
         """Tests stepping with the scripted thread plan laying over a standard
         thread plan for stepping out."""
         self.build()
         self.step_out_with_scripted_plan("Steps.StepOut")
 
-    @expectedFailureAll()
     def test_scripted_step_out(self):
         """Tests stepping with the scripted thread plan laying over an another
         scripted thread plan for stepping out."""
@@ -63,12 +62,10 @@ def test_misspelled_plan_name(self):
         # Make sure we didn't let the process run:
         self.assertEqual(stop_id, process.GetStopID(), "Process didn't run")
 
-    @expectedFailureAll()
     def test_checking_variable(self):
         """Test that we can call SBValue API's from a scripted thread plan - using SBAPI's to step"""
         self.do_test_checking_variable(False)
 
-    @expectedFailureAll()
     def test_checking_variable_cli(self):
         """Test that we can call SBValue API's from a scripted thread plan - using cli to step"""
         self.do_test_checking_variable(True)

>From a115b277f1fa4f5869aac5519e0dde2041dc0b40 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani <ismail at bennani.ma>
Date: Fri, 28 Jun 2024 01:29:23 -0700
Subject: [PATCH 2/2] [lldb/Interpreter] Fix TestStepScripted failures
 following 9a9ec22

This patch addresses the test failures in TestStepScripted following 9a9ec22.

The issue here was that the Stream* that's passed to
ThreadPlanPython::GetDescription wasn't being passed as a reference so
we never updated the caller argument using the python object.

This patch solves this issue by making a temporary `StreamSP` that will
be passed to the python method by reference, afterwhat we will copy its
content to the caller `Stream` pointer argument.

Signed-off-by: Med Ismail Bennani <ismail at bennani.ma>
---
 .../Interfaces/ScriptedThreadPlanInterface.h         |  2 +-
 lldb/include/lldb/Interpreter/ScriptInterpreter.h    |  2 +-
 lldb/source/Interpreter/ScriptInterpreter.cpp        |  9 ++++++---
 .../Python/Interfaces/ScriptedPythonInterface.cpp    | 12 ++----------
 .../Python/Interfaces/ScriptedPythonInterface.h      |  7 ++++---
 .../Interfaces/ScriptedThreadPlanPythonInterface.cpp |  4 ++--
 .../Interfaces/ScriptedThreadPlanPythonInterface.h   |  2 +-
 lldb/source/Target/ThreadPlanPython.cpp              |  7 +++++--
 8 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h
index 0f832b3b2029f..ee634d15f2a9e 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h
@@ -29,7 +29,7 @@ class ScriptedThreadPlanInterface : public ScriptedInterface {
 
   virtual lldb::StateType GetRunState() { return lldb::eStateStepping; }
 
-  virtual llvm::Error GetStopDescription(lldb_private::Stream *s) {
+  virtual llvm::Error GetStopDescription(lldb::StreamSP &stream) {
     return llvm::Error::success();
   }
 };
diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index e821a7db2c674..14a52709c1e61 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -575,7 +575,7 @@ class ScriptInterpreter : public PluginInterface {
 
   Event *GetOpaqueTypeFromSBEvent(const lldb::SBEvent &event) const;
 
-  Stream *GetOpaqueTypeFromSBStream(const lldb::SBStream &stream) const;
+  lldb::StreamSP GetOpaqueTypeFromSBStream(const lldb::SBStream &stream) const;
 
   lldb::BreakpointSP
   GetOpaqueTypeFromSBBreakpoint(const lldb::SBBreakpoint &breakpoint) const;
diff --git a/lldb/source/Interpreter/ScriptInterpreter.cpp b/lldb/source/Interpreter/ScriptInterpreter.cpp
index 75b2a39a8d11b..65e4cbddc39a5 100644
--- a/lldb/source/Interpreter/ScriptInterpreter.cpp
+++ b/lldb/source/Interpreter/ScriptInterpreter.cpp
@@ -106,10 +106,13 @@ ScriptInterpreter::GetOpaqueTypeFromSBEvent(const lldb::SBEvent &event) const {
   return event.m_opaque_ptr;
 }
 
-Stream *ScriptInterpreter::GetOpaqueTypeFromSBStream(
+lldb::StreamSP ScriptInterpreter::GetOpaqueTypeFromSBStream(
     const lldb::SBStream &stream) const {
-  if (stream.m_opaque_up)
-    return const_cast<lldb::SBStream &>(stream).m_opaque_up.get();
+  if (stream.m_opaque_up) {
+    lldb::StreamSP s = std::make_shared<lldb_private::StreamString>();
+    *s << const_cast<lldb::SBStream &>(stream).GetData();
+    return s;
+  }
 
   return nullptr;
 }
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.cpp
index 7d072212676e1..699412e437a1a 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.cpp
@@ -26,15 +26,6 @@ ScriptedPythonInterface::ScriptedPythonInterface(
     ScriptInterpreterPythonImpl &interpreter)
     : ScriptedInterface(), m_interpreter(interpreter) {}
 
-template <>
-void ScriptedPythonInterface::ReverseTransform(
-    lldb_private::Stream *&original_arg, python::PythonObject transformed_arg,
-    Status &error) {
-  Stream *s = ExtractValueFromPythonObject<Stream *>(transformed_arg, error);
-  *original_arg = *s;
-  original_arg->PutCString(static_cast<StreamString *>(s)->GetData());
-}
-
 template <>
 StructuredData::ArraySP
 ScriptedPythonInterface::ExtractValueFromPythonObject<StructuredData::ArraySP>(
@@ -74,7 +65,8 @@ Event *ScriptedPythonInterface::ExtractValueFromPythonObject<Event *>(
 }
 
 template <>
-Stream *ScriptedPythonInterface::ExtractValueFromPythonObject<Stream *>(
+lldb::StreamSP
+ScriptedPythonInterface::ExtractValueFromPythonObject<lldb::StreamSP>(
     python::PythonObject &p, Status &error) {
   if (lldb::SBStream *sb_stream = reinterpret_cast<lldb::SBStream *>(
           python::LLDBSWIGPython_CastPyObjectToSBStream(p.get())))
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
index 062bf1fcff4ac..e1a3156d10afd 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
@@ -341,8 +341,8 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
     return python::SWIGBridge::ToSWIGWrapper(arg);
   }
 
-  python::PythonObject Transform(Stream *arg) {
-    return python::SWIGBridge::ToSWIGWrapper(arg);
+  python::PythonObject Transform(lldb::StreamSP arg) {
+    return python::SWIGBridge::ToSWIGWrapper(arg.get());
   }
 
   python::PythonObject Transform(lldb::DataExtractorSP arg) {
@@ -447,7 +447,8 @@ Event *ScriptedPythonInterface::ExtractValueFromPythonObject<Event *>(
     python::PythonObject &p, Status &error);
 
 template <>
-Stream *ScriptedPythonInterface::ExtractValueFromPythonObject<Stream *>(
+lldb::StreamSP
+ScriptedPythonInterface::ExtractValueFromPythonObject<lldb::StreamSP>(
     python::PythonObject &p, Status &error);
 
 template <>
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp
index 8148e138ae564..f23858c01277c 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp
@@ -92,9 +92,9 @@ lldb::StateType ScriptedThreadPlanPythonInterface::GetRunState() {
 }
 
 llvm::Error
-ScriptedThreadPlanPythonInterface::GetStopDescription(lldb_private::Stream *s) {
+ScriptedThreadPlanPythonInterface::GetStopDescription(lldb::StreamSP &stream) {
   Status error;
-  Dispatch("stop_description", error, s);
+  Dispatch("stop_description", error, stream);
 
   if (error.Fail())
     return error.ToError();
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h
index 563874a590794..6ec89b9f59253 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h
@@ -40,7 +40,7 @@ class ScriptedThreadPlanPythonInterface : public ScriptedThreadPlanInterface,
 
   lldb::StateType GetRunState() override;
 
-  llvm::Error GetStopDescription(lldb_private::Stream *s) override;
+  llvm::Error GetStopDescription(lldb::StreamSP &stream) override;
 };
 } // namespace lldb_private
 
diff --git a/lldb/source/Target/ThreadPlanPython.cpp b/lldb/source/Target/ThreadPlanPython.cpp
index 5c0beb6409b90..dfcb67eb19022 100644
--- a/lldb/source/Target/ThreadPlanPython.cpp
+++ b/lldb/source/Target/ThreadPlanPython.cpp
@@ -182,13 +182,16 @@ void ThreadPlanPython::GetDescription(Stream *s, lldb::DescriptionLevel level) {
   if (m_implementation_sp) {
     ScriptInterpreter *script_interp = GetScriptInterpreter();
     if (script_interp) {
-      llvm::Error err = m_interface->GetStopDescription(s);
+      lldb::StreamSP stream = std::make_shared<lldb_private::StreamString>();
+      llvm::Error err = m_interface->GetStopDescription(stream);
       if (err) {
         LLDB_LOG_ERROR(GetLog(LLDBLog::Thread), std::move(err),
                        "Can't call ScriptedThreadPlan::GetStopDescription.");
         s->Printf("Python thread plan implemented by class %s.",
             m_class_name.c_str());
-      }
+      } else
+        s->PutCString(
+            reinterpret_cast<StreamString *>(stream.get())->GetData());
     }
     return;
   }



More information about the lldb-commits mailing list