HackStudio: Implement "Step Over" debugging action

The "Step Over" action continues execution without stepping into
instructions in subsequent function calls.

DevTools/HackStudio/Debugger/Debugger.cpp

@@ -205,6 +205,7 @@ bool Debugger::DebuggingState::should_stop_single_stepping(const DebugInfo::Sour
 void Debugger::remove_temporary_breakpoints()
 {
     for (auto breakpoint_address : m_state.temporary_breakpoints()) {
+        ASSERT(m_debug_session->breakpoint_exists((void*)breakpoint_address));
         bool rc = m_debug_session->remove_breakpoint((void*)breakpoint_address);
         ASSERT(rc);
     }
@@ -221,18 +222,41 @@ void Debugger::DebuggingState::add_temporary_breakpoint(u32 address)
 }
 
 void Debugger::do_step_out(const PtraceRegisters& regs)
+{
+    // To step out, we simply insert a temporary breakpoint at the
+    // instruction the current function returns to, and continue
+    // execution until we hit that instruction (or some other breakpoint).
+    insert_temporary_breakpoint_at_return_address(regs);
+}
+
+void Debugger::do_step_over(const PtraceRegisters& regs)
+{
+    // To step over, we insert a temporary breakpoint at each line in the current function,
+    // as well as at the current function's return point, and continue execution.
+    auto current_function = m_debug_session->debug_info().get_containing_function(regs.eip);
+    ASSERT(current_function.has_value());
+    auto lines_in_current_function = m_debug_session->debug_info().source_lines_in_scope(current_function.value());
+    for (const auto& line : lines_in_current_function) {
+        insert_temporary_breakpoint(line.address_of_first_statement);
+    }
+    insert_temporary_breakpoint_at_return_address(regs);
+}
+
+void Debugger::insert_temporary_breakpoint_at_return_address(const PtraceRegisters& regs)
 {
     auto frame_info = StackFrameUtils::get_info(*m_debug_session, regs.ebp);
     ASSERT(frame_info.has_value());
     u32 return_address = frame_info.value().return_address;
-    bool success = m_debug_session->insert_breakpoint(reinterpret_cast<void*>(return_address));
+    insert_temporary_breakpoint(return_address);
-    ASSERT(success);
-    m_state.add_temporary_breakpoint(return_address);
 }
 
-void Debugger::do_step_over(const PtraceRegisters&)
+void Debugger::insert_temporary_breakpoint(FlatPtr address)
 {
-    // TODO: Implement
+    if (m_debug_session->breakpoint_exists((void*)address))
+        return;
+    bool success = m_debug_session->insert_breakpoint(reinterpret_cast<void*>(address));
+    ASSERT(success);
+    m_state.add_temporary_breakpoint(address);
 }
 
 }

DevTools/HackStudio/Debugger/Debugger.h

@@ -113,6 +113,8 @@ private:
     void remove_temporary_breakpoints();
     void do_step_out(const PtraceRegisters&);
     void do_step_over(const PtraceRegisters&);
+    void insert_temporary_breakpoint(FlatPtr address);
+    void insert_temporary_breakpoint_at_return_address(const PtraceRegisters&);
 
     OwnPtr<DebugSession> m_debug_session;
     DebuggingState m_state;

DevTools/HackStudio/main.cpp

