Index: lldb/include/lldb/API/SBCommandReturnObject.h =================================================================== --- lldb/include/lldb/API/SBCommandReturnObject.h +++ lldb/include/lldb/API/SBCommandReturnObject.h @@ -15,23 +15,27 @@ #include "lldb/API/SBDefines.h" +namespace lldb_private { +class SBCommandReturnObjectImpl; +}; + namespace lldb { class LLDB_API SBCommandReturnObject { public: SBCommandReturnObject(); + SBCommandReturnObject(lldb_private::CommandReturnObject &ref); + + // rvalue ctor+assignment are incompatible with Reproducers. + SBCommandReturnObject(const lldb::SBCommandReturnObject &rhs); ~SBCommandReturnObject(); - const lldb::SBCommandReturnObject & + lldb::SBCommandReturnObject & operator=(const lldb::SBCommandReturnObject &rhs); - SBCommandReturnObject(lldb_private::CommandReturnObject *ptr); - - lldb_private::CommandReturnObject *Release(); - explicit operator bool() const; bool IsValid() const; @@ -86,6 +90,9 @@ void SetError(const char *error_cstr); + // ref() is internal for LLDB only. + lldb_private::CommandReturnObject &ref() const; + protected: friend class SBCommandInterpreter; friend class SBOptions; @@ -96,10 +103,8 @@ lldb_private::CommandReturnObject &operator*() const; - lldb_private::CommandReturnObject &ref() const; - private: - std::unique_ptr m_opaque_up; + std::unique_ptr m_opaque_up; }; } // namespace lldb Index: lldb/packages/Python/lldbsuite/test/api/command-return-object/Makefile =================================================================== --- /dev/null +++ lldb/packages/Python/lldbsuite/test/api/command-return-object/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules Index: lldb/packages/Python/lldbsuite/test/api/command-return-object/TestSBCommandReturnObject.py =================================================================== --- /dev/null +++ lldb/packages/Python/lldbsuite/test/api/command-return-object/TestSBCommandReturnObject.py @@ -0,0 +1,31 @@ +"""Test the lldb public C++ api for returning SBCommandReturnObject.""" + +from __future__ import print_function + + +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestSBCommandReturnObject(TestBase): + + mydir = TestBase.compute_mydir(__file__) + NO_DEBUG_INFO_TESTCASE = True + + @skipIfNoSBHeaders + def test_sb_command_return_object(self): + env = {self.dylibPath: self.getLLDBLibraryEnvVal()} + + self.driver_exe = self.getBuildArtifact("command-return-object") + self.buildDriver('main.cpp', self.driver_exe) + self.addTearDownHook(lambda: os.remove(self.driver_exe)) + self.signBinary(self.driver_exe) + + if self.TraceOn(): + print("Running test %s" % self.driver_exe) + check_call([self.driver_exe, self.driver_exe], env=env) + else: + with open(os.devnull, 'w') as fnull: + check_call([self.driver_exe, self.driver_exe], + env=env, stdout=fnull, stderr=fnull) Index: lldb/packages/Python/lldbsuite/test/api/command-return-object/main.cpp =================================================================== --- /dev/null +++ lldb/packages/Python/lldbsuite/test/api/command-return-object/main.cpp @@ -0,0 +1,35 @@ +#include "lldb/API/SBCommandInterpreter.h" +#include "lldb/API/SBCommandReturnObject.h" +#include "lldb/API/SBDebugger.h" + +using namespace lldb; + +static SBCommandReturnObject subcommand(SBDebugger &dbg, const char *cmd) { + SBCommandReturnObject Result; + dbg.GetCommandInterpreter().HandleCommand(cmd, Result); + return Result; +} + +class CommandCrasher : public SBCommandPluginInterface { +public: + bool DoExecute(SBDebugger dbg, char **command, + SBCommandReturnObject &result) { + // Test assignment from a different SBCommandReturnObject instance. + result = subcommand(dbg, "help"); + // Test also whether self-assignment is handled correctly. + result = result; + if (!result.Succeeded()) + return false; + return true; + } +}; + +int main() { + SBDebugger::Initialize(); + SBDebugger dbg = SBDebugger::Create(false /*source_init_files*/); + SBCommandInterpreter interp = dbg.GetCommandInterpreter(); + static CommandCrasher crasher; + interp.AddCommand("crasher", &crasher, nullptr /*help*/); + SBCommandReturnObject Result; + dbg.GetCommandInterpreter().HandleCommand("crasher", Result); +} Index: lldb/scripts/Python/python-wrapper.swig =================================================================== --- lldb/scripts/Python/python-wrapper.swig +++ lldb/scripts/Python/python-wrapper.swig @@ -650,30 +650,6 @@ return sb_ptr; } -// Currently, SBCommandReturnObjectReleaser wraps a unique pointer to an -// lldb_private::CommandReturnObject. This means that the destructor for the -// SB object will deallocate its contained CommandReturnObject. Because that -// object is used as the real return object for Python-based commands, we want -// it to stay around. Thus, we release the unique pointer before returning from -// LLDBSwigPythonCallCommand, and to guarantee that the release will occur no -// matter how we exit from the function, we have a releaser object whose -// destructor does the right thing for us -class SBCommandReturnObjectReleaser -{ -public: - SBCommandReturnObjectReleaser (lldb::SBCommandReturnObject &obj) : - m_command_return_object_ref (obj) - { - } - - ~SBCommandReturnObjectReleaser () - { - m_command_return_object_ref.Release(); - } -private: - lldb::SBCommandReturnObject &m_command_return_object_ref; -}; - SWIGEXPORT bool LLDBSwigPythonCallCommand ( @@ -686,8 +662,7 @@ ) { using namespace lldb_private; - lldb::SBCommandReturnObject cmd_retobj_sb(&cmd_retobj); - SBCommandReturnObjectReleaser cmd_retobj_sb_releaser(cmd_retobj_sb); + lldb::SBCommandReturnObject cmd_retobj_sb(cmd_retobj); lldb::SBDebugger debugger_sb(debugger); lldb::SBExecutionContext exe_ctx_sb(exe_ctx_ref_sp); @@ -724,8 +699,7 @@ ) { using namespace lldb_private; - lldb::SBCommandReturnObject cmd_retobj_sb(&cmd_retobj); - SBCommandReturnObjectReleaser cmd_retobj_sb_releaser(cmd_retobj_sb); + lldb::SBCommandReturnObject cmd_retobj_sb(cmd_retobj); lldb::SBDebugger debugger_sb(debugger); lldb::SBExecutionContext exe_ctx_sb(exe_ctx_ref_sp); Index: lldb/source/API/SBCommandInterpreter.cpp =================================================================== --- lldb/source/API/SBCommandInterpreter.cpp +++ lldb/source/API/SBCommandInterpreter.cpp @@ -162,12 +162,11 @@ protected: bool DoExecute(Args &command, CommandReturnObject &result) override { - SBCommandReturnObject sb_return(&result); + SBCommandReturnObject sb_return(result); SBCommandInterpreter sb_interpreter(&m_interpreter); SBDebugger debugger_sb(m_interpreter.GetDebugger().shared_from_this()); bool ret = m_backend->DoExecute( debugger_sb, (char **)command.GetArgumentVector(), sb_return); - sb_return.Release(); return ret; } std::shared_ptr m_backend; Index: lldb/source/API/SBCommandReturnObject.cpp =================================================================== --- lldb/source/API/SBCommandReturnObject.cpp +++ lldb/source/API/SBCommandReturnObject.cpp @@ -18,11 +18,43 @@ using namespace lldb; using namespace lldb_private; +class lldb_private::SBCommandReturnObjectImpl { +public: + SBCommandReturnObjectImpl() + : m_ptr(new CommandReturnObject()), m_owned(true) {} + SBCommandReturnObjectImpl(CommandReturnObject &ref) + : m_ptr(&ref), m_owned(false) {} + SBCommandReturnObjectImpl(const SBCommandReturnObjectImpl &rhs) + : m_ptr(new CommandReturnObject(*rhs.m_ptr)), m_owned(rhs.m_owned) {} + SBCommandReturnObjectImpl &operator=(const SBCommandReturnObjectImpl &rhs) { + SBCommandReturnObjectImpl copy(rhs); + std::swap(*this, copy); + return *this; + } + // rvalue ctor+assignment are not used by SBCommandReturnObject. + ~SBCommandReturnObjectImpl() { + if (m_owned) + delete m_ptr; + } + + CommandReturnObject &operator*() const { return *m_ptr; } + +private: + CommandReturnObject *m_ptr; + bool m_owned; +}; + SBCommandReturnObject::SBCommandReturnObject() - : m_opaque_up(new CommandReturnObject()) { + : m_opaque_up(new SBCommandReturnObjectImpl()) { LLDB_RECORD_CONSTRUCTOR_NO_ARGS(SBCommandReturnObject); } +SBCommandReturnObject::SBCommandReturnObject(CommandReturnObject &ref) + : m_opaque_up(new SBCommandReturnObjectImpl(ref)) { + LLDB_RECORD_CONSTRUCTOR(SBCommandReturnObject, + (lldb_private::CommandReturnObject &), ref); +} + SBCommandReturnObject::SBCommandReturnObject(const SBCommandReturnObject &rhs) : m_opaque_up() { LLDB_RECORD_CONSTRUCTOR(SBCommandReturnObject, @@ -31,26 +63,10 @@ m_opaque_up = clone(rhs.m_opaque_up); } -SBCommandReturnObject::SBCommandReturnObject(CommandReturnObject *ptr) - : m_opaque_up(ptr) { - LLDB_RECORD_CONSTRUCTOR(SBCommandReturnObject, - (lldb_private::CommandReturnObject *), ptr); - assert(ptr != nullptr); -} - -SBCommandReturnObject::~SBCommandReturnObject() = default; - -CommandReturnObject *SBCommandReturnObject::Release() { - LLDB_RECORD_METHOD_NO_ARGS(lldb_private::CommandReturnObject *, - SBCommandReturnObject, Release); - - return LLDB_RECORD_RESULT(m_opaque_up.release()); -} - -const SBCommandReturnObject &SBCommandReturnObject:: +SBCommandReturnObject &SBCommandReturnObject:: operator=(const SBCommandReturnObject &rhs) { LLDB_RECORD_METHOD( - const lldb::SBCommandReturnObject &, + lldb::SBCommandReturnObject &, SBCommandReturnObject, operator=,(const lldb::SBCommandReturnObject &), rhs); @@ -59,6 +75,8 @@ return LLDB_RECORD_RESULT(*this); } +SBCommandReturnObject::~SBCommandReturnObject() = default; + bool SBCommandReturnObject::IsValid() const { LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBCommandReturnObject, IsValid); return this->operator bool(); @@ -73,27 +91,27 @@ const char *SBCommandReturnObject::GetOutput() { LLDB_RECORD_METHOD_NO_ARGS(const char *, SBCommandReturnObject, GetOutput); - ConstString output(m_opaque_up->GetOutputData()); + ConstString output(ref().GetOutputData()); return output.AsCString(/*value_if_empty*/ ""); } const char *SBCommandReturnObject::GetError() { LLDB_RECORD_METHOD_NO_ARGS(const char *, SBCommandReturnObject, GetError); - ConstString output(m_opaque_up->GetErrorData()); + ConstString output(ref().GetErrorData()); return output.AsCString(/*value_if_empty*/ ""); } size_t SBCommandReturnObject::GetOutputSize() { LLDB_RECORD_METHOD_NO_ARGS(size_t, SBCommandReturnObject, GetOutputSize); - return m_opaque_up->GetOutputData().size(); + return ref().GetOutputData().size(); } size_t SBCommandReturnObject::GetErrorSize() { LLDB_RECORD_METHOD_NO_ARGS(size_t, SBCommandReturnObject, GetErrorSize); - return m_opaque_up->GetErrorData().size(); + return ref().GetErrorData().size(); } size_t SBCommandReturnObject::PutOutput(FILE *fh) { @@ -121,65 +139,63 @@ void SBCommandReturnObject::Clear() { LLDB_RECORD_METHOD_NO_ARGS(void, SBCommandReturnObject, Clear); - m_opaque_up->Clear(); + ref().Clear(); } lldb::ReturnStatus SBCommandReturnObject::GetStatus() { LLDB_RECORD_METHOD_NO_ARGS(lldb::ReturnStatus, SBCommandReturnObject, GetStatus); - return m_opaque_up->GetStatus(); + return ref().GetStatus(); } void SBCommandReturnObject::SetStatus(lldb::ReturnStatus status) { LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetStatus, (lldb::ReturnStatus), status); - m_opaque_up->SetStatus(status); + ref().SetStatus(status); } bool SBCommandReturnObject::Succeeded() { LLDB_RECORD_METHOD_NO_ARGS(bool, SBCommandReturnObject, Succeeded); - return m_opaque_up->Succeeded(); + return ref().Succeeded(); } bool SBCommandReturnObject::HasResult() { LLDB_RECORD_METHOD_NO_ARGS(bool, SBCommandReturnObject, HasResult); - return m_opaque_up->HasResult(); + return ref().HasResult(); } void SBCommandReturnObject::AppendMessage(const char *message) { LLDB_RECORD_METHOD(void, SBCommandReturnObject, AppendMessage, (const char *), message); - m_opaque_up->AppendMessage(message); + ref().AppendMessage(message); } void SBCommandReturnObject::AppendWarning(const char *message) { LLDB_RECORD_METHOD(void, SBCommandReturnObject, AppendWarning, (const char *), message); - m_opaque_up->AppendWarning(message); + ref().AppendWarning(message); } CommandReturnObject *SBCommandReturnObject::operator->() const { - return m_opaque_up.get(); + return &**m_opaque_up; } CommandReturnObject *SBCommandReturnObject::get() const { - return m_opaque_up.get(); + return &**m_opaque_up; } CommandReturnObject &SBCommandReturnObject::operator*() const { - assert(m_opaque_up.get()); - return *(m_opaque_up.get()); + return **m_opaque_up; } CommandReturnObject &SBCommandReturnObject::ref() const { - assert(m_opaque_up.get()); - return *(m_opaque_up.get()); + return **m_opaque_up; } bool SBCommandReturnObject::GetDescription(SBStream &description) { @@ -189,12 +205,12 @@ Stream &strm = description.ref(); description.Printf("Error: "); - lldb::ReturnStatus status = m_opaque_up->GetStatus(); + lldb::ReturnStatus status = ref().GetStatus(); if (status == lldb::eReturnStatusStarted) strm.PutCString("Started"); else if (status == lldb::eReturnStatusInvalid) strm.PutCString("Invalid"); - else if (m_opaque_up->Succeeded()) + else if (ref().Succeeded()) strm.PutCString("Success"); else strm.PutCString("Fail"); @@ -227,7 +243,7 @@ LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateOutputFile, (FILE *, bool), fh, transfer_ownership); - m_opaque_up->SetImmediateOutputFile(fh, transfer_ownership); + ref().SetImmediateOutputFile(fh, transfer_ownership); } void SBCommandReturnObject::SetImmediateErrorFile(FILE *fh, @@ -235,7 +251,7 @@ LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateErrorFile, (FILE *, bool), fh, transfer_ownership); - m_opaque_up->SetImmediateErrorFile(fh, transfer_ownership); + ref().SetImmediateErrorFile(fh, transfer_ownership); } void SBCommandReturnObject::PutCString(const char *string, int len) { @@ -246,9 +262,9 @@ return; } else if (len > 0) { std::string buffer(string, len); - m_opaque_up->AppendMessage(buffer.c_str()); + ref().AppendMessage(buffer.c_str()); } else - m_opaque_up->AppendMessage(string); + ref().AppendMessage(string); } const char *SBCommandReturnObject::GetOutput(bool only_if_no_immediate) { @@ -256,7 +272,7 @@ only_if_no_immediate); if (!only_if_no_immediate || - m_opaque_up->GetImmediateOutputStream().get() == nullptr) + ref().GetImmediateOutputStream().get() == nullptr) return GetOutput(); return nullptr; } @@ -266,7 +282,7 @@ only_if_no_immediate); if (!only_if_no_immediate || - m_opaque_up->GetImmediateErrorStream().get() == nullptr) + ref().GetImmediateErrorStream().get() == nullptr) return GetError(); return nullptr; } @@ -274,7 +290,7 @@ size_t SBCommandReturnObject::Printf(const char *format, ...) { va_list args; va_start(args, format); - size_t result = m_opaque_up->GetOutputStream().PrintfVarArg(format, args); + size_t result = ref().GetOutputStream().PrintfVarArg(format, args); va_end(args); return result; } @@ -286,9 +302,9 @@ fallback_error_cstr); if (error.IsValid()) - m_opaque_up->SetError(error.ref(), fallback_error_cstr); + ref().SetError(error.ref(), fallback_error_cstr); else if (fallback_error_cstr) - m_opaque_up->SetError(Status(), fallback_error_cstr); + ref().SetError(Status(), fallback_error_cstr); } void SBCommandReturnObject::SetError(const char *error_cstr) { @@ -296,7 +312,7 @@ error_cstr); if (error_cstr) - m_opaque_up->SetError(error_cstr); + ref().SetError(error_cstr); } namespace lldb_private { @@ -306,13 +322,11 @@ void RegisterMethods(Registry &R) { LLDB_REGISTER_CONSTRUCTOR(SBCommandReturnObject, ()); LLDB_REGISTER_CONSTRUCTOR(SBCommandReturnObject, - (const lldb::SBCommandReturnObject &)); + (lldb_private::CommandReturnObject &)); LLDB_REGISTER_CONSTRUCTOR(SBCommandReturnObject, - (lldb_private::CommandReturnObject *)); - LLDB_REGISTER_METHOD(lldb_private::CommandReturnObject *, - SBCommandReturnObject, Release, ()); + (const lldb::SBCommandReturnObject &)); LLDB_REGISTER_METHOD( - const lldb::SBCommandReturnObject &, + lldb::SBCommandReturnObject &, SBCommandReturnObject, operator=,(const lldb::SBCommandReturnObject &)); LLDB_REGISTER_METHOD_CONST(bool, SBCommandReturnObject, IsValid, ()); LLDB_REGISTER_METHOD_CONST(bool, SBCommandReturnObject, operator bool, ());