[Lldb-commits] [lldb] d347c56 - Revert "[lldb] Add support for large watchpoints in lldb (#79962)"
Jason Molenda via lldb-commits
lldb-commits at lists.llvm.org
Wed Jan 31 12:26:43 PST 2024
Author: Jason Molenda
Date: 2024-01-31T12:22:43-08:00
New Revision: d347c564299eeb8ad1fcb58c06914473d6a789d8
URL: https://github.com/llvm/llvm-project/commit/d347c564299eeb8ad1fcb58c06914473d6a789d8
DIFF: https://github.com/llvm/llvm-project/commit/d347c564299eeb8ad1fcb58c06914473d6a789d8.diff
LOG: Revert "[lldb] Add support for large watchpoints in lldb (#79962)"
This reverts commit 57c66b35a885b571f9897d75d18f1d974c29e533.
Added:
Modified:
lldb/include/lldb/lldb-enumerations.h
lldb/packages/Python/lldbsuite/test/concurrent_base.py
lldb/source/Breakpoint/CMakeLists.txt
lldb/source/Breakpoint/Watchpoint.cpp
lldb/source/Breakpoint/WatchpointResource.cpp
lldb/source/Commands/CommandObjectWatchpoint.cpp
lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Target/StopInfo.cpp
lldb/test/API/functionalities/watchpoint/large-watchpoint/TestLargeWatchpoint.py
lldb/unittests/Breakpoint/CMakeLists.txt
Removed:
lldb/include/lldb/Breakpoint/WatchpointAlgorithms.h
lldb/source/Breakpoint/WatchpointAlgorithms.cpp
lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/Makefile
lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/TestUnalignedLargeWatchpoint.py
lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/main.c
lldb/unittests/Breakpoint/WatchpointAlgorithmsTests.cpp
################################################################################
diff --git a/lldb/include/lldb/Breakpoint/WatchpointAlgorithms.h b/lldb/include/lldb/Breakpoint/WatchpointAlgorithms.h
deleted file mode 100644
index 9b5bb4e10f8fa..0000000000000
--- a/lldb/include/lldb/Breakpoint/WatchpointAlgorithms.h
+++ /dev/null
@@ -1,109 +0,0 @@
-//===-- WatchpointAlgorithms.h ----------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLDB_BREAKPOINT_WATCHPOINTALGORITHMS_H
-#define LLDB_BREAKPOINT_WATCHPOINTALGORITHMS_H
-
-#include "lldb/Breakpoint/WatchpointResource.h"
-#include "lldb/Utility/ArchSpec.h"
-#include "lldb/lldb-public.h"
-
-#include <vector>
-
-namespace lldb_private {
-
-class WatchpointAlgorithms {
-
-public:
- /// Convert a user's watchpoint request into an array of memory
- /// regions that can be watched by one hardware watchpoint register
- /// on the current target.
- ///
- /// \param[in] addr
- /// The start address specified by the user.
- ///
- /// \param[in] size
- /// The number of bytes the user wants to watch.
- ///
- /// \param[in] read
- /// True if we are watching for read accesses.
- ///
- /// \param[in] write
- /// True if we are watching for write accesses.
- /// \a read and \a write may both be true.
- /// There is no "modify" style for WatchpointResources -
- /// WatchpointResources are akin to the hardware watchpoint
- /// registers which are either in terms of read or write.
- /// "modify" distinction is done at the Watchpoint layer, where
- /// we check the actual range of bytes the user requested.
- ///
- /// \param[in] supported_features
- /// The bit flags in this parameter are set depending on which
- /// WatchpointHardwareFeature enum values the current target supports.
- /// The eWatchpointHardwareFeatureUnknown bit may be set if we
- /// don't have specific information about what the remote stub
- /// can support, and a reasonablec default will be used.
- ///
- /// \param[in] arch
- /// The ArchSpec of the current Target.
- ///
- /// \return
- /// A vector of WatchpointResourceSP's, one per hardware watchpoint
- /// register needed. We may return more WatchpointResources than the
- /// target can watch at once; if all resources cannot be set, the
- /// watchpoint cannot be set.
- static std::vector<lldb::WatchpointResourceSP> AtomizeWatchpointRequest(
- lldb::addr_t addr, size_t size, bool read, bool write,
- lldb::WatchpointHardwareFeature supported_features, ArchSpec &arch);
-
- struct Region {
- lldb::addr_t addr;
- size_t size;
- };
-
-protected:
- /// Convert a user's watchpoint request into an array of addr+size that
- /// can be watched with power-of-2 style hardware watchpoints.
- ///
- /// This is the default algorithm if we have no further information;
- /// most watchpoint implementations can be assumed to be able to watch up
- /// to pointer-size regions of memory in power-of-2 sizes and alingments.
- ///
- /// \param[in] user_addr
- /// The user's start address.
- ///
- /// \param[in] user_size
- /// The user's specified byte length.
- ///
- /// \param[in] min_byte_size
- /// The minimum byte size supported on this target.
- /// In most cases, this will be 1. AArch64 MASK watchpoints can
- /// watch a minimum of 8 bytes (although Byte Address Select watchpoints
- /// can watch 1 to pointer-size bytes in a pointer-size aligned granule).
- ///
- /// \param[in] max_byte_size
- /// The maximum byte size supported for one watchpoint on this target.
- ///
- /// \param[in] address_byte_size
- /// The address byte size on this target.
- static std::vector<Region> PowerOf2Watchpoints(lldb::addr_t user_addr,
- size_t user_size,
- size_t min_byte_size,
- size_t max_byte_size,
- uint32_t address_byte_size);
-};
-
-// For the unittests to have access to the individual algorithms
-class WatchpointAlgorithmsTest : public WatchpointAlgorithms {
-public:
- using WatchpointAlgorithms::PowerOf2Watchpoints;
-};
-
-} // namespace lldb_private
-
-#endif // LLDB_BREAKPOINT_WATCHPOINTALGORITHMS_H
diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h
index 50cbccba4d7c5..392d333c23a44 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -448,32 +448,6 @@ enum WatchpointWriteType {
eWatchpointWriteTypeOnModify
};
-/// The hardware and native stub capabilities for a given target,
-/// for translating a user's watchpoint request into hardware
-/// capable watchpoint resources.
-FLAGS_ENUM(WatchpointHardwareFeature){
- /// lldb will fall back to a default that assumes the target
- /// can watch up to pointer-size power-of-2 regions, aligned to
- /// power-of-2.
- eWatchpointHardwareFeatureUnknown = (1u << 0),
-
- /// Intel systems can watch 1, 2, 4, or 8 bytes (in 64-bit targets),
- /// aligned naturally.
- eWatchpointHardwareX86 = (1u << 1),
-
- /// ARM systems with Byte Address Select watchpoints
- /// can watch any consecutive series of bytes up to the
- /// size of a pointer (4 or 8 bytes), at a pointer-size
- /// alignment.
- eWatchpointHardwareArmBAS = (1u << 2),
-
- /// ARM systems with MASK watchpoints can watch any power-of-2
- /// sized region from 8 bytes to 2 gigabytes, aligned to that
- /// same power-of-2 alignment.
- eWatchpointHardwareArmMASK = (1u << 3),
-};
-LLDB_MARK_AS_BITMASK_ENUM(WatchpointHardwareFeature)
-
/// Programming language type.
///
/// These enumerations use the same language enumerations as the DWARF
diff --git a/lldb/packages/Python/lldbsuite/test/concurrent_base.py b/lldb/packages/Python/lldbsuite/test/concurrent_base.py
index 39eb27fd99747..72e04bbb20a04 100644
--- a/lldb/packages/Python/lldbsuite/test/concurrent_base.py
+++ b/lldb/packages/Python/lldbsuite/test/concurrent_base.py
@@ -166,12 +166,7 @@ def do_thread_actions(
# Initialize the (single) watchpoint on the global variable (g_watchme)
if num_watchpoint_threads + num_delay_watchpoint_threads > 0:
- # The concurrent tests have multiple threads modifying a variable
- # with the same value. The default "modify" style watchpoint will
- # only report this as 1 hit for all threads, because they all wrote
- # the same value. The testsuite needs "write" style watchpoints to
- # get the correct number of hits reported.
- self.runCmd("watchpoint set variable -w write g_watchme")
+ self.runCmd("watchpoint set variable g_watchme")
for w in self.inferior_target.watchpoint_iter():
self.thread_watchpoint = w
self.assertTrue(
diff --git a/lldb/source/Breakpoint/CMakeLists.txt b/lldb/source/Breakpoint/CMakeLists.txt
index 2fa659f803c28..3b39189e52587 100644
--- a/lldb/source/Breakpoint/CMakeLists.txt
+++ b/lldb/source/Breakpoint/CMakeLists.txt
@@ -21,7 +21,6 @@ add_lldb_library(lldbBreakpoint NO_PLUGIN_DEPENDENCIES
StoppointSite.cpp
StopPointSiteList.cpp
Watchpoint.cpp
- WatchpointAlgorithms.cpp
WatchpointList.cpp
WatchpointOptions.cpp
WatchpointResource.cpp
diff --git a/lldb/source/Breakpoint/Watchpoint.cpp b/lldb/source/Breakpoint/Watchpoint.cpp
index 7728fd09a07ae..1a8ef87c1e67a 100644
--- a/lldb/source/Breakpoint/Watchpoint.cpp
+++ b/lldb/source/Breakpoint/Watchpoint.cpp
@@ -45,16 +45,10 @@ Watchpoint::Watchpoint(Target &target, lldb::addr_t addr, uint32_t size,
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
+ if (auto ts = *type_system_or_err)
+ m_type =
+ ts->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 8 * size);
+ else
LLDB_LOG_ERROR(GetLog(LLDBLog::Watchpoints), std::move(err),
"Failed to set type: Typesystem is no longer live: {0}");
}
@@ -358,20 +352,6 @@ void Watchpoint::DumpWithLevel(Stream *s,
s->Printf("\n declare @ '%s'", m_decl_str.c_str());
if (!m_watch_spec_str.empty())
s->Printf("\n watchpoint spec = '%s'", m_watch_spec_str.c_str());
- if (IsEnabled()) {
- if (ProcessSP process_sp = m_target.GetProcessSP()) {
- auto &resourcelist = process_sp->GetWatchpointResourceList();
- size_t idx = 0;
- s->Printf("\n watchpoint resources:");
- for (WatchpointResourceSP &wpres : resourcelist.Sites()) {
- if (wpres->ConstituentsContains(this)) {
- s->Printf("\n #%zu: ", idx);
- wpres->Dump(s);
- }
- idx++;
- }
- }
- }
// Dump the snapshots we have taken.
DumpSnapshots(s, " ");
diff --git a/lldb/source/Breakpoint/WatchpointAlgorithms.cpp b/lldb/source/Breakpoint/WatchpointAlgorithms.cpp
deleted file mode 100644
index 95a978a17cbfd..0000000000000
--- a/lldb/source/Breakpoint/WatchpointAlgorithms.cpp
+++ /dev/null
@@ -1,142 +0,0 @@
-//===-- WatchpointAlgorithms.cpp ------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "lldb/Breakpoint/WatchpointAlgorithms.h"
-#include "lldb/Breakpoint/WatchpointResource.h"
-#include "lldb/Target/Process.h"
-#include "lldb/Utility/ArchSpec.h"
-
-#include <utility>
-#include <vector>
-
-using namespace lldb;
-using namespace lldb_private;
-
-std::vector<WatchpointResourceSP>
-WatchpointAlgorithms::AtomizeWatchpointRequest(
- addr_t addr, size_t size, bool read, bool write,
- WatchpointHardwareFeature supported_features, ArchSpec &arch) {
-
- std::vector<Region> entries;
-
- if (supported_features &
- WatchpointHardwareFeature::eWatchpointHardwareArmMASK) {
- entries =
- PowerOf2Watchpoints(addr, size,
- /*min_byte_size*/ 1,
- /*max_byte_size*/ INT32_MAX,
- /*address_byte_size*/ arch.GetAddressByteSize());
- } else {
- // As a fallback, assume we can watch any power-of-2
- // number of bytes up through the size of an address in the target.
- entries =
- PowerOf2Watchpoints(addr, size,
- /*min_byte_size*/ 1,
- /*max_byte_size*/ arch.GetAddressByteSize(),
- /*address_byte_size*/ arch.GetAddressByteSize());
- }
-
- std::vector<WatchpointResourceSP> resources;
- for (Region &ent : entries) {
- WatchpointResourceSP wp_res_sp =
- std::make_shared<WatchpointResource>(ent.addr, ent.size, read, write);
- resources.push_back(wp_res_sp);
- }
-
- return resources;
-}
-
-/// Convert a user's watchpoint request (\a user_addr and \a user_size)
-/// into hardware watchpoints, for a target that can watch a power-of-2
-/// region of memory (1, 2, 4, 8, etc), aligned to that same power-of-2
-/// memory address.
-///
-/// If a user asks to watch 4 bytes at address 0x1002 (0x1002-0x1005
-/// inclusive) we can implement this with two 2-byte watchpoints
-/// (0x1002 and 0x1004) or with an 8-byte watchpoint at 0x1000.
-/// A 4-byte watchpoint at 0x1002 would not be properly 4 byte aligned.
-///
-/// If a user asks to watch 16 bytes at 0x1000, and this target supports
-/// 8-byte watchpoints, we can implement this with two 8-byte watchpoints
-/// at 0x1000 and 0x1008.
-std::vector<WatchpointAlgorithms::Region>
-WatchpointAlgorithms::PowerOf2Watchpoints(addr_t user_addr, size_t user_size,
- size_t min_byte_size,
- size_t max_byte_size,
- uint32_t address_byte_size) {
-
- // Can't watch zero bytes.
- if (user_size == 0)
- return {};
-
- // The aligned watch region will be less than/equal to the size of
- // an address in this target.
- const int address_bit_size = address_byte_size * 8;
-
- size_t aligned_size = std::max(user_size, min_byte_size);
- /// Round up \a user_size to the next power-of-2 size
- /// user_size == 8 -> aligned_size == 8
- /// user_size == 9 -> aligned_size == 16
- /// user_size == 15 -> aligned_size == 16
- /// user_size == 192 -> aligned_size == 256
- /// Could be `std::bit_ceil(aligned_size)` when we build with C++20?
-
- aligned_size = 1ULL << (address_bit_size - __builtin_clzll(aligned_size - 1));
-
- addr_t aligned_start = user_addr & ~(aligned_size - 1);
-
- // Does this power-of-2 memory range, aligned to power-of-2 that the
- // hardware can watch, completely cover the requested region.
- if (aligned_size <= max_byte_size &&
- aligned_start + aligned_size >= user_addr + user_size)
- return {{aligned_start, aligned_size}};
-
- // If the maximum region we can watch is larger than the aligned
- // size, try increasing the region size by one power of 2 and see
- // if aligning to that amount can cover the requested region.
- //
- // Increasing the aligned_size repeatedly instead of splitting the
- // watchpoint can result in us watching large regions of memory
- // unintentionally when we could use small two watchpoints. e.g.
- // user_addr 0x3ff8 user_size 32
- // can be watched with four 8-byte watchpoints or if it's done with one
- // MASK watchpoint, it would need to be a 32KB watchpoint (a 16KB
- // watchpoint at 0x0 only covers 0x0000-0x4000). A user request
- // at the end of a power-of-2 region can lead to these undesirably
- // large watchpoints and many false positive hits to ignore.
- if (max_byte_size >= (aligned_size << 1)) {
- aligned_size <<= 1;
- aligned_start = user_addr & ~(aligned_size - 1);
- if (aligned_size <= max_byte_size &&
- aligned_start + aligned_size >= user_addr + user_size)
- return {{aligned_start, aligned_size}};
-
- // Go back to our original aligned size, to try the multiple
- // watchpoint approach.
- aligned_size >>= 1;
- }
-
- // We need to split the user's watchpoint into two or more watchpoints
- // that can be monitored by hardware, because of alignment and/or size
- // reasons.
- aligned_size = std::min(aligned_size, max_byte_size);
- aligned_start = user_addr & ~(aligned_size - 1);
-
- std::vector<Region> result;
- addr_t current_address = aligned_start;
- const addr_t user_end_address = user_addr + user_size;
- while (current_address + aligned_size < user_end_address) {
- result.push_back({current_address, aligned_size});
- current_address += aligned_size;
- }
-
- if (current_address < user_end_address)
- result.push_back({current_address, aligned_size});
-
- return result;
-}
diff --git a/lldb/source/Breakpoint/WatchpointResource.cpp b/lldb/source/Breakpoint/WatchpointResource.cpp
index 8f15fc7c49583..c1eb50c0358b3 100644
--- a/lldb/source/Breakpoint/WatchpointResource.cpp
+++ b/lldb/source/Breakpoint/WatchpointResource.cpp
@@ -9,7 +9,6 @@
#include <assert.h>
#include "lldb/Breakpoint/WatchpointResource.h"
-#include "lldb/Utility/Stream.h"
#include <algorithm>
@@ -114,8 +113,7 @@ bool WatchpointResource::ShouldStop(StoppointCallbackContext *context) {
}
void WatchpointResource::Dump(Stream *s) const {
- s->Printf("addr = 0x%8.8" PRIx64 " size = %zu", m_addr, m_size);
- return;
+ return; // LWP_TODO
}
wp_resource_id_t WatchpointResource::GetNextID() {
diff --git a/lldb/source/Commands/CommandObjectWatchpoint.cpp b/lldb/source/Commands/CommandObjectWatchpoint.cpp
index 438a16c50bd67..c80868d33905e 100644
--- a/lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -1139,22 +1139,9 @@ class CommandObjectWatchpointSetExpression : public CommandObjectRaw {
// Fetch the type from the value object, the type of the watched object is
// the pointee type
- /// of the expression, so convert to that if we found a valid type.
+ /// of the expression, so convert to that if we found a valid type.
CompilerType compiler_type(valobj_sp->GetCompilerType());
- std::optional<uint64_t> valobj_size = valobj_sp->GetByteSize();
- // Set the type as a uint8_t array if the size being watched is
- // larger than the ValueObject's size (which is probably the size
- // of a pointer).
- if (valobj_size && size > *valobj_size) {
- auto type_system = compiler_type.GetTypeSystem();
- if (type_system) {
- CompilerType clang_uint8_type =
- type_system->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 8);
- compiler_type = clang_uint8_type.GetArrayType(size);
- }
- }
-
Status error;
WatchpointSP watch_sp =
target->CreateWatchpoint(addr, size, &compiler_type, watch_type, error);
diff --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
index d756354f9bd27..565941d3168f1 100644
--- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -491,13 +491,14 @@ static StopInfoSP GetStopInfoForHardwareBP(Thread &thread, Target *target,
uint64_t exc_sub_sub_code) {
// Try hardware watchpoint.
if (target) {
+ // LWP_TODO: We need to find the WatchpointResource that matches
+ // the address, and evaluate its Watchpoints.
+
// The exc_sub_code indicates the data break address.
- WatchpointResourceSP wp_rsrc_sp =
- target->GetProcessSP()->GetWatchpointResourceList().FindByAddress(
- (addr_t)exc_sub_code);
- if (wp_rsrc_sp && wp_rsrc_sp->GetNumberOfConstituents() > 0) {
- return StopInfo::CreateStopReasonWithWatchpointID(
- thread, wp_rsrc_sp->GetConstituentAtIndex(0)->GetID());
+ lldb::WatchpointSP wp_sp =
+ target->GetWatchpointList().FindByAddress((lldb::addr_t)exc_sub_code);
+ if (wp_sp && wp_sp->IsEnabled()) {
+ return StopInfo::CreateStopReasonWithWatchpointID(thread, wp_sp->GetID());
}
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 4e3447e767c35..fd724350b1554 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -24,7 +24,6 @@
#include <sys/types.h>
#include "lldb/Breakpoint/Watchpoint.h"
-#include "lldb/Breakpoint/WatchpointAlgorithms.h"
#include "lldb/Breakpoint/WatchpointResource.h"
#include "lldb/Core/Debugger.h"
#include "lldb/Core/Module.h"
@@ -3154,22 +3153,23 @@ Status ProcessGDBRemote::EnableWatchpoint(WatchpointSP wp_sp, bool notify) {
bool write = wp_sp->WatchpointWrite() || wp_sp->WatchpointModify();
size_t size = wp_sp->GetByteSize();
- ArchSpec target_arch = GetTarget().GetArchitecture();
- WatchpointHardwareFeature supported_features =
- eWatchpointHardwareFeatureUnknown;
-
- // LWP_TODO: enable MASK watchpoint for arm64 debugserver
- // when it reports that it supports them.
- if (target_arch.GetTriple().getOS() == llvm::Triple::MacOSX &&
- target_arch.GetTriple().getArch() == llvm::Triple::aarch64) {
-#if 0
- supported_features |= eWatchpointHardwareArmMASK;
-#endif
- }
-
- std::vector<WatchpointResourceSP> resources =
- WatchpointAlgorithms::AtomizeWatchpointRequest(
- addr, size, read, write, supported_features, target_arch);
+ // New WatchpointResources needed to implement this Watchpoint.
+ std::vector<WatchpointResourceSP> resources;
+
+ // LWP_TODO: Break up the user's request into pieces that can be watched
+ // given the capabilities of the target cpu / stub software.
+ // As a default, breaking the watched region up into target-pointer-sized,
+ // aligned, groups.
+ //
+ // Beyond the default, a stub can / should inform us of its capabilities,
+ // e.g. a stub that can do AArch64 power-of-2 MASK watchpoints.
+ //
+ // And the cpu may have unique capabilities. AArch64 BAS watchpoints
+ // can watch any sequential bytes in a doubleword, but Intel watchpoints
+ // can only watch 1, 2, 4, 8 bytes within a doubleword.
+ WatchpointResourceSP wp_res_sp =
+ std::make_shared<WatchpointResource>(addr, size, read, write);
+ resources.push_back(wp_res_sp);
// LWP_TODO: Now that we know the WP Resources needed to implement this
// Watchpoint, we need to look at currently allocated Resources in the
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 95f78056b1644..3b65d661c1abd 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -987,10 +987,8 @@ class StopInfoWatchpoint : public StopInfo {
// 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();
+ if (m_should_stop && !wp_sp->WatchedValueReportable(exe_ctx))
m_should_stop = false;
- }
// Finally, if we are going to stop, print out the new & old values:
if (m_should_stop) {
diff --git a/lldb/test/API/functionalities/watchpoint/large-watchpoint/TestLargeWatchpoint.py b/lldb/test/API/functionalities/watchpoint/large-watchpoint/TestLargeWatchpoint.py
index f7ceb47c0b615..c5e161497e628 100644
--- a/lldb/test/API/functionalities/watchpoint/large-watchpoint/TestLargeWatchpoint.py
+++ b/lldb/test/API/functionalities/watchpoint/large-watchpoint/TestLargeWatchpoint.py
@@ -25,11 +25,6 @@ def continue_and_report_stop_reason(self, process, iter_str):
@skipIf(archs=no_match(["arm64", "arm64e", "aarch64"]))
@skipUnlessDarwin
- # LWP_TODO: until debugserver advertises that it supports
- # MASK watchpoints, this test can't be enabled, lldb won't
- # try to send watchpoints larger than 8 bytes.
- @skipIfDarwin
-
# debugserver only gained the ability to watch larger regions
# with this patch.
@skipIfOutOfTreeDebugserver
diff --git a/lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/Makefile b/lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/Makefile
deleted file mode 100644
index 10495940055b6..0000000000000
--- a/lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/Makefile
+++ /dev/null
@@ -1,3 +0,0 @@
-C_SOURCES := main.c
-
-include Makefile.rules
diff --git a/lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/TestUnalignedLargeWatchpoint.py b/lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/TestUnalignedLargeWatchpoint.py
deleted file mode 100644
index fea8f62b4be9c..0000000000000
--- a/lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/TestUnalignedLargeWatchpoint.py
+++ /dev/null
@@ -1,101 +0,0 @@
-"""
-Watch a large unaligned memory region that
-lldb will need multiple hardware watchpoints
-to cover.
-"""
-
-
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-
-class UnalignedLargeWatchpointTestCase(TestBase):
- def continue_and_report_stop_reason(self, process, iter_str):
- if self.TraceOn():
- self.runCmd("script print('continue')")
- process.Continue()
- self.assertIn(
- process.GetState(), [lldb.eStateStopped, lldb.eStateExited], iter_str
- )
- thread = process.GetSelectedThread()
- return thread.GetStopReason()
-
- NO_DEBUG_INFO_TESTCASE = True
-
- # Test on 64-bit targets where we probably have
- # four watchpoint registers that can watch doublewords (8-byte).
- @skipIf(archs=no_match(["arm64", "arm64e", "aarch64", "x86_64"]))
- def test_unaligned_large_watchpoint(self):
- """Test watching an unaligned region of memory that requires multiple watchpoints."""
- self.build()
- self.main_source_file = lldb.SBFileSpec("main.c")
- (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
- self, "break here", self.main_source_file
- )
- self.runCmd("break set -p done")
- self.runCmd("break set -p exiting")
-
- frame = thread.GetFrameAtIndex(0)
-
- array_addr = frame.GetValueForVariablePath("array").GetValueAsUnsigned()
-
- # Don't assume that the heap allocated array is aligned
- # to a 1024 byte boundary to begin with, force alignment.
- # wa_addr = (array_addr + 1024) & ~(1024 - 1)
- wa_addr = array_addr
-
- # Now make the start address unaligned.
- wa_addr = wa_addr + 7
-
- err = lldb.SBError()
- wp_opts = lldb.SBWatchpointOptions()
- wp_opts.SetWatchpointTypeWrite(lldb.eWatchpointWriteTypeOnModify)
- wp = target.WatchpointCreateByAddress(wa_addr, 22, wp_opts, err)
- self.assertTrue(wp.IsValid())
- self.assertSuccess(err)
- if self.TraceOn():
- self.runCmd("watch list -v")
-
- c_count = 0
- reason = self.continue_and_report_stop_reason(process, "continue #%d" % c_count)
- while reason == lldb.eStopReasonWatchpoint:
- c_count = c_count + 1
- reason = self.continue_and_report_stop_reason(
- process, "continue #%d" % c_count
- )
- self.assertLessEqual(c_count, 22)
-
- self.assertEqual(c_count, 22)
- self.expect("watchpoint list -v", substrs=["hit_count = 22"])
- self.assertEqual(wp.GetHitCount(), 22)
-
- target.DeleteWatchpoint(wp.GetID())
-
- # Now try watching a 16 byte variable
- # (not unaligned, but a good check to do anyway)
- #
- frame = thread.GetFrameAtIndex(0)
- err = lldb.SBError()
- wp = frame.locals["variable"][0].Watch(True, False, True, err)
- self.assertSuccess(err)
- if self.TraceOn():
- self.runCmd("frame select 0")
- self.runCmd("watchpoint list")
-
- c_count = 0
- reason = self.continue_and_report_stop_reason(process, "continue #%d" % c_count)
- while reason == lldb.eStopReasonWatchpoint:
- c_count = c_count + 1
- reason = self.continue_and_report_stop_reason(
- process, "continue #%d" % c_count
- )
- self.assertLessEqual(c_count, 4)
-
- if self.TraceOn():
- self.runCmd("frame select 0")
-
- self.assertEqual(c_count, 4)
- self.expect("watchpoint list -v", substrs=["hit_count = 4"])
- self.assertEqual(wp.GetHitCount(), 4)
diff --git a/lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/main.c b/lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/main.c
deleted file mode 100644
index 4c8c66bfc6762..0000000000000
--- a/lldb/test/API/functionalities/watchpoint/unaligned-large-watchpoint/main.c
+++ /dev/null
@@ -1,35 +0,0 @@
-#include <stdint.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-
-struct obj {
- uint32_t one;
- uint32_t two;
- uint32_t three;
- uint32_t four;
-};
-
-int main() {
- const int count = 16776960;
- uint8_t *array = (uint8_t *)malloc(count);
- memset(array, 0, count);
- struct obj variable;
- variable.one = variable.two = variable.three = variable.four = 0;
-
- puts("break here");
-
- for (int i = 0; i < count; i++)
- array[i]++;
-
- puts("done iterating");
-
- variable.one = 1;
- variable.two = 2;
- variable.three = 3;
- variable.four = 4;
-
- printf("variable value is %d\n",
- variable.one + variable.two + variable.three + variable.four);
- puts("exiting.");
-}
diff --git a/lldb/unittests/Breakpoint/CMakeLists.txt b/lldb/unittests/Breakpoint/CMakeLists.txt
index 757c2da1a4d9d..3164af237a772 100644
--- a/lldb/unittests/Breakpoint/CMakeLists.txt
+++ b/lldb/unittests/Breakpoint/CMakeLists.txt
@@ -1,6 +1,5 @@
add_lldb_unittest(LLDBBreakpointTests
BreakpointIDTest.cpp
- WatchpointAlgorithmsTests.cpp
LINK_LIBS
lldbBreakpoint
diff --git a/lldb/unittests/Breakpoint/WatchpointAlgorithmsTests.cpp b/lldb/unittests/Breakpoint/WatchpointAlgorithmsTests.cpp
deleted file mode 100644
index 6617d36f1d9bc..0000000000000
--- a/lldb/unittests/Breakpoint/WatchpointAlgorithmsTests.cpp
+++ /dev/null
@@ -1,156 +0,0 @@
-//===-- WatchpointAlgorithmsTests.cpp -------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "gtest/gtest.h"
-
-#include "lldb/Breakpoint/WatchpointAlgorithms.h"
-
-#include <utility>
-#include <vector>
-
-using namespace lldb;
-using namespace lldb_private;
-
-struct testcase {
- WatchpointAlgorithms::Region user; // What the user requested
- std::vector<WatchpointAlgorithms::Region>
- hw; // The hardware watchpoints we'll use
-};
-
-void check_testcase(testcase test,
- std::vector<WatchpointAlgorithms::Region> result,
- size_t min_byte_size, size_t max_byte_size,
- uint32_t address_byte_size) {
-
- EXPECT_EQ(result.size(), test.hw.size());
- for (size_t i = 0; i < result.size(); i++) {
- EXPECT_EQ(result[i].addr, test.hw[i].addr);
- EXPECT_EQ(result[i].size, test.hw[i].size);
- }
-}
-
-TEST(WatchpointAlgorithmsTests, PowerOf2Watchpoints) {
-
- // clang-format off
- std::vector<testcase> doubleword_max = {
- {
- {0x1012, 8},
- {{0x1010, 8}, {0x1018, 8}}
- },
- {
- {0x1002, 4},
- {{0x1000, 8}}
- },
- {
- {0x1006, 4},
- {{0x1004, 4}, {0x1008, 4}}
- },
- {
- {0x1006, 8},
- {{0x1000, 8}, {0x1008, 8}}
- },
- {
- {0x1000, 24},
- {{0x1000, 8}, {0x1008, 8}, {0x1010, 8}}
- },
- {
- {0x1014, 26},
- {{0x1010, 8}, {0x1018, 8}, {0x1020, 8}, {0x1028, 8}}
- },
- };
- // clang-format on
- for (testcase test : doubleword_max) {
- addr_t user_addr = test.user.addr;
- size_t user_size = test.user.size;
- size_t min_byte_size = 1;
- size_t max_byte_size = 8;
- size_t address_byte_size = 8;
- auto result = WatchpointAlgorithmsTest::PowerOf2Watchpoints(
- user_addr, user_size, min_byte_size, max_byte_size, address_byte_size);
-
- check_testcase(test, result, min_byte_size, max_byte_size,
- address_byte_size);
- }
-
- // clang-format off
- std::vector<testcase> word_max = {
- {
- {0x1002, 4},
- {{0x1000, 4}, {0x1004, 4}}
- },
- };
- // clang-format on
- for (testcase test : word_max) {
- addr_t user_addr = test.user.addr;
- size_t user_size = test.user.size;
- size_t min_byte_size = 1;
- size_t max_byte_size = 4;
- size_t address_byte_size = 4;
- auto result = WatchpointAlgorithmsTest::PowerOf2Watchpoints(
- user_addr, user_size, min_byte_size, max_byte_size, address_byte_size);
-
- check_testcase(test, result, min_byte_size, max_byte_size,
- address_byte_size);
- }
-
- // clang-format off
- std::vector<testcase> twogig_max = {
- {
- {0x1010, 16},
- {{0x1010, 16}}
- },
- {
- {0x1010, 24},
- {{0x1000, 64}}
- },
-
- // We increase 36 to the aligned 64 byte size, but
- // 0x1000-0x1040 doesn't cover the requested region. Then
- // we expand to 128 bytes starting at 0x1000 that does
- // cover it. Is this a good tradeoff for a 36 byte region?
- {
- {0x1024, 36},
- {{0x1000, 128}}
- },
- {
- {0x1000, 192},
- {{0x1000, 256}}
- },
- {
- {0x1080, 192},
- {{0x1000, 512}}
- },
-
- // In this case, our aligned size is 128, and increasing it to 256
- // still can't watch the requested region. The algorithm
- // falls back to using two 128 byte watchpoints.
- // The alternative would be to use a 1024B watchpoint
- // starting at 0x1000, to watch this 120 byte user request.
- //
- // This still isn't ideal. The user is asking to watch 0x12e0-1358
- // and could be optimally handled by a
- // 16-byte watchpoint at 0x12e0 and a 128-byte watchpoint at 0x1300
- {
- {0x12e0, 120},
- {{0x1280, 128}, {0x1300, 128}}
- },
- };
- // clang-format on
- for (testcase test : twogig_max) {
- addr_t user_addr = test.user.addr;
- size_t user_size = test.user.size;
- size_t min_byte_size = 1;
- size_t max_byte_size = INT32_MAX;
- size_t address_byte_size = 8;
- auto result = WatchpointAlgorithmsTest::PowerOf2Watchpoints(
- user_addr, user_size, min_byte_size, max_byte_size, address_byte_size);
-
- check_testcase(test, result, min_byte_size, max_byte_size,
- address_byte_size);
- }
-}
More information about the lldb-commits
mailing list