Skip to content

[lldb][NFC] Refactor StopInfoWatchpoint::PerformAction #163696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Parent: [lldb][NFC] Refactor Watchpoint class
wants to merge 1 commit into
base: users/dlav-sc/lldb_refactor_watchpoint_class
Choose a base branch
from
+52 −197
Open

[lldb][NFC] Refactor StopInfoWatchpoint::PerformAction #163696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions lldb/include/lldb/Breakpoint/Watchpoint.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
void SetDeclInfo(const std::string &str);
std::string GetWatchSpec() const;
void SetWatchSpec(const std::string &str);
bool WatchedValueReportable(const ExecutionContext &exe_ctx);

// This function determines whether we should report a watchpoint value
// change. Specifically, it checks the watchpoint condition (if present),
Expand All @@ -102,7 +101,6 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
// Snapshot management interface.
bool IsWatchVariable() const;
void SetWatchVariable(bool val);
bool CaptureWatchedValue(const ExecutionContext &exe_ctx);

/// \struct WatchpointVariableContext
/// \brief Represents the context of a watchpoint variable.
Expand Down Expand Up @@ -205,7 +203,6 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
private:
friend class Target;
friend class WatchpointList;
friend class StopInfoWatchpoint; // This needs to call UndoHitCount()

lldb::ValueObjectSP CalculateWatchedValue() const;

Expand All @@ -223,8 +220,6 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
m_new_value_sp.reset();
}

void UndoHitCount() { m_hit_counter.Decrement(); }

Target &m_target;
bool m_enabled; // Is this watchpoint enabled
bool m_is_hardware; // Is this a hardware watchpoint
Expand Down
56 changes: 0 additions & 56 deletions lldb/source/Breakpoint/Watchpoint.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,66 +260,10 @@ bool Watchpoint::IsWatchVariable() const { return m_is_watch_variable; }

void Watchpoint::SetWatchVariable(bool val) { m_is_watch_variable = val; }

bool Watchpoint::CaptureWatchedValue(const ExecutionContext &exe_ctx) {
ConstString g_watch_name("$__lldb__watch_value");
m_old_value_sp = m_new_value_sp;
Address watch_address(GetLoadAddress());
if (!m_type.IsValid()) {
// Don't know how to report new & old values, since we couldn't make a
// scalar type for this watchpoint. This works around an assert in
// ValueObjectMemory::Create.
// FIXME: This should not happen, but if it does in some case we care about,
// we can go grab the value raw and print it as unsigned.
return false;
}
m_new_value_sp = ValueObjectMemory::Create(
exe_ctx.GetBestExecutionContextScope(), g_watch_name.GetStringRef(),
watch_address, m_type);
m_new_value_sp = m_new_value_sp->CreateConstantValue(g_watch_name);
return (m_new_value_sp && m_new_value_sp->GetError().Success());
}

bool Watchpoint::WatchedValueReportable(const ExecutionContext &exe_ctx) {
if (!WatchpointModify() || WatchpointRead())
return true;
if (!m_type.IsValid())
return true;

ConstString g_watch_name("$__lldb__watch_value");
Address watch_address(GetLoadAddress());
ValueObjectSP newest_valueobj_sp = ValueObjectMemory::Create(
exe_ctx.GetBestExecutionContextScope(), g_watch_name.GetStringRef(),
watch_address, m_type);
newest_valueobj_sp = newest_valueobj_sp->CreateConstantValue(g_watch_name);
Status error;

DataExtractor new_data;
DataExtractor old_data;

newest_valueobj_sp->GetData(new_data, error);
if (error.Fail())
return true;
m_new_value_sp->GetData(old_data, error);
if (error.Fail())
return true;

if (new_data.GetByteSize() != old_data.GetByteSize() ||
new_data.GetByteSize() == 0)
return true;

if (memcmp(new_data.GetDataStart(), old_data.GetDataStart(),
old_data.GetByteSize()) == 0)
return false; // Value has not changed, user requested modify watchpoint

return true;
}

