[Lldb-commits] [lldb] r373775 - [lldb] Fix crash on SBCommandReturnObject & assignment
Jan Kratochvil via lldb-commits
lldb-commits at lists.llvm.org
Fri Oct 4 12:32:57 PDT 2019
Author: jankratochvil
Date: Fri Oct 4 12:32:57 2019
New Revision: 373775
URL: http://llvm.org/viewvc/llvm-project?rev=373775&view=rev
Log:
[lldb] Fix crash on SBCommandReturnObject & assignment
I was writing an SB API client and it was crashing on:
bool DoExecute(SBDebugger dbg, char **command, SBCommandReturnObject &result) {
result = subcommand(dbg, "help");
That is because SBCommandReturnObject &result gets initialized inside LLDB by:
bool DoExecute(Args &command, CommandReturnObject &result) override {
// std::unique_ptr gets initialized here from `&result`!!!
SBCommandReturnObject sb_return(&result);
DoExecute(...);
sb_return.Release();
Differential revision: https://reviews.llvm.org/D67589
Added:
lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/
lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/Makefile
lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/TestSBCommandReturnObject.py
lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/main.cpp
Modified:
lldb/trunk/include/lldb/API/SBCommandReturnObject.h
lldb/trunk/scripts/Python/python-wrapper.swig
lldb/trunk/source/API/SBCommandInterpreter.cpp
lldb/trunk/source/API/SBCommandReturnObject.cpp
Modified: lldb/trunk/include/lldb/API/SBCommandReturnObject.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBCommandReturnObject.h?rev=373775&r1=373774&r2=373775&view=diff
==============================================================================
--- lldb/trunk/include/lldb/API/SBCommandReturnObject.h (original)
+++ lldb/trunk/include/lldb/API/SBCommandReturnObject.h Fri Oct 4 12:32:57 2019
@@ -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 @@ public:
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 @@ protected:
lldb_private::CommandReturnObject &operator*() const;
- lldb_private::CommandReturnObject &ref() const;
-
private:
- std::unique_ptr<lldb_private::CommandReturnObject> m_opaque_up;
+ std::unique_ptr<lldb_private::SBCommandReturnObjectImpl> m_opaque_up;
};
} // namespace lldb
Added: lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/Makefile
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/Makefile?rev=373775&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/Makefile (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/Makefile Fri Oct 4 12:32:57 2019
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Added: lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/TestSBCommandReturnObject.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/TestSBCommandReturnObject.py?rev=373775&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/TestSBCommandReturnObject.py (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/TestSBCommandReturnObject.py Fri Oct 4 12:32:57 2019
@@ -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)
Added: lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/main.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/main.cpp?rev=373775&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/main.cpp (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/api/command-return-object/main.cpp Fri Oct 4 12:32:57 2019
@@ -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);
+}
Modified: lldb/trunk/scripts/Python/python-wrapper.swig
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Python/python-wrapper.swig?rev=373775&r1=373774&r2=373775&view=diff
==============================================================================
--- lldb/trunk/scripts/Python/python-wrapper.swig (original)
+++ lldb/trunk/scripts/Python/python-wrapper.swig Fri Oct 4 12:32:57 2019
@@ -671,30 +671,6 @@ LLDBSWIGPython_CastPyObjectToSBValue
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
(
@@ -707,8 +683,7 @@ LLDBSwigPythonCallCommand
)
{
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);
@@ -745,8 +720,7 @@ LLDBSwigPythonCallCommandObject
)
{
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);
Modified: lldb/trunk/source/API/SBCommandInterpreter.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBCommandInterpreter.cpp?rev=373775&r1=373774&r2=373775&view=diff
==============================================================================
--- lldb/trunk/source/API/SBCommandInterpreter.cpp (original)
+++ lldb/trunk/source/API/SBCommandInterpreter.cpp Fri Oct 4 12:32:57 2019
@@ -162,12 +162,11 @@ public:
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<lldb::SBCommandPluginInterface> m_backend;
Modified: lldb/trunk/source/API/SBCommandReturnObject.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBCommandReturnObject.cpp?rev=373775&r1=373774&r2=373775&view=diff
==============================================================================
--- lldb/trunk/source/API/SBCommandReturnObject.cpp (original)
+++ lldb/trunk/source/API/SBCommandReturnObject.cpp Fri Oct 4 12:32:57 2019
@@ -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 @@ SBCommandReturnObject::SBCommandReturnOb
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 @@ operator=(const SBCommandReturnObject &r
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 @@ SBCommandReturnObject::operator bool() c
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 @@ size_t SBCommandReturnObject::PutError(F
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 @@ bool SBCommandReturnObject::GetDescripti
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 @@ void SBCommandReturnObject::SetImmediate
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 @@ void SBCommandReturnObject::SetImmediate
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 @@ void SBCommandReturnObject::PutCString(c
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 @@ const char *SBCommandReturnObject::GetOu
only_if_no_immediate);
if (!only_if_no_immediate ||
- m_opaque_up->GetImmediateOutputStream().get() == nullptr)
+ ref().GetImmediateOutputStream().get() == nullptr)
return GetOutput();
return nullptr;
}
@@ -265,8 +281,7 @@ const char *SBCommandReturnObject::GetEr
LLDB_RECORD_METHOD(const char *, SBCommandReturnObject, GetError, (bool),
only_if_no_immediate);
- if (!only_if_no_immediate ||
- m_opaque_up->GetImmediateErrorStream().get() == nullptr)
+ if (!only_if_no_immediate || ref().GetImmediateErrorStream().get() == nullptr)
return GetError();
return nullptr;
}
@@ -274,7 +289,7 @@ const char *SBCommandReturnObject::GetEr
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 +301,9 @@ void SBCommandReturnObject::SetError(lld
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 +311,7 @@ void SBCommandReturnObject::SetError(con
error_cstr);
if (error_cstr)
- m_opaque_up->SetError(error_cstr);
+ ref().SetError(error_cstr);
}
namespace lldb_private {
@@ -306,13 +321,11 @@ template <>
void RegisterMethods<SBCommandReturnObject>(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, ());
More information about the lldb-commits
mailing list