[lib/inferior_control] Remove size parameter from s/w breakpoints
The size is fixed, so simplify by removing it.
Also rename "kind" parameter of Breakpoint to "size", since
that's what it is.
Tested: Run example from garnet/bin/debugserver/README.md.
fx shell debugserver 7000 /system/test/sys/debugger-test segfault
Then on host: gdb
(gdb) target extended-remote 192.168.42.72:7000
(gdb) file out/build-zircon/build-x64/system/utest/debugger/debugger.elf
(gdb) run
Change-Id: I4b8923893e6897c09c3939ef0ed1b3c3f32f1d79
diff --git a/garnet/bin/debugserver/cmd_handler.cc b/garnet/bin/debugserver/cmd_handler.cc
index cd4f72e..4f4503a 100644
--- a/garnet/bin/debugserver/cmd_handler.cc
+++ b/garnet/bin/debugserver/cmd_handler.cc
@@ -780,9 +780,14 @@
switch (type) {
case 0:
- if (insert)
- return InsertSoftwareBreakpoint(addr, kind, optional_params, std::move(callback));
- return RemoveSoftwareBreakpoint(addr, kind, std::move(callback));
+ if (kind != inferior_control::SoftwareBreakpoint::Size()) {
+ FXL_LOG(ERROR) << "zZ0: unsupported kind field: " << kind;
+ return ReplyWithError(ErrorCode::INVAL, std::move(callback));
+ }
+ if (insert) {
+ return InsertSoftwareBreakpoint(addr, optional_params, std::move(callback));
+ }
+ return RemoveSoftwareBreakpoint(addr, std::move(callback));
default:
break;
}
@@ -1306,10 +1311,10 @@
}
bool CommandHandler::InsertSoftwareBreakpoint(
- uintptr_t addr, size_t kind, const fxl::StringView& optional_params,
+ uintptr_t addr, const fxl::StringView& optional_params,
ResponseCallback callback) {
FXL_VLOG(1) << fxl::StringPrintf(
- "Insert software breakpoint at %" PRIxPTR ", kind: %lu", addr, kind);
+ "Insert software breakpoint at 0x%lx", addr);
inferior_control::Process* current_process = server_->current_process();
if (!current_process) {
@@ -1319,7 +1324,7 @@
// TODO(armansito): Handle |optional_params|.
- if (!current_process->breakpoints()->InsertSoftwareBreakpoint(addr, kind)) {
+ if (!current_process->breakpoints()->InsertSoftwareBreakpoint(addr)) {
FXL_LOG(ERROR) << "Failed to insert software breakpoint";
return ReplyWithError(ErrorCode::PERM, std::move(callback));
}
@@ -1327,10 +1332,10 @@
return ReplyOK(std::move(callback));
}
-bool CommandHandler::RemoveSoftwareBreakpoint(uintptr_t addr, size_t kind,
+bool CommandHandler::RemoveSoftwareBreakpoint(uintptr_t addr,
ResponseCallback callback) {
- FXL_VLOG(1) << fxl::StringPrintf("Remove software breakpoint at %" PRIxPTR,
- addr);
+ FXL_VLOG(1) << fxl::StringPrintf(
+ "Remove software breakpoint at 0x%lx", addr);
inferior_control::Process* current_process = server_->current_process();
if (!current_process) {
diff --git a/garnet/bin/debugserver/cmd_handler.h b/garnet/bin/debugserver/cmd_handler.h
index 617b567..5b07795 100644
--- a/garnet/bin/debugserver/cmd_handler.h
+++ b/garnet/bin/debugserver/cmd_handler.h
@@ -81,11 +81,10 @@
bool Handle_vRun(const fxl::StringView& packet, ResponseCallback callback);
// Breakpoints
- bool InsertSoftwareBreakpoint(uintptr_t addr, size_t kind,
+ bool InsertSoftwareBreakpoint(uintptr_t addr,
const fxl::StringView& optional_params,
ResponseCallback callback);
- bool RemoveSoftwareBreakpoint(uintptr_t addr, size_t kind,
- ResponseCallback callback);
+ bool RemoveSoftwareBreakpoint(uintptr_t addr, ResponseCallback callback);
// The root Server instance that owns us.
RspServer* server_; // weak
diff --git a/garnet/lib/inferior_control/breakpoint.cc b/garnet/lib/inferior_control/breakpoint.cc
index 186eb8a..e5c9774 100644
--- a/garnet/lib/inferior_control/breakpoint.cc
+++ b/garnet/lib/inferior_control/breakpoint.cc
@@ -6,36 +6,94 @@
#include <cinttypes>
+#include "garnet/lib/debugger_utils/breakpoints.h"
#include "lib/fxl/logging.h"
#include "lib/fxl/strings/string_printf.h"
+#include "process.h"
+
namespace inferior_control {
-Breakpoint::Breakpoint(uintptr_t address, size_t kind)
- : address_(address), kind_(kind) {}
+Breakpoint::Breakpoint(uintptr_t address, size_t size)
+ : address_(address), size_(size) {}
-ProcessBreakpoint::ProcessBreakpoint(uintptr_t address, size_t kind,
+ProcessBreakpoint::ProcessBreakpoint(uintptr_t address, size_t size,
ProcessBreakpointSet* owner)
- : Breakpoint(address, kind), owner_(owner) {
+ : Breakpoint(address, size), owner_(owner) {
FXL_DCHECK(owner_);
}
-SoftwareBreakpoint::SoftwareBreakpoint(uintptr_t address, size_t kind,
+SoftwareBreakpoint::SoftwareBreakpoint(uintptr_t address,
ProcessBreakpointSet* owner)
- : ProcessBreakpoint(address, kind, owner) {}
+ : ProcessBreakpoint(address, Size(), owner) {}
SoftwareBreakpoint::~SoftwareBreakpoint() {
if (IsInserted())
Remove();
}
+size_t SoftwareBreakpoint::Size() {
+ return debugger_utils::GetBreakpointInstructionSize();
+}
+
+bool SoftwareBreakpoint::Insert() {
+ // TODO(PT-103): Handle breakpoints in unloaded solibs.
+
+ if (IsInserted()) {
+ FXL_LOG(WARNING) << "Breakpoint already inserted";
+ return false;
+ }
+
+ // Read the current contents at the address that we're about to overwrite, so
+ // that it can be restored later.
+ size_t num_bytes = Size();
+ std::vector<uint8_t> orig;
+ orig.resize(num_bytes);
+ if (!owner()->process()->ReadMemory(address(), orig.data(), num_bytes)) {
+ FXL_LOG(ERROR) << "Failed to obtain current contents of memory";
+ return false;
+ }
+
+ // Insert the breakpoint instruction.
+ const uint8_t* insn = debugger_utils::GetBreakpointInstruction();
+ if (!owner()->process()->WriteMemory(address(), insn, num_bytes)) {
+ FXL_LOG(ERROR) << "Failed to insert software breakpoint";
+ return false;
+ }
+
+ original_bytes_ = std::move(orig);
+ return true;
+}
+
+bool SoftwareBreakpoint::Remove() {
+ if (!IsInserted()) {
+ FXL_LOG(WARNING) << "Breakpoint not inserted";
+ return false;
+ }
+
+ FXL_DCHECK(original_bytes_.size() == Size());
+
+ // Restore the original contents.
+ if (!owner()->process()->WriteMemory(
+ address(), original_bytes_.data(), Size())) {
+ FXL_LOG(ERROR) << "Failed to restore original instructions";
+ return false;
+ }
+
+ original_bytes_.clear();
+ return true;
+}
+
+bool SoftwareBreakpoint::IsInserted() const {
+ return !original_bytes_.empty();
+}
+
ProcessBreakpointSet::ProcessBreakpointSet(Process* process)
: process_(process) {
FXL_DCHECK(process_);
}
-bool ProcessBreakpointSet::InsertSoftwareBreakpoint(uintptr_t address,
- size_t kind) {
+bool ProcessBreakpointSet::InsertSoftwareBreakpoint(uintptr_t address) {
if (breakpoints_.find(address) != breakpoints_.end()) {
FXL_LOG(ERROR) << fxl::StringPrintf(
"Breakpoint already inserted at address: 0x%" PRIxPTR, address);
@@ -43,7 +101,7 @@
}
std::unique_ptr<ProcessBreakpoint> breakpoint(
- new SoftwareBreakpoint(address, kind, this));
+ new SoftwareBreakpoint(address, this));
if (!breakpoint->Insert()) {
FXL_LOG(ERROR) << "Failed to insert software breakpoint";
return false;
@@ -70,9 +128,9 @@
return true;
}
-ThreadBreakpoint::ThreadBreakpoint(uintptr_t address, size_t kind,
+ThreadBreakpoint::ThreadBreakpoint(uintptr_t address, size_t size,
ThreadBreakpointSet* owner)
- : Breakpoint(address, kind), owner_(owner) {
+ : Breakpoint(address, size), owner_(owner) {
FXL_DCHECK(owner_);
}
diff --git a/garnet/lib/inferior_control/breakpoint.h b/garnet/lib/inferior_control/breakpoint.h
index 5b466f2..8bd2468 100644
--- a/garnet/lib/inferior_control/breakpoint.h
+++ b/garnet/lib/inferior_control/breakpoint.h
@@ -23,7 +23,7 @@
// Represents a breakpoint.
class Breakpoint {
public:
- Breakpoint(uintptr_t address, size_t kind);
+ Breakpoint(uintptr_t address, size_t size);
virtual ~Breakpoint() = default;
// Inserts the breakpoint at the memory address it was initialized with.
@@ -38,20 +38,20 @@
virtual bool IsInserted() const = 0;
uintptr_t address() const { return address_; }
- size_t kind() const { return kind_; }
+ size_t size() const { return size_; }
private:
Breakpoint() = default;
uintptr_t address_;
- size_t kind_;
+ size_t size_;
FXL_DISALLOW_COPY_AND_ASSIGN(Breakpoint);
};
class ProcessBreakpoint : public Breakpoint {
protected:
- ProcessBreakpoint(uintptr_t address, size_t kind,
+ ProcessBreakpoint(uintptr_t address, size_t size,
ProcessBreakpointSet* owner);
ProcessBreakpointSet* owner() const { return owner_; }
@@ -64,8 +64,10 @@
// Represents a software breakpoint.
class SoftwareBreakpoint final : public ProcessBreakpoint {
public:
- SoftwareBreakpoint(uintptr_t address, size_t kind,
- ProcessBreakpointSet* owner);
+ // Return the size in bytes of a s/w breakpoint.
+ static size_t Size();
+
+ SoftwareBreakpoint(uintptr_t address, ProcessBreakpointSet* owner);
~SoftwareBreakpoint() override;
// Breakpoint overrides
@@ -92,11 +94,9 @@
// Returns a pointer to the process that this object belongs to.
Process* process() const { return process_; }
- // Inserts a software breakpoint at the specified memory address with the
- // given kind. |kind| is an architecture dependent parameter that specifies
- // how many bytes the software breakpoint spans. Returns true on success or
- // false on failure.
- bool InsertSoftwareBreakpoint(uintptr_t address, size_t kind);
+ // Inserts a software breakpoint at the specified memory address.
+ // Returns true on success or false on failure.
+ bool InsertSoftwareBreakpoint(uintptr_t address);
// Removes the software breakpoint that was previously inserted at the given
// address. Returns false if there is an error of a breakpoint was not
@@ -114,7 +114,7 @@
class ThreadBreakpoint : public Breakpoint {
protected:
- ThreadBreakpoint(uintptr_t address, size_t kind, ThreadBreakpointSet* owner);
+ ThreadBreakpoint(uintptr_t address, size_t size, ThreadBreakpointSet* owner);
ThreadBreakpointSet* owner() const { return owner_; }
private:
@@ -151,8 +151,8 @@
// Returns a pointer to the thread that this object belongs to.
Thread* thread() const { return thread_; }
- // Inserts a single-step breakpoint at the specified memory address with the
- // given kind. |address| is recorded as the current pc value, at the moment
+ // Inserts a single-step breakpoint at the specified memory address.
+ // |address| is recorded as the current pc value, at the moment
// for bookkeeping purposes.
// Returns true on success or false on failure.
bool InsertSingleStepBreakpoint(uintptr_t address);
diff --git a/garnet/lib/inferior_control/breakpoint_arm64.cc b/garnet/lib/inferior_control/breakpoint_arm64.cc
index 730a885..5a81ce0 100644
--- a/garnet/lib/inferior_control/breakpoint_arm64.cc
+++ b/garnet/lib/inferior_control/breakpoint_arm64.cc
@@ -4,34 +4,11 @@
#include "breakpoint.h"
+#include "garnet/lib/debugger_utils/breakpoints.h"
#include "lib/fxl/logging.h"
namespace inferior_control {
-namespace {
-
-// TODO(dje): This isn't needed until we support debugserver-injected
-// breakpoints.
-// The arm64 s/w breakpoint instruction.
-// const uint32_t kBreakpoint = 0xd4200000;
-
-} // namespace
-
-bool SoftwareBreakpoint::Insert() {
- FXL_NOTIMPLEMENTED();
- return false;
-}
-
-bool SoftwareBreakpoint::Remove() {
- FXL_NOTIMPLEMENTED();
- return false;
-}
-
-bool SoftwareBreakpoint::IsInserted() const {
- FXL_NOTIMPLEMENTED();
- return false;
-}
-
bool SingleStepBreakpoint::Insert() {
FXL_NOTIMPLEMENTED();
return false;
diff --git a/garnet/lib/inferior_control/breakpoint_x64.cc b/garnet/lib/inferior_control/breakpoint_x64.cc
index 0e86503..775e3f7 100644
--- a/garnet/lib/inferior_control/breakpoint_x64.cc
+++ b/garnet/lib/inferior_control/breakpoint_x64.cc
@@ -4,6 +4,7 @@
#include "breakpoint.h"
+#include "garnet/lib/debugger_utils/breakpoints.h"
#include "lib/fxl/logging.h"
#include "process.h"
@@ -14,65 +15,6 @@
namespace {
-// The i386 Int3 instruction.
-const uint8_t kInt3 = 0xCC;
-
-} // namespace
-
-bool SoftwareBreakpoint::Insert() {
- // TODO: Handle breakpoints in unloaded solibs.
-
- if (IsInserted()) {
- FXL_LOG(WARNING) << "Breakpoint already inserted";
- return false;
- }
-
- // We only support inserting the single byte Int3 instruction.
- if (kind() != 1) {
- FXL_LOG(ERROR) << "Software breakpoint kind must be 1 on X64";
- return false;
- }
-
- // Read the current contents at the address that we're about to overwrite, so
- // that it can be restored later.
- uint8_t orig;
- if (!owner()->process()->ReadMemory(address(), &orig, 1)) {
- FXL_LOG(ERROR) << "Failed to obtain current contents of memory";
- return false;
- }
-
- // Insert the Int3 instruction.
- if (!owner()->process()->WriteMemory(address(), &kInt3, 1)) {
- FXL_LOG(ERROR) << "Failed to insert software breakpoint";
- return false;
- }
-
- original_bytes_.push_back(orig);
- return true;
-}
-
-bool SoftwareBreakpoint::Remove() {
- if (!IsInserted()) {
- FXL_LOG(WARNING) << "Breakpoint not inserted";
- return false;
- }
-
- FXL_DCHECK(original_bytes_.size() == 1);
-
- // Restore the original contents.
- if (!owner()->process()->WriteMemory(address(), original_bytes_.data(), 1)) {
- FXL_LOG(ERROR) << "Failed to restore original instructions";
- return false;
- }
-
- original_bytes_.clear();
- return true;
-}
-
-bool SoftwareBreakpoint::IsInserted() const { return !original_bytes_.empty(); }
-
-namespace {
-
// Set the TF bit in the RFLAGS register of |thread|.
bool SetRflagsTF(Thread* thread, bool enable) {
