[Lldb-commits] [lldb] [lldb-dap] Add 'source' references to stack frames without source files. (PR #128268)

John Harrison via lldb-commits lldb-commits at lists.llvm.org
Mon Feb 24 14:31:54 PST 2025


https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/128268

>From 4378be2df5da154420306bc65d95a4b714b05fae Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Fri, 21 Feb 2025 17:45:17 -0800
Subject: [PATCH 1/8] [lldb-dap] Add 'source' references to stack frames
 without source files.

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.
---
 .../lldb-dap/Handler/SourceRequestHandler.cpp | 41 ++++++++++++++++++-
 lldb/tools/lldb-dap/JSONUtils.cpp             | 38 ++++++++++-------
 2 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp
index 03561c9e64922..7a1ef7f33b6fb 100644
--- a/lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp
@@ -9,7 +9,15 @@
 #include "DAP.h"
 #include "EventHelper.h"
 #include "JSONUtils.h"
+#include "LLDBUtils.h"
 #include "RequestHandler.h"
+#include "lldb/API/SBFrame.h"
+#include "lldb/API/SBInstructionList.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/API/SBStream.h"
+#include "lldb/API/SBTarget.h"
+#include "lldb/API/SBThread.h"
+#include "llvm/Support/JSON.h"
 
 namespace lldb_dap {
 
@@ -74,8 +82,37 @@ namespace lldb_dap {
 void SourceRequestHandler::operator()(const llvm::json::Object &request) {
   llvm::json::Object response;
   FillResponse(request, response);
-  llvm::json::Object body{{"content", ""}};
-  response.try_emplace("body", std::move(body));
+  const auto *arguments = request.getObject("arguments");
+  const auto *source = arguments->getObject("source");
+  llvm::json::Object body;
+  int64_t source_ref = GetUnsigned(
+      source, "sourceReference", GetUnsigned(arguments, "sourceReference", 0));
+
+  if (source_ref) {
+    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));
+    }
+  } else {
+    response["success"] = false;
+    response["message"] =
+        "invalid arguments, expected source.sourceReference to be set";
+  }
+
   dap.SendJSON(llvm::json::Value(std::move(response)));
 }
 
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();

>From fad22ceb10f6e01505fc54f8da31119945d83079 Mon Sep 17 00:00:00 2001
From: John Harrison <ash at greaterthaninfinity.com>
Date: Sat, 22 Feb 2025 13:18:18 -0800
Subject: [PATCH 2/8] Apply suggestions from code review

Co-authored-by: Jonas Devlieghere <jonas at devlieghere.com>
---
 lldb/tools/lldb-dap/JSONUtils.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index ee8fcef6f2503..86e5852b9a6c9 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -819,7 +819,7 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
                                frame.GetSymbol().GetStartAddress().GetOffset();
     lldb::addr_t inst_line =
         inst_offset / (frame.GetThread().GetProcess().GetAddressByteSize() / 2);
-    // lines are base-1 indexed
+    // Line numbers are 1-based. 
     object.try_emplace("line", inst_line + 1);
     object.try_emplace("column", 1);
   }

>From aff8b1949f9cb0ceb78d505bad39ccc2b1e996bc Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Sat, 22 Feb 2025 15:16:24 -0800
Subject: [PATCH 3/8] Adding unit tests and included the 'path' on the frames
 with disassembly formatted like '<module>`<symbol>' for example
 '/usr/lib/system/libsystem_c.dylib`_qsort'.

---
 .../test/tools/lldb-dap/dap_server.py         | 13 +++
 lldb/test/API/tools/lldb-dap/source/Makefile  |  3 +
 .../tools/lldb-dap/source/TestDAP_source.py   | 86 +++++++++++++++++++
 lldb/test/API/tools/lldb-dap/source/main.c    | 20 +++++
 .../lldb-dap/stackTrace/TestDAP_stackTrace.py | 44 +++++++++-
 .../test/API/tools/lldb-dap/stackTrace/main.c | 15 ++++
 lldb/tools/lldb-dap/JSONUtils.cpp             | 46 +++++-----
 lldb/tools/lldb-dap/lldb-dap.cpp              |  1 +
 8 files changed, 204 insertions(+), 24 deletions(-)
 create mode 100644 lldb/test/API/tools/lldb-dap/source/Makefile
 create mode 100644 lldb/test/API/tools/lldb-dap/source/TestDAP_source.py
 create mode 100644 lldb/test/API/tools/lldb-dap/source/main.c

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 00e068d0954ea..391378cf027bc 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -1068,6 +1068,19 @@ def request_stackTrace(
                 print("[%3u] %s" % (idx, name))
         return response
 
+    def request_source(self, sourceReference):
+        """Request a source from a 'Source' reference."""
+        command_dict = {
+            "command": "source",
+            "type": "request",
+            "arguments": {
+                "source": {"sourceReference": sourceReference},
+                # legacy version of the request
+                "sourceReference": sourceReference,
+            },
+        }
+        return self.send_recv(command_dict)
+
     def request_threads(self):
         """Request a list of all threads and combine any information from any
         "stopped" events since those contain more information about why a
diff --git a/lldb/test/API/tools/lldb-dap/source/Makefile b/lldb/test/API/tools/lldb-dap/source/Makefile
new file mode 100644
index 0000000000000..10495940055b6
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/source/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/source/TestDAP_source.py b/lldb/test/API/tools/lldb-dap/source/TestDAP_source.py
new file mode 100644
index 0000000000000..28479ccd11ec9
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/source/TestDAP_source.py
@@ -0,0 +1,86 @@
+"""
+Test lldb-dap source request
+"""
+
+
+import os
+
+import lldbdap_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TestDAP_source(lldbdap_testcase.DAPTestCaseBase):
+    @skipIfWindows
+    def test_stackTrace(self):
+        """
+        Tests the 'source' packet.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.c"
+        self.source_path = os.path.join(os.getcwd(), source)
+        self.qsort_call = line_number(source, "qsort call")
+
+        lines = [self.qsort_call]
+        breakpoint_ids = self.set_source_breakpoints(source, lines)
+        self.assertEqual(
+            len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
+        )
+
+        self.continue_to_breakpoints(breakpoint_ids)
+
+        response = self.dap_server.request_source(sourceReference=0)
+        self.assertFalse(response["success"], "verify invalid sourceReference fails")
+
+        (stackFrames, totalFrames) = self.get_stackFrames_and_totalFramesCount()
+        frameCount = len(stackFrames)
+        self.assertGreaterEqual(
+            frameCount, 3, "verify we get frames from system librarys (libc qsort)"
+        )
+        self.assertEqual(
+            totalFrames,
+            frameCount,
+            "verify total frames returns a speculative page size",
+        )
+        expectedFrames = [
+            {
+                "name": "comp",
+                "line": 14,
+                "sourceName": "main.c",
+                "containsSourceReference": False,
+            },
+            {"name": "qsort", "sourceName": "qsort", "containsSourceReference": True},
+            {
+                "name": "main",
+                "line": 25,
+                "sourceName": "main.c",
+                "containsSourceReference": False,
+            },
+        ]
+        for idx, expected in enumerate(expectedFrames):
+            frame = stackFrames[idx]
+            frame_name = self.get_dict_value(frame, ["name"])
+            self.assertRegex(frame_name, expected["name"])
+            source_name = self.get_dict_value(frame, ["source", "name"])
+            self.assertRegex(source_name, expected["sourceName"])
+            if expected["containsSourceReference"]:
+                sourceReference = self.get_dict_value(
+                    frame, ["source", "sourceReference"]
+                )
+                response = self.dap_server.request_source(
+                    sourceReference=sourceReference
+                )
+                self.assertTrue(response["success"])
+                self.assertGreater(
+                    len(self.get_dict_value(response, ["body", "content"])),
+                    0,
+                    "verify content returned",
+                )
+                self.assertEqual(
+                    self.get_dict_value(response, ["body", "mimeType"]),
+                    "text/x-lldb.disassembly",
+                    "verify mime type returned",
+                )
+            else:
+                self.assertNotIn("sourceReference", frame["source"])
diff --git a/lldb/test/API/tools/lldb-dap/source/main.c b/lldb/test/API/tools/lldb-dap/source/main.c
new file mode 100644
index 0000000000000..6d8b9461b2998
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/source/main.c
@@ -0,0 +1,20 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+int comp(const void *first, const void *second) {
+  const int a = *((const int *)first);
+  const int b = *((const int *)second);
+  if (a == b) // qsort call
+    return 0;
+  if (a > b)
+    return 1;
+  return -1;
+}
+
+int main(int argc, char const *argv[]) {
+  int numbers[] = {4, 5, 2, 3, 1, 0, 9, 8, 6, 7};
+  qsort(numbers, sizeof(numbers) / sizeof(int), sizeof(int), comp);
+
+  return 0;
+}
diff --git a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
index 56ed1ebdf7ab4..4490e6eec2d8e 100644
--- a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
+++ b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
@@ -69,8 +69,9 @@ def test_stackTrace(self):
         self.recurse_end = line_number(source, "recurse end")
         self.recurse_call = line_number(source, "recurse call")
         self.recurse_invocation = line_number(source, "recurse invocation")
+        self.qsort_call = line_number(source, "qsort call")
 
-        lines = [self.recurse_end]
+        lines = [self.recurse_end, self.qsort_call]
 
         # Set breakpoint at a point of deepest recuusion
         breakpoint_ids = self.set_source_breakpoints(source, lines)
@@ -195,7 +196,7 @@ def test_stackTrace(self):
         )
         self.verify_stackFrames(startFrame, stackFrames)
 
-        # Verify we get not frames when startFrame is too high
+        # Verify we do not recive frames when startFrame is out of range
         startFrame = 1000
         levels = 1
         stackFrames = self.get_stackFrames(startFrame=startFrame, levels=levels)
@@ -203,6 +204,45 @@ def test_stackTrace(self):
             0, len(stackFrames), "verify zero frames with startFrame out of bounds"
         )
 
+        # Verify a stack frame from an external library (libc`qsort) to ensure
+        # frames without source code return a valid source reference.
+        self.continue_to_breakpoints(breakpoint_ids)
+        (stackFrames, totalFrames) = self.get_stackFrames_and_totalFramesCount()
+        frameCount = len(stackFrames)
+        self.assertGreaterEqual(
+            frameCount, 3, "verify we get frames from system librarys (libc qsort)"
+        )
+        self.assertEqual(
+            totalFrames,
+            frameCount,
+            "verify total frames returns a speculative page size",
+        )
+        expectedFrames = [
+            {
+                "name": "comp",
+                "line": 14,
+                "sourceName": "main.c",
+                "containsSourceReference": False,
+            },
+            {"name": "qsort", "sourceName": "qsort", "containsSourceReference": True},
+            {
+                "name": "main",
+                "line": 25,
+                "sourceName": "main.c",
+                "containsSourceReference": False,
+            },
+        ]
+        for idx, expected in enumerate(expectedFrames):
+            frame = stackFrames[idx]
+            frame_name = self.get_dict_value(frame, ["name"])
+            self.assertRegex(frame_name, expected["name"])
+            source_name = self.get_dict_value(frame, ["source", "name"])
+            self.assertRegex(source_name, expected["sourceName"])
+            if expected["containsSourceReference"]:
+                self.assertIn("sourceReference", frame["source"])
+            else:
+                self.assertNotIn("sourceReference", frame["source"])
+
     @skipIfWindows
     def test_functionNameWithArgs(self):
         """
diff --git a/lldb/test/API/tools/lldb-dap/stackTrace/main.c b/lldb/test/API/tools/lldb-dap/stackTrace/main.c
index 25d81be08e778..2c623ce374b38 100644
--- a/lldb/test/API/tools/lldb-dap/stackTrace/main.c
+++ b/lldb/test/API/tools/lldb-dap/stackTrace/main.c
@@ -1,4 +1,5 @@
 #include <stdio.h>
+#include <stdlib.h>
 #include <unistd.h>
 
 int recurse(int x) {
@@ -7,7 +8,21 @@ int recurse(int x) {
   return recurse(x - 1) + x; // recurse call
 }
 
+int comp(const void *first, const void *second) {
+  const int a = *((const int *)first);
+  const int b = *((const int *)second);
+  if (a == b) // qsort call
+    return 0;
+  if (a > b)
+    return 1;
+  return -1;
+}
+
 int main(int argc, char const *argv[]) {
   recurse(40); // recurse invocation
+
+  int numbers[] = {4, 5, 2, 3, 1, 0, 9, 8, 6, 7};
+  qsort(numbers, sizeof(numbers) / sizeof(int), sizeof(int), comp);
+
   return 0;
 }
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 86e5852b9a6c9..28350981312fc 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -41,6 +41,7 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
@@ -50,6 +51,7 @@
 #include <optional>
 #include <sstream>
 #include <string>
+#include <sys/syslimits.h>
 #include <utility>
 #include <vector>
 
@@ -697,24 +699,6 @@ llvm::json::Value CreateSource(llvm::StringRef source_path) {
   return llvm::json::Value(std::move(source));
 }
 
-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);
-
-  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": {
 //   "type": "object",
 //   "description": "A Stackframe contains the source location.",
@@ -806,20 +790,38 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
 
   EmplaceSafeString(object, "name", frame_name);
 
-  object.try_emplace("source", CreateSource(frame, frame_name));
   auto line_entry = frame.GetLineEntry();
-  if (line_entry.IsValid() &&
+  // 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 ||
        line_entry.GetLine() != LLDB_INVALID_LINE_NUMBER)) {
+    object.try_emplace("source", CreateSource(line_entry));
     object.try_emplace("line", line_entry.GetLine());
     auto column = line_entry.GetColumn();
     object.try_emplace("column", column);
-  } else {
+  } else if (frame.GetSymbol().IsValid()) {
+    // If no source is associated with the frame, use the DAPFrameID to track
+    // the 'source' and generate assembly.
+    llvm::json::Object source;
+    EmplaceSafeString(source, "name", frame_name);
+    char buf[PATH_MAX] = {0};
+    size_t size = frame.GetModule().GetFileSpec().GetPath(buf, PATH_MAX);
+    EmplaceSafeString(source, "path",
+                      std::string(buf, size) + '`' + frame_name);
+    source.try_emplace("sourceReference", MakeDAPFrameID(frame));
+    // Markthe source as deemphasized since users will only be able to view
+    // assembly for these frames.
+    EmplaceSafeString(source, "presentationHint", "deemphasize");
+    object.try_emplace("source", std::move(source));
+
+    // 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);
-    // Line numbers are 1-based. 
+    // Line numbers are 1-based.
     object.try_emplace("line", inst_line + 1);
     object.try_emplace("column", 1);
   }
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 7f196a958654c..b27f22896d3a8 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -46,6 +46,7 @@
 #include <algorithm>
 #include <array>
 #include <condition_variable>
+#include <climits>
 #include <cstdint>
 #include <cstdio>
 #include <cstdlib>

>From 0b495177eab78b3ab2ab4358cab260a77cf84aa9 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Sat, 22 Feb 2025 15:24:06 -0800
Subject: [PATCH 4/8] Fixing includes for linux.

---
 lldb/tools/lldb-dap/JSONUtils.cpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 28350981312fc..b9ba722b7ceff 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -7,7 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "JSONUtils.h"
-
 #include "BreakpointBase.h"
 #include "DAP.h"
 #include "ExceptionBreakpoint.h"
@@ -51,7 +50,6 @@
 #include <optional>
 #include <sstream>
 #include <string>
-#include <sys/syslimits.h>
 #include <utility>
 #include <vector>
 

>From c68deeea6f3abb521ebcbf9bd00359daa505c82b Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Sat, 22 Feb 2025 22:46:04 -0800
Subject: [PATCH 5/8] Fixing the tests on linux that depends on libc details.

---
 .../lldb-dap/stackTrace/TestDAP_stackTrace.py | 40 +++++++------------
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
index 4490e6eec2d8e..2821c0ef425d3 100644
--- a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
+++ b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
@@ -217,31 +217,21 @@ def test_stackTrace(self):
             frameCount,
             "verify total frames returns a speculative page size",
         )
-        expectedFrames = [
-            {
-                "name": "comp",
-                "line": 14,
-                "sourceName": "main.c",
-                "containsSourceReference": False,
-            },
-            {"name": "qsort", "sourceName": "qsort", "containsSourceReference": True},
-            {
-                "name": "main",
-                "line": 25,
-                "sourceName": "main.c",
-                "containsSourceReference": False,
-            },
-        ]
-        for idx, expected in enumerate(expectedFrames):
-            frame = stackFrames[idx]
-            frame_name = self.get_dict_value(frame, ["name"])
-            self.assertRegex(frame_name, expected["name"])
-            source_name = self.get_dict_value(frame, ["source", "name"])
-            self.assertRegex(source_name, expected["sourceName"])
-            if expected["containsSourceReference"]:
-                self.assertIn("sourceReference", frame["source"])
-            else:
-                self.assertNotIn("sourceReference", frame["source"])
+
+        frame = stackFrames.pop(0)
+        frame_name = self.get_dict_value(frame, ["name"])
+        self.assertRegex(frame_name, 'comp')
+        self.assertEqual(self.get_dict_value(frame, ['line']), 14)
+        self.assertNotIn('sourceReference', frame['source'])
+
+        # libc`qsort may not be the first frame below comp, search upwards
+        found_qsort = False
+        for frame in stackFrames:
+            if 'qsort' not in frame['name']:
+                continue
+            found_qsort = True
+            self.assertIn("sourceReference", frame["source"])
+        self.assertTrue(found_qsort, 'verify we found the qsort frame')
 
     @skipIfWindows
     def test_functionNameWithArgs(self):

>From 7879ae3db723b14752c2d0d42013cf742906f921 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Mon, 24 Feb 2025 09:32:53 -0800
Subject: [PATCH 6/8] Using '__attribute__((nodebug))' instead of a libc call
 to help verify stack frames without sources return disassembly.

---
 .../tools/lldb-dap/source/TestDAP_source.py   | 74 ++++++++++++-------
 lldb/test/API/tools/lldb-dap/source/main.c    | 20 ++---
 .../lldb-dap/stackTrace/TestDAP_stackTrace.py | 34 +--------
 .../test/API/tools/lldb-dap/stackTrace/main.c | 15 ----
 4 files changed, 56 insertions(+), 87 deletions(-)

diff --git a/lldb/test/API/tools/lldb-dap/source/TestDAP_source.py b/lldb/test/API/tools/lldb-dap/source/TestDAP_source.py
index 28479ccd11ec9..edf0af0bba2ba 100644
--- a/lldb/test/API/tools/lldb-dap/source/TestDAP_source.py
+++ b/lldb/test/API/tools/lldb-dap/source/TestDAP_source.py
@@ -12,17 +12,16 @@
 
 class TestDAP_source(lldbdap_testcase.DAPTestCaseBase):
     @skipIfWindows
-    def test_stackTrace(self):
+    def test_source(self):
         """
         Tests the 'source' packet.
         """
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(program)
-        source = "main.c"
-        self.source_path = os.path.join(os.getcwd(), source)
-        self.qsort_call = line_number(source, "qsort call")
+        source = self.getSourcePath("main.c")
+        breakpoint_line = line_number(source, "breakpoint")
 
-        lines = [self.qsort_call]
+        lines = [breakpoint_line]
         breakpoint_ids = self.set_source_breakpoints(source, lines)
         self.assertEqual(
             len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
@@ -35,38 +34,59 @@ def test_stackTrace(self):
 
         (stackFrames, totalFrames) = self.get_stackFrames_and_totalFramesCount()
         frameCount = len(stackFrames)
-        self.assertGreaterEqual(
-            frameCount, 3, "verify we get frames from system librarys (libc qsort)"
-        )
+        self.assertGreaterEqual(frameCount, 3, "verify we got up to main at least")
         self.assertEqual(
             totalFrames,
             frameCount,
             "verify total frames returns a speculative page size",
         )
-        expectedFrames = [
+        wantFrames = [
+            {
+                "name": "handler",
+                "line": 8,
+                "source": {
+                    "name": "main.c",
+                    "path": source,
+                    "containsSourceReference": False,
+                },
+            },
             {
-                "name": "comp",
-                "line": 14,
-                "sourceName": "main.c",
-                "containsSourceReference": False,
+                "name": "add",
+                "source": {
+                    "name": "add",
+                    "path": program + "`add",
+                    "containsSourceReference": True,
+                },
             },
-            {"name": "qsort", "sourceName": "qsort", "containsSourceReference": True},
             {
                 "name": "main",
-                "line": 25,
-                "sourceName": "main.c",
-                "containsSourceReference": False,
+                "line": 12,
+                "source": {
+                    "name": "main.c",
+                    "path": source,
+                    "containsSourceReference": False,
+                },
             },
         ]
-        for idx, expected in enumerate(expectedFrames):
-            frame = stackFrames[idx]
-            frame_name = self.get_dict_value(frame, ["name"])
-            self.assertRegex(frame_name, expected["name"])
-            source_name = self.get_dict_value(frame, ["source", "name"])
-            self.assertRegex(source_name, expected["sourceName"])
-            if expected["containsSourceReference"]:
+        for idx, want in enumerate(wantFrames):
+            got = stackFrames[idx]
+            name = self.get_dict_value(got, ["name"])
+            self.assertEqual(name, want["name"])
+
+            if "line" in want:
+                line = self.get_dict_value(got, ["line"])
+                self.assertEqual(line, want["line"])
+
+            wantSource = want["source"]
+            source_name = self.get_dict_value(got, ["source", "name"])
+            self.assertEqual(source_name, wantSource["name"])
+
+            source_path = self.get_dict_value(got, ["source", "path"])
+            self.assertEqual(source_path, wantSource["path"])
+
+            if wantSource["containsSourceReference"]:
                 sourceReference = self.get_dict_value(
-                    frame, ["source", "sourceReference"]
+                    got, ["source", "sourceReference"]
                 )
                 response = self.dap_server.request_source(
                     sourceReference=sourceReference
@@ -75,7 +95,7 @@ def test_stackTrace(self):
                 self.assertGreater(
                     len(self.get_dict_value(response, ["body", "content"])),
                     0,
-                    "verify content returned",
+                    "verify content returned disassembly",
                 )
                 self.assertEqual(
                     self.get_dict_value(response, ["body", "mimeType"]),
@@ -83,4 +103,4 @@ def test_stackTrace(self):
                     "verify mime type returned",
                 )
             else:
-                self.assertNotIn("sourceReference", frame["source"])
+                self.assertNotIn("sourceReference", got["source"])
diff --git a/lldb/test/API/tools/lldb-dap/source/main.c b/lldb/test/API/tools/lldb-dap/source/main.c
index 6d8b9461b2998..39420dba7874f 100644
--- a/lldb/test/API/tools/lldb-dap/source/main.c
+++ b/lldb/test/API/tools/lldb-dap/source/main.c
@@ -1,20 +1,14 @@
 #include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
 
-int comp(const void *first, const void *second) {
-  const int a = *((const int *)first);
-  const int b = *((const int *)second);
-  if (a == b) // qsort call
-    return 0;
-  if (a > b)
-    return 1;
-  return -1;
+__attribute__((nodebug)) static void add(int i, int j, void handler(int)) {
+  handler(i + j);
 }
 
-int main(int argc, char const *argv[]) {
-  int numbers[] = {4, 5, 2, 3, 1, 0, 9, 8, 6, 7};
-  qsort(numbers, sizeof(numbers) / sizeof(int), sizeof(int), comp);
+static void handler(int result) {
+  printf("result %d\n", result); // breakpoint
+}
 
+int main(int argc, char const *argv[]) {
+  add(2, 3, handler);
   return 0;
 }
diff --git a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
index 2821c0ef425d3..56ed1ebdf7ab4 100644
--- a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
+++ b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
@@ -69,9 +69,8 @@ def test_stackTrace(self):
         self.recurse_end = line_number(source, "recurse end")
         self.recurse_call = line_number(source, "recurse call")
         self.recurse_invocation = line_number(source, "recurse invocation")
-        self.qsort_call = line_number(source, "qsort call")
 
-        lines = [self.recurse_end, self.qsort_call]
+        lines = [self.recurse_end]
 
         # Set breakpoint at a point of deepest recuusion
         breakpoint_ids = self.set_source_breakpoints(source, lines)
@@ -196,7 +195,7 @@ def test_stackTrace(self):
         )
         self.verify_stackFrames(startFrame, stackFrames)
 
-        # Verify we do not recive frames when startFrame is out of range
+        # Verify we get not frames when startFrame is too high
         startFrame = 1000
         levels = 1
         stackFrames = self.get_stackFrames(startFrame=startFrame, levels=levels)
@@ -204,35 +203,6 @@ def test_stackTrace(self):
             0, len(stackFrames), "verify zero frames with startFrame out of bounds"
         )
 
-        # Verify a stack frame from an external library (libc`qsort) to ensure
-        # frames without source code return a valid source reference.
-        self.continue_to_breakpoints(breakpoint_ids)
-        (stackFrames, totalFrames) = self.get_stackFrames_and_totalFramesCount()
-        frameCount = len(stackFrames)
-        self.assertGreaterEqual(
-            frameCount, 3, "verify we get frames from system librarys (libc qsort)"
-        )
-        self.assertEqual(
-            totalFrames,
-            frameCount,
-            "verify total frames returns a speculative page size",
-        )
-
-        frame = stackFrames.pop(0)
-        frame_name = self.get_dict_value(frame, ["name"])
-        self.assertRegex(frame_name, 'comp')
-        self.assertEqual(self.get_dict_value(frame, ['line']), 14)
-        self.assertNotIn('sourceReference', frame['source'])
-
-        # libc`qsort may not be the first frame below comp, search upwards
-        found_qsort = False
-        for frame in stackFrames:
-            if 'qsort' not in frame['name']:
-                continue
-            found_qsort = True
-            self.assertIn("sourceReference", frame["source"])
-        self.assertTrue(found_qsort, 'verify we found the qsort frame')
-
     @skipIfWindows
     def test_functionNameWithArgs(self):
         """