// RETURNS - true if we should stop at this breakpoint, false if we
// should continue.

bool Watchpoint::ShouldStop(StoppointCallbackContext *context) {
m_hit_counter.Increment();

return IsEnabled();
}

Expand Down
188 changes: 52 additions & 136 deletions lldb/source/Target/StopInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
#include "lldb/Utility/StreamString.h"
#include "lldb/ValueObject/ValueObject.h"

#include "llvm/ADT/ScopeExit.h"

using namespace lldb;
using namespace lldb_private;

Expand Down Expand Up @@ -704,6 +706,16 @@ class StopInfoBreakpoint : public StopInfo {

// StopInfoWatchpoint

static void ReportWatchpointHit(const ExecutionContext &exe_ctx,
lldb::WatchpointSP wp_sp) {
Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger();
StreamSP output_sp = debugger.GetAsyncOutputStream();
if (wp_sp->DumpSnapshots(output_sp.get())) {
output_sp->EOL();
output_sp->Flush();
}
}

class StopInfoWatchpoint : public StopInfo {
public:
// Make sure watchpoint is properly disabled and subsequently enabled while
Expand Down Expand Up @@ -959,159 +971,63 @@ class StopInfoWatchpoint : public StopInfo {
}

void PerformAction(Event *event_ptr) override {
Log *log = GetLog(LLDBLog::Watchpoints);
// We're going to calculate if we should stop or not in some way during the
// course of this code. Also by default we're going to stop, so set that
// here.
m_should_stop = true;


ThreadSP thread_sp(m_thread_wp.lock());
if (thread_sp) {

WatchpointSP wp_sp(
thread_sp->CalculateTarget()->GetWatchpointList().FindByID(
GetValue()));
if (wp_sp) {
// This sentry object makes sure the current watchpoint is disabled
// while performing watchpoint actions, and it is then enabled after we
// are finished.
ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
ProcessSP process_sp = exe_ctx.GetProcessSP();

WatchpointSentry sentry(process_sp, wp_sp);

if (m_silently_skip_wp) {
m_should_stop = false;
wp_sp->UndoHitCount();
}

if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount()) {
m_should_stop = false;
m_should_stop_is_valid = true;
}

Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger();

if (m_should_stop && wp_sp->GetConditionText() != nullptr) {
// We need to make sure the user sees any parse errors in their
// condition, so we'll hook the constructor errors up to the
// debugger's Async I/O.
ExpressionResults result_code;
EvaluateExpressionOptions expr_options;
expr_options.SetUnwindOnError(true);
expr_options.SetIgnoreBreakpoints(true);
ValueObjectSP result_value_sp;
result_code = UserExpression::Evaluate(
exe_ctx, expr_options, wp_sp->GetConditionText(),
llvm::StringRef(), result_value_sp);

if (result_code == eExpressionCompleted) {
if (result_value_sp) {
Scalar scalar_value;
if (result_value_sp->ResolveValue(scalar_value)) {
if (scalar_value.ULongLong(1) == 0) {
// The condition failed, which we consider "not having hit
// the watchpoint" so undo the hit count here.
wp_sp->UndoHitCount();
m_should_stop = false;
} else
m_should_stop = true;
LLDB_LOGF(log,
"Condition successfully evaluated, result is %s.\n",
m_should_stop ? "true" : "false");
} else {
m_should_stop = true;
LLDB_LOGF(
log,
"Failed to get an integer result from the expression.");
}
}
} else {
const char *err_str = "<unknown error>";
if (result_value_sp)
err_str = result_value_sp->GetError().AsCString();

LLDB_LOGF(log, "Error evaluating condition: \"%s\"\n", err_str);

StreamString strm;
strm << "stopped due to an error evaluating condition of "
"watchpoint ";
wp_sp->GetDescription(&strm, eDescriptionLevelBrief);
strm << ": \"" << wp_sp->GetConditionText() << "\"\n";
strm << err_str;

Debugger::ReportError(strm.GetString().str(),
exe_ctx.GetTargetRef().GetDebugger().GetID());
}
}

// If the condition says to stop, we run the callback to further decide
// whether to stop.
if (m_should_stop) {
// FIXME: For now the callbacks have to run in async mode - the
// first time we restart we need
// to get out of there. So set it here.
// When we figure out how to nest watchpoint hits then this will
// change.

bool old_async = debugger.GetAsyncExecution();
debugger.SetAsyncExecution(true);

StoppointCallbackContext context(event_ptr, exe_ctx, false);
bool stop_requested = wp_sp->InvokeCallback(&context);
if (!thread_sp)
return;

debugger.SetAsyncExecution(old_async);
auto Deferred = llvm::make_scope_exit([this]() {
Log *log = GetLog(LLDBLog::Watchpoints);
LLDB_LOGF(log,
"Process::%s returning from action with m_should_stop: %d.",
__FUNCTION__, m_should_stop);
m_should_stop_is_valid = true;
});

// Also make sure that the callback hasn't continued the target. If
// it did, when we'll set m_should_stop to false and get out of here.
if (HasTargetRunSinceMe())
m_should_stop = false;
WatchpointSP wp_sp(
thread_sp->CalculateTarget()->GetWatchpointList().FindByID(GetValue()));
if (!wp_sp) {
Log *log_process(GetLog(LLDBLog::Process));

if (m_should_stop && !stop_requested) {
// We have been vetoed by the callback mechanism.
m_should_stop = false;
}
}
LLDB_LOGF(log_process,
"Process::%s could not find watchpoint id: %" PRId64 "...",
__FUNCTION__, m_value);
m_should_stop = true;
return;
}

// Don't stop if the watched region value is unmodified, and
// this is a Modify-type watchpoint.
if (m_should_stop && !wp_sp->WatchedValueReportable(exe_ctx)) {
wp_sp->UndoHitCount();
m_should_stop = false;
}
// We're going to calculate if we should stop or not in some way during the
// course of this code. Also by default we're not going to stop, so set
// that here.
m_should_stop = false;

// Finally, if we are going to stop, print out the new & old values:
if (m_should_stop) {
wp_sp->CaptureWatchedValue(exe_ctx);
ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));

Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger();
StreamUP output_up = debugger.GetAsyncOutputStream();
if (wp_sp->DumpSnapshots(output_up.get()))
output_up->EOL();
}
// This sentry object makes sure the current watchpoint is disabled
// while performing watchpoint actions, and it is then enabled after we
// are finished.
WatchpointSentry sentry(exe_ctx.GetProcessSP(), wp_sp);

} else {
Log *log_process(GetLog(LLDBLog::Process));
// Hardware watchpoint may want to be skipped, so check it here.
if (m_silently_skip_wp)
return;

LLDB_LOGF(log_process,
"Process::%s could not find watchpoint id: %" PRId64 "...",
__FUNCTION__, m_value);
}
LLDB_LOGF(log,
"Process::%s returning from action with m_should_stop: %d.",
__FUNCTION__, m_should_stop);
// Check watchpoint condition, ignore count and other staff related to
// watchpoint here.
StoppointCallbackContext context(event_ptr, exe_ctx, false);
if (!wp_sp->ShouldReport(context))
return;

m_should_stop_is_valid = true;
}
// Finally, if we are going to stop, print out the new & old values:
m_should_stop = true;
ReportWatchpointHit(exe_ctx, wp_sp);
}

private:
void SetStepOverPlanComplete() {
assert(m_using_step_over_plan);
m_step_over_plan_complete = true;
}

bool m_should_stop = false;
bool m_should_stop_is_valid = false;
// A false watchpoint hit has happened -
Expand Down
Loading