From b35925f2043fdde5f2ae3a1d4ffd220d8cbd84b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cristi=C3=A1n=20Donoso?= <donosoc@google.com>
Date: Sat, 27 Apr 2019 00:05:42 +0000
Subject: [PATCH] [debugger] Extended breakpoint Add IPC request to support
watchpoints.
This required to extract a bit the "breakpoint type" from the settings.
This change is a no-op but requires to touch quite a bit of files because
of our "use the ipc type as our internal storage" strategy.
TEST=host + target tests.
Change-Id: Ia24efea188224a330e38ff1c5a44585f0021da8b
---
src/developer/debug/debug_agent/breakpoint.cc | 10 ++--
src/developer/debug/debug_agent/breakpoint.h | 10 +++-
.../debug/debug_agent/breakpoint_unittest.cc | 18 ++++---
.../debug/debug_agent/debug_agent.cc | 49 +++++++++++++++----
src/developer/debug/debug_agent/debug_agent.h | 6 +++
.../debug/debug_agent/hardware_breakpoint.cc | 2 +-
.../integration_tests/breakpoint_test.cc | 2 +-
.../integration_tests/debugged_job_test.cc | 2 +-
.../debug/debug_agent/process_breakpoint.cc | 4 +-
.../process_breakpoint_unittest.cc | 22 +++++----
src/developer/debug/ipc/agent_protocol.cc | 13 +++--
src/developer/debug/ipc/client_protocol.cc | 2 +-
src/developer/debug/ipc/protocol.h | 5 ++
src/developer/debug/ipc/protocol_unittests.cc | 2 +
src/developer/debug/ipc/records.cc | 16 ++++++
src/developer/debug/ipc/records.h | 6 +--
.../debug/zxdb/client/breakpoint_impl.cc | 2 +-
.../debug/zxdb/console/command_utils.cc | 9 ----
.../debug/zxdb/console/command_utils.h | 1 -
19 files changed, 127 insertions(+), 54 deletions(-)
diff --git a/src/developer/debug/debug_agent/breakpoint.cc b/src/developer/debug/debug_agent/breakpoint.cc
index e022d3b272d..b405c80c324 100644
--- a/src/developer/debug/debug_agent/breakpoint.cc
+++ b/src/developer/debug/debug_agent/breakpoint.cc
@@ -30,10 +30,16 @@ Breakpoint::~Breakpoint() {
}
zx_status_t Breakpoint::SetSettings(
+ debug_ipc::BreakpointType type,
const debug_ipc::BreakpointSettings& settings) {
- zx_status_t result = ZX_OK;
+ FXL_DCHECK(type == debug_ipc::BreakpointType::kSoftware ||
+ type == debug_ipc::BreakpointType::kHardware)
+ << "Got: " << debug_ipc::BreakpointTypeToString(type);
+ type_ = type;
settings_ = settings;
+ zx_status_t result = ZX_OK;
+
// The stats needs to reference the current ID. We assume setting the
// settings doesn't update the stats (an option to do this may need to be
// added in the future).
@@ -64,8 +70,6 @@ zx_status_t Breakpoint::SetSettings(
return result;
}
-
-
bool Breakpoint::AppliesToThread(zx_koid_t pid,
zx_koid_t tid) const {
for (auto& location : settings_.locations) {
diff --git a/src/developer/debug/debug_agent/breakpoint.h b/src/developer/debug/debug_agent/breakpoint.h
index 81969691e4e..8e45d59ec70 100644
--- a/src/developer/debug/debug_agent/breakpoint.h
+++ b/src/developer/debug/debug_agent/breakpoint.h
@@ -57,7 +57,14 @@ class Breakpoint {
const debug_ipc::BreakpointStats& stats() const { return stats_; }
// Sets the initial settings, or updates settings.
- zx_status_t SetSettings(const debug_ipc::BreakpointSettings& settings);
+ zx_status_t SetSettings(debug_ipc::BreakpointType,
+ const debug_ipc::BreakpointSettings& settings);
+
+ debug_ipc::BreakpointType type() const { return type_; }
+
+ // The setter is used mostly for testing. Normal setting should go through
+ // SetSettings.
+ void set_type(debug_ipc::BreakpointType type) { type_ = type; }
const debug_ipc::BreakpointSettings& settings() const { return settings_; }
// A breakpoint can be set to apply to a specific set of threads. A thread
@@ -73,6 +80,7 @@ class Breakpoint {
ProcessDelegate* process_delegate_; // Non-owning.
+ debug_ipc::BreakpointType type_ = debug_ipc::BreakpointType::kLast;
debug_ipc::BreakpointSettings settings_;
debug_ipc::BreakpointStats stats_;
diff --git a/src/developer/debug/debug_agent/breakpoint_unittest.cc b/src/developer/debug/debug_agent/breakpoint_unittest.cc
index cfa2840e855..d2dc605846f 100644
--- a/src/developer/debug/debug_agent/breakpoint_unittest.cc
+++ b/src/developer/debug/debug_agent/breakpoint_unittest.cc
@@ -55,7 +55,8 @@ TEST(Breakpoint, Registration) {
pr_settings.address = kAddress1;
// Apply the settings.
- ASSERT_EQ(ZX_OK, bp.SetSettings(settings));
+ ASSERT_EQ(ZX_OK, bp.SetSettings(debug_ipc::BreakpointType::kSoftware,
+ settings));
EXPECT_EQ(CallVector({CallPair{kProcess1, kAddress1}}),
delegate.register_calls());
EXPECT_TRUE(delegate.unregister_calls().empty());
@@ -69,7 +70,8 @@ TEST(Breakpoint, Registration) {
pr_settings.thread_koid = 0;
pr_settings.address = kAddress2;
- ASSERT_EQ(ZX_OK, bp.SetSettings(settings));
+ ASSERT_EQ(ZX_OK, bp.SetSettings(debug_ipc::BreakpointType::kSoftware,
+ settings));
EXPECT_EQ(CallVector({CallPair{kProcess2, kAddress2}}),
delegate.register_calls());
EXPECT_EQ(CallVector({CallPair{kProcess1, kAddress1}}),
@@ -96,7 +98,8 @@ TEST(Breakpoint, Registration) {
settings.locations.push_back(old_pr_settings);
settings.locations.push_back(new_pr_settings);
- ASSERT_EQ(ZX_OK, bp.SetSettings(settings));
+ ASSERT_EQ(ZX_OK, bp.SetSettings(debug_ipc::BreakpointType::kSoftware,
+ settings));
EXPECT_EQ(CallVector({{kProcess1, kAddress1}, {kProcess3, kAddress3}}),
delegate.register_calls());
@@ -121,7 +124,8 @@ TEST(Breakpoint, Destructor) {
pr_settings.address = kAddress1;
// Apply the settings.
- ASSERT_EQ(ZX_OK, bp->SetSettings(settings));
+ ASSERT_EQ(ZX_OK, bp->SetSettings(debug_ipc::BreakpointType::kSoftware,
+ settings));
EXPECT_EQ(CallVector({CallPair{kProcess1, kAddress1}}),
delegate.register_calls());
EXPECT_TRUE(delegate.unregister_calls().empty());
@@ -153,7 +157,8 @@ TEST(Breakpoint, HitCount) {
pr_settings.address = kAddress1;
// Apply the settings.
- ASSERT_EQ(ZX_OK, bp->SetSettings(settings));
+ ASSERT_EQ(ZX_OK, bp->SetSettings(debug_ipc::BreakpointType::kSoftware,
+ settings));
delegate.Clear();
EXPECT_EQ(kBreakpointId, bp->stats().breakpoint_id);
@@ -189,7 +194,8 @@ TEST(Breakpoint, OneShot) {
pr_settings.address = kAddress;
// Apply the settings.
- ASSERT_EQ(ZX_OK, bp->SetSettings(settings));
+ ASSERT_EQ(ZX_OK, bp->SetSettings(debug_ipc::BreakpointType::kSoftware,
+ settings));
delegate.Clear();
EXPECT_EQ(kBreakpointId, bp->stats().breakpoint_id);
diff --git a/src/developer/debug/debug_agent/debug_agent.cc b/src/developer/debug/debug_agent/debug_agent.cc
index 1f67aad3f17..ad439a63ab6 100644
--- a/src/developer/debug/debug_agent/debug_agent.cc
+++ b/src/developer/debug/debug_agent/debug_agent.cc
@@ -346,17 +346,18 @@ void DebugAgent::OnWriteRegisters(
void DebugAgent::OnAddOrChangeBreakpoint(
const debug_ipc::AddOrChangeBreakpointRequest& request,
debug_ipc::AddOrChangeBreakpointReply* reply) {
- TIME_BLOCK();
- uint32_t id = request.breakpoint.breakpoint_id;
- auto found = breakpoints_.find(id);
- if (found == breakpoints_.end()) {
- found = breakpoints_
- .emplace(std::piecewise_construct, std::forward_as_tuple(id),
- std::forward_as_tuple(this))
- .first;
+ switch (request.breakpoint_type) {
+ case debug_ipc::BreakpointType::kSoftware:
+ case debug_ipc::BreakpointType::kHardware:
+ return SetupBreakpoint(request, reply);
+ case debug_ipc::BreakpointType::kWatchpoint:
+ return SetupWatchpoint(request, reply);
+ case debug_ipc::BreakpointType::kLast:
+ break;
}
- reply->status = found->second.SetSettings(request.breakpoint);
+
+ FXL_NOTREACHED() << "Invalid Breakpoint Type.";
}
void DebugAgent::OnRemoveBreakpoint(
@@ -418,6 +419,21 @@ void DebugAgent::UnregisterBreakpoint(Breakpoint* bp, zx_koid_t process_koid,
proc->UnregisterBreakpoint(bp, address);
}
+void DebugAgent::SetupBreakpoint(
+ const debug_ipc::AddOrChangeBreakpointRequest& request,
+ debug_ipc::AddOrChangeBreakpointReply* reply) {
+ uint32_t id = request.breakpoint.breakpoint_id;
+ auto found = breakpoints_.find(id);
+ if (found == breakpoints_.end()) {
+ found = breakpoints_
+ .emplace(std::piecewise_construct, std::forward_as_tuple(id),
+ std::forward_as_tuple(this))
+ .first;
+ }
+ reply->status = found->second.SetSettings(request.breakpoint_type,
+ request.breakpoint);
+}
+
zx_status_t DebugAgent::RegisterWatchpoint(
Watchpoint* wp, zx_koid_t process_koid,
const debug_ipc::AddressRange& range) {
@@ -442,6 +458,21 @@ void DebugAgent::UnregisterWatchpoint(Watchpoint* wp, zx_koid_t process_koid,
process->UnregisterWatchpoint(wp, range);
}
+void DebugAgent::SetupWatchpoint(
+ const debug_ipc::AddOrChangeBreakpointRequest& request,
+ debug_ipc::AddOrChangeBreakpointReply* reply) {
+ auto id = request.watchpoint.watchpoint_id;
+
+ auto wp_it = watchpoints_.find(id);
+ if (wp_it == watchpoints_.end()) {
+ wp_it = watchpoints_
+ .emplace(std::piecewise_construct, std::forward_as_tuple(id),
+ std::forward_as_tuple(this))
+ .first;
+ }
+ reply->status = wp_it->second.SetSettings(request.watchpoint);
+}
+
void DebugAgent::OnAddressSpace(const debug_ipc::AddressSpaceRequest& request,
debug_ipc::AddressSpaceReply* reply) {
TIME_BLOCK();
diff --git a/src/developer/debug/debug_agent/debug_agent.h b/src/developer/debug/debug_agent/debug_agent.h
index 2aad02405bb..dbe5f7e935a 100644
--- a/src/developer/debug/debug_agent/debug_agent.h
+++ b/src/developer/debug/debug_agent/debug_agent.h
@@ -110,6 +110,9 @@ class DebugAgent : public RemoteAPI,
void UnregisterBreakpoint(Breakpoint* bp, zx_koid_t process_koid,
uint64_t address) override;
+ void SetupBreakpoint(const debug_ipc::AddOrChangeBreakpointRequest& request,
+ debug_ipc::AddOrChangeBreakpointReply* reply);
+
// Watchpoint::ProcessDelegate implementation --------------------------------
zx_status_t RegisterWatchpoint(Watchpoint*, zx_koid_t process_koid,
@@ -117,6 +120,9 @@ class DebugAgent : public RemoteAPI,
void UnregisterWatchpoint(Watchpoint*, zx_koid_t process_koid,
const debug_ipc::AddressRange&) override;
+ void SetupWatchpoint(const debug_ipc::AddOrChangeBreakpointRequest& request,
+ debug_ipc::AddOrChangeBreakpointReply* reply);
+
// Job/Process/Thread Management ---------------------------------------------
zx_status_t AddDebuggedJob(zx_koid_t job_koid, zx::job zx_job);
diff --git a/src/developer/debug/debug_agent/hardware_breakpoint.cc b/src/developer/debug/debug_agent/hardware_breakpoint.cc
index ab7aa3341ee..64bce53cf8d 100644
--- a/src/developer/debug/debug_agent/hardware_breakpoint.cc
+++ b/src/developer/debug/debug_agent/hardware_breakpoint.cc
@@ -169,7 +169,7 @@ std::set<zx_koid_t> HWThreadsTargeted(const ProcessBreakpoint& pb) {
bool all_threads = false;
for (Breakpoint* bp : pb.breakpoints()) {
// We only care about hardware breakpoints.
- if (bp->settings().type != debug_ipc::BreakpointType::kHardware)
+ if (bp->type() != debug_ipc::BreakpointType::kHardware)
continue;
for (auto& location : bp->settings().locations) {
diff --git a/src/developer/debug/debug_agent/integration_tests/breakpoint_test.cc b/src/developer/debug/debug_agent/integration_tests/breakpoint_test.cc
index 1a70af13c6e..3ff19b18f68 100644
--- a/src/developer/debug/debug_agent/integration_tests/breakpoint_test.cc
+++ b/src/developer/debug/debug_agent/integration_tests/breakpoint_test.cc
@@ -254,9 +254,9 @@ TEST(BreakpointIntegration, HWBreakpoint) {
location.address = module_function;
debug_ipc::AddOrChangeBreakpointRequest breakpoint_request = {};
+ breakpoint_request.breakpoint_type = debug_ipc::BreakpointType::kHardware;
breakpoint_request.breakpoint.breakpoint_id = kBreakpointId;
breakpoint_request.breakpoint.one_shot = true;
- breakpoint_request.breakpoint.type = debug_ipc::BreakpointType::kHardware;
breakpoint_request.breakpoint.locations.push_back(location);
debug_ipc::AddOrChangeBreakpointReply breakpoint_reply;
remote_api->OnAddOrChangeBreakpoint(breakpoint_request, &breakpoint_reply);
diff --git a/src/developer/debug/debug_agent/integration_tests/debugged_job_test.cc b/src/developer/debug/debug_agent/integration_tests/debugged_job_test.cc
index ab8e538139d..85a0aa681eb 100644
--- a/src/developer/debug/debug_agent/integration_tests/debugged_job_test.cc
+++ b/src/developer/debug/debug_agent/integration_tests/debugged_job_test.cc
@@ -356,8 +356,8 @@ TEST(DebuggedJobIntegrationTest, DISABLED_RepresentativeScenario) {
location.process_koid = process_koid;
location.address = function_address;
AddOrChangeBreakpointRequest breakpoint_request;
+ breakpoint_request.breakpoint_type = debug_ipc::BreakpointType::kSoftware;
breakpoint_request.breakpoint.breakpoint_id = breakpoint_id;
- breakpoint_request.breakpoint.type = debug_ipc::BreakpointType::kSoftware;
breakpoint_request.breakpoint.locations.push_back(location);
AddOrChangeBreakpointReply breakpoint_reply;
remote_api->OnAddOrChangeBreakpoint(breakpoint_request, &breakpoint_reply);
diff --git a/src/developer/debug/debug_agent/process_breakpoint.cc b/src/developer/debug/debug_agent/process_breakpoint.cc
index 212d11c86b8..0277274ccc6 100644
--- a/src/developer/debug/debug_agent/process_breakpoint.cc
+++ b/src/developer/debug/debug_agent/process_breakpoint.cc
@@ -73,7 +73,7 @@ void ProcessBreakpoint::OnHit(
hit_breakpoints->clear();
for (Breakpoint* breakpoint : breakpoints_) {
// Only care for breakpoints that match the exception type.
- if (breakpoint->settings().type != exception_type)
+ if (breakpoint->type() != exception_type)
continue;
breakpoint->OnHit();
@@ -137,7 +137,7 @@ zx_status_t ProcessBreakpoint::Update() {
// regardless of which threads are targeted.
int sw_bp_count = 0;
for (Breakpoint* bp : breakpoints_) {
- if (bp->settings().type == debug_ipc::BreakpointType::kSoftware)
+ if (bp->type() == debug_ipc::BreakpointType::kSoftware)
sw_bp_count++;
}
diff --git a/src/developer/debug/debug_agent/process_breakpoint_unittest.cc b/src/developer/debug/debug_agent/process_breakpoint_unittest.cc
index 6da0d342383..9ba3a9de7d1 100644
--- a/src/developer/debug/debug_agent/process_breakpoint_unittest.cc
+++ b/src/developer/debug/debug_agent/process_breakpoint_unittest.cc
@@ -152,6 +152,7 @@ const char
TEST(ProcessBreakpoint, InstallAndFixup) {
TestProcessDelegate process_delegate;
Breakpoint main_breakpoint(&process_delegate);
+ main_breakpoint.set_type(debug_ipc::BreakpointType::kSoftware);
zx_koid_t process_koid = 0x1234;
MockProcess process(process_koid);
@@ -188,6 +189,7 @@ TEST(ProcessBreakpoint, InstallAndFixup) {
TEST(ProcessBreakpoint, StepMultiple) {
TestProcessDelegate process_delegate;
Breakpoint main_breakpoint(&process_delegate);
+ main_breakpoint.set_type(debug_ipc::BreakpointType::kSoftware);
zx_koid_t process_koid = 0x1234;
MockProcess process(process_koid);
@@ -245,14 +247,16 @@ TEST(ProcessBreakpoint, HitCount) {
// (corresponds to two logical breakpoints at the same address).
std::unique_ptr<Breakpoint> main_breakpoint1 =
std::make_unique<Breakpoint>(&process_delegate);
- zx_status_t status = main_breakpoint1->SetSettings(settings);
+ zx_status_t status = main_breakpoint1->SetSettings(
+ debug_ipc::BreakpointType::kSoftware, settings);
ASSERT_EQ(ZX_OK, status);
std::unique_ptr<Breakpoint> main_breakpoint2 =
std::make_unique<Breakpoint>(&process_delegate);
constexpr uint32_t kBreakpointId2 = 13;
settings.breakpoint_id = kBreakpointId2;
- status = main_breakpoint2->SetSettings(settings);
+ status = main_breakpoint2->SetSettings(debug_ipc::BreakpointType::kSoftware,
+ settings);
ASSERT_EQ(ZX_OK, status);
// There should only be one address with a breakpoint.
@@ -307,10 +311,10 @@ TEST(ProcessBreakpoint, HWBreakpointForAllThreads) {
auto breakpoint = std::make_unique<Breakpoint>(&process_delegate);
debug_ipc::BreakpointSettings settings1 = {};
settings1.breakpoint_id = kBreakpointId1;
- settings1.type = debug_ipc::BreakpointType::kHardware;
// This location is for all threads.
settings1.locations.push_back({kProcessId, 0, kAddress});
- zx_status_t status = breakpoint->SetSettings(settings1);
+ zx_status_t status =
+ breakpoint->SetSettings(debug_ipc::BreakpointType::kHardware, settings1);
ASSERT_EQ(status, ZX_OK);
// Should have installed the breakpoint.
@@ -354,9 +358,9 @@ TEST(ProcessBreakpoint, HWBreakpointWithThreadId) {
auto breakpoint1 = std::make_unique<Breakpoint>(&process_delegate);
debug_ipc::BreakpointSettings settings1 = {};
settings1.breakpoint_id = kBreakpointId1;
- settings1.type = debug_ipc::BreakpointType::kHardware;
settings1.locations.push_back({kProcessId, kThreadId1, kAddress});
- zx_status_t status = breakpoint1->SetSettings(settings1);
+ zx_status_t status =
+ breakpoint1->SetSettings(debug_ipc::BreakpointType::kHardware, settings1);
ASSERT_EQ(status, ZX_OK);
// Should have installed the process breakpoint.
ASSERT_EQ(process_delegate.bps().size(), 1u);
@@ -374,13 +378,12 @@ TEST(ProcessBreakpoint, HWBreakpointWithThreadId) {
auto breakpoint2 = std::make_unique<Breakpoint>(&process_delegate);
debug_ipc::BreakpointSettings settings2 = {};
settings2.breakpoint_id = kBreakpointId2;
- settings2.type = debug_ipc::BreakpointType::kHardware;
settings2.locations.push_back({kProcessId, kThreadId2, kAddress});
// This breakpoint has another location for another thread.
// In practice, this should not happen, but it's important that no HW
// breakpoint get installed if for the wrong location.
settings2.locations.push_back({kProcessId, kThreadId3, kOtherAddress});
- breakpoint2->SetSettings(settings2);
+ breakpoint2->SetSettings(debug_ipc::BreakpointType::kHardware, settings2);
// Registering this breakpoint should create a new ProcessBreakpoint.
ASSERT_EQ(process_delegate.bps().size(), 2u);
auto& process_bp2 = (process_delegate.bps().begin()++)->second;
@@ -409,9 +412,8 @@ TEST(ProcessBreakpoint, HWBreakpointWithThreadId) {
auto sw_breakpoint = std::make_unique<Breakpoint>(&process_delegate);
debug_ipc::BreakpointSettings sw_settings = {};
sw_settings.breakpoint_id = kSwBreakpointId;
- sw_settings.type = debug_ipc::BreakpointType::kSoftware;
sw_settings.locations.push_back({kProcessId, 0, kAddress});
- sw_breakpoint->SetSettings(sw_settings);
+ sw_breakpoint->SetSettings(debug_ipc::BreakpointType::kSoftware, sw_settings);
// Should have installed only a SW breakpoint.
ASSERT_EQ(arch_provider->TotalBreakpointInstallCalls(), 3u);
ASSERT_EQ(arch_provider->TotalBreakpointUninstallCalls(), 1u);
diff --git a/src/developer/debug/ipc/agent_protocol.cc b/src/developer/debug/ipc/agent_protocol.cc
index 08896f0a696..e4c4543d984 100644
--- a/src/developer/debug/ipc/agent_protocol.cc
+++ b/src/developer/debug/ipc/agent_protocol.cc
@@ -31,11 +31,6 @@ bool Deserialize(MessageReader* reader, BreakpointSettings* settings) {
return false;
settings->stop = static_cast<Stop>(stop);
- uint32_t type;
- if (!reader->ReadUint32(&type))
- return false;
- settings->type = static_cast<BreakpointType>(type);
-
return Deserialize(reader, &settings->locations);
}
@@ -350,6 +345,14 @@ bool ReadRequest(MessageReader* reader, AddOrChangeBreakpointRequest* request,
if (!reader->ReadHeader(&header))
return false;
*transaction_id = header.transaction_id;
+
+ uint32_t breakpoint_type;
+ if (!reader->ReadUint32(&breakpoint_type) ||
+ breakpoint_type >= static_cast<uint32_t>(BreakpointType::kLast)) {
+ return false;
+ }
+ request->breakpoint_type = static_cast<BreakpointType>(breakpoint_type);
+
return Deserialize(reader, &request->breakpoint);
}
diff --git a/src/developer/debug/ipc/client_protocol.cc b/src/developer/debug/ipc/client_protocol.cc
index 22deddfc4a7..20d6544e008 100644
--- a/src/developer/debug/ipc/client_protocol.cc
+++ b/src/developer/debug/ipc/client_protocol.cc
@@ -130,7 +130,6 @@ void Serialize(const BreakpointSettings& settings, MessageWriter* writer) {
writer->WriteUint32(settings.breakpoint_id);
writer->WriteBool(settings.one_shot);
writer->WriteUint32(static_cast<uint32_t>(settings.stop));
- writer->WriteUint32(static_cast<uint32_t>(settings.type));
Serialize(settings.locations, writer);
}
@@ -414,6 +413,7 @@ bool ReadReply(MessageReader* reader, WriteRegistersReply* reply,
void WriteRequest(const AddOrChangeBreakpointRequest& request,
uint32_t transaction_id, MessageWriter* writer) {
writer->WriteHeader(MsgHeader::Type::kAddOrChangeBreakpoint, transaction_id);
+ writer->WriteUint32(static_cast<uint32_t>(request.breakpoint_type));
Serialize(request.breakpoint, writer);
}
diff --git a/src/developer/debug/ipc/protocol.h b/src/developer/debug/ipc/protocol.h
index 8912de60183..a72b7539b17 100644
--- a/src/developer/debug/ipc/protocol.h
+++ b/src/developer/debug/ipc/protocol.h
@@ -222,7 +222,12 @@ struct ReadMemoryReply {
};
struct AddOrChangeBreakpointRequest {
+ // What kind of request this is.
+ BreakpointType breakpoint_type = BreakpointType::kSoftware;
+
+ // Only one of these should be valid at a time.
BreakpointSettings breakpoint;
+ WatchpointSettings watchpoint;
};
struct AddOrChangeBreakpointReply {
// A variety of race conditions could cause a breakpoint modification or
diff --git a/src/developer/debug/ipc/protocol_unittests.cc b/src/developer/debug/ipc/protocol_unittests.cc
index 09e8abca3bd..07f715fe65a 100644
--- a/src/developer/debug/ipc/protocol_unittests.cc
+++ b/src/developer/debug/ipc/protocol_unittests.cc
@@ -379,6 +379,7 @@ TEST(Protocol, ReadMemoryReply) {
TEST(Protocol, AddOrChangeBreakpointRequest) {
AddOrChangeBreakpointRequest initial;
+ initial.breakpoint_type = BreakpointType::kHardware;
initial.breakpoint.breakpoint_id = 8976;
initial.breakpoint.stop = debug_ipc::Stop::kProcess;
initial.breakpoint.locations.resize(1);
@@ -391,6 +392,7 @@ TEST(Protocol, AddOrChangeBreakpointRequest) {
AddOrChangeBreakpointRequest second;
ASSERT_TRUE(SerializeDeserializeRequest(initial, &second));
+ EXPECT_EQ(initial.breakpoint_type, second.breakpoint_type);
EXPECT_EQ(initial.breakpoint.breakpoint_id, second.breakpoint.breakpoint_id);
EXPECT_EQ(initial.breakpoint.stop, second.breakpoint.stop);
ASSERT_EQ(initial.breakpoint.locations.size(),
diff --git a/src/developer/debug/ipc/records.cc b/src/developer/debug/ipc/records.cc
index 0659ad7e4b8..935435566a1 100644
--- a/src/developer/debug/ipc/records.cc
+++ b/src/developer/debug/ipc/records.cc
@@ -77,6 +77,22 @@ const char* RegisterCategory::TypeToString(RegisterCategory::Type type) {
return nullptr;
}
+const char* BreakpointTypeToString(BreakpointType type) {
+ switch (type) {
+ case BreakpointType::kSoftware:
+ return "Software";
+ case BreakpointType::kHardware:
+ return "Hardware";
+ case BreakpointType::kWatchpoint:
+ return "Watchpoint";
+ case BreakpointType::kLast:
+ return "Last";
+ }
+
+ FXL_NOTREACHED();
+ return nullptr;
+}
+
RegisterCategory::Type RegisterCategory::RegisterIDToCategory(RegisterID id) {
uint32_t val = static_cast<uint32_t>(id);
diff --git a/src/developer/debug/ipc/records.h b/src/developer/debug/ipc/records.h
index 835719274c4..143803ef0d5 100644
--- a/src/developer/debug/ipc/records.h
+++ b/src/developer/debug/ipc/records.h
@@ -151,7 +151,10 @@ enum class Stop : uint32_t {
enum class BreakpointType : uint32_t {
kSoftware,
kHardware,
+ kWatchpoint,
+ kLast,
};
+const char* BreakpointTypeToString(BreakpointType);
struct BreakpointSettings {
// The ID if this breakpoint. This is assigned by the client. This is
@@ -166,9 +169,6 @@ struct BreakpointSettings {
// What should stop when the breakpoint is hit.
Stop stop = Stop::kAll;
- // What kind of breakpoint this is.
- BreakpointType type = BreakpointType::kSoftware;
-
// Processes to which this breakpoint applies.
//
// If any process specifies a nonzero thread_koid, it must be the only
diff --git a/src/developer/debug/zxdb/client/breakpoint_impl.cc b/src/developer/debug/zxdb/client/breakpoint_impl.cc
index 1e040054fe2..5958015b2f6 100644
--- a/src/developer/debug/zxdb/client/breakpoint_impl.cc
+++ b/src/developer/debug/zxdb/client/breakpoint_impl.cc
@@ -260,10 +260,10 @@ void BreakpointImpl::SendBackendAddOrChange(
backend_installed_ = true;
debug_ipc::AddOrChangeBreakpointRequest request;
+ request.breakpoint_type = settings_.type;
request.breakpoint.breakpoint_id = backend_id_;
request.breakpoint.stop = SettingsStopToIpcStop(settings_.stop_mode);
request.breakpoint.one_shot = settings_.one_shot;
- request.breakpoint.type = settings_.type;
for (const auto& proc : procs_) {
for (const auto& pair : proc.second.locs) {
diff --git a/src/developer/debug/zxdb/console/command_utils.cc b/src/developer/debug/zxdb/console/command_utils.cc
index dd3dfe80ed4..e0319e4eb6a 100644
--- a/src/developer/debug/zxdb/console/command_utils.cc
+++ b/src/developer/debug/zxdb/console/command_utils.cc
@@ -330,15 +330,6 @@ const char* BreakpointEnabledToString(bool enabled) {
return enabled ? "Enabled" : "Disabled";
}
-const char* BreakpointTypeToString(debug_ipc::BreakpointType type) {
- switch (type) {
- case debug_ipc::BreakpointType::kSoftware:
- return "Software";
- case debug_ipc::BreakpointType::kHardware:
- return "Hardware";
- }
-}
-
std::string DescribeJobContext(const ConsoleContext* context,
const JobContext* job_context) {
int id = context->IdForJobContext(job_context);
diff --git a/src/developer/debug/zxdb/console/command_utils.h b/src/developer/debug/zxdb/console/command_utils.h
index ccd9886d89b..d7dec3089cd 100644
--- a/src/developer/debug/zxdb/console/command_utils.h
+++ b/src/developer/debug/zxdb/console/command_utils.h
@@ -86,7 +86,6 @@ std::string BreakpointScopeToString(const ConsoleContext* context,
const BreakpointSettings& settings);
std::string BreakpointStopToString(BreakpointSettings::StopMode mode);
const char* BreakpointEnabledToString(bool enabled);
-const char* BreakpointTypeToString(debug_ipc::BreakpointType);
std::string DescribeTarget(const ConsoleContext* context, const Target* target);
--
GitLab