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'.

ashgti · ashgti · commit aff8b1949f9c · 2025-02-24T14:07:39.000-08:00
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,14 +196,53 @@ 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)
         self.assertEqual(
             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
@@ -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
@@ -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
@@ -46,6 +46,7 @@
 #include <algorithm>
 #include <array>
 #include <condition_variable>
+#include <climits>
 #include <cstdint>
 #include <cstdio>
 #include <cstdlib>

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+C_SOURCES := main.c`
	`2`	`+`
	`3`	`+include Makefile.rules`