[lldb-dap] Support StackFrameFormat #137113

JDevlieghere · 2025-04-24T03:10:18Z

The debug adapter protocol supports an option to provide formatting information for a stack frames as part of the StackTrace request. lldb-dap incorrectly advertises it supports this, but until this PR that support wasn't actually implemented.

Fixes #137057

The debug adapter protocol supports an option to provide formatting information for a stack frames as part of the StackTrace request. lldb-dap incorrectly advertises it supports this, but until this PR that support wasn't actually implemented. Fixes llvm#137057

llvmbot · 2025-04-24T03:10:55Z

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

The debug adapter protocol supports an option to provide formatting information for a stack frames as part of the StackTrace request. lldb-dap incorrectly advertises it supports this, but until this PR that support wasn't actually implemented.

Fixes #137057

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

4 Files Affected:

(modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+3-1)
(modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+14-4)
(modified) lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py (+30)
(modified) lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp (+37-5)

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 a9915ba2f6de6..dadf6b1f8774c 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
@@ -1046,7 +1046,7 @@ def request_modules(self):
         return self.send_recv({"command": "modules", "type": "request"})
 
     def request_stackTrace(
-        self, threadId=None, startFrame=None, levels=None, dump=False
+        self, threadId=None, startFrame=None, levels=None, format=None, dump=False
     ):
         if threadId is None:
             threadId = self.get_thread_id()
