2 files changed, 98 insertions, 39 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<u32*>(address), (original_bytes.value() & ~(uint32_t)0xff) | BREAKPOINT_INSTRUCTION))
+    BreakPoint breakpoint { address, original_bytes.value(), BreakPointState::Disabled };
+
+    m_breakpoints.set(address, breakpoint);
+
+    enable_breakpoint(breakpoint);
+
+    return true;
+}
+
+bool DebugSession::disable_breakpoint(const BreakPoint& breakpoint)
+{
+    ASSERT(m_breakpoints.contains(breakpoint.address));
+    if (!poke(reinterpret_cast<u32*>(reinterpret_cast<char*>(breakpoint.address)), breakpoint.original_first_word))
         return false;
 
-    m_breakpoints.set(address, { address, original_bytes.value() });
+    auto bp = m_breakpoints.get(breakpoint.address).value();
+    bp.state = BreakPointState::Disabled;
+    m_breakpoints.set(bp.address, bp);
     return true;
 }
 
-void DebugSession::remove_breakpoint(const BreakPoint& breakpoint)
+bool DebugSession::enable_breakpoint(const BreakPoint& breakpoint)
 {
     ASSERT(m_breakpoints.contains(breakpoint.address));
-    poke(reinterpret_cast<u32*>(reinterpret_cast<char*>(breakpoint.address)), breakpoint.original_first_word);
-    m_breakpoints.remove(breakpoint.address);
+
+    if (!poke(reinterpret_cast<u32*>(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::BreakPoint> DebugSession::get_matching_breakpoint(const PtraceRegisters& regs) const
-{
-    return m_breakpoints.get(reinterpret_cast<void*>(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<u32> 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<BreakPoint> 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<typename Callback>
     void run(Callback callback);
@@ -100,40 +107,57 @@ private:
 template<typename Callback>
 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<PtraceRegisters>());
-            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<PtraceRegisters>());
+                m_is_debugee_dead = true;
+                break;
+            }
         }
 
         auto regs = get_registers();
+        Optional<BreakPoint> 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<u32>(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<u32>(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;
     }
 }