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 <ism...@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 <ism...@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 <ism...@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 <ism...@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; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits