[lldb] add software watchpoints support #151195
Conversation
|
@llvm/pr-subscribers-lldb Author: None (dlav-sc) ChangesThis patch adds support for software watchpoints in LLDB. All logic is implemented at the LLDB client level, as the To set a software watchpoint, use the Note: For now, this patch doesn't extend lldb watchpoint Patch is 182.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/151195.diff 59 Files Affected:
diff --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h
index 0f788ff602b70..d2785db9a0932 100644
--- a/lldb/include/lldb/API/SBValue.h
+++ b/lldb/include/lldb/API/SBValue.h
@@ -356,7 +356,7 @@ class LLDB_API SBValue {
/// return due to a value not being contained in memory, too
/// large, or watchpoint resources are not available or all in
/// use.
- lldb::SBWatchpoint Watch(bool resolve_location, bool read, bool write,
+ lldb::SBWatchpoint Watch(bool resolve_location, SBWatchpointOptions options,
SBError &error);
// Backward compatibility fix in the interim.
@@ -386,8 +386,8 @@ class LLDB_API SBValue {
/// return due to a value not being contained in memory, too
/// large, or watchpoint resources are not available or all in
/// use.
- lldb::SBWatchpoint WatchPointee(bool resolve_location, bool read, bool write,
- SBError &error);
+ lldb::SBWatchpoint WatchPointee(bool resolve_location,
+ SBWatchpointOptions options, SBError &error);
/// If this value represents a C++ class that has a vtable, return an value
/// that represents the virtual function table.
diff --git a/lldb/include/lldb/API/SBWatchpointOptions.h b/lldb/include/lldb/API/SBWatchpointOptions.h
index 5d1d6ab9e09ff..ad682314e86c9 100644
--- a/lldb/include/lldb/API/SBWatchpointOptions.h
+++ b/lldb/include/lldb/API/SBWatchpointOptions.h
@@ -33,6 +33,9 @@ class LLDB_API SBWatchpointOptions {
void SetWatchpointTypeWrite(lldb::WatchpointWriteType write_type);
lldb::WatchpointWriteType GetWatchpointTypeWrite() const;
+ void SetWatchpointMode(lldb::WatchpointMode mode);
+ lldb::WatchpointMode GetWatchpointMode() const;
+
private:
// This auto_pointer is made in the constructor and is always valid.
mutable std::unique_ptr<WatchpointOptionsImpl> m_opaque_up;
diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h b/lldb/include/lldb/Breakpoint/Watchpoint.h
index ca48a0114888a..74245f0052899 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -15,6 +15,7 @@
#include "lldb/Breakpoint/StoppointSite.h"
#include "lldb/Breakpoint/WatchpointOptions.h"
#include "lldb/Symbol/CompilerType.h"
+#include "lldb/Target/Process.h"
#include "lldb/Target/Target.h"
#include "lldb/Utility/UserID.h"
#include "lldb/lldb-private.h"
@@ -58,37 +59,95 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
const WatchpointEventData &operator=(const WatchpointEventData &) = delete;
};
+ // Make sure watchpoint is properly disabled and subsequently enabled while
+ // performing watchpoint actions.
+ class WatchpointSentry {
+ public:
+ WatchpointSentry(lldb::ProcessSP p_sp, lldb::WatchpointSP w_sp)
+ : process_sp(p_sp), watchpoint_sp(w_sp) {
+ lldbassert(process_sp && watchpoint_sp && "Ill-formed WatchpointSentry!");
+
+ constexpr bool notify = false;
+ watchpoint_sp->TurnOnEphemeralMode();
+ process_sp->DisableWatchpoint(watchpoint_sp, notify);
+ process_sp->AddPreResumeAction(SentryPreResumeAction, this);
+ }
+
+ void DoReenable() {
+ bool was_disabled = watchpoint_sp->IsDisabledDuringEphemeralMode();
+ watchpoint_sp->TurnOffEphemeralMode();
+ constexpr bool notify = false;
+ if (was_disabled) {
+ process_sp->DisableWatchpoint(watchpoint_sp, notify);
+ } else {
+ process_sp->EnableWatchpoint(watchpoint_sp, notify);
+ }
+ }
+
+ ~WatchpointSentry() {
+ DoReenable();
+ process_sp->ClearPreResumeAction(SentryPreResumeAction, this);
+ }
+
+ static bool SentryPreResumeAction(void *sentry_void) {
+ WatchpointSentry *sentry = static_cast<WatchpointSentry *>(sentry_void);
+ sentry->DoReenable();
+ return true;
+ }
+
+ private:
+ lldb::ProcessSP process_sp;
+ lldb::WatchpointSP watchpoint_sp;
+ };
+
Watchpoint(Target &target, lldb::addr_t addr, uint32_t size,
- const CompilerType *type, bool hardware = true);
+ const CompilerType *type, lldb::WatchpointMode mode);
+
+ Watchpoint(Target &target, llvm::StringRef expr, uint32_t size,
+ ExecutionContext &exe_ctx);
~Watchpoint() override;
bool IsEnabled() const;
- // This doesn't really enable/disable the watchpoint. It is currently just
- // for use in the Process plugin's {Enable,Disable}Watchpoint, which should
- // be used instead.
+ // This doesn't really enable/disable the watchpoint. It is currently
+ // just for use in the Process plugin's {Enable,Disable}Watchpoint, which
+ // should be used instead.
void SetEnabled(bool enabled, bool notify = true);
- bool IsHardware() const override;
+ bool IsHardware() const override {
+ return m_mode == lldb::eWatchpointModeHardware;
+ }
bool ShouldStop(StoppointCallbackContext *context) override;
- bool WatchpointRead() const;
- bool WatchpointWrite() const;
- bool WatchpointModify() const;
+ bool WatchpointRead() const { return m_watch_type & LLDB_WATCH_TYPE_READ; }
+ bool WatchpointWrite() const { return m_watch_type & LLDB_WATCH_TYPE_WRITE; }
+ bool WatchpointModify() const {
+ return m_watch_type & LLDB_WATCH_TYPE_MODIFY;
+ }
+
uint32_t GetIgnoreCount() const;
void SetIgnoreCount(uint32_t n);
void SetWatchpointType(uint32_t type, bool notify = true);
void SetDeclInfo(const std::string &str);
- std::string GetWatchSpec();
+ std::string GetWatchSpec() const;
void SetWatchSpec(const std::string &str);
+
+ // This function determines whether we should report a watchpoint value
+ // change. Specifically, it checks the watchpoint condition (if present),
+ // ignore count and so on.
+ //
+ // \param[in] exe_ctx This should represent the current execution context
+ // where execution stopped. It's used only for watchpoint condition
+ // evaluation.
+ //
+ // \return Returns true if we should report a watchpoint hit.
bool WatchedValueReportable(const ExecutionContext &exe_ctx);
// 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.
@@ -124,7 +183,7 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
void *baton, lldb_private::StoppointCallbackContext *context,
lldb::user_id_t break_id, lldb::user_id_t break_loc_id);
- void GetDescription(Stream *s, lldb::DescriptionLevel level);
+ void GetDescription(Stream *s, lldb::DescriptionLevel level) const;
void Dump(Stream *s) const override;
bool DumpSnapshots(Stream *s, const char *prefix = nullptr) const;
void DumpWithLevel(Stream *s, lldb::DescriptionLevel description_level) const;
@@ -186,26 +245,100 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
bool IsDisabledDuringEphemeralMode();
- const CompilerType &GetCompilerType() { return m_type; }
+ CompilerType GetCompilerType() const;
private:
friend class Target;
friend class WatchpointList;
- friend class StopInfoWatchpoint; // This needs to call UndoHitCount()
+
+ lldb::ValueObjectSP CalculateWatchedValue() const;
+
+ void CaptureWatchedValue(lldb::ValueObjectSP new_value_sp) {
+ m_old_value_sp = m_new_value_sp;
+ m_new_value_sp = new_value_sp;
+ }
+
+ bool CheckWatchpointCondition(const ExecutionContext &exe_ctx) const;
+
+ // This class facilitates retrieving a watchpoint's watched value.
+ //
+ // It's used by both hardware and software watchpoints to access
+ // values stored in the process memory.
+ //
+ // To retrieve the value located in the memory, the value's memory address
+ // and its CompilerType are required. ExecutionContext in this case should
+ // contain information about current process, so CalculateWatchedValue
+ // function first of all create ExecutionContext from the process of m_target.
+ class AddressWatchpointCalculateStrategy {
+ public:
+ AddressWatchpointCalculateStrategy(Target &target, lldb::addr_t addr,
+ uint32_t size, const CompilerType *type)
+ : m_target{target}, m_addr{addr},
+ m_type{CreateCompilerType(target, size, type)} {}
+
+ lldb::ValueObjectSP CalculateWatchedValue() const;
+
+ CompilerType GetCompilerType() const { return m_type; };
+
+ private:
+ static CompilerType CreateCompilerType(Target &target, uint32_t size,
+ const CompilerType *type) {
+ if (type && type->IsValid())
+ return *type;
+ // If we don't have a known type, then we force it to unsigned int of the
+ // right size.
+ return DeriveCompilerType(target, size);
+ }
+
+ static CompilerType DeriveCompilerType(Target &target, uint32_t size);
+
+ Target &m_target;
+ lldb::addr_t m_addr;
+ CompilerType m_type;
+ };
+
+ // This class facilitates retrieving a watchpoint's watched value.
+ //
+ // It's used only by software watchpoints to obtain arbitral watched
+ // value, in particular not stored in the process memory.
+ //
+ // To retrieve such values, this class evaluates watchpoint's exression,
+ // therefor it is required that ExecutionContext should know about
+ // stack frame in which watched expression was specified.
+ class ExpressionWatchpointCalculateStrategy {
+ public:
+ ExpressionWatchpointCalculateStrategy(Target &target, llvm::StringRef expr,
+ ExecutionContext exe_ctx)
+ : m_target{target}, m_expr{expr}, m_exe_ctx{exe_ctx} {
+ lldbassert(
+ m_exe_ctx.GetFramePtr() &&
+ "ExecutionContext should contain information about stack frame");
+ }
+
+ lldb::ValueObjectSP CalculateWatchedValue() const;
+
+ private:
+ Target &m_target;
+ llvm::StringRef m_expr; // ref on watchpoint's m_watch_spec_str
+ ExecutionContext m_exe_ctx; // The execution context associated
+ // with watched value.
+ };
+
+ using WatchpointCalculateStrategy =
+ std::variant<AddressWatchpointCalculateStrategy,
+ ExpressionWatchpointCalculateStrategy>;
void ResetHistoricValues() {
m_old_value_sp.reset();
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
- bool m_is_watch_variable; // True if set via 'watchpoint set variable'.
- bool m_is_ephemeral; // True if the watchpoint is in the ephemeral mode,
- // meaning that it is
+ bool m_enabled; // Is this watchpoint enabled
+ lldb::WatchpointMode m_mode; // Is this hardware or software watchpoint
+ bool m_is_watch_variable; // True if set via 'watchpoint set variable'.
+ bool m_is_ephemeral; // True if the watchpoint is in the ephemeral mode,
+ // meaning that it is
// undergoing a pair of temporary disable/enable actions to avoid recursively
// triggering further watchpoint events.
uint32_t m_disabled_count; // Keep track of the count that the watchpoint is
@@ -213,15 +346,19 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
// At the end of the ephemeral mode when the watchpoint is to be enabled
// again, we check the count, if it is more than 1, it means the user-
// supplied actions actually want the watchpoint to be disabled!
- uint32_t m_watch_read : 1, // 1 if we stop when the watched data is read from
- m_watch_write : 1, // 1 if we stop when the watched data is written to
- m_watch_modify : 1; // 1 if we stop when the watched data is changed
+ uint32_t m_watch_type;
uint32_t m_ignore_count; // Number of times to ignore this watchpoint
+ std::string m_watch_spec_str; // Spec for the watchpoint. It is optional for a
+ // hardware watchpoint, in which it is used only
+ // for dumping, but required for a software
+ // watchpoint calculation
+ WatchpointCalculateStrategy m_calculate_strategy;
std::string m_decl_str; // Declaration information, if any.
- std::string m_watch_spec_str; // Spec for the watchpoint.
lldb::ValueObjectSP m_old_value_sp;
lldb::ValueObjectSP m_new_value_sp;
- CompilerType m_type;
+ lldb::ValueObjectSP
+ m_previous_hit_value_sp; // Used in software watchpoints to ensure proper
+ // ignore count behavior
Status m_error; // An error object describing errors associated with this
// watchpoint.
WatchpointOptions m_options; // Settable watchpoint options, which is a
diff --git a/lldb/include/lldb/Interpreter/OptionGroupWatchpoint.h b/lldb/include/lldb/Interpreter/OptionGroupWatchpoint.h
index 527a2612b189b..67d5f5661d828 100644
--- a/lldb/include/lldb/Interpreter/OptionGroupWatchpoint.h
+++ b/lldb/include/lldb/Interpreter/OptionGroupWatchpoint.h
@@ -44,6 +44,7 @@ class OptionGroupWatchpoint : public OptionGroup {
WatchType watch_type;
OptionValueUInt64 watch_size;
bool watch_type_specified;
+ lldb::WatchpointMode watch_mode;
lldb::LanguageType language_type;
private:
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index a8892e9c43225..3fe134e664c26 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2256,6 +2256,8 @@ class Process : public std::enable_shared_from_this<Process>,
return m_watchpoint_resource_list;
}
+ llvm::SmallVector<lldb::WatchpointSP> GetEnabledSoftwareWatchpoint();
+
// When ExtendedBacktraces are requested, the HistoryThreads that are created
// need an owner -- they're saved here in the Process. The threads in this
// list are not iterated over - driver programs need to request the extended
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 0d4e11b65339e..116d4c78e09f7 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -795,9 +795,17 @@ class Target : public std::enable_shared_from_this<Target>,
bool resolve_indirect_symbols);
// Use this to create a watchpoint:
- lldb::WatchpointSP CreateWatchpoint(lldb::addr_t addr, size_t size,
- const CompilerType *type, uint32_t kind,
- Status &error);
+ lldb::WatchpointSP CreateWatchpointByAddress(lldb::addr_t addr, size_t size,
+ const CompilerType *type,
+ uint32_t kind,
+ lldb::WatchpointMode mode,
+ Status &error);
+
+ // Can create only software watchpoints
+ lldb::WatchpointSP CreateWatchpointByExpression(llvm::StringRef expr,
+ size_t size,
+ ExecutionContext &exe_ctx,
+ uint32_t kind, Status &error);
lldb::WatchpointSP GetLastCreatedWatchpoint() {
return m_last_created_watchpoint;
diff --git a/lldb/include/lldb/Target/ThreadPlan.h b/lldb/include/lldb/Target/ThreadPlan.h
index a7bac8cc5ecf6..0aec457d03f73 100644
--- a/lldb/include/lldb/Target/ThreadPlan.h
+++ b/lldb/include/lldb/Target/ThreadPlan.h
@@ -313,6 +313,7 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>,
eKindStepThrough,
eKindStepUntil,
eKindSingleThreadTimeout,
+ eKindWatchpointStepInstruction
};
virtual ~ThreadPlan();
diff --git a/lldb/include/lldb/Target/ThreadPlanStepInstruction.h b/lldb/include/lldb/Target/ThreadPlanStepInstruction.h
index 52a5a2efc0a47..508d50b996766 100644
--- a/lldb/include/lldb/Target/ThreadPlanStepInstruction.h
+++ b/lldb/include/lldb/Target/ThreadPlanStepInstruction.h
@@ -17,8 +17,10 @@ namespace lldb_private {
class ThreadPlanStepInstruction : public ThreadPlan {
public:
- ThreadPlanStepInstruction(Thread &thread, bool step_over, bool stop_others,
- Vote report_stop_vote, Vote report_run_vote);
+ ThreadPlanStepInstruction(
+ Thread &thread, bool step_over, bool stop_others, Vote report_stop_vote,
+ Vote report_run_vote,
+ ThreadPlanKind kind = ThreadPlan::eKindStepInstruction);
~ThreadPlanStepInstruction() override;
@@ -36,6 +38,8 @@ class ThreadPlanStepInstruction : public ThreadPlan {
void SetUpState();
+ lldb::addr_t GetInstructionAddr() const { return m_instruction_addr; };
+
private:
friend lldb::ThreadPlanSP Thread::QueueThreadPlanForStepSingleInstruction(
bool step_over, bool abort_other_plans, bool stop_other_threads,
diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h
index 69e8671b6e21b..370e5dbb1f3c8 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -1076,6 +1076,9 @@ enum InstructionControlFlowKind {
FLAGS_ENUM(WatchpointKind){eWatchpointKindWrite = (1u << 0),
eWatchpointKindRead = (1u << 1)};
+/// TODO: Maybe we should extend this to any stoppoint.
+enum WatchpointMode { eWatchpointModeHardware, eWatchpointModeSoftware };
+
enum GdbSignal {
eGdbSignalBadAccess = 0x91,
eGdbSignalBadInstruction = 0x92,
diff --git a/lldb/packages/Python/lldbsuite/test/lldbwatchpointutils.py b/lldb/packages/Python/lldbsuite/test/lldbwatchpointutils.py
new file mode 100644
index 0000000000000..9d313040dae07
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lldbwatchpointutils.py
@@ -0,0 +1,49 @@
+import os
+import os.path
+from enum import Enum
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.gdbclientutils import *
+
+class WatchpointType(list[str], Enum):
+ READ = ['r', 'read']
+ WRITE = ['w', 'write']
+ READ_WRITE = ['rw', 'read_write']
+ MODIFY = ['m', 'modify']
+
+class WatchpointCLICommandVariant(str, Enum):
+ EXPRESSION = "expression"
+ VARIABLE = "variable"
+
+def _get_SBWatchpointOptions(wp_type, wp_mode):
+ wp_opts = lldb.SBWatchpointOptions()
+
+ if (wp_type == WatchpointType.READ or wp_type == WatchpointType.READ_WRITE):
+ wp_opts.SetWatchpointTypeRead(True)
+ if (wp_type == WatchpointType.WRITE or wp_type == WatchpointType.READ_WRITE):
+ wp_opts.SetWatchpointTypeWrite(lldb.eWatchpointWriteTypeAlways)
+ if (wp_type == WatchpointType.MODIFY):
+ wp_opts.SetWatchpointTypeWrite(lldb.eWatchpointWriteTypeOnModify)
+
+ wp_opts.SetWatchpointMode(wp_mode)
+
+ return wp_opts
+
+def get_set_watchpoint_CLI_command(command_variant, wp_type, wp_mode):
+ cmd = ["watchpoint set", command_variant.value, "-w", wp_type.value[1]]
+ if (wp_mode == lldb.eWatchpointModeSoftware):
+ cmd.append("-S")
+ return " ".join(map(str, cmd))
+
+def set_watchpoint_at_value(value, wp_type, wp_mode, error):
+ wp_opts = _get_SBWatchpointOptions(wp_type, wp_mode)
+ return value.Watch(True, wp_opts, error)
+
+def set_watchpoint_at_pointee(value, wp_type, wp_mode, error):
+ wp_opts = _get_SBWatchpointOptions(wp_type, wp_mode)
+ return value.WatchPointee(True, wp_opts, error)
+
+def set_watchpoint_by_address(target, addr, size, wp_type, wp_mode, error):
+ wp_opts = _get_SBWatchpointOptions(wp_type, wp_mode)
+ return target.WatchpointCreateByAddress(addr, size, wp_opts, error)
+
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index f26f7951edc6f..df69926b15b8e 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1301,8 +1301,7 @@ lldb::SBWatchpoint SBTarget::WatchAddress(lldb::addr_t addr, size_t size,
SBWatchpointOptions options;
options.SetWatchpointTypeRead(read);
- if (modify)
- options.SetWatchpointTypeWrite(eWatchpointWriteTypeOnModify);
+ options.SetWatchpointTypeWrite(eWatchpointWriteTypeOnModify);
return WatchpointCreateByAddress(addr, size, options, error);
}
@@ -1313,7 +1312,10 @@ S...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
✅ With the latest revision this PR passed the Python code formatter. |
lldb/include/lldb/API/SBValue.h
Outdated
There was a problem hiding this comment.
You can't change extant SB API's - we maintain strict binary compatibility. You'll have to add these as overloads.
There was a problem hiding this comment.
If I want to watch a global variable, what stack frame is relevant?
daniilavdeev
force-pushed
the
dlav-sc/lldb_software_watchpoints
branch
from
1876b81 to
e522e29
Compare
This patch adds support for software watchpoints in LLDB. This functionality has long been available in GDB, so it is also necessary for LLDB. All logic is implemented at the LLDB client level, as the GDB remote protocol currently does not support packets that establish software watchpoints. When software watchpoints are present, LLDB will perform instruction steps and check the watchpoints for hits. To set a software watchpoints, use the -S flag: wa s v -S local_variable wa s e -S -- global_ptr Note: For now, this patch doesn't extend lldb watchpoint functionality. It simply provides the option to use software watchpoints instead of hardware ones in the same scenarios. However, I plan to extend this functionality in the future. For example, we could use software watchpoints to monitor registers and other arbitrary expressions that aren't placed in memory.
This patch adds support for software breakpoints in the Python API. SBWatchpointOptions, in addition to the watchpoint type, also contains information on whether the breakpoint is hardware or software. The interfaces SBValue::Watch and SBValue::WatchPointee have been changed. Now, instead of using bool Read and bool Write to define the breakpoint type, these functions take SBWatchpointOptions as an argument. Before patch: lldb::SBWatchpoint SBValue::Watch(bool resolve_location, bool read, bool write,SBError &error); After patch: lldb::SBWatchpoint SBValue::Watch(bool resolve_location, SBWatchpointOptions options, SBError &error);
daniilavdeev
force-pushed
the
dlav-sc/lldb_software_watchpoints
branch
from
e522e29 to
db300f0
Compare
This patch adds software watchpoints tests
Commit cbb4e99 introduced a regression in lldb software watchpoint tests. The issue occurs in the following scenario: 1. Set a software watchpoint 2. Step over a function call 3. The watchpoint triggers inside the function being stepped over 4. After continuing execution, we expect to stop after returning from the function (as with a normal step-over operation) However, with the mentioned patch, execution no longer stops at the end of the function. The patch seems to have modified ThreadPlanStepOut's stop condition - it now only stops when the stop reason is eStopReasonBreakpoint. The problem occurs because ThreadPlanStepOut only enables its internal breakpoint when it's at the top of the ThreadPlanStack. With software watchpoints, execution proceeds step-by-step while checking if any watched values change after each step. In this case, ThreadPlanWatchpointStepInstruction remains higher on the stack than ThreadPlanStepOut, therefore the internal breakpoint stays disabled. As a result, we stop due to eStopReasonTrace instead of the desired eStopReasonBreakpoint, and consequently fail to stop at the intended location. This patch removes the requirement for ThreadPlanStepOut to be the current plan and enables the internal breakpoint unconditionally. I believe this check is legacy code that can be safely removed - I was quite surprised to find this breakpoint isn't always enabled by default. However, if issues arise, we could consider modifying the condition introduced by the patch. For example, instead of checking the stop reason, we could verify whether the stop address matches the target address of the breakpoint that ThreadPlanStepOut uses to stop at the end of the step.
daniilavdeev
force-pushed
the
dlav-sc/lldb_software_watchpoints
branch
from
db300f0 to
343b749
Compare
Currently, lldb silently disables watchpoints on local variables when execution leaves the variable's scope. This patch makes lldb emit a warning to the user when a watchpoint leaves its scope.
daniilavdeev
force-pushed
the
dlav-sc/lldb_software_watchpoints
branch
from
343b749 to
d0fa83c
Compare
|
@jimingham , @DavidSpickett, @labath, @jasonmolenda, @clayborg, @Michael137 could you take a look at this patch, please? I’d be glad to hear your thoughts and recommendations. |
|
I did a quick read-through but need to read more closely. Generally speaking, a software implemented watchpoint is going to be so slow that I doubt it will be useful in any real program debug session, but that's not a reason to not consider the change. I appreciate that it looks like you need to request a software watchpoint explicitly, I would never want someone setting too many watchpoints on a target with a limited number to end up with a software watchpoint silently when the hardware resources are exhausted. I do wonder if we can reliably instruction step over every instruction on every target (atomics instructions/sequences seem to be a sore point), or if we might need to have a insn-stepping-deny-list of instructions for a given target and fall back to a breakpoint & continue approach on those. On a target where we do instruction emulation to determine the next pc value, put a breakpoint there, and continue the thread for "instruction step", we're less likely to hit that kind of issue I suspect. |
Well, yeah, they're quite slow, especially on targets that don't support hardware stepping. I've tested this patch on both riscv and x86 and the software watchpoints work significantly faster on x86 compared to riscv. I believe this difference comes from x86 using hardware stepping while riscv relies on software stepping. However, such performance is specific to software watchpoints. Since gdb already supports software watchpoints, I don't see why we shouldn't add them to lldb as well.
The original goal of my work was to enable watchpoint functionality at any cost on targets without hardware watchpoint support, in particular riscv. Now, with some limitations and speed tradeoffs of course, software watchpoints are available. Moreover, while hardware watchpoints only work for memory locations, software watchpoints don't have this limitation. In the future, we could use them to monitor registers and other complex expressions, like gdb do.
Yep. I might be wrong, but as I recall, gdb silently starts to use software watchpoints. In my opinion, this is a poor decision - users should manually, explicitly set software watchpoints.
It's hard to say for sure, but generally there are no guarantees. While developing this patch, I encountered several bugs related to stepping (#127506) and discovered that lldb lacks proper support for stepping through atomic sequences on riscv (#127505). |
|
Will need some time to give this proper attention, but -
Can you be specific here, what is the state of watchpoints for RISC-V? Proposal, ratified, available in hardware, only available in simulation? If this were the only reason to have software watchpoints, I would be hesitant to accept them if in a few years time RISC-V was going to have hardware watchpoints anyway. See also hardware single stepping being missing for a while, but in that case at least the software step is not user visible. So we could remove it once the majority of hardware does support it. Windows on Arm in theory has hardware watchpoints but the APIs to discover them are immature. Happy to wait for Microsoft to fix that though, wouldn't justify this feature on its own.
This sounds like a more persuasive use case for me.
I am interested in what they have done with them. Can you link any relevant documentation for it? A set of examples would be ideal.
I take from this that GDB also does not use extra remote protocol packets to implement this? Which is fine, we would aim to match GDB first before extending anything. GDB code breaks actually can have a condition list that's evaluated on the target side: Which is along the same lines, but they'd need the debug server to be aware that a software watch was set for this to be used in that situation. |
If it was just single instructions, we realise we can't step it accurately, disable all software watchpoints, and tell the user what happened. The problem with sequences is we don't know that we're in them. Very few people have complained about atomic sequences, but then again, most people don't instruction single step a lot of code. I can't think of another "sequence" thing we don't handle, but I feel like there can't be more than 2/3 things. That combined with v8.1 having single instruction atomics, mitigates it further. Let me do some research to see if it really is a handful of sequence types. |
For context regarding the debug specification status:
That specification also covers facilities for hardware-assisted single-stepping. But again, the relevant Linux-side support is missing at the moment.
Oh, I should probably be more accurate when describing the motivation. While it’s true that support for hardware-assisted watchpoints should be available in a year or so, there are still limitations:
In addition to my thoughs I think gdb's documentation provides a
|
Yes
LLDB supports conditions on watchpoints too and there's nothing special about software watchpoints in this regard. LLDB evaluates conditions on the target using the expression mechanism, so lldb-server doesn't even know about conditions of its watchpoints. Originally, the condition checking logic was implemented in |
I found this a bit confusing when reading this patch. There are really two "conditions" being talked about here. There's the primary "hidden" condition that tests "did the region we are watching change value" and then there's the watchpoint condition set with Anyway, by itself, I don't think in this case running either of these conditions on the stub side on its own will matter. So long as lldb is driving the single stepping you would still be doing the most expensive thing - stopping to talk to lldb... We'd need to add a |
|
One kind of testing we need to add here - given the performance degradation this feature entails - is that if you have no software watchpoints, we don't do this single step all threads, then if you set a software watchpoint we start single-stepping all threads and then when you remove the software watchpoint we stop the single-stepping again. |
I'm not completely clear if this bytecode is executed in the inferior process (gdb inserts/compiles in the agent that runs the expression, inserts jumps to the agent) or if this is executed in gdbserver (it has bytecode associated with certain breakpoints, runs that, collects data or resumes execution unless a condition is matched). For a conditional breakpoint today, we hit the breakpoint trap, gdbserver/lldb-server/debugserver are invoked with that. They send a packet up to lldb saying we've stopped. lldb recognizes the breakpoint, finds a condition, does a memory/register read, checks if the condition has been met. If not, it tells the debug stub to resume the inferior, the debug stub resumes the inferior (remove breakpoint, instruction step, re-insert breakpoint, continue) If the Agent Expression is evaluated at the debug stub level, this gets lldb out of the equation but we're still halting the inferior process and context switching over to the debug stub. We're still going to be quite slow. This does allow for the debugger to (in theory) disconnect from the inferior process -- so if you were developing an app that needed to be outdoors when it was executing, being able to disconnect lldb while it is collecting data at these sample points, and downloading it later when you're back at your computer could be valuable. From a "stopping the inferior and resuming again is slow" perspective, I'm not sure if eliminating lldb from the equation speeds it up enough to fundamentally change the perf tradeoffs with an approach like this. (of course in this case we're talking about instruction stepping the process so we can't do it entirely in-process in the inferior, we need the debug agent of lldb-server/debugserver evaluating the agent expression to detect if we should report a stop to lldb) |
I only skimmed the Debug Specification 1.0 so I may have missed it, but is this only designed for JTAG style debugging of a device? On AArch64 there's "external debug" (JTAG) and "self hosted debug" where lldb/gdb are running on the same system as the inferior debuggee, and controlling it. When the inferior process hits a breakpoint/watchpoint/hardware breakpoint trap, it traps into kernel execution level which packages the exception up and sends it to the debugger for handling. I may have skimmed the doc too quickly, but I didn't see a self-hosted side to this spec, how is Linux going to support it? It's not related to this PR, I'm just curious. Also, their Program Buffer is kind of interesting, a group of instructions that can be run when a watchpoint is triggered. I didn't read in more detail about what you can do with a Program Buffer bit of code, but it sounds like it could be interesting for use cases where you can't halt for very long. |
The debug specification describes two extensions
By default access to Trigger Module is available from M mode. So Linux is going to request SBI firmware to access this resource. This respected API is available in OpenSBI since 1.5 |
Program buffer is for external debugger, unfortunately. |
The first condition you mentioned is generally speaking only necessary for modify-type watchpoints. With hardware watchpoints, lldb-server can only detect read/write access to the watched region but cannot determine the actual value. Therefore, to implement modify-type watchpoint logic, lldb-client needs to perform additional checks to verify that the watched value has actually changed (ensuring we're not writing the same value) after lldb-server reports write access to the watched region. For software watchpoints, the entire logic is implemented on the lldb-client side. Since we cannot trigger on memory access (read or write), software watchpoints can only be of the modify type (gdb also only supports modify-type for software watchpoints). lldb-client checks whether the watched value has changed, similar to hardware modify-type watchpoints, but after each step in this case. The second condition is the one I referenced in my previous comment. It is checked by lldb-client using the standard expression mechanism (similar to when you type expr -- ). Please take a look at |
Yeah, I have consider adding such tests as well, but I currently have no idea how to implement them. I used lldb step logs to observe the thread plan stack and to ensure that the WatchpointThreadPlan is used only when necessary. However, I haven't found a way to obtain the state of the thread plan stack via the python api or the lldb command-line interface to use in a test. We could potentially parse the logs in tests to get the thread plan stack, but that seems too unreliable. Please share your thoughts, if you have any ideas on the matter. |
|
I plan to look at the PR in detail at some point, but for now...
Could we check at a lower level? The reverse debugging tests wrap a real lldb-server in a Python class, you could do the same and check for single step and software break set packets during the time where lldb should not be using them. (or fake the entire debug server but I bet you'll have to provide all sorts of things to get it working) |
|
There's |
|
There isn't an SB API for this, but it would be easy to add if you wanted to inspect it that way. |
6 participants
This patch adds support for software watchpoints in LLDB.
This functionality has long been available in GDB, so it
is also necessary for LLDB.
All logic is implemented at the LLDB client level, as the
GDB remote protocol currently does not support packets
that establish software watchpoints. When software
watchpoints are present, LLDB will perform instruction
steps and check the watchpoints for hits.
To set a software watchpoint, use the
-Sflag:Note: For now, this patch doesn't extend lldb watchpoint
functionality. It simply provides the option to use software watchpoints
instead of hardware ones in the same scenarios. However, I plan to
extend this functionality in the future. For example, we could use
software watchpoints to monitor registers and other arbitrary
expressions that aren't placed in memory.