[lldb-dap] Add 'source' references to stack frames without source files.

[ashgti]

This adds 'source' references to all stack frames. When opening a stack frame users will see the disassembly of the frame if the source is not available.

This works around the odd behavior of navigating frames without the VSCode disassembly view open, which causes 'step' to step in the first frame with a source instead of the active frame.

This fixes #128260

Old behavior:
https://github.com/user-attachments/assets/3f40582d-ac96-451a-a5ae-498a323bf30e

New behavior:
https://github.com/user-attachments/assets/3a3f9ac6-3e6c-4795-9bb2-1132b3916b6f

[llvmbot]

@llvm/pr-subscribers-lldb

Author: John Harrison (ashgti)

Changes

This adds 'source' references to all stack frames. When opening a stack frame users will see the disassembly of the frame if the source is not available.

This works around the odd behavior of navigating frames without the VSCode disassembly view open, which causes 'step' to step in the first frame with a source instead of the active frame.

This fixes #128260

Old behavior:
https://github.com/user-attachments/assets/3f40582d-ac96-451a-a5ae-498a323bf30e

New behavior:
https://github.com/user-attachments/assets/3a3f9ac6-3e6c-4795-9bb2-1132b3916b6f

---

Full diff: https://github.com/llvm/llvm-project/pull/128268.diff

2 Files Affected:

• (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+23-15)
• (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+25-3)

diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 6ca4dfb4711a1..ee8fcef6f2503 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -45,7 +45,6 @@
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 #include <chrono>
-#include <climits>
 #include <cstddef>
 #include <iomanip>
 #include <optional>
@@ -698,14 +697,22 @@ llvm::json::Value CreateSource(llvm::StringRef source_path) {
   return llvm::json::Value(std::move(source));
 }
 
-static std::optional<llvm::json::Value> CreateSource(lldb::SBFrame &frame) {
+static llvm::json::Value CreateSource(lldb::SBFrame &frame,
+                                      llvm::StringRef frame_name) {
   auto line_entry = frame.GetLineEntry();
   // A line entry of 0 indicates the line is compiler generated i.e. no source
   // file is associated with the frame.
   if (line_entry.GetFileSpec().IsValid() && line_entry.GetLine() != 0)
     return CreateSource(line_entry);
 
-  return {};
+  llvm::json::Object source;
+  EmplaceSafeString(source, "name", frame_name);
+  source.try_emplace("sourceReference", MakeDAPFrameID(frame));
+  // If we don't have a filespec then we don't have the original source. Mark
+  // the source as deemphasized since users will only be able to view assembly
+  // for these frames.
+  EmplaceSafeString(source, "presentationHint", "deemphasize");
+  return std::move(source);
 }
 
 // "StackFrame": {
@@ -799,21 +806,22 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
 
   EmplaceSafeString(object, "name", frame_name);
 
-  auto source = CreateSource(frame);
-
-  if (source) {
-    object.try_emplace("source", *source);
-    auto line_entry = frame.GetLineEntry();
-    auto line = line_entry.GetLine();
-    if (line && line != LLDB_INVALID_LINE_NUMBER)
-      object.try_emplace("line", line);
-    else
-      object.try_emplace("line", 0);
+  object.try_emplace("source", CreateSource(frame, frame_name));
+  auto line_entry = frame.GetLineEntry();
+  if (line_entry.IsValid() &&
+      (line_entry.GetLine() != 0 ||
+       line_entry.GetLine() != LLDB_INVALID_LINE_NUMBER)) {
+    object.try_emplace("line", line_entry.GetLine());
     auto column = line_entry.GetColumn();
     object.try_emplace("column", column);
   } else {
-    object.try_emplace("line", 0);
-    object.try_emplace("column", 0);
+    lldb::addr_t inst_offset = frame.GetPCAddress().GetOffset() -
+                               frame.GetSymbol().GetStartAddress().GetOffset();
+    lldb::addr_t inst_line =
+        inst_offset / (frame.GetThread().GetProcess().GetAddressByteSize() / 2);
+    // lines are base-1 indexed
+    object.try_emplace("line", inst_line + 1);
+    object.try_emplace("column", 1);
   }
 
   const auto pc = frame.GetPC();
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index e323990d8b6ed..1da8358eb5224 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -13,7 +13,9 @@
 #include "RunInTerminal.h"
 #include "Watchpoint.h"
 #include "lldb/API/SBDeclaration.h"
+#include "lldb/API/SBDefines.h"
 #include "lldb/API/SBEvent.h"
+#include "lldb/API/SBFile.h"
 #include "lldb/API/SBInstruction.h"
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
@@ -38,9 +40,6 @@
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <array>
-#include <cassert>
-#include <climits>
-#include <cstdarg>
 #include <cstdint>
 #include <cstdio>
 #include <cstdlib>
@@ -3488,6 +3487,29 @@ void request_source(DAP &dap, const llvm::json::Object &request) {
   llvm::json::Object response;
   FillResponse(request, response);
   llvm::json::Object body{{"content", ""}};
+
+  const auto *arguments = request.getObject("arguments");
+  const auto *source = arguments->getObject("source");
+  int64_t source_ref = GetUnsigned(
+      source, "sourceReference", GetUnsigned(arguments, "sourceReference", 0));
+
+  lldb::SBProcess process = dap.target.GetProcess();
+  // Upper 32 bits is the thread index ID
+  lldb::SBThread thread =
+      process.GetThreadByIndexID(GetLLDBThreadIndexID(source_ref));
+  // Lower 32 bits is the frame index
+  lldb::SBFrame frame = thread.GetFrameAtIndex(GetLLDBFrameID(source_ref));
+  if (!frame.IsValid()) {
+    response["success"] = false;
+    response["message"] = "source not found";
+  } else {
+    lldb::SBInstructionList insts = frame.GetSymbol().GetInstructions(dap.target);
+    lldb::SBStream stream;
+    insts.GetDescription(stream);
+    body["content"] = stream.GetData();
+    body["mimeType"] = "text/x-lldb.disassembly";
+  }
+
   response.try_emplace("body", std::move(body));
   dap.SendJSON(llvm::json::Value(std::move(response)));
 }

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[JDevlieghere]

This will need a test.

[JDevlieghere]

I assume this function is used elsewhere? The line number and column have a different meaning based on whether we have source info and having the logic spread across this function and CreateStackFrame seems like a pretty big foot gun. Any chance we can localize this logic?

[ashgti]

Merged this into the only place that called the helper (CreateStackFrame) to make sure its more clear.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[labath]

Could we make the test self contained (i.e. without the qsort reference)? If all you need is a function without debug info, then __attribute__((nodebug)) should suffice.

[ashgti]

Could we make the test self contained (i.e. without the qsort reference)? If all you need is a function without debug info, then __attribute__((nodebug)) should suffice.

Thanks for the suggestion! That works a lot more consistently than depending on the implementation details of libc's qsort.

[JDevlieghere]

LGTM!

[ashgti]

Pulled and rebased my changes on the request handler refactor.

[JDevlieghere]

Totally optional & I didn't think about it earlier, but this seems like something you might want to advertise through a release note.