diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h b/lldb/include/lldb/Breakpoint/Watchpoint.h index ca48a0114888a..06ff1161f733c 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,93 @@ class Watchpoint : public std::enable_shared_from_this, const WatchpointEventData &operator=(const WatchpointEventData &) = delete; }; + /// \class WatchpointSentry + /// \brief 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(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); + + /// \brief 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 +181,7 @@ class Watchpoint : public std::enable_shared_from_this, 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 +243,72 @@ class Watchpoint : public std::enable_shared_from_this, 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; + + /// \brief This class facilitates retrieving a watchpoint's watched value. + /// + /// \details 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; 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 +316,19 @@ class Watchpoint : public std::enable_shared_from_this, // 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, 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->CreateConstantValue(g_watch_name); - return (m_new_value_sp && m_new_value_sp->GetError().Success()); +lldb::ValueObjectSP Watchpoint::CalculateWatchedValue() const { + auto caller = [](const auto &strategy) { + return strategy.CalculateWatchedValue(); + }; + auto value_obj_sp = std::visit(caller, m_calculate_strategy); + if (!value_obj_sp) + Debugger::ReportError("Watchpoint %u: Failed to calculate watched value! " + "The behavior of this watchpoint may be disrupted!", + m_id); + return value_obj_sp; } -bool Watchpoint::WatchedValueReportable(const ExecutionContext &exe_ctx) { - if (!m_watch_modify || m_watch_read) - return true; - if (!m_type.IsValid()) - return true; +CompilerType Watchpoint::GetCompilerType() const { + const AddressWatchpointCalculateStrategy *addr_strategy = + std::get_if(&m_calculate_strategy); + if (!addr_strategy) + return CompilerType(); + return addr_strategy->GetCompilerType(); +} - 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; +bool Watchpoint::CheckWatchpointCondition( + const ExecutionContext &exe_ctx) const { + if (GetConditionText() == nullptr) + return true; - DataExtractor new_data; - DataExtractor old_data; + Log *log = GetLog(LLDBLog::Watchpoints); - newest_valueobj_sp->GetData(new_data, error); - if (error.Fail()) - return true; - m_new_value_sp->GetData(old_data, error); - if (error.Fail()) + // 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. + EvaluateExpressionOptions expr_options; + expr_options.SetUnwindOnError(true); + expr_options.SetIgnoreBreakpoints(true); + ValueObjectSP result_value_sp; + ExpressionResults result_code = m_target.EvaluateExpression( + GetConditionText(), exe_ctx.GetBestExecutionContextScope(), + result_value_sp, expr_options); + + if (!result_value_sp || result_code != eExpressionCompleted) { + LLDB_LOGF(log, "Error evaluating condition:\n"); + + StreamString strm; + strm << "stopped due to an error evaluating condition of " + "watchpoint "; + GetDescription(&strm, eDescriptionLevelBrief); + strm << ": \"" << GetConditionText() << "\"\n"; + + Debugger::ReportError(strm.GetString().str(), + exe_ctx.GetTargetRef().GetDebugger().GetID()); return true; + } - if (new_data.GetByteSize() != old_data.GetByteSize() || - new_data.GetByteSize() == 0) + Scalar scalar_value; + if (!result_value_sp->ResolveValue(scalar_value)) { + LLDB_LOGF(log, "Failed to get an integer result from the expression."); return true; + } + + bool should_stop = scalar_value.ULongLong(1) != 0; + + LLDB_LOGF(log, "Condition successfully evaluated, result is %s.\n", + should_stop ? "true" : "false"); + + return should_stop; +} + +bool Watchpoint::WatchedValueReportable(const ExecutionContext &exe_ctx) { + if (!CheckWatchpointCondition(exe_ctx)) { + // The condition failed, which we consider "not having hit + // the watchpoint". + return false; + } + + lldb::ValueObjectSP current_value_sp = CalculateWatchedValue(); + if (!current_value_sp || current_value_sp->GetError().Fail()) + return false; + + if (IsHardware()) { + // Don't stop if the watched region value is unmodified, and + // this is a Modify-type watchpoint. + if (WatchpointModify() && !WatchpointRead() && + IsValueObjectsEqual(current_value_sp, m_new_value_sp)) + return false; + } else { + if (m_new_value_sp && IsValueObjectsEqual(current_value_sp, m_new_value_sp)) + return false; - if (memcmp(new_data.GetDataStart(), old_data.GetDataStart(), - old_data.GetByteSize()) == 0) - return false; // Value has not changed, user requested modify watchpoint + if (m_previous_hit_value_sp && + IsValueObjectsEqual(current_value_sp, m_previous_hit_value_sp)) + return false; + + m_previous_hit_value_sp = current_value_sp; + } + + // All checks are passed. Hit! + m_hit_counter.Increment(); + + // Even if ignore count > 0 consider it as a hit anyway, just + // don't stop at this watchpoint. + if (m_ignore_count) { + --m_ignore_count; + return false; + } + CaptureWatchedValue(current_value_sp); return true; } @@ -257,12 +357,10 @@ bool Watchpoint::WatchedValueReportable(const ExecutionContext &exe_ctx) { // should continue. bool Watchpoint::ShouldStop(StoppointCallbackContext *context) { - m_hit_counter.Increment(); - return IsEnabled(); } -void Watchpoint::GetDescription(Stream *s, lldb::DescriptionLevel level) { +void Watchpoint::GetDescription(Stream *s, lldb::DescriptionLevel level) const { DumpWithLevel(s, level); } @@ -276,7 +374,7 @@ bool Watchpoint::DumpSnapshots(Stream *s, const char *prefix) const { bool printed_anything = false; // For read watchpoints, don't display any before/after value changes. - if (m_watch_read && !m_watch_modify && !m_watch_write) + if (WatchpointRead() && !WatchpointModify() && !WatchpointWrite()) return printed_anything; s->Printf("\n"); @@ -349,11 +447,12 @@ void Watchpoint::DumpWithLevel(Stream *s, assert(description_level >= lldb::eDescriptionLevelBrief && description_level <= lldb::eDescriptionLevelVerbose); - s->Printf("Watchpoint %u: addr = 0x%8.8" PRIx64 + s->Printf("%s Watchpoint %u: addr = 0x%8.8" PRIx64 " size = %u state = %s type = %s%s%s", - GetID(), GetLoadAddress(), m_byte_size, - IsEnabled() ? "enabled" : "disabled", m_watch_read ? "r" : "", - m_watch_write ? "w" : "", m_watch_modify ? "m" : ""); + IsHardware() ? "Hardware" : "Software", GetID(), GetLoadAddress(), + m_byte_size, IsEnabled() ? "enabled" : "disabled", + WatchpointRead() ? "r" : "", WatchpointWrite() ? "w" : "", + WatchpointModify() ? "m" : ""); if (description_level >= lldb::eDescriptionLevelFull) { if (!m_decl_str.empty()) @@ -417,32 +516,35 @@ void Watchpoint::SetEnabled(bool enabled, bool notify) { // Within StopInfo.cpp, we purposely do disable/enable watchpoint while // performing watchpoint actions. } + bool changed = enabled != m_enabled; m_enabled = enabled; if (notify && !m_is_ephemeral && changed) SendWatchpointChangedEvent(enabled ? eWatchpointEventTypeEnabled : eWatchpointEventTypeDisabled); + + if (IsHardware() || !enabled) + return; + + // If we enable a software watchpoint, we should update + // m_previous_hit_value_sp + m_previous_hit_value_sp = CalculateWatchedValue(); } void Watchpoint::SetWatchpointType(uint32_t type, bool notify) { - int old_watch_read = m_watch_read; - int old_watch_write = m_watch_write; - int old_watch_modify = m_watch_modify; - m_watch_read = (type & LLDB_WATCH_TYPE_READ) != 0; - m_watch_write = (type & LLDB_WATCH_TYPE_WRITE) != 0; - m_watch_modify = (type & LLDB_WATCH_TYPE_MODIFY) != 0; - if (notify && - (old_watch_read != m_watch_read || old_watch_write != m_watch_write || - old_watch_modify != m_watch_modify)) + if (!IsHardware() && + (type & LLDB_WATCH_TYPE_READ || type & LLDB_WATCH_TYPE_WRITE)) { + Debugger::ReportWarning("Software watchpoint can only be a modify type, " + "setting watchpoint type to modify", + m_target.GetDebugger().GetID()); + type = LLDB_WATCH_TYPE_MODIFY; + } + + if (notify && m_watch_type != type) SendWatchpointChangedEvent(eWatchpointEventTypeTypeChanged); + m_watch_type = type; } -bool Watchpoint::WatchpointRead() const { return m_watch_read != 0; } - -bool Watchpoint::WatchpointWrite() const { return m_watch_write != 0; } - -bool Watchpoint::WatchpointModify() const { return m_watch_modify != 0; } - uint32_t Watchpoint::GetIgnoreCount() const { return m_ignore_count; } void Watchpoint::SetIgnoreCount(uint32_t n) { diff --git a/lldb/source/Commands/CommandObjectWatchpoint.cpp b/lldb/source/Commands/CommandObjectWatchpoint.cpp index 12effed12a3cf..8facda2e5acca 100644 --- a/lldb/source/Commands/CommandObjectWatchpoint.cpp +++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp @@ -805,7 +805,7 @@ corresponding to the byte size of the data type."); void DoExecute(Args &command, CommandReturnObject &result) override { Target &target = GetTarget(); - StackFrame *frame = m_exe_ctx.GetFramePtr(); + StackFrameSP frame_sp = m_exe_ctx.GetFrameSP(); // If no argument is present, issue an error message. There's no way to // set a watchpoint. @@ -839,7 +839,7 @@ corresponding to the byte size of the data type."); uint32_t expr_path_options = StackFrame::eExpressionPathOptionCheckPtrVsMember | StackFrame::eExpressionPathOptionsAllowDirectIVarAccess; - valobj_sp = frame->GetValueForVariableExpressionPath( + valobj_sp = frame_sp->GetValueForVariableExpressionPath( command.GetArgumentAtIndex(0), eNoDynamicValues, expr_path_options, var_sp, error); @@ -902,11 +902,15 @@ corresponding to the byte size of the data type."); error.Clear(); WatchpointSP watch_sp = - target.CreateWatchpoint(addr, size, &compiler_type, watch_type, error); + target.CreateWatchpointByAddress(addr, size, &compiler_type, watch_type, + m_option_watchpoint.watch_mode, error); if (!watch_sp) { result.AppendErrorWithFormat( - "Watchpoint creation failed (addr=0x%" PRIx64 ", size=%" PRIu64 + "%s watchpoint creation failed (addr=0x%" PRIx64 ", size=%" PRIu64 ", variable expression='%s').\n", + m_option_watchpoint.watch_mode == lldb::eWatchpointModeHardware + ? "Hardware" + : "Software", addr, static_cast(size), command.GetArgumentAtIndex(0)); if (const char *error_message = error.AsCString(nullptr)) result.AppendError(error_message); @@ -923,7 +927,7 @@ corresponding to the byte size of the data type."); watch_sp->SetDeclInfo(std::string(ss.GetString())); } if (var_sp->GetScope() == eValueTypeVariableLocal) - watch_sp->SetupVariableWatchpointDisabler(m_exe_ctx.GetFrameSP()); + watch_sp->SetupVariableWatchpointDisabler(frame_sp); } output_stream.Printf("Watchpoint created: "); watch_sp->GetDescription(&output_stream, lldb::eDescriptionLevelFull); @@ -1094,21 +1098,28 @@ class CommandObjectWatchpointSetExpression : public CommandObjectRaw { Status error; WatchpointSP watch_sp = - target.CreateWatchpoint(addr, size, &compiler_type, watch_type, error); - if (watch_sp) { - watch_sp->SetWatchSpec(std::string(expr)); - Stream &output_stream = result.GetOutputStream(); - output_stream.Printf("Watchpoint created: "); - watch_sp->GetDescription(&output_stream, lldb::eDescriptionLevelFull); - output_stream.EOL(); - result.SetStatus(eReturnStatusSuccessFinishResult); - } else { - result.AppendErrorWithFormat("Watchpoint creation failed (addr=0x%" PRIx64 - ", size=%" PRIu64 ").\n", - addr, (uint64_t)size); - if (error.AsCString(nullptr)) - result.AppendError(error.AsCString()); + target.CreateWatchpointByAddress(addr, size, &compiler_type, watch_type, + m_option_watchpoint.watch_mode, error); + if (!watch_sp) { + result.AppendErrorWithFormat( + "%s watchpoint creation failed (addr=0x%" PRIx64 ", size=%" PRIu64 + ", expression='%s').\n", + m_option_watchpoint.watch_mode == eWatchpointModeHardware + ? "Hardware" + : "Software", + addr, static_cast(size), expr.data()); + if (const char *error_message = error.AsCString(nullptr)) + result.AppendError(error_message); + return; } + + watch_sp->SetWatchSpec(std::string(expr)); + + Stream &output_stream = result.GetOutputStream(); + output_stream.Printf("Watchpoint created: "); + watch_sp->GetDescription(&output_stream, lldb::eDescriptionLevelFull); + output_stream.EOL(); + result.SetStatus(eReturnStatusSuccessFinishResult); } private: diff --git a/lldb/source/Interpreter/OptionGroupWatchpoint.cpp b/lldb/source/Interpreter/OptionGroupWatchpoint.cpp index 219f550ee5562..f589f1a945c90 100644 --- a/lldb/source/Interpreter/OptionGroupWatchpoint.cpp +++ b/lldb/source/Interpreter/OptionGroupWatchpoint.cpp @@ -43,8 +43,15 @@ static constexpr OptionDefinition g_option_table[] = { {LLDB_OPT_SET_1, false, "watch", 'w', OptionParser::eRequiredArgument, nullptr, OptionEnumValues(g_watch_type), 0, eArgTypeWatchType, "Specify the type of watching to perform."}, - {LLDB_OPT_SET_1, false, "size", 's', OptionParser::eRequiredArgument, - nullptr, {}, 0, eArgTypeByteSize, + {LLDB_OPT_SET_1, + false, + "size", + 's', + OptionParser::eRequiredArgument, + nullptr, + {}, + 0, + eArgTypeByteSize, "Number of bytes to use to watch a region."}, {LLDB_OPT_SET_2, false, @@ -103,6 +110,7 @@ OptionGroupWatchpoint::SetOptionValue(uint32_t option_idx, void OptionGroupWatchpoint::OptionParsingStarting( ExecutionContext *execution_context) { watch_type_specified = false; + watch_mode = eWatchpointModeHardware; watch_type = eWatchInvalid; watch_size.Clear(); language_type = eLanguageTypeUnknown; diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 91f3a6ce383b1..7b24359087cf9 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -3317,6 +3317,12 @@ Status ProcessGDBRemote::EnableWatchpoint(WatchpointSP wp_sp, bool notify) { return error; } + // Handle a software watchpoint + if (!wp_sp->IsHardware()) { + wp_sp->SetEnabled(true, notify); + return error; + } + bool read = wp_sp->WatchpointRead(); bool write = wp_sp->WatchpointWrite() || wp_sp->WatchpointModify(); size_t size = wp_sp->GetByteSize(); @@ -3423,33 +3429,38 @@ Status ProcessGDBRemote::DisableWatchpoint(WatchpointSP wp_sp, bool notify) { return error; } - if (wp_sp->IsHardware()) { - bool disabled_all = true; + // Handle a software watchpoint + if (!wp_sp->IsHardware()) { + wp_sp->SetEnabled(false, notify); + return error; + } - std::vector unused_resources; - for (const auto &wp_res_sp : m_watchpoint_resource_list.Sites()) { - if (wp_res_sp->ConstituentsContains(wp_sp)) { - GDBStoppointType type = GetGDBStoppointType(wp_res_sp); - addr_t addr = wp_res_sp->GetLoadAddress(); - size_t size = wp_res_sp->GetByteSize(); - if (m_gdb_comm.SendGDBStoppointTypePacket(type, false, addr, size, - GetInterruptTimeout())) { - disabled_all = false; - } else { - wp_res_sp->RemoveConstituent(wp_sp); - if (wp_res_sp->GetNumberOfConstituents() == 0) - unused_resources.push_back(wp_res_sp); - } + bool disabled_all = true; + + std::vector unused_resources; + for (const auto &wp_res_sp : m_watchpoint_resource_list.Sites()) { + if (wp_res_sp->ConstituentsContains(wp_sp)) { + GDBStoppointType type = GetGDBStoppointType(wp_res_sp); + addr_t addr = wp_res_sp->GetLoadAddress(); + size_t size = wp_res_sp->GetByteSize(); + if (m_gdb_comm.SendGDBStoppointTypePacket(type, false, addr, size, + GetInterruptTimeout())) { + disabled_all = false; + } else { + wp_res_sp->RemoveConstituent(wp_sp); + if (wp_res_sp->GetNumberOfConstituents() == 0) + unused_resources.push_back(wp_res_sp); } } - for (auto &wp_res_sp : unused_resources) - m_watchpoint_resource_list.Remove(wp_res_sp->GetID()); - - wp_sp->SetEnabled(false, notify); - if (!disabled_all) - error = Status::FromErrorString( - "Failure disabling one of the watchpoint locations"); } + for (auto &wp_res_sp : unused_resources) + m_watchpoint_resource_list.Remove(wp_res_sp->GetID()); + + wp_sp->SetEnabled(false, notify); + if (!disabled_all) + error = Status::FromErrorString( + "Failure disabling one of the watchpoint locations"); + return error; } diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index f47dae2b2465d..5b4a075fba53e 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -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; @@ -685,50 +687,6 @@ class StopInfoBreakpoint : public StopInfo { class StopInfoWatchpoint : public StopInfo { public: - // Make sure watchpoint is properly disabled and subsequently enabled while - // performing watchpoint actions. - class WatchpointSentry { - public: - WatchpointSentry(ProcessSP p_sp, WatchpointSP w_sp) : process_sp(p_sp), - watchpoint_sp(w_sp) { - if (process_sp && watchpoint_sp) { - const bool notify = false; - watchpoint_sp->TurnOnEphemeralMode(); - process_sp->DisableWatchpoint(watchpoint_sp, notify); - process_sp->AddPreResumeAction(SentryPreResumeAction, this); - } - } - - void DoReenable() { - if (process_sp && watchpoint_sp) { - bool was_disabled = watchpoint_sp->IsDisabledDuringEphemeralMode(); - watchpoint_sp->TurnOffEphemeralMode(); - const bool notify = false; - if (was_disabled) { - process_sp->DisableWatchpoint(watchpoint_sp, notify); - } else { - process_sp->EnableWatchpoint(watchpoint_sp, notify); - } - } - } - - ~WatchpointSentry() { - DoReenable(); - if (process_sp) - process_sp->ClearPreResumeAction(SentryPreResumeAction, this); - } - - static bool SentryPreResumeAction(void *sentry_void) { - WatchpointSentry *sentry = (WatchpointSentry *) sentry_void; - sentry->DoReenable(); - return true; - } - - private: - ProcessSP process_sp; - WatchpointSP watchpoint_sp; - }; - StopInfoWatchpoint(Thread &thread, break_id_t watch_id, bool silently_skip_wp) : StopInfo(thread, watch_id), m_silently_skip_wp(silently_skip_wp) {} @@ -938,159 +896,106 @@ 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) + return; + 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; + }); - ThreadSP thread_sp(m_thread_wp.lock()); - if (thread_sp) { + WatchpointSP wp_sp( + thread_sp->CalculateTarget()->GetWatchpointList().FindByID(GetValue())); + if (!wp_sp) { + Log *log_process(GetLog(LLDBLog::Process)); - 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(); + LLDB_LOGF(log_process, + "Process::%s could not find watchpoint id: %" PRId64 "...", + __FUNCTION__, m_value); + m_should_stop = true; + return; + } - WatchpointSentry sentry(process_sp, wp_sp); + // 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; - if (m_silently_skip_wp) { - m_should_stop = false; - wp_sp->UndoHitCount(); - } + ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0)); - if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount()) { - m_should_stop = false; - m_should_stop_is_valid = true; - } + // This sentry object makes sure the current watchpoint is disabled + // while performing watchpoint actions, and it is then enabled after we + // are finished. + ProcessSP process_sp = exe_ctx.GetProcessSP(); + Watchpoint::WatchpointSentry sentry(process_sp, wp_sp); - 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 = ""; - 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 (wp_sp->IsHardware()) { + // Hardware watchpoint may want to be skipped, so check it here. + if (m_silently_skip_wp) + return; - // 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. + // Watchpoint condition, ignore count and other watchpoint's staff of a + // hardware watchpoint haven't still been checked, so do checks here too. + if (!wp_sp->WatchedValueReportable(exe_ctx)) + return; + } - bool old_async = debugger.GetAsyncExecution(); - debugger.SetAsyncExecution(true); + // Watchpoint hit! + // Now run a watchpoint callback if it is preserved and report a hit. + if (!RunWatchpointCallback(exe_ctx, wp_sp, event_ptr)) + return; - StoppointCallbackContext context(event_ptr, exe_ctx, false); - bool stop_requested = wp_sp->InvokeCallback(&context); + // 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()) + return; - debugger.SetAsyncExecution(old_async); + // Finally, if we are going to stop, print out the new & old values: + m_should_stop = true; + ReportWatchpointHit(exe_ctx, wp_sp); + } - // 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; +private: + void SetStepOverPlanComplete() { + assert(m_using_step_over_plan); + m_step_over_plan_complete = true; + } - if (m_should_stop && !stop_requested) { - // We have been vetoed by the callback mechanism. - m_should_stop = false; - } - } + 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(); + } + } - // 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; - } + static bool RunWatchpointCallback(const ExecutionContext &exe_ctx, + WatchpointSP wp_sp, Event *event_ptr) { + // 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. - // Finally, if we are going to stop, print out the new & old values: - if (m_should_stop) { - wp_sp->CaptureWatchedValue(exe_ctx); + Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger(); - Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger(); - StreamUP output_up = debugger.GetAsyncOutputStream(); - if (wp_sp->DumpSnapshots(output_up.get())) - output_up->EOL(); - } + bool old_async = debugger.GetAsyncExecution(); + debugger.SetAsyncExecution(true); - } else { - Log *log_process(GetLog(LLDBLog::Process)); + StoppointCallbackContext context(event_ptr, exe_ctx, false); + bool stop_requested = wp_sp->InvokeCallback(&context); - 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); + debugger.SetAsyncExecution(old_async); - m_should_stop_is_valid = true; - } + return stop_requested; } -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 - diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index fa98c24606492..fdd28044d665e 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -924,9 +924,9 @@ bool Target::ProcessIsValid() { return (m_process_sp && m_process_sp->IsAlive()); } -static bool CheckIfWatchpointsSupported(Target *target, Status &error) { +static bool CheckIfHardwareWatchpointsSupported(Target &target, Status &error) { std::optional num_supported_hardware_watchpoints = - target->GetProcessSP()->GetWatchpointSlotCount(); + target.GetProcessSP()->GetWatchpointSlotCount(); // If unable to determine the # of watchpoints available, // assume they are supported. @@ -942,91 +942,111 @@ static bool CheckIfWatchpointsSupported(Target *target, Status &error) { return true; } -// See also Watchpoint::SetWatchpointType(uint32_t type) and the -// OptionGroupWatchpoint::WatchType enum type. -WatchpointSP Target::CreateWatchpoint(lldb::addr_t addr, size_t size, - const CompilerType *type, uint32_t kind, - Status &error) { - Log *log = GetLog(LLDBLog::Watchpoints); - LLDB_LOGF(log, - "Target::%s (addr = 0x%8.8" PRIx64 " size = %" PRIu64 - " type = %u)\n", - __FUNCTION__, addr, (uint64_t)size, kind); - - WatchpointSP wp_sp; - if (!ProcessIsValid()) { - error = Status::FromErrorString("process is not alive"); - return wp_sp; - } +static bool IsModifyWatchpointKind(uint32_t kind) { + return (kind & LLDB_WATCH_TYPE_READ) == 0 && + (kind & LLDB_WATCH_TYPE_WRITE) == 0; +} - if (addr == LLDB_INVALID_ADDRESS || size == 0) { - if (size == 0) - error = Status::FromErrorString( - "cannot set a watchpoint with watch_size of 0"); - else - error = Status::FromErrorStringWithFormat( - "invalid watch address: %" PRIu64, addr); - return wp_sp; +static bool CheckSoftwareWatchpointParameters(uint32_t kind, Status &error) { + if (!IsModifyWatchpointKind(kind)) { + error = Status::FromErrorString( + "software watchpoint can be only \"modify\" type"); + return false; } + return true; +} +static bool CheckCommonWatchpointParameters(uint32_t kind, size_t size, + Status &error) { if (!LLDB_WATCH_TYPE_IS_VALID(kind)) { error = Status::FromErrorStringWithFormat("invalid watchpoint type: %d", kind); + return false; } - if (!CheckIfWatchpointsSupported(this, error)) - return wp_sp; + if (size == 0) { + error = + Status::FromErrorString("cannot set a watchpoint with watch_size of 0"); + return false; + } - // Currently we only support one watchpoint per address, with total number of - // watchpoints limited by the hardware which the inferior is running on. + return true; +} +// LWP_TODO this sequence is looking for an existing watchpoint +// at the exact same user-specified address. If addr/size/type match, +// reuse the matched watchpoint. If type/size/mode differ, return an error. +// This isn't correct, we need both watchpoints to use a shared +// WatchpointResource in the target, and expand the WatchpointResource +// to handle the needs of both Watchpoints. +// Also, even if the addresses don't match, they may need to be +// supported by the same WatchpointResource, e.g. a watchpoint +// watching 1 byte at 0x102 and a watchpoint watching 1 byte at 0x103. +// They're in the same word and must be watched by a single hardware +// watchpoint register. +static WatchpointSP CheckMatchedWatchpoint(Target &target, lldb::addr_t addr, + uint32_t kind, size_t size, + WatchpointMode mode, Status &error) { // Grab the list mutex while doing operations. - const bool notify = false; // Don't notify about all the state changes we do - // on creating the watchpoint. + std::unique_lock lock; + target.GetWatchpointList().GetListMutex(lock); - // Mask off ignored bits from watchpoint address. - if (ABISP abi = m_process_sp->GetABI()) - addr = abi->FixDataAddress(addr); + WatchpointList &wp_list = target.GetWatchpointList(); + WatchpointSP matched_sp = wp_list.FindByAddress(addr); + if (!matched_sp) + return nullptr; - // LWP_TODO this sequence is looking for an existing watchpoint - // at the exact same user-specified address, disables the new one - // if addr/size/type match. If type/size differ, disable old one. - // This isn't correct, we need both watchpoints to use a shared - // WatchpointResource in the target, and expand the WatchpointResource - // to handle the needs of both Watchpoints. - // Also, even if the addresses don't match, they may need to be - // supported by the same WatchpointResource, e.g. a watchpoint - // watching 1 byte at 0x102 and a watchpoint watching 1 byte at 0x103. - // They're in the same word and must be watched by a single hardware - // watchpoint register. + size_t old_size = matched_sp->GetByteSize(); + uint32_t old_type = + (matched_sp->WatchpointRead() ? LLDB_WATCH_TYPE_READ : 0) | + (matched_sp->WatchpointWrite() ? LLDB_WATCH_TYPE_WRITE : 0) | + (matched_sp->WatchpointModify() ? LLDB_WATCH_TYPE_MODIFY : 0); + WatchpointMode old_mode = matched_sp->IsHardware() ? eWatchpointModeHardware + : eWatchpointModeSoftware; + // Return the existing watchpoint if size, type and mode match. + if (size == old_size && kind == old_type && mode == old_mode) { + Debugger::ReportWarning(llvm::formatv( + "Address 0x{0:x} is already monitored by Watchpoint {1} with " + "matching parameters: size ({2}), type ({3}{4}{5}), and mode ({6}). " + "Reusing existing watchpoint.", + addr, matched_sp->GetID(), size, + matched_sp->WatchpointRead() ? "r" : "", + matched_sp->WatchpointWrite() ? "w" : "", + matched_sp->WatchpointModify() ? "m" : "", + matched_sp->IsHardware() ? "Hardware" : "Software")); + WatchpointSP wp_sp = matched_sp; + return wp_sp; + } + error = Status::FromErrorStringWithFormat( + "Address 0x%lx is already monitored by Watchpoint %u with " + "diffrent size(%zu), type(%s%s%s) or mode(%s).\n" + "Multiple watchpoints on the same address are not supported. " + "You should manually delete Watchpoint %u before setting a " + "a new watchpoint on this address.", + addr, matched_sp->GetID(), size, matched_sp->WatchpointRead() ? "r" : "", + matched_sp->WatchpointWrite() ? "w" : "", + matched_sp->WatchpointModify() ? "m" : "", + matched_sp->IsHardware() ? "Hardware" : "Software", matched_sp->GetID()); + return nullptr; +} + +static bool AddWatchpointToList(Target &target, WatchpointSP wp_sp, + Status &error) { + lldbassert(wp_sp); + + Log *log = GetLog(LLDBLog::Watchpoints); + constexpr bool notify = true; + + WatchpointList &wp_list = target.GetWatchpointList(); + + // Grab the list mutex while doing operations. std::unique_lock lock; - this->GetWatchpointList().GetListMutex(lock); - WatchpointSP matched_sp = m_watchpoint_list.FindByAddress(addr); - if (matched_sp) { - size_t old_size = matched_sp->GetByteSize(); - uint32_t old_type = - (matched_sp->WatchpointRead() ? LLDB_WATCH_TYPE_READ : 0) | - (matched_sp->WatchpointWrite() ? LLDB_WATCH_TYPE_WRITE : 0) | - (matched_sp->WatchpointModify() ? LLDB_WATCH_TYPE_MODIFY : 0); - // Return the existing watchpoint if both size and type match. - if (size == old_size && kind == old_type) { - wp_sp = matched_sp; - wp_sp->SetEnabled(false, notify); - } else { - // Nil the matched watchpoint; we will be creating a new one. - m_process_sp->DisableWatchpoint(matched_sp, notify); - m_watchpoint_list.Remove(matched_sp->GetID(), true); - } - } + wp_list.GetListMutex(lock); - if (!wp_sp) { - wp_sp = std::make_shared(*this, addr, size, type); - wp_sp->SetWatchpointType(kind, notify); - m_watchpoint_list.Add(wp_sp, true); - } + wp_list.Add(wp_sp, notify); - error = m_process_sp->EnableWatchpoint(wp_sp, notify); + error = target.GetProcessSP()->EnableWatchpoint(wp_sp, /*notify=*/false); LLDB_LOGF(log, "Target::%s (creation of watchpoint %s with id = %u)\n", __FUNCTION__, error.Success() ? "succeeded" : "failed", wp_sp->GetID()); @@ -1034,10 +1054,77 @@ WatchpointSP Target::CreateWatchpoint(lldb::addr_t addr, size_t size, if (error.Fail()) { // Enabling the watchpoint on the device side failed. Remove the said // watchpoint from the list maintained by the target instance. - m_watchpoint_list.Remove(wp_sp->GetID(), true); - wp_sp.reset(); - } else - m_last_created_watchpoint = wp_sp; + wp_list.Remove(wp_sp->GetID(), notify); + return false; + } + return true; +} + +// See also Watchpoint::SetWatchpointType(uint32_t type) and the +// OptionGroupWatchpoint::WatchType enum type. +WatchpointSP Target::CreateWatchpointByAddress(lldb::addr_t addr, size_t size, + const CompilerType *type, + uint32_t kind, + WatchpointMode mode, + Status &error) { + Log *log = GetLog(LLDBLog::Watchpoints); + LLDB_LOGF(log, + "Target::%s (addr = 0x%8.8" PRIx64 " size = %" PRIu64 + " type = %u)\n", + __FUNCTION__, addr, static_cast(size), kind); + + if (!ProcessIsValid()) { + error = Status::FromErrorString("process is not alive"); + return nullptr; + } + + if (!CheckCommonWatchpointParameters(kind, size, error)) + return nullptr; + + if (addr == LLDB_INVALID_ADDRESS) { + error = Status::FromErrorStringWithFormat("invalid watch address: %" PRIu64, + addr); + return nullptr; + } + + // Mask off ignored bits from watchpoint address. + if (ABISP abi = m_process_sp->GetABI(); abi != nullptr) + addr = abi->FixDataAddress(addr); + + if (mode == eWatchpointModeHardware) { + // Hardware wathpoint specific checks + if (!CheckIfHardwareWatchpointsSupported(*this, error)) + return nullptr; + } else { + // Software watchpoint specific checks + if (!CheckSoftwareWatchpointParameters(kind, error)) + return nullptr; + } + + WatchpointSP wp_sp = + CheckMatchedWatchpoint(*this, addr, kind, size, mode, error); + if (error.Fail()) { + // A watchpoint already exists at this address with conflicting parameters + // (size, type, or mode). Since multiple watchpoints cannot share the same + // address, return an error here and suggest the user to remove the existing + // watchpoint if user still wants to create a new one. + return nullptr; + } + + if (wp_sp) { + // A watchpoint with desired parameters already exists at this address. + // In this case, enable it if needed and reuse it. + error = m_process_sp->EnableWatchpoint(wp_sp, /*notify=*/false); + return wp_sp; + } + + wp_sp = std::make_shared(*this, addr, size, type, mode); + wp_sp->SetWatchpointType(kind, /*notify=*/false); + + if (!AddWatchpointToList(*this, wp_sp, error)) + return nullptr; + + m_last_created_watchpoint = wp_sp; return wp_sp; } diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp index 8c3e19725f8cb..5a96979f6b80c 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -621,63 +621,72 @@ void Thread::WillStop() { } bool Thread::SetupToStepOverBreakpointIfNeeded(RunDirection direction) { - if (GetResumeState() != eStateSuspended) { - // First check whether this thread is going to "actually" resume at all. - // For instance, if we're stepping from one level to the next of an - // virtual inlined call stack, we just change the inlined call stack index - // without actually running this thread. In that case, for this thread we - // shouldn't push a step over breakpoint plan or do that work. - if (GetCurrentPlan()->IsVirtualStep()) - return false; + if (GetResumeState() == eStateSuspended) + return false; - // If we're at a breakpoint push the step-over breakpoint plan. Do this - // before telling the current plan it will resume, since we might change - // what the current plan is. - - lldb::RegisterContextSP reg_ctx_sp(GetRegisterContext()); - ProcessSP process_sp(GetProcess()); - if (reg_ctx_sp && process_sp && direction == eRunForward) { - const addr_t thread_pc = reg_ctx_sp->GetPC(); - BreakpointSiteSP bp_site_sp = - process_sp->GetBreakpointSiteList().FindByAddress(thread_pc); - // If we're at a BreakpointSite which we have either - // 1. already triggered/hit, or - // 2. the Breakpoint was added while stopped, or the pc was moved - // to this BreakpointSite - // Step past the breakpoint before resuming. - // If we stopped at a breakpoint instruction/BreakpointSite location - // without hitting it, and we're still at that same address on - // resuming, then we want to hit the BreakpointSite when we resume. - if (bp_site_sp && m_stopped_at_unexecuted_bp != thread_pc) { - // Note, don't assume there's a ThreadPlanStepOverBreakpoint, the - // target may not require anything special to step over a breakpoint. - - ThreadPlan *cur_plan = GetCurrentPlan(); - - bool push_step_over_bp_plan = false; - if (cur_plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { - ThreadPlanStepOverBreakpoint *bp_plan = - (ThreadPlanStepOverBreakpoint *)cur_plan; - if (bp_plan->GetBreakpointLoadAddress() != thread_pc) - push_step_over_bp_plan = true; - } else - push_step_over_bp_plan = true; - - if (push_step_over_bp_plan) { - ThreadPlanSP step_bp_plan_sp(new ThreadPlanStepOverBreakpoint(*this)); - if (step_bp_plan_sp) { - step_bp_plan_sp->SetPrivate(true); - - if (GetCurrentPlan()->RunState() != eStateStepping) { - ThreadPlanStepOverBreakpoint *step_bp_plan = - static_cast( - step_bp_plan_sp.get()); - step_bp_plan->SetAutoContinue(true); - } - QueueThreadPlan(step_bp_plan_sp, false); - return true; - } + // First check whether this thread is going to "actually" resume at all. + // For instance, if we're stepping from one level to the next of an + // virtual inlined call stack, we just change the inlined call stack index + // without actually running this thread. In that case, for this thread we + // shouldn't push a step over breakpoint plan or do that work. + + if (GetCurrentPlan()->IsVirtualStep()) + return false; + + if (direction != eRunForward) + return false; + + ProcessSP process_sp(GetProcess()); + if (!process_sp) + return false; + + lldb::RegisterContextSP reg_ctx_sp(GetRegisterContext()); + if (!reg_ctx_sp) + return false; + + // If we're at a breakpoint push the step-over breakpoint plan. Do this + // before telling the current plan it will resume, since we might change + // what the current plan is. + + const addr_t thread_pc = reg_ctx_sp->GetPC(); + BreakpointSiteSP bp_site_sp = + process_sp->GetBreakpointSiteList().FindByAddress(thread_pc); + // If we're at a BreakpointSite which we have either + // 1. already triggered/hit, or + // 2. the Breakpoint was added while stopped, or the pc was moved + // to this BreakpointSite + // Step past the breakpoint before resuming. + // If we stopped at a breakpoint instruction/BreakpointSite location + // without hitting it, and we're still at that same address on + // resuming, then we want to hit the BreakpointSite when we resume. + if (bp_site_sp && m_stopped_at_unexecuted_bp != thread_pc) { + // Note, don't assume there's a ThreadPlanStepOverBreakpoint, the + // target may not require anything special to step over a breakpoint. + + ThreadPlan *cur_plan = GetCurrentPlan(); + + bool push_step_over_bp_plan = false; + if (cur_plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { + ThreadPlanStepOverBreakpoint *bp_plan = + (ThreadPlanStepOverBreakpoint *)cur_plan; + if (bp_plan->GetBreakpointLoadAddress() != thread_pc) + push_step_over_bp_plan = true; + } else + push_step_over_bp_plan = true; + + if (push_step_over_bp_plan) { + ThreadPlanSP step_bp_plan_sp(new ThreadPlanStepOverBreakpoint(*this)); + if (step_bp_plan_sp) { + step_bp_plan_sp->SetPrivate(true); + + if (GetCurrentPlan()->RunState() != eStateStepping) { + ThreadPlanStepOverBreakpoint *step_bp_plan = + static_cast( + step_bp_plan_sp.get()); + step_bp_plan->SetAutoContinue(true); } + QueueThreadPlan(step_bp_plan_sp, false); + return true; } } } diff --git a/lldb/test/API/commands/watchpoints/hello_watchlocation/TestWatchLocation.py b/lldb/test/API/commands/watchpoints/hello_watchlocation/TestWatchLocation.py index 8e3133864ee59..cfbf95baecaf7 100644 --- a/lldb/test/API/commands/watchpoints/hello_watchlocation/TestWatchLocation.py +++ b/lldb/test/API/commands/watchpoints/hello_watchlocation/TestWatchLocation.py @@ -74,7 +74,8 @@ def test_hello_watchlocation(self): # Get a hold of the watchpoint id just created, it is used later on to # match the watchpoint id which is expected to be fired. match = re.match( - "Watchpoint created: Watchpoint (.*):", self.res.GetOutput().splitlines()[0] + "Watchpoint created: Hardware Watchpoint (.*):", + self.res.GetOutput().splitlines()[0], ) if match: expected_wp_id = int(match.group(1), 0) diff --git a/lldb/test/API/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py b/lldb/test/API/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py index d40d14bbe6d66..ee78a763934bb 100644 --- a/lldb/test/API/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py +++ b/lldb/test/API/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py @@ -213,7 +213,7 @@ def test_rw_watchpoint_set_ignore_count(self): # Use the '-v' option to do verbose listing of the watchpoint. # Expect to find a hit_count of 2 as well. - self.expect("watchpoint list -v", substrs=["hit_count = 2", "ignore_count = 2"]) + self.expect("watchpoint list -v", substrs=["hit_count = 2", "ignore_count = 0"]) # Read-write watchpoints not supported on SystemZ @expectedFailureAll(archs=["s390x"])