diff --git a/lldb/test/API/tools/lldb-dap/stackTrace/main.c b/lldb/test/API/tools/lldb-dap/stackTrace/main.c
index 2c623ce374b38..25d81be08e778 100644
--- a/lldb/test/API/tools/lldb-dap/stackTrace/main.c
+++ b/lldb/test/API/tools/lldb-dap/stackTrace/main.c
@@ -1,5 +1,4 @@
 #include <stdio.h>
-#include <stdlib.h>
 #include <unistd.h>
 
 int recurse(int x) {
@@ -8,21 +7,7 @@ int recurse(int x) {
   return recurse(x - 1) + x; // recurse call
 }
 
-int comp(const void *first, const void *second) {
-  const int a = *((const int *)first);
-  const int b = *((const int *)second);
-  if (a == b) // qsort call
-    return 0;
-  if (a > b)
-    return 1;
-  return -1;
-}
-
 int main(int argc, char const *argv[]) {
   recurse(40); // recurse invocation
-
-  int numbers[] = {4, 5, 2, 3, 1, 0, 9, 8, 6, 7};
-  qsort(numbers, sizeof(numbers) / sizeof(int), sizeof(int), comp);
-
   return 0;
 }

>From bcf2e2cef98ce09570a1a30345dfe49f5f134c03 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Mon, 24 Feb 2025 09:41:05 -0800
Subject: [PATCH 7/8] Applying formatting.

---
 lldb/tools/lldb-dap/lldb-dap.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index b27f22896d3a8..7f196a958654c 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -46,7 +46,6 @@
 #include <algorithm>
 #include <array>
 #include <condition_variable>
-#include <climits>
 #include <cstdint>
 #include <cstdio>
 #include <cstdlib>

>From 9b6fdd7ebc6daf6e95f461bfea6971c97cd6d103 Mon Sep 17 00:00:00 2001
From: John Harrison <ash at greaterthaninfinity.com>
Date: Mon, 24 Feb 2025 10:22:21 -0800
Subject: [PATCH 8/8] Update lldb/tools/lldb-dap/JSONUtils.cpp

Co-authored-by: Jonas Devlieghere <jonas at devlieghere.com>
---
 lldb/tools/lldb-dap/JSONUtils.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index b9ba722b7ceff..9f08efb2a3ac1 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -808,7 +808,7 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
     EmplaceSafeString(source, "path",
                       std::string(buf, size) + '`' + frame_name);
     source.try_emplace("sourceReference", MakeDAPFrameID(frame));
-    // Markthe source as deemphasized since users will only be able to view
+    // Mark the source as deemphasized since users will only be able to view
     // assembly for these frames.
     EmplaceSafeString(source, "presentationHint", "deemphasize");
     object.try_emplace("source", std::move(source));



More information about the lldb-commits mailing list