[Lldb-commits] [lldb] 480f1a4 - [lldb-dap] fix wrong assembly line number x64 (#136486)

[via lldb-commits]

Author: Ely Ronnen
Date: 2025-04-25T09:34:17-07:00
New Revision: 480f1a4980de4d1a3691329bbe0b5b9e4321a3f3

URL: https://github.com/llvm/llvm-project/commit/480f1a4980de4d1a3691329bbe0b5b9e4321a3f3
DIFF: https://github.com/llvm/llvm-project/commit/480f1a4980de4d1a3691329bbe0b5b9e4321a3f3.diff

LOG: [lldb-dap] fix wrong assembly line number x64 (#136486)

Fix wrong assembly line number calculation that assumes the instruction
size is `GetAddressByteSize() / 2`

Fixes #136495

Added: 
    lldb/test/API/tools/lldb-dap/stackTrace-x86/Makefile
    lldb/test/API/tools/lldb-dap/stackTrace-x86/TestDAP_source_x86.py
    lldb/test/API/tools/lldb-dap/stackTrace-x86/main.c

Modified: 
    lldb/tools/lldb-dap/JSONUtils.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/test/API/tools/lldb-dap/stackTrace-x86/Makefile b/lldb/test/API/tools/lldb-dap/stackTrace-x86/Makefile
new file mode 100644
index 0000000000000..10495940055b6
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/stackTrace-x86/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

diff  --git a/lldb/test/API/tools/lldb-dap/stackTrace-x86/TestDAP_source_x86.py b/lldb/test/API/tools/lldb-dap/stackTrace-x86/TestDAP_source_x86.py
new file mode 100644
index 0000000000000..6239e1af53ecd
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/stackTrace-x86/TestDAP_source_x86.py
@@ -0,0 +1,65 @@
+"""
+Test lldb-dap stack trace containing x86 assembly
+"""
+
+import lldbdap_testcase
+from lldbsuite.test import lldbplatformutil
+from lldbsuite.test.decorators import skipUnlessArch, skipUnlessPlatform
+from lldbsuite.test.lldbtest import line_number
+
+
+class TestDAP_stacktrace_x86(lldbdap_testcase.DAPTestCaseBase):
+    @skipUnlessArch("x86_64")
+    @skipUnlessPlatform(["linux"] + lldbplatformutil.getDarwinOSTriples())
+    def test_stacktrace_x86(self):
+        """
+        Tests that lldb-dap steps through correctly and the source lines are correct in x86 assembly.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(
+            program,
+            initCommands=[
+                "settings set target.process.thread.step-in-avoid-nodebug false"
+            ],
+        )
+
+        source = "main.c"
+        breakpoint_ids = self.set_source_breakpoints(
+            source,
+            [line_number(source, "// Break here")],
+        )
+        self.continue_to_breakpoints(breakpoint_ids)
+        self.stepIn()
+
+        frame = self.get_stackFrames()[0]
+        self.assertEqual(
+            frame["name"],
+            "no_branch_func",
+            "verify we are in the no_branch_func function",
+        )
+
+        self.assertEqual(frame["line"], 1, "verify we are at the start of the function")
+        minimum_assembly_lines = (
+            line_number(source, "Assembly end")
+            - line_number(source, "Assembly start")
+            + 1
+        )
+        self.assertLessEqual(
+            10,
+            minimum_assembly_lines,
+            "verify we have a reasonable number of assembly lines",
+        )
+
+        for i in range(2, minimum_assembly_lines):
+            self.stepIn()
+            frame = self.get_stackFrames()[0]
+            self.assertEqual(
+                frame["name"],
+                "no_branch_func",
+                "verify we are still in the no_branch_func function",
+            )
+            self.assertEqual(
+                frame["line"],
+                i,
+                f"step in should advance a single line in the function to {i}",
+            )

diff  --git a/lldb/test/API/tools/lldb-dap/stackTrace-x86/main.c b/lldb/test/API/tools/lldb-dap/stackTrace-x86/main.c
new file mode 100644
index 0000000000000..52ee3132d0df0
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/stackTrace-x86/main.c
@@ -0,0 +1,29 @@
+#include <stdio.h>
+
+__attribute__((nodebug)) int no_branch_func(void) {
+  int result = 0;
+
+  __asm__ __volatile__("movl $0, %%eax;" // Assembly start
+                       "incl %%eax;"
+                       "incl %%eax;"
+                       "incl %%eax;"
+                       "incl %%eax;"
+                       "incl %%eax;"
+                       "incl %%eax;"
+                       "incl %%eax;"
+                       "incl %%eax;"
+                       "incl %%eax;"
+                       "incl %%eax;"
+                       "movl %%eax, %0;" // Assembly end
+                       : "=r"(result)
+                       :
+                       : "%eax");
+
+  return result;
+}
+
+int main(void) {
+  int result = no_branch_func(); // Break here
+  printf("Result: %d\n", result);
+  return 0;
+}

diff  --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 621492a882bca..4b8e43cf23960 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -19,6 +19,7 @@
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBFrame.h"
 #include "lldb/API/SBFunction.h"
+#include "lldb/API/SBInstructionList.h"
 #include "lldb/API/SBLineEntry.h"
 #include "lldb/API/SBModule.h"
 #include "lldb/API/SBQueue.h"
@@ -776,10 +777,11 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
 
     // Calculate the line of the current PC from the start of the current
     // symbol.
-    lldb::addr_t inst_offset = frame.GetPCAddress().GetOffset() -
-                               frame.GetSymbol().GetStartAddress().GetOffset();
-    lldb::addr_t inst_line =
-        inst_offset / (frame.GetThread().GetProcess().GetAddressByteSize() / 2);
+    lldb::SBTarget target = frame.GetThread().GetProcess().GetTarget();
+    lldb::SBInstructionList inst_list = target.ReadInstructions(
+        frame.GetSymbol().GetStartAddress(), frame.GetPCAddress(), nullptr);
+    size_t inst_line = inst_list.GetSize();
+
     // Line numbers are 1-based.
     object.try_emplace("line", inst_line + 1);
     object.try_emplace("column", 1);