@@ -1055,6 +1055,8 @@ def request_stackTrace(
             args_dict["startFrame"] = startFrame
         if levels is not None:
             args_dict["levels"] = levels
+        if format is not None:
+            args_dict["format"] = format
         command_dict = {
             "command": "stackTrace",
             "type": "request",
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index 70b04b051e0ec..b5b55b336d535 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -161,10 +161,14 @@ def get_dict_value(self, d, key_path):
         return value
 
     def get_stackFrames_and_totalFramesCount(
-        self, threadId=None, startFrame=None, levels=None, dump=False
+        self, threadId=None, startFrame=None, levels=None, format=None, dump=False
     ):
         response = self.dap_server.request_stackTrace(
-            threadId=threadId, startFrame=startFrame, levels=levels, dump=dump
+            threadId=threadId,
+            startFrame=startFrame,
+            levels=levels,
+            format=format,
+            dump=dump,
         )
         if response:
             stackFrames = self.get_dict_value(response, ["body", "stackFrames"])
@@ -177,9 +181,15 @@ def get_stackFrames_and_totalFramesCount(
             return (stackFrames, totalFrames)
         return (None, 0)
 
-    def get_stackFrames(self, threadId=None, startFrame=None, levels=None, dump=False):
+    def get_stackFrames(
+        self, threadId=None, startFrame=None, levels=None, format=None, dump=False
+    ):
         (stackFrames, totalFrames) = self.get_stackFrames_and_totalFramesCount(
-            threadId=threadId, startFrame=startFrame, levels=levels, dump=dump
+            threadId=threadId,
+            startFrame=startFrame,
+            levels=levels,
+            format=format,
+            dump=dump,
         )
         return stackFrames
 
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..713b5d841cfcd 100644
--- a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
+++ b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
@@ -217,3 +217,33 @@ def test_functionNameWithArgs(self):
         self.continue_to_next_stop()
         frame = self.get_stackFrames()[0]
         self.assertEqual(frame["name"], "recurse(x=1)")
+
+    @skipIfWindows
+    def test_StackFrameFormat(self):
+        """
+        Test the StackFrameFormat.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.c"
+
+        self.set_source_breakpoints(source, [line_number(source, "recurse end")])
+
+        self.continue_to_next_stop()
+        frame = self.get_stackFrames(format={"includeAll": True})[0]
+        self.assertEqual(frame["name"], "a.out main.c:6:5 recurse(x=1)")
+
+        frame = self.get_stackFrames(format={"parameters": True})[0]
+        self.assertEqual(frame["name"], "recurse(x=1)")
+
+        frame = self.get_stackFrames(format={"parameterNames": True})[0]
+        self.assertEqual(frame["name"], "recurse(x=1)")
+
+        frame = self.get_stackFrames(format={"parameterValues": True})[0]
+        self.assertEqual(frame["name"], "recurse(x=1)")
+
+        frame = self.get_stackFrames(format={"parameters": False, "line": True})[0]
+        self.assertEqual(frame["name"], "main.c:6:5 recurse")
+
+        frame = self.get_stackFrames(format={"parameters": False, "module": True})[0]
+        self.assertEqual(frame["name"], "a.out recurse")
diff --git a/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
index a58e3325af100..359237f1db0b4 100644
--- a/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
@@ -49,6 +49,7 @@ static constexpr int StackPageSize = 20;
 //
 // s=3,l=3 = [th0->s3, label1, th1->s0]
 static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
+                            lldb::SBFormat &frame_format,
                             llvm::json::Array &stack_frames, int64_t &offset,
                             const int64_t start_frame, const int64_t levels) {
   bool reached_end_of_stack = false;
@@ -56,7 +57,7 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
        static_cast<int64_t>(stack_frames.size()) < levels; i++) {
     if (i == -1) {
       stack_frames.emplace_back(
-          CreateExtendedStackFrameLabel(thread, dap.frame_format));
+          CreateExtendedStackFrameLabel(thread, frame_format));
       continue;
     }
 
@@ -67,7 +68,7 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
       break;
     }
 
-    stack_frames.emplace_back(CreateStackFrame(frame, dap.frame_format));
+    stack_frames.emplace_back(CreateStackFrame(frame, frame_format));
   }
 
   if (dap.configuration.displayExtendedBacktrace && reached_end_of_stack) {
@@ -80,7 +81,7 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
         continue;
 
       reached_end_of_stack = FillStackFrames(
-          dap, backtrace, stack_frames, offset,
+          dap, backtrace, frame_format, stack_frames, offset,
           (start_frame - offset) > 0 ? start_frame - offset : -1, levels);
       if (static_cast<int64_t>(stack_frames.size()) >= levels)
         break;
@@ -178,14 +179,45 @@ void StackTraceRequestHandler::operator()(
   llvm::json::Array stack_frames;
   llvm::json::Object body;
 
+  lldb::SBFormat frame_format = dap.frame_format;
+
+  if (const auto *format = arguments->getObject("format")) {
+    const bool parameters = GetBoolean(format, "parameters").value_or(false);
+    const bool parameter_names =
+        GetBoolean(format, "parameterNames").value_or(false);
+    const bool parameter_values =
+        GetBoolean(format, "parameterValues").value_or(false);
+    const bool line = GetBoolean(format, "line").value_or(false);
+    const bool module = GetBoolean(format, "module").value_or(false);
+    const bool include_all = GetBoolean(format, "includeAll").value_or(false);
+
+    std::string format_str;
+    llvm::raw_string_ostream os(format_str);
+
+    if (include_all || module)
+      os << "{${module.file.basename} }";
+
+    if (include_all || line)
+      os << "{${line.file.basename}:${line.number}:${line.column} }";
+
+    if (include_all || parameters || parameter_names || parameter_values)
+      os << "{${function.name-with-args}}";
+    else
+      os << "{${function.name-without-args}}";
+
+    lldb::SBError error;
+    frame_format = lldb::SBFormat(format_str.c_str(), error);
+    assert(error.Success());
+  }
+
   if (thread.IsValid()) {
     const auto start_frame =
         GetInteger<uint64_t>(arguments, "startFrame").value_or(0);
     const auto levels = GetInteger<uint64_t>(arguments, "levels").value_or(0);
     int64_t offset = 0;
     bool reached_end_of_stack =
-        FillStackFrames(dap, thread, stack_frames, offset, start_frame,
-                        levels == 0 ? INT64_MAX : levels);
+        FillStackFrames(dap, thread, frame_format, stack_frames, offset,
+                        start_frame, levels == 0 ? INT64_MAX : levels);
     body.try_emplace("totalFrames",
                      start_frame + stack_frames.size() +
                          (reached_end_of_stack ? 0 : StackPageSize));

ashgti · 2025-04-24T15:46:43Z

lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp

+        GetBoolean(format, "parameterValues").value_or(false);
+    const bool line = GetBoolean(format, "line").value_or(false);
+    const bool module = GetBoolean(format, "module").value_or(false);
+    const bool include_all = GetBoolean(format, "includeAll").value_or(false);


I think this is maybe a different meaning than the spec meant for includeAll.

It says:

Includes all stack frames, including those the debug adapter might otherwise hide.

Which to me, sounds like its about hiding frames that we might not find interesting (e.g. frames from system libraries).

I think if this is false you could skip showing the extended backtraces.

Great observation, you're right.

Its worth noting that today I don't think we skip any frames, however we do use the 'presentationHint' to tweak the frames a little.

deemphasize is used when we don't have the source info for a frame

llvm-project/lldb/tools/lldb-dap/JSONUtils.cpp

Line 774 in feaa5aa

EmplaceSafeString(source, "presentationHint", "deemphasize");

subtle is used if the frame is artificial or hidden

llvm-project/lldb/tools/lldb-dap/JSONUtils.cpp

Line 795 in feaa5aa

object.try_emplace("presentationHint", "subtle");

ashgti · 2025-04-24T16:01:01Z

lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp

@@ -178,14 +179,45 @@ void StackTraceRequestHandler::operator()(
  llvm::json::Array stack_frames;
  llvm::json::Object body;

+  lldb::SBFormat frame_format = dap.frame_format;
+
+  if (const auto *format = arguments->getObject("format")) {


Another thing I notice is the StackFrameFormat inherits from ValueFormat and thus includes the hex?: boolean; field as well.

I'm not sure what that would mean in terms of a stack frame however, just thought I would note that for completeness.

github-actions · 2025-04-24T16:48:05Z

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

ashgti · 2025-04-24T16:57:21Z

lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp

@@ -180,44 +184,53 @@ void StackTraceRequestHandler::operator()(
  llvm::json::Object body;

  lldb::SBFormat frame_format = dap.frame_format;
+  bool include_all = false;


Should this default to dap.configuration.displayExtendedBacktrace and then it simplifies line 75-77.

da-viper · 2025-04-24T19:07:57Z

lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py

@@ -217,3 +216,30 @@ def test_functionNameWithArgs(self):
        self.continue_to_next_stop()
        frame = self.get_stackFrames()[0]
        self.assertEqual(frame["name"], "recurse(x=1)")
+
+    @skipIfWindows


LGTM
But I could not see why this test is skipped on windows
there is a #include <unistd.h> but it is not used any where

Fair enough. All the tests in this file are skipped on Windows for as far as the history goes back. I can try removing all of them in a followup and see what the Windows bot say.

lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py

The debug adapter protocol supports an option to provide formatting information for a stack frames as part of the StackTrace request. lldb-dap incorrectly advertises it supports this, but until this PR that support wasn't actually implemented. Fixes llvm#137057 Back-ported from commit 262158b

[lldb-dap] Support StackFrameFormat (llvm#137113)

The debug adapter protocol supports an option to provide formatting information for a stack frames as part of the StackTrace request. lldb-dap incorrectly advertises it supports this, but until this PR that support wasn't actually implemented. Fixes llvm#137057

JDevlieghere requested review from ashgti, vogelsgesang and da-viper April 24, 2025 03:10

llvmbot added lldb lldb-dap labels Apr 24, 2025

ashgti reviewed Apr 24, 2025

View reviewed changes

Fix incorrect interpretation of includeAll

5b1378f

JDevlieghere requested a review from ashgti April 24, 2025 16:46

Black & Darker disagree

3cf5b4f

ashgti approved these changes Apr 24, 2025

View reviewed changes

Simplify dap.configuration.displayExtendedBacktrace

66f2253

da-viper approved these changes Apr 24, 2025

View reviewed changes

Fix decorator

0584669

JDevlieghere merged commit 262158b into llvm:main Apr 24, 2025
10 checks passed

JDevlieghere deleted the issue-137057 branch April 24, 2025 23:25

JDevlieghere added a commit to swiftlang/llvm-project that referenced this pull request Apr 29, 2025

Merge pull request #10571 from swiftlang/StackFrameFormat-backport

c205676

[lldb-dap] Support StackFrameFormat (llvm#137113)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lldb-dap] Support StackFrameFormat #137113

[lldb-dap] Support StackFrameFormat #137113

Uh oh!

JDevlieghere commented Apr 24, 2025

Uh oh!

llvmbot commented Apr 24, 2025

Uh oh!

ashgti Apr 24, 2025

Uh oh!

JDevlieghere Apr 24, 2025

Uh oh!

ashgti Apr 24, 2025

Uh oh!

ashgti Apr 24, 2025

Uh oh!

github-actions bot commented Apr 24, 2025 •

edited

Loading

Uh oh!

ashgti Apr 24, 2025

Uh oh!

da-viper Apr 24, 2025

Uh oh!

JDevlieghere Apr 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[lldb-dap] Support StackFrameFormat #137113

[lldb-dap] Support StackFrameFormat #137113

Uh oh!

Conversation

JDevlieghere commented Apr 24, 2025

Uh oh!

llvmbot commented Apr 24, 2025

Uh oh!

ashgti Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

JDevlieghere Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

ashgti Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

ashgti Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ashgti Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

da-viper Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

JDevlieghere Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Apr 24, 2025 •

edited

Loading