https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/137113
>From 21681616560eceb9b46ef4c370817099fe149701 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jo...@devlieghere.com>
Date: Wed, 23 Apr 2025 20:05:19 -0700
Subject: [PATCH 1/4] [lldb-dap] Support StackFrameFormat
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
---
.../test/tools/lldb-dap/dap_server.py | 4 +-
.../test/tools/lldb-dap/lldbdap_testcase.py | 18 ++++++--
.../lldb-dap/stackTrace/TestDAP_stackTrace.py | 30 +++++++++++++
.../Handler/StackTraceRequestHandler.cpp | 42 ++++++++++++++++---
4 files changed, 84 insertions(+), 10 deletions(-)
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));
>From 5b1378f178fb38d8fd158571bcdee61a6b5d6eda Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jo...@devlieghere.com>
Date: Thu, 24 Apr 2025 09:45:18 -0700
Subject: [PATCH 2/4] Fix incorrect interpretation of includeAll
---
.../TestDAP_extendedStackTrace.py | 36 +++++++++---
.../lldb-dap/stackTrace/TestDAP_stackTrace.py | 4 --
.../Handler/StackTraceRequestHandler.cpp | 57 ++++++++++++-------
3 files changed, 64 insertions(+), 33 deletions(-)
diff --git
a/lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py
b/lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py
index f6b613da964b8..7c616a72d3c79 100644
---
a/lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py
+++
b/lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py
@@ -2,7 +2,6 @@
Test lldb-dap stackTrace request with an extended backtrace thread.
"""
-
import os
import lldbdap_testcase
@@ -12,11 +11,8 @@
class TestDAP_extendedStackTrace(lldbdap_testcase.DAPTestCaseBase):
- @skipUnlessDarwin
- def test_stackTrace(self):
- """
- Tests the 'stackTrace' packet on a thread with an extended backtrace.
- """
+
+ def build_and_run(self, displayExtendedBacktrace=True):
backtrace_recording_lib = findBacktraceRecordingDylib()
if not backtrace_recording_lib:
self.skipTest(
@@ -36,7 +32,7 @@ def test_stackTrace(self):
"DYLD_LIBRARY_PATH=/usr/lib/system/introspection",
"DYLD_INSERT_LIBRARIES=" + backtrace_recording_lib,
],
- displayExtendedBacktrace=True,
+ displayExtendedBacktrace=displayExtendedBacktrace,
)
source = "main.m"
breakpoint = line_number(source, "breakpoint 1")
@@ -47,6 +43,12 @@ def test_stackTrace(self):
len(breakpoint_ids), len(lines), "expect correct number of
breakpoints"
)
+ @skipUnlessDarwin
+ def test_stackTrace(self):
+ """
+ Tests the 'stackTrace' packet on a thread with an extended backtrace.
+ """
+ self.build_and_run()
events = self.continue_to_next_stop()
stackFrames, totalFrames = self.get_stackFrames_and_totalFramesCount(
@@ -102,3 +104,23 @@ def test_stackTrace(self):
self.assertGreaterEqual(
totalFrames, i, "total frames should include a pagination
offset"
)
+
+ @skipIfWindows
+ def test_stackTraceWithFormat(self):
+ """
+ Tests the 'stackTrace' packet on a thread with an extended backtrace
using stack trace formats.
+ """
+ self.build_and_run(displayExtendedBacktrace=False)
+ events = self.continue_to_next_stop()
+
+ stackFrames, _ = self.get_stackFrames_and_totalFramesCount(
+ threadId=events[0]["body"]["threadId"], format={"includeAll": True}
+ )
+
+ stackLabels = [
+ (i, frame)
+ for i, frame in enumerate(stackFrames)
+ if frame.get("presentationHint", "") == "label"
+ ]
+
+ self.assertEqual(len(stackLabels), 2, "expected two label stack
frames")
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 713b5d841cfcd..3a11df7505994 100644
--- a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
+++ b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
@@ -2,7 +2,6 @@
Test lldb-dap stackTrace request
"""
-
import os
import lldbdap_testcase
@@ -230,9 +229,6 @@ def test_StackFrameFormat(self):
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)")
diff --git a/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
b/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
index 359237f1db0b4..06e1c06545b73 100644
--- a/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
@@ -51,7 +51,8 @@ static constexpr int StackPageSize = 20;
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) {
+ const int64_t start_frame, const int64_t levels,
+ const bool include_all) {
bool reached_end_of_stack = false;
for (int64_t i = start_frame;
static_cast<int64_t>(stack_frames.size()) < levels; i++) {
@@ -71,7 +72,9 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
stack_frames.emplace_back(CreateStackFrame(frame, frame_format));
}
- if (dap.configuration.displayExtendedBacktrace && reached_end_of_stack) {
+ const bool include_extended_backtrace =
+ include_all || dap.configuration.displayExtendedBacktrace;
+ if (include_extended_backtrace && reached_end_of_stack) {
// Check for any extended backtraces.
for (uint32_t bt = 0;
bt < thread.GetProcess().GetNumExtendedBacktraceTypes(); bt++) {
@@ -82,7 +85,8 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
reached_end_of_stack = FillStackFrames(
dap, backtrace, frame_format, stack_frames, offset,
- (start_frame - offset) > 0 ? start_frame - offset : -1, levels);
+ (start_frame - offset) > 0 ? start_frame - offset : -1, levels,
+ include_all);
if (static_cast<int64_t>(stack_frames.size()) >= levels)
break;
}
@@ -180,34 +184,43 @@ void StackTraceRequestHandler::operator()(
llvm::json::Object body;
lldb::SBFormat frame_format = dap.frame_format;
+ bool include_all = false;
if (const auto *format = arguments->getObject("format")) {
+ // Indicates that all stack frames should be included, even those the debug
+ // adapter might otherwise hide.
+ include_all = GetBoolean(format, "includeAll").value_or(false);
+
+ // Parse the properties that have a corresponding format string.
+ // FIXME: Support "parameterTypes" and "hex".
+ const bool module = GetBoolean(format, "module").value_or(false);
+ const bool line = GetBoolean(format, "line").value_or(false);
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);
+ // Only change the format string if we have to.
+ if (module || line || parameters || parameter_names || parameter_values) {
+ std::string format_str;
+ llvm::raw_string_ostream os(format_str);
- if (include_all || module)
- os << "{${module.file.basename} }";
+ if (module)
+ os << "{${module.file.basename} }";
- if (include_all || line)
- os << "{${line.file.basename}:${line.number}:${line.column} }";
+ if (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}}";
+ if (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());
+ lldb::SBError error;
+ frame_format = lldb::SBFormat(format_str.c_str(), error);
+ assert(error.Success());
+ }
}
if (thread.IsValid()) {
@@ -215,9 +228,9 @@ void StackTraceRequestHandler::operator()(
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, frame_format, stack_frames, offset,
- start_frame, levels == 0 ? INT64_MAX : levels);
+ bool reached_end_of_stack = FillStackFrames(
+ dap, thread, frame_format, stack_frames, offset, start_frame,
+ levels == 0 ? INT64_MAX : levels, include_all);
body.try_emplace("totalFrames",
start_frame + stack_frames.size() +
(reached_end_of_stack ? 0 : StackPageSize));
>From 3cf5b4ffa290c79093411141deb5175c0457d8b9 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jo...@devlieghere.com>
Date: Thu, 24 Apr 2025 09:50:32 -0700
Subject: [PATCH 3/4] Black & Darker disagree
---
.../lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py | 1 -
1 file changed, 1 deletion(-)
diff --git
a/lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py
b/lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py
index 7c616a72d3c79..d91428a41edac 100644
---
a/lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py
+++
b/lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py
@@ -11,7 +11,6 @@
class TestDAP_extendedStackTrace(lldbdap_testcase.DAPTestCaseBase):
-
def build_and_run(self, displayExtendedBacktrace=True):
backtrace_recording_lib = findBacktraceRecordingDylib()
if not backtrace_recording_lib:
>From 66f22534cdfa385808129117ccd84bd3ca9a8cd5 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jo...@devlieghere.com>
Date: Thu, 24 Apr 2025 09:59:54 -0700
Subject: [PATCH 4/4] Simplify dap.configuration.displayExtendedBacktrace
---
lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
b/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
index 06e1c06545b73..4ea4cd1e517d4 100644
--- a/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
@@ -72,9 +72,7 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
stack_frames.emplace_back(CreateStackFrame(frame, frame_format));
}
- const bool include_extended_backtrace =
- include_all || dap.configuration.displayExtendedBacktrace;
- if (include_extended_backtrace && reached_end_of_stack) {
+ if (include_all && reached_end_of_stack) {
// Check for any extended backtraces.
for (uint32_t bt = 0;
bt < thread.GetProcess().GetNumExtendedBacktraceTypes(); bt++) {
@@ -184,7 +182,7 @@ void StackTraceRequestHandler::operator()(
llvm::json::Object body;
lldb::SBFormat frame_format = dap.frame_format;
- bool include_all = false;
+ bool include_all = dap.configuration.displayExtendedBacktrace;
if (const auto *format = arguments->getObject("format")) {
// Indicates that all stack frames should be included, even those the debug
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
