HackStudio: Use AK::unwind_stack_from_frame_pointer

Author: https://github.com/spholz
Commit: https://github.com/SerenityOS/serenity/commit/3059ac71f3
Pull-request: https://github.com/SerenityOS/serenity/pull/24292
Reviewed-by: https://github.com/ADKaster ✅

Userland/DevTools/HackStudio/Debugger/BacktraceModel.cpp

@@ -6,7 +6,7 @@
 
 #include "BacktraceModel.h"
 #include "Debugger.h"
-#include <LibDebug/StackFrameUtils.h>
+#include <AK/StackUnwinder.h>
 
 namespace HackStudio {
 
@@ -33,38 +33,47 @@ GUI::ModelIndex BacktraceModel::index(int row, int column, const GUI::ModelIndex
 
 Vector<BacktraceModel::FrameInfo> BacktraceModel::create_backtrace(Debug::ProcessInspector const& inspector, PtraceRegisters const& regs)
 {
-    FlatPtr current_frame_pointer = regs.bp();
-    FlatPtr current_program_counter = regs.ip();
     Vector<BacktraceModel::FrameInfo> frames;
-    size_t frame_index = 0;
-    do {
-        auto lib = inspector.library_at(current_program_counter);
+
+    auto add_frame = [&frames, &inspector](FlatPtr address, FlatPtr frame_pointer) {
+        auto const* lib = inspector.library_at(address);
 
         if (lib) {
-            // After the first frame, current_instruction holds the return address from the function call.
-            // We need to go back to the 'call' instruction to get accurate source position information.
-            if (frame_index > 0)
-                --current_program_counter;
-            ByteString name = lib->debug_info->elf().symbolicate(current_program_counter - lib->base_address);
+            ByteString name = lib->debug_info->elf().symbolicate(address - lib->base_address);
             if (name.is_empty()) {
-                dbgln("BacktraceModel: couldn't find containing function for address: {:p} (library={})", current_program_counter, lib->name);
+                dbgln("BacktraceModel: couldn't find containing function for address: {:p} (library={})", address, lib->name);
                 name = "<missing>";
             }
 
-            auto source_position = lib->debug_info->get_source_position(current_program_counter - lib->base_address);
+            auto source_position = lib->debug_info->get_source_position(address - lib->base_address);
 
-            frames.append({ name, current_program_counter, current_frame_pointer, source_position });
+            frames.append({ name, address, frame_pointer, source_position });
         } else {
-            dbgln("BacktraceModel: couldn't find containing library for address: {:p}", current_program_counter);
-            frames.append({ "<missing>", current_program_counter, current_frame_pointer, {} });
+            dbgln("BacktraceModel: couldn't find containing library for address: {:p}", address);
+            frames.append({ "<missing>", address, frame_pointer, {} });
         }
+    };
+
+    add_frame(regs.ip(), regs.bp());
+
+    MUST(AK::unwind_stack_from_frame_pointer(
+        regs.bp(),
+        [&](FlatPtr address) -> ErrorOr<FlatPtr> {
+            auto maybe_value = inspector.peek(address);
+            if (!maybe_value.has_value())
+                return EFAULT;
+
+            return maybe_value.value();
+        },
+        [&add_frame](AK::StackFrame stack_frame) -> ErrorOr<IterationDecision> {
+            // Subtract one from return_address to go back to the calling instruction to get accurate source position information.
+            auto address = stack_frame.return_address - 1;
+
+            add_frame(address, stack_frame.previous_frame_pointer);
+
+            return IterationDecision::Continue;
+        }));
 
-        auto frame_info = Debug::StackFrameUtils::get_info(inspector, current_frame_pointer);
-        VERIFY(frame_info.has_value());
-        current_program_counter = frame_info.value().return_address;
-        current_frame_pointer = frame_info.value().next_ebp;
-        ++frame_index;
-    } while (current_frame_pointer && current_program_counter);
     return frames;
 }
 

Userland/DevTools/HackStudio/Debugger/Debugger.cpp

@@ -6,7 +6,7 @@
  */
 
 #include "Debugger.h"
-#include <LibDebug/StackFrameUtils.h>
+#include <AK/StackUnwinder.h>
 
 namespace HackStudio {
 
@@ -313,10 +313,23 @@ void Debugger::do_step_over(PtraceRegisters const& regs)
 
 void Debugger::insert_temporary_breakpoint_at_return_address(PtraceRegisters const& regs)
 {
-    auto frame_info = Debug::StackFrameUtils::get_info(*m_debug_session, regs.bp());
-    VERIFY(frame_info.has_value());
-    FlatPtr return_address = frame_info.value().return_address;
-    insert_temporary_breakpoint(return_address);
+    Optional<FlatPtr> return_address;
+    MUST(AK::unwind_stack_from_frame_pointer(
+        regs.bp(),
+        [this](FlatPtr address) -> ErrorOr<FlatPtr> {
+            auto maybe_value = m_debug_session->peek(address);
+            if (!maybe_value.has_value())
+                return EFAULT;
+
+            return maybe_value.value();
+        },
+        [&return_address](AK::StackFrame stack_frame) -> ErrorOr<IterationDecision> {
+            return_address = stack_frame.return_address;
+            return IterationDecision::Break;
+        }));
+
+    VERIFY(return_address.has_value());
+    insert_temporary_breakpoint(return_address.value());
 }
 
 void Debugger::insert_temporary_breakpoint(FlatPtr address)

Userland/Libraries/LibDebug/CMakeLists.txt

@@ -9,7 +9,6 @@ set(SOURCES
     Dwarf/DwarfInfo.cpp
     Dwarf/LineProgram.cpp
     ProcessInspector.cpp
-    StackFrameUtils.cpp
 )
 
 serenity_lib(LibDebug debug)

Userland/Libraries/LibDebug/DebugSession.h

@@ -227,6 +227,7 @@ void DebugSession::run(DesiredInitialDebugeeState initial_debugee_state, Callbac
 #    error Unknown architecture
 #endif
 
+                // FIXME: Use AK::unwind_stack_from_frame_pointer
                 do {
                     if (current_ebp == required_ebp) {
                         found_ebp = true;

Userland/Libraries/LibDebug/StackFrameUtils.cpp

@@ -1,22 +0,0 @@
-/*
- * Copyright (c) 2020, Itamar S. <itamar8910@gmail.com>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#include "StackFrameUtils.h"
-
-namespace Debug::StackFrameUtils {
-
-Optional<StackFrameInfo> get_info(ProcessInspector const& inspector, FlatPtr current_ebp)
-{
-    auto return_address = inspector.peek(current_ebp + sizeof(FlatPtr));
-    auto next_ebp = inspector.peek(current_ebp);
-    if (!return_address.has_value() || !next_ebp.has_value())
-        return {};
-
-    StackFrameInfo info = { return_address.value(), next_ebp.value() };
-    return info;
-}
-
-}

Userland/Libraries/LibDebug/StackFrameUtils.h

@@ -1,22 +0,0 @@
-/*
- * Copyright (c) 2020, Itamar S. <itamar8910@gmail.com>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#pragma once
-
-#include <AK/Optional.h>
-#include <AK/Types.h>
-#include <LibDebug/DebugSession.h>
-
-namespace Debug::StackFrameUtils {
-
-struct StackFrameInfo {
-    FlatPtr return_address;
-    FlatPtr next_ebp;
-};
-
-Optional<StackFrameInfo> get_info(ProcessInspector const&, FlatPtr current_ebp);
-
-}