Skip to content

[lldb-dap] fix wrong assembly line number x64 #136486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions lldb/test/API/tools/lldb-dap/stackTrace-x86/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
C_SOURCES := main.c

include Makefile.rules
Original file line number Diff line number Diff line change
@@ -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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we explicitly step with granularity='instruction'?

Copy link
Contributor Author

@eronnen eronnen Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be the same because it's a function without debug symbols so each step should execute one assembly instruction

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}",
)
29 changes: 29 additions & 0 deletions lldb/test/API/tools/lldb-dap/stackTrace-x86/main.c
Original file line number Diff line number Diff line change
@@ -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;
}
10 changes: 6 additions & 4 deletions lldb/tools/lldb-dap/JSONUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Expand Down
Loading