[Lldb-commits] [lldb] [lldb] refactor watchpoint functionality (PR #159807)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Sep 19 09:09:04 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: None (dlav-sc)
<details>
<summary>Changes</summary>
This patch is the first in a series of patches that add software watchpoint support to LLDB.
The main purpose of this patch is to refactor watchpoint functionality in LLDB, making it easier to implement software watchpoint logic.
---
Patch is 66.95 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/159807.diff
15 Files Affected:
- (modified) lldb/include/lldb/Breakpoint/Watchpoint.h (+132-26)
- (modified) lldb/include/lldb/Interpreter/OptionGroupWatchpoint.h (+1)
- (modified) lldb/include/lldb/Target/Target.h (+5-3)
- (modified) lldb/include/lldb/lldb-enumerations.h (+2)
- (modified) lldb/source/API/SBTarget.cpp (+2-2)
- (modified) lldb/source/API/SBValue.cpp (+2-2)
- (modified) lldb/source/Breakpoint/Watchpoint.cpp (+208-106)
- (modified) lldb/source/Commands/CommandObjectWatchpoint.cpp (+32-20)
- (modified) lldb/source/Interpreter/OptionGroupWatchpoint.cpp (+10-2)
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+34-23)
- (modified) lldb/source/Target/StopInfo.cpp (+81-176)
- (modified) lldb/source/Target/Target.cpp (+159-75)
- (modified) lldb/source/Target/Thread.cpp (+64-55)
- (modified) lldb/test/API/commands/watchpoints/hello_watchlocation/TestWatchLocation.py (+1-1)
- (modified) lldb/test/API/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py (+1-1)
``````````diff
diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h b/lldb/include/lldb/Breakpoint/Watchpoint.h
index ca48a0114888a..62435fe7dcce2 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,92 @@ 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() 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 +180,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 +242,72 @@ 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;
+ };
+
+ using WatchpointCalculateStrategy =
+ std::variant<AddressWatchpointCalculateStrategy>;
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 +315,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_decl_str; // Declaration information, if any.
- std::string m_watch_spec_str; // Spec for the 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.
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/Target.h b/lldb/include/lldb/Target/Target.h
index 14a09f29094d5..bce2f178d4799 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -796,9 +796,11 @@ 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);
lldb::WatchpointSP GetLastCreatedWatchpoint() {
return m_last_created_watchpoint;
diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h
index fec9fdef44df9..f4604f9ac74b3 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -1081,6 +1081,8 @@ enum InstructionControlFlowKind {
FLAGS_ENUM(WatchpointKind){eWatchpointKindWrite = (1u << 0),
eWatchpointKindRead = (1u << 1)};
+enum WatchpointMode { eWatchpointModeHardware, eWatchpointModeSoftware };
+
enum GdbSignal {
eGdbSignalBadAccess = 0x91,
eGdbSignalBadInstruction = 0x92,
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index eb56337de3c44..4ea6f25906876 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1349,8 +1349,8 @@ SBTarget::WatchpointCreateByAddress(lldb::addr_t addr, size_t size,
Status cw_error;
// This API doesn't take in a type, so we can't figure out what it is.
CompilerType *type = nullptr;
- watchpoint_sp =
- target_sp->CreateWatchpoint(addr, size, type, watch_type, cw_error);
+ watchpoint_sp = target_sp->CreateWatchpointByAddress(
+ addr, size, type, watch_type, lldb::eWatchpointModeHardware, cw_error);
error.SetError(std::move(cw_error));
sb_watchpoint.SetSP(watchpoint_sp);
}
diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index e300ecee3f8ac..1670883062173 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -1491,8 +1491,8 @@ lldb::SBWatchpoint SBValue::Watch(bool resolve_location, bool read, bool write,
Status rc;
CompilerType type(value_sp->GetCompilerType());
- WatchpointSP watchpoint_sp =
- target_sp->CreateWatchpoint(addr, byte_size, &type, watch_type, rc);
+ WatchpointSP watchpoint_sp = target_sp->CreateWatchpointByAddress(
+ addr, byte_size, &type, watch_type, lldb::eWatchpointModeHardware, rc);
error.SetError(std::move(rc));
if (watchpoint_sp) {
diff --git a/lldb/source/Breakpoint/Watchpoint.cpp b/lldb/source/Breakpoint/Watchpoint.cpp
index f1366ca538075..dad3155ff1348 100644
--- a/lldb/source/Breakpoint/Watchpoint.cpp
+++ b/lldb/source/Breakpoint/Watchpoint.cpp
@@ -10,6 +10,7 @@
#include "lldb/Breakpoint/StoppointCallbackContext.h"
#include "lldb/Breakpoint/WatchpointResource.h"
+#include "lldb/Core/Debugger.h"
#include "lldb/Core/Value.h"
#include "lldb/DataFormatters/DumpValueObjectOptions.h"
#include "lldb/Expression/UserExpression.h"
@@ -26,45 +27,95 @@
using namespace lldb;
using namespace lldb_private;
-Watchpoint::Watchpoint(Target &target, lldb::addr_t addr, uint32_t size,
- const CompilerType *type, bool hardware)
- : StoppointSite(0, addr, size, hardware), m_target(target),
- m_enabled(false), m_is_hardware(hardware), m_is_watch_variable(false),
- m_is_ephemeral(false), m_disabled_count(0), m_watch_read(0),
- m_watch_write(0), m_watch_modify(0), m_ignore_count(0) {
-
- if (type && type->IsValid())
- m_type = *type;
- else {
- // If we don't have a known type, then we force it to unsigned int of the
- // right size.
- auto type_system_or_err =
- target.GetScratchTypeSystemForLanguage(eLanguageTypeC);
- if (auto err = type_system_or_err.takeError()) {
- LLDB_LOG_ERROR(GetLog(LLDBLog::Watchpoints), std::move(err),
- "Failed to set type: {0}");
- } else {
- if (auto ts = *type_system_or_err) {
- if (size <= target.GetArchitecture().GetAddressByteSize()) {
- m_type =
- ts->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 8 * size);
- } else {
- CompilerType clang_uint8_type =
- ts->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 8);
- m_type = clang_uint8_type.GetArrayType(size);
- }
- } else
- LLDB_LOG_ERROR(GetLog(LLDBLog::Watchpoints), std::move(err),
- "Failed to set type: Typesystem is no longer live: {0}");
- }
+static bool IsValueObjectsEqual(lldb::ValueObjectSP lhs,
+ lldb::ValueObjectSP rhs) {
+ lldbassert(lhs);
+ lldbassert(rhs);
+
+ Status error;
+
+ DataExtractor lhs_data;
+ lhs->GetData(lhs_data, error);
+ if (error.Fail())
+ return false;
+
+ DataExtractor rhs_data;
+ rhs->GetData(rhs_data, error);
+ if (error.Fail())
+ return false;
+
+ return llvm::equal(
+ llvm::iterator_range(lhs_data.GetDataStart(), lhs_data.GetDataEnd()),
+ llvm::iterator_range(rhs_data.GetDataStart(), rhs_data.GetDataEnd()));
+}
+
+CompilerType Watchpoint::AddressWatchpointCalculateStrategy::DeriveCompilerType(
+ Target &target, uint32_t size) {
+ auto type_system_or_err =
+ target.GetScratchTypeSystemForLanguage(eLanguageTypeC);
+
+ auto err = type_system_or_err.takeError();
+ if (err) {
+ LLDB_LOG_ERROR(GetLog(LLDBLog::Watchpoints), std::move(err),
+ "Failed to set type: {0}");
+ return CompilerType();
}
- // Set the initial value of the watched variable:
- if (m_target.GetProcessSP()) {
- ExecutionContext exe_ctx;
- m_target.GetProcessSP()->CalculateExecutionContext(exe_ctx);
- CaptureWatchedValue(exe_ctx);
+ auto ts = *type_system_or_err;
+ if (!ts) {
+ LLDB_LOG_ERROR(GetLog(LLDBLog::Watchpoints), std::move(err),
+ "Failed to set type: Typesystem is no longer live: {0}");
+ return CompilerType();
+ }
+
+ if (size <= target.GetArchitecture().GetAddressByteSize())
+ return ts->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 8 * size);
+
+ CompilerType clang_uint8_type =
+ ts->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 8);
+ return clang_uint8_type.GetArrayType(size);
+}
+
+lldb::ValueObjectSP
+Watchpoint::AddressWatchpointCalculateStrategy::CalculateWatchedValue() const {
+ 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 nullptr;
}
+
+ // To obtain a value that represents memory at a given address, we only need
+ // an information about process.
+ // Create here an ExecutionContext from the current process.
+ ExecutionContext exe_ctx;
+ lldb::ProcessSP process_sp = m_target.GetProcessSP();
+ if (!process_sp)
+ return nullptr;
+ process_sp->CalculateExecutionContext(exe_ctx);
+
+ ConstString g_watch_name("$__lldb__watch_value");
+ Address watch_address(m_addr);
+ auto new_value_sp = ValueObjectMemory::Create(
+ exe_ctx.GetBestExecutionContextScope(), g_watch_name.GetStringRef(),
+ watch_address, m_type);
+ new_value_sp = new_value_sp->CreateConstantValue(g_watch_name);
+ return new_value_sp;
+}
+
+Watchpoint::Watchpoint(Target &target, lldb::addr_t addr, uint32_t size,
+ const CompilerType *type, lldb::WatchpointMode mode)
+ : StoppointSite(0, addr, size,
+ mode == lldb::eWatchpointModeHardware ? true : false),
+ m_target(target), m_enabled(false), m_mode(mode),
+ m_is_watch_variable(false), m_is_ephemeral(false), m_disabled_count(0),
+ m_ignore_count(0), m_watch_spec_str{},
+ m_calculate_strategy{
+ AddressWatchpointCalculateStrategy{target, addr, size, type}} {
+ // Set the initial value of the watched variable:
+ CaptureWatchedValue(CalculateWatchedValue());
}
Watchpoint::~Watchpoint() = default;
@@ -184,72 +235,121 @@ void Watchpoint::ClearCallback() {
void Watchpoint::SetDeclInfo(const std::string &str) { m_decl_str = str; }
-std::string Watchpoint::GetWatchSpec() { return m_watch_spec_str; }
+std::string Watchpoint::GetWatchSpec() const { return m_watch_spec_str; }
void Watchpoint::SetWatchSpec(const std::string &str) {
m_watch_spec_str = str;
}
-bool Watchpoint::IsHardware() const {
- lldbassert(m_is_hardware || !HardwareRequired());
- return m_is_hardware;
-}
-
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->CreateCons...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/159807
More information about the lldb-commits
mailing list