From ce726fe027150a3aea8126d294183631b34114c9 Mon Sep 17 00:00:00 2001
From: Itamar <itamar8910@gmail.com>
Date: Fri, 19 Nov 2021 16:13:07 +0200
Subject: [PATCH] HackStudio: Improve backtrace accuracy

We now decrement the return address of the previous frame by one to get
the address of the call instruction and use this address in the
backtrace.

This results in more accurate source position information than what we
previously had when using the return address.
---
 .../HackStudio/Debugger/BacktraceModel.cpp      | 17 +++++++++++++----
 .../HackStudio/Debugger/BacktraceModel.h        |  5 +++--
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/Userland/DevTools/HackStudio/Debugger/BacktraceModel.cpp b/Userland/DevTools/HackStudio/Debugger/BacktraceModel.cpp
index 0440fc028e..0ae6152e44 100644
--- a/Userland/DevTools/HackStudio/Debugger/BacktraceModel.cpp
+++ b/Userland/DevTools/HackStudio/Debugger/BacktraceModel.cpp
@@ -36,21 +36,30 @@ Vector<BacktraceModel::FrameInfo> BacktraceModel::create_backtrace(Debug::Proces
     FlatPtr current_ebp = regs.bp();
     FlatPtr current_instruction = regs.ip();
     Vector<BacktraceModel::FrameInfo> frames;
+    size_t frame_index = 0;
     do {
-        auto lib = inspector.library_at(regs.ip());
+        auto lib = inspector.library_at(current_instruction);
         if (!lib)
             continue;
-        String name = lib->debug_info->name_of_containing_function(current_instruction - lib->base_address);
+
+        // 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_instruction;
+        String name = lib->debug_info->elf().symbolicate(current_instruction - lib->base_address);
         if (name.is_null()) {
-            dbgln("BacktraceModel: couldn't find containing function for address: {:p}", current_instruction);
+            dbgln("BacktraceModel: couldn't find containing function for address: {:p} (library={})", current_instruction, lib->name);
             name = "<missing>";
         }
 
-        frames.append({ name, current_instruction, current_ebp });
+        auto source_position = lib->debug_info->get_source_position(current_instruction - lib->base_address);
+
+        frames.append({ name, current_instruction, current_ebp, source_position });
         auto frame_info = Debug::StackFrameUtils::get_info(inspector, current_ebp);
         VERIFY(frame_info.has_value());
         current_instruction = frame_info.value().return_address;
         current_ebp = frame_info.value().next_ebp;
+        ++frame_index;
     } while (current_ebp && current_instruction);
     return frames;
 }
diff --git a/Userland/DevTools/HackStudio/Debugger/BacktraceModel.h b/Userland/DevTools/HackStudio/Debugger/BacktraceModel.h
index a335b3c58e..4eefb54a89 100644
--- a/Userland/DevTools/HackStudio/Debugger/BacktraceModel.h
+++ b/Userland/DevTools/HackStudio/Debugger/BacktraceModel.h
@@ -38,8 +38,9 @@ public:
 
     struct FrameInfo {
         String function_name;
-        FlatPtr instruction_address;
-        FlatPtr frame_base;
+        FlatPtr instruction_address { 0 };
+        FlatPtr frame_base { 0 };
+        Optional<Debug::DebugInfo::SourcePosition> m_source_position;
     };
 
     const Vector<FrameInfo>& frames() const { return m_frames; }