@@ -614,8 +614,6 @@ int main_impl(int argc, char** argv)
     RefPtr<EditorWrapper> current_editor_in_execution;
     Debugger::initialize(
         [&](const PtraceRegisters& regs) {
-            dbg() << "Program stopped";
-
             ASSERT(Debugger::the().session());
             const auto& debug_session = *Debugger::the().session();
             auto source_position = debug_session.debug_info().get_source_position(regs.eip);
@@ -639,7 +637,6 @@ int main_impl(int argc, char** argv)
             return Debugger::HasControlPassedToUser::Yes;
         },
         [&]() {
-            dbg() << "Program continued";
             Core::EventLoop::main().post_event(*g_window, make<Core::DeferredInvocationEvent>([&](auto&) {
                 debug_info_widget.set_debug_actions_enabled(false);
                 if (current_editor_in_execution) {
@@ -649,7 +646,6 @@ int main_impl(int argc, char** argv)
             Core::EventLoop::wake();
         },
         [&]() {
-            dbg() << "Program exited";
             Core::EventLoop::main().post_event(*g_window, make<Core::DeferredInvocationEvent>([&](auto&) {
                 debug_info_widget.program_stopped();
                 hide_action_tabs();

Libraries/LibDebug/DebugInfo.cpp

@@ -138,7 +138,7 @@ Optional<DebugInfo::SourcePosition> DebugInfo::get_source_position(u32 target_ad
     // TODO: We can do a binray search here
     for (size_t i = 0; i < m_sorted_lines.size() - 1; ++i) {
         if (m_sorted_lines[i + 1].address > target_address) {
-            return Optional<SourcePosition>({ m_sorted_lines[i].file, m_sorted_lines[i].line, m_sorted_lines[i].address });
+            return SourcePosition::from_line_info(m_sorted_lines[i]);
         }
     }
     return {};
@@ -300,11 +300,38 @@ OwnPtr<DebugInfo::VariableInfo> DebugInfo::create_variable_info(const Dwarf::DIE
 }
 
 String DebugInfo::name_of_containing_function(u32 address) const
+{
+    auto function = get_containing_function(address);
+    if (!function.has_value())
+        return {};
+    return function.value().name;
+}
+
+Optional<DebugInfo::VariablesScope> DebugInfo::get_containing_function(u32 address) const
 {
     for (const auto& scope : m_scopes) {
         if (!scope.is_function || address < scope.address_low || address >= scope.address_high)
             continue;
-        return scope.name;
+        return scope;
     }
     return {};
 }
+
+Vector<DebugInfo::SourcePosition> DebugInfo::source_lines_in_scope(const VariablesScope& scope) const
+{
+    Vector<DebugInfo::SourcePosition> source_lines;
+    for (const auto& line : m_sorted_lines) {
+        if (line.address < scope.address_low)
+            continue;
+
+        if (line.address >= scope.address_high)
+            break;
+        source_lines.append(SourcePosition::from_line_info(line));
+    }
+    return source_lines;
+}
+
+DebugInfo::SourcePosition DebugInfo::SourcePosition::from_line_info(const LineProgram::LineInfo& line)
+{
+    return { line.file, line.line, line.address };
+}

Libraries/LibDebug/DebugInfo.h

@@ -47,6 +47,8 @@ public:
 
         bool operator==(const SourcePosition& other) const { return file_path == other.file_path && line_number == other.line_number; }
         bool operator!=(const SourcePosition& other) const { return !(*this == other); }
+
+        static SourcePosition from_line_info(const LineProgram::LineInfo&);
     };
 
     struct VariableInfo {
@@ -80,7 +82,7 @@ public:
         bool is_function { false };
         String name;
         u32 address_low { 0 };
-        u32 address_high { 0 };
+        u32 address_high { 0 }; // Non-inclusive - the lowest address after address_low that's not in this scope
         Vector<Dwarf::DIE> dies_of_variables;
     };
 
@@ -104,6 +106,8 @@ public:
     }
 
     String name_of_containing_function(u32 address) const;
+    Vector<SourcePosition> source_lines_in_scope(const VariablesScope&) const;
+    Optional<VariablesScope> get_containing_function(u32 address) const;
 
 private:
     void prepare_variable_scopes();

Libraries/LibDebug/DebugSession.cpp

@@ -175,6 +175,8 @@ bool DebugSession::enable_breakpoint(void* address)
     auto breakpoint = m_breakpoints.get(address);
     ASSERT(breakpoint.has_value());
 
+    ASSERT(breakpoint.value().state == BreakPointState::Disabled);
+
     if (!poke(reinterpret_cast<u32*>(breakpoint.value().address), (breakpoint.value().original_first_word & ~(uint32_t)0xff) | BREAKPOINT_INSTRUCTION))
         return false;
 

Libraries/LibDebug/DebugSession.h

@@ -207,10 +207,17 @@ void DebugSession::run(Callback callback)
 
         // Re-enable the breakpoint if it wasn't removed by the user
         if (current_breakpoint.has_value() && m_breakpoints.contains(current_breakpoint.value().address)) {
-            // The current breakpoint was removed in order to make it transparrent to the user.
+            // The current breakpoint was removed to make it transparrent to the user.
             // We now want to re-enable it - the code execution flow could hit it again.
             // To re-enable the breakpoint, we first perform a single step and execute the
             // instruction of the breakpoint, and then redo the INT3 patch in its first byte.
+
+            // If the user manually inserted a breakpoint at were we breaked at originally,
+            // we need to disable that breakpoint because we want to singlestep over it to execute the
+            // instruction we breaked on (we re-enable it again later anyways).
+            if (m_breakpoints.contains(current_breakpoint.value().address) && m_breakpoints.get(current_breakpoint.value().address).value().state == BreakPointState::Enabled) {
+                disable_breakpoint(current_breakpoint.value().address);
+            }
             auto stopped_address = single_step();
             enable_breakpoint(current_breakpoint.value().address);
             did_single_step = true;