Fix lldb-dap non-leaf frame source resolution issue #165944
Conversation
|
@llvm/pr-subscribers-lldb Author: None (jeffreytan81) ChangesSummaryWhile dogfooding lldb-dap, I observed that VSCode frequently displays certain stack frames as greyed out. Although these frames have valid debug information, double-clicking them shows disassembly instead of source code. However, running Root CauseInvestigation revealed that The correct approach is to use the symbol context's line entry, which LLDB resolves by effectively checking PC-1 for non-leaf frames, properly identifying the line information for the call instruction rather than the return address. TestingManually tested with VSCode debugging sessions on production workloads. Verified that non-leaf frames now correctly display source code instead of disassembly view. Before the change symptom: And here is after the fix: Full diff: https://github.com/llvm/llvm-project/pull/165944.diff 3 Files Affected:
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index f009a902f79e7..ebafda95ff583 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -657,18 +657,31 @@ std::optional<protocol::Source> DAP::ResolveSource(const lldb::SBFrame &frame) {
if (!frame.IsValid())
return std::nullopt;
- const lldb::SBAddress frame_pc = frame.GetPCAddress();
- if (DisplayAssemblySource(debugger, frame_pc))
+ // IMPORTANT: Get line entry from symbol context, NOT from PC address.
+ // When a frame's PC points to a return address (the instruction
+ // after a call), that address may have line number 0 for compiler generated
+ // code.
+ //
+ // EXAMPLE: If PC is at 0x1004 (frame return address after the call
+ // instruction) with no line info, but 0x1003 (in the middle of previous call
+ // instruction) is at line 42, symbol context returns line 42.
+ //
+ // NOTE: This issue is non-deterministic and depends on compiler debug info
+ // generation, making it difficult to create a reliable automated test.
+ const lldb::SBLineEntry frame_line_entry = frame.GetLineEntry();
+ if (DisplayAssemblySource(debugger, frame_line_entry)) {
+ const lldb::SBAddress frame_pc = frame.GetPCAddress();
return ResolveAssemblySource(frame_pc);
+ }
- return CreateSource(frame.GetLineEntry().GetFileSpec());
+ return CreateSource(frame_line_entry.GetFileSpec());
}
std::optional<protocol::Source> DAP::ResolveSource(lldb::SBAddress address) {
- if (DisplayAssemblySource(debugger, address))
+ lldb::SBLineEntry line_entry = GetLineEntryForAddress(target, address);
+ if (DisplayAssemblySource(debugger, line_entry))
return ResolveAssemblySource(address);
- lldb::SBLineEntry line_entry = GetLineEntryForAddress(target, address);
if (!line_entry.IsValid())
return std::nullopt;
diff --git a/lldb/tools/lldb-dap/ProtocolUtils.cpp b/lldb/tools/lldb-dap/ProtocolUtils.cpp
index 868c67ca72986..acf31b03f7af0 100644
--- a/lldb/tools/lldb-dap/ProtocolUtils.cpp
+++ b/lldb/tools/lldb-dap/ProtocolUtils.cpp
@@ -27,7 +27,7 @@ using namespace lldb_dap::protocol;
namespace lldb_dap {
static bool ShouldDisplayAssemblySource(
- lldb::SBAddress address,
+ lldb::SBLineEntry line_entry,
lldb::StopDisassemblyType stop_disassembly_display) {
if (stop_disassembly_display == lldb::eStopDisassemblyTypeNever)
return false;
@@ -37,7 +37,6 @@ static bool ShouldDisplayAssemblySource(
// A line entry of 0 indicates the line is compiler generated i.e. no source
// file is associated with the frame.
- auto line_entry = address.GetLineEntry();
auto file_spec = line_entry.GetFileSpec();
if (!file_spec.IsValid() || line_entry.GetLine() == 0 ||
line_entry.GetLine() == LLDB_INVALID_LINE_NUMBER)
@@ -174,10 +173,10 @@ bool IsAssemblySource(const protocol::Source &source) {
}
bool DisplayAssemblySource(lldb::SBDebugger &debugger,
- lldb::SBAddress address) {
+ lldb::SBLineEntry line_entry) {
const lldb::StopDisassemblyType stop_disassembly_display =
GetStopDisassemblyDisplay(debugger);
- return ShouldDisplayAssemblySource(address, stop_disassembly_display);
+ return ShouldDisplayAssemblySource(line_entry, stop_disassembly_display);
}
std::string GetLoadAddressString(const lldb::addr_t addr) {
diff --git a/lldb/tools/lldb-dap/ProtocolUtils.h b/lldb/tools/lldb-dap/ProtocolUtils.h
index a1f7ae0661914..f4d576ba9f608 100644
--- a/lldb/tools/lldb-dap/ProtocolUtils.h
+++ b/lldb/tools/lldb-dap/ProtocolUtils.h
@@ -53,7 +53,8 @@ std::optional<protocol::Source> CreateSource(const lldb::SBFileSpec &file);
/// Checks if the given source is for assembly code.
bool IsAssemblySource(const protocol::Source &source);
-bool DisplayAssemblySource(lldb::SBDebugger &debugger, lldb::SBAddress address);
+bool DisplayAssemblySource(lldb::SBDebugger &debugger,
+ lldb::SBLineEntry line_entry);
/// Get the address as a 16-digit hex string, e.g. "0x0000000000012345"
std::string GetLoadAddressString(const lldb::addr_t addr);
|
|
Thanks. // filename test.cpp
bool foo();
void bar() {
int val = 32; // breakpoint here
}
int main(int argc, char const *argv[]) {
auto val = foo();
return 0;
}
bool foo() {
bar();
#line 0 "test.cpp"
return true;
}
|
|
You can put the test case in https://github.com/llvm/llvm-project/blob/main/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py |
|
@da-viper, thanks for sharing the idea! I will add the test. My only concern is that the source line after the |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
jeffreytan81
force-pushed
the
fix_dap_frame_resolve_source
branch
3 times, most recently
from
9fdc072 to
7b59d42
Compare
I do not think in this test case the compiler will emit any code after the The tests are run without optimisation by default. the bool foo() { // line 1
bar(); // line 2
#line 0 "test.cpp"
return true; // line 0
} // line 1
// line 2
int main () { // line 3
return 0; // line 4
} // line 5that's why I forward declared |
| int foo() { | ||
| bar(); // foo call bar | ||
| #line 0 "test.cpp" | ||
| return 0; |
There was a problem hiding this comment.
I would suggest using a function call for more predictable code generation
void at_line_zero() {}
int foo() {
bar(); // foo call bar
#line 0 "test.cpp"
at_line_zero()
7b59d42 to
6f7f37f
Compare
|
|
||
| void at_line_zero() {} | ||
|
|
||
| int foo() { |
There was a problem hiding this comment.
please, forward declare foo() and move the definition to bottom of the file so the line directive does not affect the main() function
|
|
||
|
|
||
| class TestDAP_stackTraceCompilerGeneratedCode(lldbdap_testcase.DAPTestCaseBase): | ||
| @skipIfWindows |
There was a problem hiding this comment.
| @skipIfWindows |
No reason to skip window as it is not platform specific
There was a problem hiding this comment.
The reason is that all other TestDAP_stackTraceXXX tests are using skipIfWindows so it probably safer to follow the pattern. I can try to remove this, but I do not have a Windows machine to verify so if windows test bots failed, I will probably have to add skipIfWindows back to unblock.
| # sourceReference to retrieve disassembly source file. | ||
| # Verify that this didn't happen - the path should be a real file path. | ||
| foo_path = foo_frame.get("source", {}).get("path") | ||
| self.assertNotIn("`", foo_path, "Expected foo source path to not contain `") |
There was a problem hiding this comment.
| self.assertNotIn("`", foo_path, "Expected foo source path to not contain `") | |
| self.assertNotIn("`", foo_path, "Expected foo source path to not contain `") | |
| self.continue_to_exit() |
lldb/tools/lldb-dap/DAP.cpp
Outdated
| // IMPORTANT: Get line entry from symbol context, NOT from PC address. | ||
| // When a frame's PC points to a return address (the instruction | ||
| // after a call), that address may have line number 0 for compiler generated | ||
| // code. | ||
| // | ||
| // EXAMPLE: If PC is at 0x1004 (frame return address after the call | ||
| // instruction) with no line info, but 0x1003 (in the middle of previous call | ||
| // instruction) is at line 42, symbol context returns line 42. | ||
|
|
There was a problem hiding this comment.
| // IMPORTANT: Get line entry from symbol context, NOT from PC address. | |
| // When a frame's PC points to a return address (the instruction | |
| // after a call), that address may have line number 0 for compiler generated | |
| // code. | |
| // | |
| // EXAMPLE: If PC is at 0x1004 (frame return address after the call | |
| // instruction) with no line info, but 0x1003 (in the middle of previous call | |
| // instruction) is at line 42, symbol context returns line 42. |
we could probably remove this as we now have a test case covering it.
|
✅ With the latest revision this PR passed the Python code formatter. |
jeffreytan81
force-pushed
the
fix_dap_frame_resolve_source
branch
from
7d07981 to
de0fac2
Compare
jeffreytan81
force-pushed
the
fix_dap_frame_resolve_source
branch
from
7e8e28c to
c5983ac
Compare
jeffreytan81
force-pushed
the
fix_dap_frame_resolve_source
branch
from
c5983ac to
3b56010
Compare
4 participants
Summary
While dogfooding lldb-dap, I observed that VSCode frequently displays certain stack frames as greyed out. Although these frames have valid debug information, double-clicking them shows disassembly instead of source code. However, running
btfrom the LLDB command line correctly displays source file and line information for these same frames, indicating this is an lldb-dap specific issue.Root Cause
Investigation revealed that
DAP::ResolveSource()incorrectly uses a frame's PC address directly to determine whether valid source line information exists. This approach works for leaf frames, but fails for non-leaf (caller) frames where the PC points to the return address immediately after a call instruction. This return address may fall into compiler-generated code with no associated line information, even though the actual call site has valid source location data.The correct approach is to use the symbol context's line entry, which LLDB resolves by effectively checking PC-1 for non-leaf frames, properly identifying the line information for the call instruction rather than the return address.
Testing
Manually tested with VSCode debugging sessions on production workloads. Verified that non-leaf frames now correctly display source code instead of disassembly view.
Before the change symptom:
And here is after the fix:
