From c3faaeb9b935ff3c656a87e7c04d955ed7d1f92a Mon Sep 17 00:00:00 2001 From: Itamar Date: Mon, 13 Apr 2020 17:07:18 +0300 Subject: [PATCH] Debugger: Breakpoints now persist after being tripped Previously, a breakpoint was removed after it was tripped. After a breakpoint trips, we have to undo the 'int3' patch from the instruction in order to continue the exceution. To make a breakpoint persist, we switch to "single step" mode, which stops the execution after a single instruction, and then we insert the breakpoint at the previous instruction. There is also some code that deals with an edge case where there are breakpoints in two consecutive instructions. --- Applications/Debugger/DebugSession.cpp | 57 ++++++++++++++---- Applications/Debugger/DebugSession.h | 82 +++++++++++++++++--------- 2 files changed, 99 insertions(+), 40 deletions(-) diff --git a/Applications/Debugger/DebugSession.cpp b/Applications/Debugger/DebugSession.cpp index 14e673cc5e..5519c73289 100644 --- a/Applications/Debugger/DebugSession.cpp +++ b/Applications/Debugger/DebugSession.cpp @@ -132,18 +132,38 @@ bool DebugSession::insert_breakpoint(void* address) if (!original_bytes.has_value()) return false; - if (!poke(reinterpret_cast(address), (original_bytes.value() & ~(uint32_t)0xff) | BREAKPOINT_INSTRUCTION)) - return false; + BreakPoint breakpoint { address, original_bytes.value(), BreakPointState::Disabled }; + + m_breakpoints.set(address, breakpoint); + + enable_breakpoint(breakpoint); - m_breakpoints.set(address, { address, original_bytes.value() }); return true; } -void DebugSession::remove_breakpoint(const BreakPoint& breakpoint) +bool DebugSession::disable_breakpoint(const BreakPoint& breakpoint) { ASSERT(m_breakpoints.contains(breakpoint.address)); - poke(reinterpret_cast(reinterpret_cast(breakpoint.address)), breakpoint.original_first_word); - m_breakpoints.remove(breakpoint.address); + if (!poke(reinterpret_cast(reinterpret_cast(breakpoint.address)), breakpoint.original_first_word)) + return false; + + auto bp = m_breakpoints.get(breakpoint.address).value(); + bp.state = BreakPointState::Disabled; + m_breakpoints.set(bp.address, bp); + return true; +} + +bool DebugSession::enable_breakpoint(const BreakPoint& breakpoint) +{ + ASSERT(m_breakpoints.contains(breakpoint.address)); + + if (!poke(reinterpret_cast(breakpoint.address), (breakpoint.original_first_word & ~(uint32_t)0xff) | BREAKPOINT_INSTRUCTION)) + return false; + + auto bp = m_breakpoints.get(breakpoint.address).value(); + bp.state = BreakPointState::Enabled; + m_breakpoints.set(bp.address, bp); + return true; } PtraceRegisters DebugSession::get_registers() const @@ -164,11 +184,6 @@ void DebugSession::set_registers(const PtraceRegisters& regs) } } -Optional DebugSession::get_matching_breakpoint(const PtraceRegisters& regs) const -{ - return m_breakpoints.get(reinterpret_cast(regs.eip - 1)); -} - void DebugSession::continue_debugee() { if (ptrace(PT_CONTINUE, m_debugee_pid, 0, 0) < 0) { @@ -176,3 +191,23 @@ void DebugSession::continue_debugee() ASSERT_NOT_REACHED(); } } + +void* DebugSession::single_step() +{ + auto regs = get_registers(); + constexpr u32 TRAP_FLAG = 0x100; + regs.eflags |= TRAP_FLAG; + set_registers(regs); + + continue_debugee(); + + if (waitpid(m_debugee_pid, 0, WSTOPPED) != m_debugee_pid) { + perror("waitpid"); + ASSERT_NOT_REACHED(); + } + + regs = get_registers(); + regs.eflags &= ~(TRAP_FLAG); + set_registers(regs); + return (void*)regs.eip; +} diff --git a/Applications/Debugger/DebugSession.h b/Applications/Debugger/DebugSession.h index 2b683c6ecd..43488754c5 100644 --- a/Applications/Debugger/DebugSession.h +++ b/Applications/Debugger/DebugSession.h @@ -53,19 +53,26 @@ public: bool poke(u32* address, u32 data); Optional peek(u32* address) const; + enum class BreakPointState { + Enabled, + Disabled, + }; + struct BreakPoint { void* address; u32 original_first_word; + BreakPointState state; }; bool insert_breakpoint(void* address); - void remove_breakpoint(const BreakPoint&); - Optional get_matching_breakpoint(const PtraceRegisters&) const; + bool disable_breakpoint(const BreakPoint&); + bool enable_breakpoint(const BreakPoint&); PtraceRegisters get_registers() const; void set_registers(const PtraceRegisters&); void continue_debugee(); + void* single_step(); template void run(Callback callback); @@ -100,40 +107,57 @@ private: template void DebugSession::run(Callback callback) { + bool in_consecutive_breakpoint = false; for (;;) { - continue_debugee(); + if (!in_consecutive_breakpoint) { + continue_debugee(); - int wstatus = 0; - if (waitpid(m_debugee_pid, &wstatus, WSTOPPED | WEXITED) != m_debugee_pid) { - perror("waitpid"); - ASSERT_NOT_REACHED(); - } + int wstatus = 0; + if (waitpid(m_debugee_pid, &wstatus, WSTOPPED | WEXITED) != m_debugee_pid) { + perror("waitpid"); + ASSERT_NOT_REACHED(); + } - // FIXME: This check actually only checks whether the debugee - // Is stopped because it hit a breakpoint or not - if (WSTOPSIG(wstatus) != SIGTRAP) { - callback(DebugBreakReason::Exited, Optional()); - m_is_debugee_dead = true; - break; + // FIXME: This check actually only checks whether the debugee + // Is stopped because it hit a breakpoint or not + if (WSTOPSIG(wstatus) != SIGTRAP) { + callback(DebugBreakReason::Exited, Optional()); + m_is_debugee_dead = true; + break; + } } auto regs = get_registers(); + Optional current_breakpoint; - auto current_breakpoint = get_matching_breakpoint(regs); - if (current_breakpoint.has_value()) { - // FIXME: The current implementation removes a breakpoint - // after the first time it has been triggered. - remove_breakpoint(current_breakpoint.value()); - - // We need to re-execute the instruction we patched for the breakpoint, - // so we rollback the instruction pointer to the breakpoint's address - regs.eip = reinterpret_cast(current_breakpoint.value().address); - set_registers(regs); - DebugDecision decision = callback(DebugBreakReason::Breakpoint, regs); - if (decision != DebugDecision::Continue) { - // FIXME: implement detach & kill - ASSERT_NOT_REACHED(); - } + if (in_consecutive_breakpoint) { + current_breakpoint = m_breakpoints.get((void*)regs.eip); + } else { + current_breakpoint = m_breakpoints.get((void*)((u32)regs.eip - 1)); } + + ASSERT(current_breakpoint.has_value()); + + // We want to make the breakpoint transparrent to the user of the debugger + + regs.eip = reinterpret_cast(current_breakpoint.value().address); + set_registers(regs); + disable_breakpoint(current_breakpoint.value()); + + DebugDecision decision = callback(DebugBreakReason::Breakpoint, regs); + if (decision != DebugDecision::Continue) { + // FIXME: implement detach & kill + ASSERT_NOT_REACHED(); + } + + // Re-enable the breakpoint + auto stopped_address = single_step(); + enable_breakpoint(current_breakpoint.value()); + + // If there is another breakpoint after the current one, + // Then we are already on it (because of single_step) + auto breakpoint_at_next_instruction = m_breakpoints.get(stopped_address); + in_consecutive_breakpoint = breakpoint_at_next_instruction.has_value() + && breakpoint_at_next_instruction.value().state == BreakPointState::Enabled; } }