Some more pointer safety in Breakpoint.
Plumb unique_ptrs<> all the way through the baton interface. NFC, this is a minor improvement to remove the possibility of an accidental pointer ownership issue. Reviewed By: jingham Differential Revision: https://reviews.llvm.org/D24495 llvm-svn: 281360
This commit is contained in:
parent
8ea02f4e1c
commit
4e4fbe8211
|
@ -65,14 +65,10 @@ public:
|
|||
}
|
||||
};
|
||||
|
||||
class CommandBaton : public Baton {
|
||||
class CommandBaton : public TypedBaton<CommandData> {
|
||||
public:
|
||||
CommandBaton(CommandData *data) : Baton(data) {}
|
||||
|
||||
~CommandBaton() override {
|
||||
delete ((CommandData *)m_data);
|
||||
m_data = nullptr;
|
||||
}
|
||||
explicit CommandBaton(std::unique_ptr<CommandData> Data)
|
||||
: TypedBaton(std::move(Data)) {}
|
||||
|
||||
void GetDescription(Stream *s, lldb::DescriptionLevel level) const override;
|
||||
};
|
||||
|
@ -341,7 +337,7 @@ public:
|
|||
/// A new'ed CommandData object. The breakpoint will take ownership
|
||||
/// of this object.
|
||||
//------------------------------------------------------------------
|
||||
void SetCommandDataCallback(CommandData *cmd_data);
|
||||
void SetCommandDataCallback(std::unique_ptr<CommandData> cmd_data);
|
||||
|
||||
protected:
|
||||
//------------------------------------------------------------------
|
||||
|
|
|
@ -217,14 +217,10 @@ public:
|
|||
bool stop_on_error;
|
||||
};
|
||||
|
||||
class CommandBaton : public Baton {
|
||||
class CommandBaton : public TypedBaton<CommandData> {
|
||||
public:
|
||||
CommandBaton(CommandData *data) : Baton(data) {}
|
||||
|
||||
~CommandBaton() override {
|
||||
delete ((CommandData *)m_data);
|
||||
m_data = nullptr;
|
||||
}
|
||||
CommandBaton(std::unique_ptr<CommandData> Data)
|
||||
: TypedBaton(std::move(Data)) {}
|
||||
|
||||
void GetDescription(Stream *s, lldb::DescriptionLevel level) const override;
|
||||
};
|
||||
|
|
|
@ -32,22 +32,42 @@ namespace lldb_private {
|
|||
//----------------------------------------------------------------------
|
||||
class Baton {
|
||||
public:
|
||||
explicit Baton(void *p) : m_data(p) {}
|
||||
Baton() {}
|
||||
virtual ~Baton() {}
|
||||
|
||||
virtual ~Baton() {
|
||||
// The default destructor for a baton does NOT attempt to clean up
|
||||
// anything in m_baton
|
||||
virtual void *data() = 0;
|
||||
|
||||
virtual void GetDescription(Stream *s,
|
||||
lldb::DescriptionLevel level) const = 0;
|
||||
};
|
||||
|
||||
class UntypedBaton : public Baton {
|
||||
public:
|
||||
UntypedBaton(void *Data) : m_data(Data) {}
|
||||
virtual ~UntypedBaton() {
|
||||
// The default destructor for an untyped baton does NOT attempt to clean up
|
||||
// anything in m_data.
|
||||
}
|
||||
|
||||
virtual void GetDescription(Stream *s, lldb::DescriptionLevel level) const;
|
||||
void *data() override { return m_data; }
|
||||
void GetDescription(Stream *s, lldb::DescriptionLevel level) const override;
|
||||
|
||||
void *m_data; // Leave baton public for easy access
|
||||
};
|
||||
|
||||
private:
|
||||
//------------------------------------------------------------------
|
||||
// For Baton only
|
||||
//------------------------------------------------------------------
|
||||
DISALLOW_COPY_AND_ASSIGN(Baton);
|
||||
template <typename T> class TypedBaton : public Baton {
|
||||
public:
|
||||
explicit TypedBaton(std::unique_ptr<T> Item) : Item(std::move(Item)) {}
|
||||
|
||||
T *getItem() { return Item.get(); }
|
||||
const T *getItem() const { return Item.get(); }
|
||||
|
||||
void *data() override { return Item.get(); }
|
||||
virtual void GetDescription(Stream *s,
|
||||
lldb::DescriptionLevel level) const override {}
|
||||
|
||||
protected:
|
||||
std::unique_ptr<T> Item;
|
||||
};
|
||||
|
||||
} // namespace lldb_private
|
||||
|
|
|
@ -38,6 +38,8 @@
|
|||
|
||||
#include "lldb/lldb-enumerations.h"
|
||||
|
||||
#include "llvm/ADT/STLExtras.h"
|
||||
|
||||
using namespace lldb;
|
||||
using namespace lldb_private;
|
||||
|
||||
|
@ -46,23 +48,13 @@ struct CallbackData {
|
|||
void *callback_baton;
|
||||
};
|
||||
|
||||
class SBBreakpointCallbackBaton : public Baton {
|
||||
class SBBreakpointCallbackBaton : public TypedBaton<CallbackData> {
|
||||
public:
|
||||
SBBreakpointCallbackBaton(SBBreakpoint::BreakpointHitCallback callback,
|
||||
void *baton)
|
||||
: Baton(new CallbackData) {
|
||||
CallbackData *data = (CallbackData *)m_data;
|
||||
data->callback = callback;
|
||||
data->callback_baton = baton;
|
||||
}
|
||||
|
||||
~SBBreakpointCallbackBaton() override {
|
||||
CallbackData *data = (CallbackData *)m_data;
|
||||
|
||||
if (data) {
|
||||
delete data;
|
||||
m_data = nullptr;
|
||||
}
|
||||
: TypedBaton(llvm::make_unique<CallbackData>()) {
|
||||
getItem()->callback = callback;
|
||||
getItem()->callback_baton = baton;
|
||||
}
|
||||
};
|
||||
|
||||
|
|
|
@ -352,7 +352,7 @@ void Breakpoint::SetCallback(BreakpointHitCallback callback, void *baton,
|
|||
bool is_synchronous) {
|
||||
// The default "Baton" class will keep a copy of "baton" and won't free
|
||||
// or delete it when it goes goes out of scope.
|
||||
m_options_up->SetCallback(callback, BatonSP(new Baton(baton)),
|
||||
m_options_up->SetCallback(callback, std::make_shared<UntypedBaton>(baton),
|
||||
is_synchronous);
|
||||
|
||||
SendBreakpointChangedEvent(eBreakpointEventTypeCommandChanged);
|
||||
|
|
|
@ -171,8 +171,8 @@ void BreakpointLocation::SetCallback(BreakpointHitCallback callback,
|
|||
void *baton, bool is_synchronous) {
|
||||
// The default "Baton" class will keep a copy of "baton" and won't free
|
||||
// or delete it when it goes goes out of scope.
|
||||
GetLocationOptions()->SetCallback(callback, BatonSP(new Baton(baton)),
|
||||
is_synchronous);
|
||||
GetLocationOptions()->SetCallback(
|
||||
callback, std::make_shared<UntypedBaton>(baton), is_synchronous);
|
||||
SendBreakpointLocationChangedEvent(eBreakpointEventTypeCommandChanged);
|
||||
}
|
||||
|
||||
|
|
|
@ -23,6 +23,8 @@
|
|||
#include "lldb/Target/Target.h"
|
||||
#include "lldb/Target/ThreadSpec.h"
|
||||
|
||||
#include "llvm/ADT/STLExtras.h"
|
||||
|
||||
using namespace lldb;
|
||||
using namespace lldb_private;
|
||||
|
||||
|
@ -212,11 +214,10 @@ std::unique_ptr<BreakpointOptions> BreakpointOptions::CreateFromStructuredData(
|
|||
}
|
||||
}
|
||||
|
||||
BreakpointOptions *bp_options = new BreakpointOptions(
|
||||
auto bp_options = llvm::make_unique<BreakpointOptions>(
|
||||
condition_text.c_str(), enabled, ignore_count, one_shot);
|
||||
if (cmd_data)
|
||||
bp_options->SetCommandDataCallback(cmd_data.release());
|
||||
return std::unique_ptr<BreakpointOptions>(bp_options);
|
||||
bp_options->SetCommandDataCallback(std::move(cmd_data));
|
||||
return bp_options;
|
||||
}
|
||||
|
||||
StructuredData::ObjectSP BreakpointOptions::SerializeToStructuredData() {
|
||||
|
@ -230,10 +231,10 @@ StructuredData::ObjectSP BreakpointOptions::SerializeToStructuredData() {
|
|||
options_dict_sp->AddStringItem(GetKey(OptionNames::ConditionText),
|
||||
m_condition_text);
|
||||
if (m_baton_is_command_baton) {
|
||||
CommandData *cmd_data =
|
||||
static_cast<CommandData *>(m_callback_baton_sp->m_data);
|
||||
auto cmd_baton =
|
||||
std::static_pointer_cast<CommandBaton>(m_callback_baton_sp);
|
||||
StructuredData::ObjectSP commands_sp =
|
||||
cmd_data->SerializeToStructuredData();
|
||||
cmd_baton->getItem()->SerializeToStructuredData();
|
||||
if (commands_sp) {
|
||||
options_dict_sp->AddItem(
|
||||
BreakpointOptions::CommandData::GetSerializationKey(), commands_sp);
|
||||
|
@ -249,6 +250,17 @@ StructuredData::ObjectSP BreakpointOptions::SerializeToStructuredData() {
|
|||
void BreakpointOptions::SetCallback(BreakpointHitCallback callback,
|
||||
const lldb::BatonSP &callback_baton_sp,
|
||||
bool callback_is_synchronous) {
|
||||
// FIXME: This seems unsafe. If BatonSP actually *is* a CommandBaton, but
|
||||
// in a shared_ptr<Baton> instead of a shared_ptr<CommandBaton>, then we
|
||||
// will set m_baton_is_command_baton to false, which is incorrect.
|
||||
// One possible solution is to make the base Baton class provide a method
|
||||
// such as:
|
||||
// virtual StringRef getBatonId() const { return ""; }
|
||||
// and have CommandBaton override this to return something unique, and then
|
||||
// check for it here. Another option might be to make Baton using the llvm
|
||||
// casting infrastructure, so that we could write something like:
|
||||
// if (llvm::isa<CommandBaton>(callback_baton_sp))
|
||||
// at relevant callsites instead of storing a boolean.
|
||||
m_callback_is_synchronous = callback_is_synchronous;
|
||||
m_callback = callback;
|
||||
m_callback_baton_sp = callback_baton_sp;
|
||||
|
@ -282,7 +294,7 @@ bool BreakpointOptions::InvokeCallback(StoppointCallbackContext *context,
|
|||
lldb::user_id_t break_id,
|
||||
lldb::user_id_t break_loc_id) {
|
||||
if (m_callback && context->is_synchronous == IsCallbackSynchronous()) {
|
||||
return m_callback(m_callback_baton_sp ? m_callback_baton_sp->m_data
|
||||
return m_callback(m_callback_baton_sp ? m_callback_baton_sp->data()
|
||||
: nullptr,
|
||||
context, break_id, break_loc_id);
|
||||
} else
|
||||
|
@ -379,7 +391,7 @@ void BreakpointOptions::GetDescription(Stream *s,
|
|||
|
||||
void BreakpointOptions::CommandBaton::GetDescription(
|
||||
Stream *s, lldb::DescriptionLevel level) const {
|
||||
CommandData *data = (CommandData *)m_data;
|
||||
const CommandData *data = getItem();
|
||||
|
||||
if (level == eDescriptionLevelBrief) {
|
||||
s->Printf(", commands = %s",
|
||||
|
@ -404,8 +416,9 @@ void BreakpointOptions::CommandBaton::GetDescription(
|
|||
s->IndentLess();
|
||||
}
|
||||
|
||||
void BreakpointOptions::SetCommandDataCallback(CommandData *cmd_data) {
|
||||
CommandBatonSP baton_sp(new CommandBaton(cmd_data));
|
||||
void BreakpointOptions::SetCommandDataCallback(
|
||||
std::unique_ptr<CommandData> cmd_data) {
|
||||
auto baton_sp = std::make_shared<CommandBaton>(std::move(cmd_data));
|
||||
SetCallback(BreakpointOptions::BreakpointOptionsCallbackFunction, baton_sp);
|
||||
}
|
||||
|
||||
|
|
|
@ -61,7 +61,8 @@ void Watchpoint::SetCallback(WatchpointHitCallback callback, void *baton,
|
|||
bool is_synchronous) {
|
||||
// The default "Baton" class will keep a copy of "baton" and won't free
|
||||
// or delete it when it goes goes out of scope.
|
||||
m_options.SetCallback(callback, BatonSP(new Baton(baton)), is_synchronous);
|
||||
m_options.SetCallback(callback, std::make_shared<UntypedBaton>(baton),
|
||||
is_synchronous);
|
||||
|
||||
SendWatchpointChangedEvent(eWatchpointEventTypeCommandChanged);
|
||||
}
|
||||
|
|
|
@ -106,7 +106,7 @@ const Baton *WatchpointOptions::GetBaton() const {
|
|||
bool WatchpointOptions::InvokeCallback(StoppointCallbackContext *context,
|
||||
lldb::user_id_t watch_id) {
|
||||
if (m_callback && context->is_synchronous == IsCallbackSynchronous()) {
|
||||
return m_callback(m_callback_baton_sp ? m_callback_baton_sp->m_data
|
||||
return m_callback(m_callback_baton_sp ? m_callback_baton_sp->data()
|
||||
: nullptr,
|
||||
context, watch_id);
|
||||
} else
|
||||
|
@ -173,7 +173,7 @@ void WatchpointOptions::GetDescription(Stream *s,
|
|||
|
||||
void WatchpointOptions::CommandBaton::GetDescription(
|
||||
Stream *s, lldb::DescriptionLevel level) const {
|
||||
CommandData *data = (CommandData *)m_data;
|
||||
const CommandData *data = getItem();
|
||||
|
||||
if (level == eDescriptionLevelBrief) {
|
||||
s->Printf(", commands = %s",
|
||||
|
|
|
@ -24,6 +24,8 @@
|
|||
#include "lldb/Target/Target.h"
|
||||
#include "lldb/Target/Thread.h"
|
||||
|
||||
#include "llvm/ADT/STLExtras.h"
|
||||
|
||||
using namespace lldb;
|
||||
using namespace lldb_private;
|
||||
|
||||
|
@ -215,10 +217,9 @@ are no syntax errors may indicate that a function was declared but never called.
|
|||
if (!bp_options)
|
||||
continue;
|
||||
|
||||
BreakpointOptions::CommandData *cmd_data =
|
||||
new BreakpointOptions::CommandData();
|
||||
auto cmd_data = llvm::make_unique<BreakpointOptions::CommandData>();
|
||||
cmd_data->user_source.SplitIntoLines(line.c_str(), line.size());
|
||||
bp_options->SetCommandDataCallback(cmd_data);
|
||||
bp_options->SetCommandDataCallback(std::move(cmd_data));
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -238,8 +239,7 @@ are no syntax errors may indicate that a function was declared but never called.
|
|||
SetBreakpointCommandCallback(std::vector<BreakpointOptions *> &bp_options_vec,
|
||||
const char *oneliner) {
|
||||
for (auto bp_options : bp_options_vec) {
|
||||
BreakpointOptions::CommandData *cmd_data =
|
||||
new BreakpointOptions::CommandData();
|
||||
auto cmd_data = llvm::make_unique<BreakpointOptions::CommandData>();
|
||||
|
||||
// It's necessary to set both user_source and script_source to the
|
||||
// oneliner.
|
||||
|
@ -251,7 +251,7 @@ are no syntax errors may indicate that a function was declared but never called.
|
|||
cmd_data->script_source.assign(oneliner);
|
||||
cmd_data->stop_on_error = m_options.m_stop_on_error;
|
||||
|
||||
bp_options->SetCommandDataCallback(cmd_data);
|
||||
bp_options->SetCommandDataCallback(std::move(cmd_data));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -207,8 +207,8 @@ are no syntax errors may indicate that a function was declared but never called.
|
|||
new WatchpointOptions::CommandData());
|
||||
if (data_ap) {
|
||||
data_ap->user_source.SplitIntoLines(line);
|
||||
BatonSP baton_sp(
|
||||
new WatchpointOptions::CommandBaton(data_ap.release()));
|
||||
auto baton_sp = std::make_shared<WatchpointOptions::CommandBaton>(
|
||||
std::move(data_ap));
|
||||
wp_options->SetCallback(WatchpointOptionsCallbackFunction, baton_sp);
|
||||
}
|
||||
}
|
||||
|
@ -239,7 +239,8 @@ are no syntax errors may indicate that a function was declared but never called.
|
|||
data_ap->script_source.assign(oneliner);
|
||||
data_ap->stop_on_error = m_options.m_stop_on_error;
|
||||
|
||||
BatonSP baton_sp(new WatchpointOptions::CommandBaton(data_ap.release()));
|
||||
auto baton_sp =
|
||||
std::make_shared<WatchpointOptions::CommandBaton>(std::move(data_ap));
|
||||
wp_options->SetCallback(WatchpointOptionsCallbackFunction, baton_sp);
|
||||
}
|
||||
|
||||
|
|
|
@ -18,4 +18,5 @@
|
|||
using namespace lldb;
|
||||
using namespace lldb_private;
|
||||
|
||||
void Baton::GetDescription(Stream *s, lldb::DescriptionLevel level) const {}
|
||||
void UntypedBaton::GetDescription(Stream *s,
|
||||
lldb::DescriptionLevel level) const {}
|
||||
|
|
|
@ -412,24 +412,23 @@ void ScriptInterpreterPython::IOHandlerInputComplete(IOHandler &io_handler,
|
|||
if (!bp_options)
|
||||
continue;
|
||||
|
||||
std::unique_ptr<BreakpointOptions::CommandData> data_ap(
|
||||
new BreakpointOptions::CommandData());
|
||||
if (data_ap.get()) {
|
||||
data_ap->user_source.SplitIntoLines(data);
|
||||
auto data_ap = llvm::make_unique<BreakpointOptions::CommandData>();
|
||||
if (!data_ap)
|
||||
break;
|
||||
data_ap->user_source.SplitIntoLines(data);
|
||||
|
||||
if (GenerateBreakpointCommandCallbackData(data_ap->user_source,
|
||||
data_ap->script_source)
|
||||
.Success()) {
|
||||
BreakpointOptions::CommandBatonSP baton_sp(
|
||||
new BreakpointOptions::CommandBaton(data_ap.release()));
|
||||
bp_options->SetCallback(
|
||||
ScriptInterpreterPython::BreakpointCallbackFunction, baton_sp);
|
||||
} else if (!batch_mode) {
|
||||
StreamFileSP error_sp = io_handler.GetErrorStreamFile();
|
||||
if (error_sp) {
|
||||
error_sp->Printf("Warning: No command attached to breakpoint.\n");
|
||||
error_sp->Flush();
|
||||
}
|
||||
if (GenerateBreakpointCommandCallbackData(data_ap->user_source,
|
||||
data_ap->script_source)
|
||||
.Success()) {
|
||||
auto baton_sp = std::make_shared<BreakpointOptions::CommandBaton>(
|
||||
std::move(data_ap));
|
||||
bp_options->SetCallback(
|
||||
ScriptInterpreterPython::BreakpointCallbackFunction, baton_sp);
|
||||
} else if (!batch_mode) {
|
||||
StreamFileSP error_sp = io_handler.GetErrorStreamFile();
|
||||
if (error_sp) {
|
||||
error_sp->Printf("Warning: No command attached to breakpoint.\n");
|
||||
error_sp->Flush();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -438,23 +437,20 @@ void ScriptInterpreterPython::IOHandlerInputComplete(IOHandler &io_handler,
|
|||
case eIOHandlerWatchpoint: {
|
||||
WatchpointOptions *wp_options =
|
||||
(WatchpointOptions *)io_handler.GetUserData();
|
||||
std::unique_ptr<WatchpointOptions::CommandData> data_ap(
|
||||
new WatchpointOptions::CommandData());
|
||||
if (data_ap.get()) {
|
||||
data_ap->user_source.SplitIntoLines(data);
|
||||
auto data_ap = llvm::make_unique<WatchpointOptions::CommandData>();
|
||||
data_ap->user_source.SplitIntoLines(data);
|
||||
|
||||
if (GenerateWatchpointCommandCallbackData(data_ap->user_source,
|
||||
data_ap->script_source)) {
|
||||
BatonSP baton_sp(
|
||||
new WatchpointOptions::CommandBaton(data_ap.release()));
|
||||
wp_options->SetCallback(
|
||||
ScriptInterpreterPython::WatchpointCallbackFunction, baton_sp);
|
||||
} else if (!batch_mode) {
|
||||
StreamFileSP error_sp = io_handler.GetErrorStreamFile();
|
||||
if (error_sp) {
|
||||
error_sp->Printf("Warning: No command attached to breakpoint.\n");
|
||||
error_sp->Flush();
|
||||
}
|
||||
if (GenerateWatchpointCommandCallbackData(data_ap->user_source,
|
||||
data_ap->script_source)) {
|
||||
auto baton_sp =
|
||||
std::make_shared<WatchpointOptions::CommandBaton>(std::move(data_ap));
|
||||
wp_options->SetCallback(
|
||||
ScriptInterpreterPython::WatchpointCallbackFunction, baton_sp);
|
||||
} else if (!batch_mode) {
|
||||
StreamFileSP error_sp = io_handler.GetErrorStreamFile();
|
||||
if (error_sp) {
|
||||
error_sp->Printf("Warning: No command attached to breakpoint.\n");
|
||||
error_sp->Flush();
|
||||
}
|
||||
}
|
||||
m_active_io_handler = eIOHandlerNone;
|
||||
|
@ -1238,8 +1234,7 @@ void ScriptInterpreterPython::SetBreakpointCommandCallbackFunction(
|
|||
// Set a Python one-liner as the callback for the breakpoint.
|
||||
Error ScriptInterpreterPython::SetBreakpointCommandCallback(
|
||||
BreakpointOptions *bp_options, const char *command_body_text) {
|
||||
std::unique_ptr<BreakpointOptions::CommandData> data_ap(
|
||||
new BreakpointOptions::CommandData());
|
||||
auto data_ap = llvm::make_unique<BreakpointOptions::CommandData>();
|
||||
|
||||
// Split the command_body_text into lines, and pass that to
|
||||
// GenerateBreakpointCommandCallbackData. That will
|
||||
|
@ -1251,8 +1246,8 @@ Error ScriptInterpreterPython::SetBreakpointCommandCallback(
|
|||
Error error = GenerateBreakpointCommandCallbackData(data_ap->user_source,
|
||||
data_ap->script_source);
|
||||
if (error.Success()) {
|
||||
BreakpointOptions::CommandBatonSP baton_sp(
|
||||
new BreakpointOptions::CommandBaton(data_ap.release()));
|
||||
auto baton_sp =
|
||||
std::make_shared<BreakpointOptions::CommandBaton>(std::move(data_ap));
|
||||
bp_options->SetCallback(ScriptInterpreterPython::BreakpointCallbackFunction,
|
||||
baton_sp);
|
||||
return error;
|
||||
|
@ -1263,8 +1258,7 @@ Error ScriptInterpreterPython::SetBreakpointCommandCallback(
|
|||
// Set a Python one-liner as the callback for the watchpoint.
|
||||
void ScriptInterpreterPython::SetWatchpointCommandCallback(
|
||||
WatchpointOptions *wp_options, const char *oneliner) {
|
||||
std::unique_ptr<WatchpointOptions::CommandData> data_ap(
|
||||
new WatchpointOptions::CommandData());
|
||||
auto data_ap = llvm::make_unique<WatchpointOptions::CommandData>();
|
||||
|
||||
// It's necessary to set both user_source and script_source to the oneliner.
|
||||
// The former is used to generate callback description (as in watchpoint
|
||||
|
@ -1277,7 +1271,8 @@ void ScriptInterpreterPython::SetWatchpointCommandCallback(
|
|||
|
||||
if (GenerateWatchpointCommandCallbackData(data_ap->user_source,
|
||||
data_ap->script_source)) {
|
||||
BatonSP baton_sp(new WatchpointOptions::CommandBaton(data_ap.release()));
|
||||
auto baton_sp =
|
||||
std::make_shared<WatchpointOptions::CommandBaton>(std::move(data_ap));
|
||||
wp_options->SetCallback(ScriptInterpreterPython::WatchpointCallbackFunction,
|
||||
baton_sp);
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue