Author: Med Ismail Bennani Date: 2024-06-28T01:40:03-07:00 New Revision: 1130e923e2d7fe046101bf639bc5ebcde194c005
URL: https://github.com/llvm/llvm-project/commit/1130e923e2d7fe046101bf639bc5ebcde194c005 DIFF: https://github.com/llvm/llvm-project/commit/1130e923e2d7fe046101bf639bc5ebcde194c005.diff LOG: [lldb/Interpreter] Discard ScriptedThreadPlan::GetStopDescription return value (#96985) This patch changes `ScriptedThreadPlan::GetStopDescription` behavior by discarding its return value since it is optional in the first place (the user doesn't need to provide a return value in their implementation). This patch also addresses the test failures in TestStepScripted following 9a9ec22 and re-enables the tests that were XFAIL'd previously. The issue here was that the `Stream*` that's passed to `ThreadPlanPython::GetDescription` wasn't being passed by reference to the python method so it was never updated to reflect how the python method interacted with it. This patch solves this issue by making a temporary `StreamSP` that will be passed to the python method by reference, after what we will copy its content to the caller `Stream` pointer argument. --------- Signed-off-by: Med Ismail Bennani <ism...@bennani.ma> Added: Modified: lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h lldb/include/lldb/Interpreter/ScriptInterpreter.h lldb/source/Interpreter/ScriptInterpreter.cpp lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.cpp lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h lldb/source/Target/ThreadPlanPython.cpp lldb/test/API/functionalities/step_scripted/TestStepScripted.py Removed: ################################################################################ diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h index 9130f9412cb0b..ee634d15f2a9e 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::StreamSP &stream) { + return llvm::Error::success(); } }; } // namespace lldb_private 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 b7e475812f22b..f23858c01277c 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp @@ -91,15 +91,15 @@ lldb::StateType ScriptedThreadPlanPythonInterface::GetRunState() { static_cast<uint32_t>(lldb::eStateStepping))); } -llvm::Expected<bool> -ScriptedThreadPlanPythonInterface::GetStopDescription(lldb_private::Stream *s) { +llvm::Error +ScriptedThreadPlanPythonInterface::GetStopDescription(lldb::StreamSP &stream) { Status error; - Dispatch("stop_description", error, s); + Dispatch("stop_description", error, stream); 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..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::Expected<bool> 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 373555324ba6e..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) { - 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(), + 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; } 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) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
