-
Notifications
You must be signed in to change notification settings - Fork 11.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb-dap] Add an option to provide a format for stack frames #71843
[lldb-dap] Add an option to provide a format for stack frames #71843
Conversation
@llvm/pr-subscribers-lldb Author: Walter Erquinigo (walter-erquinigo) ChangesWhen this option is enabled, display names of stack frames are generated using the This option is disabled by default because of its performance cost. It's a good option for non-gigantic programs. Full diff: https://github.com/llvm/llvm-project/pull/71843.diff 11 Files Affected:
diff --git a/lldb/include/lldb/API/SBFrame.h b/lldb/include/lldb/API/SBFrame.h
index 7c4477f9125d1cd..75e04d794baf848 100644
--- a/lldb/include/lldb/API/SBFrame.h
+++ b/lldb/include/lldb/API/SBFrame.h
@@ -87,8 +87,15 @@ class LLDB_API SBFrame {
// display to a user
const char *GetDisplayFunctionName();
+ /// Similar to \a GetDisplayFunctionName() but with function arguments and
+ /// their values inserted into the function display name whenever possible.
+ ///
+ /// \param[in] output
+ /// The stream where the display name is written to.
+ void GetDisplayFunctionNameWithArgs(SBStream &output);
+
const char *GetFunctionName() const;
-
+
// Return the frame function's language. If there isn't a function, then
// guess the language type from the mangled name.
lldb::LanguageType GuessLanguage() const;
diff --git a/lldb/include/lldb/Target/StackFrame.h b/lldb/include/lldb/Target/StackFrame.h
index 6824d916030a024..7b8510dfb4b40c6 100644
--- a/lldb/include/lldb/Target/StackFrame.h
+++ b/lldb/include/lldb/Target/StackFrame.h
@@ -14,6 +14,7 @@
#include "lldb/Utility/Flags.h"
+#include "lldb/Core/FormatEntity.h"
#include "lldb/Core/ValueObjectList.h"
#include "lldb/Symbol/SymbolContext.h"
#include "lldb/Target/ExecutionContextScope.h"
@@ -324,8 +325,23 @@ class StackFrame : public ExecutionContextScope,
/// C string with the assembly instructions for this function.
const char *Disassemble();
+ /// Print a description of this frame using the provided frame format.
+ /// If the format is invalid, then the default formatter will be used (see \a
+ /// StackFrame::Dump()), in which case \b false is returned. Otherwise, \b
+ /// true is returned.
+ ///
+ /// \param[in] strm
+ /// The Stream to print the description to.
+ ///
+ /// \param[in] frame_marker
+ /// Optional string that will be prepended to the frame output description.
+ bool DumpUsingFormat(Stream &strm,
+ const lldb_private::FormatEntity::Entry *format,
+ llvm::StringRef frame_marker = {});
+
/// Print a description for this frame using the frame-format formatter
- /// settings.
+ /// settings. If the current frame-format settings are invalid, then the
+ /// default formatter will be used (see \a StackFrame::Dump()).
///
/// \param [in] strm
/// The Stream to print the description to.
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 d1fb478bc8bb9ee..0c305fdd1ad9bbe 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
@@ -732,6 +732,7 @@ def request_launch(
enableAutoVariableSummaries=False,
enableSyntheticChildDebugging=False,
commandEscapePrefix="`",
+ showFramesWithFunctionArgs=False,
):
args_dict = {"program": program}
if args:
@@ -773,6 +774,7 @@ def request_launch(
args_dict["runInTerminal"] = runInTerminal
if postRunCommands:
args_dict["postRunCommands"] = postRunCommands
+ args_dict["showFramesWithFunctionArgs"] = showFramesWithFunctionArgs
args_dict["enableAutoVariableSummaries"] = enableAutoVariableSummaries
args_dict["enableSyntheticChildDebugging"] = enableSyntheticChildDebugging
args_dict["commandEscapePrefix"] = commandEscapePrefix
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 aa89ffe24c3e026..0775922500ea4a8 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
@@ -352,6 +352,7 @@ def launch(
enableAutoVariableSummaries=False,
enableSyntheticChildDebugging=False,
commandEscapePrefix="`",
+ showFramesWithFunctionArgs=False,
):
"""Sending launch request to dap"""
@@ -391,6 +392,7 @@ def cleanup():
enableAutoVariableSummaries=enableAutoVariableSummaries,
enableSyntheticChildDebugging=enableSyntheticChildDebugging,
commandEscapePrefix=commandEscapePrefix,
+ showFramesWithFunctionArgs=showFramesWithFunctionArgs,
)
if expectFailure:
@@ -428,6 +430,7 @@ def build_and_launch(
enableAutoVariableSummaries=False,
enableSyntheticChildDebugging=False,
commandEscapePrefix="`",
+ showFramesWithFunctionArgs=False,
):
"""Build the default Makefile target, create the DAP debug adaptor,
and launch the process.
@@ -459,4 +462,5 @@ def build_and_launch(
enableAutoVariableSummaries=enableAutoVariableSummaries,
enableSyntheticChildDebugging=enableSyntheticChildDebugging,
commandEscapePrefix=commandEscapePrefix,
+ showFramesWithFunctionArgs=showFramesWithFunctionArgs,
)
diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp
index da5c6075e8f7b4b..814966f966ef7f2 100644
--- a/lldb/source/API/SBFrame.cpp
+++ b/lldb/source/API/SBFrame.cpp
@@ -601,8 +601,8 @@ SBValue SBFrame::FindValue(const char *name, ValueType value_type,
stop_if_block_is_inlined_function,
[frame](Variable *v) { return v->IsInScope(frame); },
&variable_list);
- if (value_type == eValueTypeVariableGlobal
- || value_type == eValueTypeVariableStatic) {
+ if (value_type == eValueTypeVariableGlobal ||
+ value_type == eValueTypeVariableStatic) {
const bool get_file_globals = true;
VariableList *frame_vars = frame->GetVariableList(get_file_globals,
nullptr);
@@ -814,9 +814,11 @@ SBValueList SBFrame::GetVariables(const lldb::SBVariablesOptions &options) {
if (num_variables) {
size_t num_produced = 0;
for (const VariableSP &variable_sp : *variable_list) {
- if (INTERRUPT_REQUESTED(dbg,
- "Interrupted getting frame variables with {0} of {1} "
- "produced.", num_produced, num_variables))
+ if (INTERRUPT_REQUESTED(
+ dbg,
+ "Interrupted getting frame variables with {0} of {1} "
+ "produced.",
+ num_produced, num_variables))
return {};
if (variable_sp) {
@@ -1232,6 +1234,32 @@ const char *SBFrame::GetFunctionName() const {
return name;
}
+void SBFrame::GetDisplayFunctionNameWithArgs(SBStream &output) {
+ Stream &strm = output.ref();
+
+ std::unique_lock<std::recursive_mutex> lock;
+ ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
+
+ StackFrame *frame = nullptr;
+ Target *target = exe_ctx.GetTargetPtr();
+ Process *process = exe_ctx.GetProcessPtr();
+
+ if (target && process) {
+ Process::StopLocker stop_locker;
+ if (stop_locker.TryLock(&process->GetRunLock())) {
+ frame = exe_ctx.GetFramePtr();
+ if (frame) {
+ FormatEntity::Entry format;
+ Status s = FormatEntity::Parse("${function.name-with-args}", format);
+ assert(
+ s.Success() &&
+ "The ${function.name-with-args} format must be parsed correctly");
+ frame->DumpUsingFormat(strm, &format);
+ }
+ }
+ }
+}
+
const char *SBFrame::GetDisplayFunctionName() {
LLDB_INSTRUMENT_VA(this);
diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp
index 11ada92348ecee2..71e7f356245f1c3 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -1779,18 +1779,32 @@ void StackFrame::CalculateExecutionContext(ExecutionContext &exe_ctx) {
exe_ctx.SetContext(shared_from_this());
}
+bool StackFrame::DumpUsingFormat(Stream &strm,
+ const FormatEntity::Entry *format,
+ llvm::StringRef frame_marker) {
+ GetSymbolContext(eSymbolContextEverything);
+ ExecutionContext exe_ctx(shared_from_this());
+ StreamString s;
+ s.PutCString(frame_marker);
+
+ if (format && FormatEntity::Format(*format, s, &m_sc, &exe_ctx, nullptr,
+ nullptr, false, false)) {
+ strm.PutCString(s.GetString());
+ return true;
+ }
+ Dump(&strm, true, false);
+ strm.EOL();
+ return false;
+}
+
void StackFrame::DumpUsingSettingsFormat(Stream *strm, bool show_unique,
const char *frame_marker) {
if (strm == nullptr)
return;
- GetSymbolContext(eSymbolContextEverything);
ExecutionContext exe_ctx(shared_from_this());
StreamString s;
- if (frame_marker)
- s.PutCString(frame_marker);
-
const FormatEntity::Entry *frame_format = nullptr;
Target *target = exe_ctx.GetTargetPtr();
if (target) {
@@ -1800,13 +1814,7 @@ void StackFrame::DumpUsingSettingsFormat(Stream *strm, bool show_unique,
frame_format = target->GetDebugger().GetFrameFormat();
}
}
- if (frame_format && FormatEntity::Format(*frame_format, s, &m_sc, &exe_ctx,
- nullptr, nullptr, false, false)) {
- strm->PutCString(s.GetString());
- } else {
- Dump(strm, true, false);
- strm->EOL();
- }
+ DumpUsingFormat(*strm, frame_format, frame_marker);
}
void StackFrame::Dump(Stream *strm, bool show_frame_index,
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 245b3f34b70c868..153dd77169b3a8d 100644
--- a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
+++ b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
@@ -3,12 +3,13 @@
"""
+import os
+
import dap_server
+import lldbdap_testcase
+from lldbsuite.test import lldbutil
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-import lldbdap_testcase
-import os
class TestDAP_stackTrace(lldbdap_testcase.DAPTestCaseBase):
@@ -187,3 +188,19 @@ def test_stackTrace(self):
self.assertEquals(
0, len(stackFrames), "verify zero frames with startFrame out of bounds"
)
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_functionNameWithArgs(self):
+ """
+ Test that the stack frame without a function name is given its pc in the response.
+ """
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program, showFramesWithFunctionArgs=True)
+ source = "main.c"
+
+ self.set_source_breakpoints(source, [line_number(source, "recurse end")])
+
+ self.continue_to_next_stop()
+ frame = self.get_stackFrames()[0]
+ self.assertEquals(frame["name"], "recurse(x=1)")
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index b00c103c33b7a92..765e64a321f1530 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -189,6 +189,7 @@ struct DAP {
ReplMode repl_mode;
bool auto_repl_mode_collision_warning;
std::string command_escape_prefix = "`";
+ bool show_frames_with_function_args = false;
DAP();
~DAP();
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 2ff17616c2e9986..4dcd792b3ec4213 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -785,11 +785,18 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame) {
int64_t frame_id = MakeDAPFrameID(frame);
object.try_emplace("id", frame_id);
- // `function_name` can be a nullptr, which throws an error when assigned to an
- // `std::string`.
- const char *function_name = frame.GetDisplayFunctionName();
- std::string frame_name =
- function_name == nullptr ? std::string() : function_name;
+ std::string frame_name;
+ if (g_dap.show_frames_with_function_args) {
+ lldb::SBStream stream;
+ frame.GetDisplayFunctionNameWithArgs(stream);
+ frame_name = stream.GetData();
+ } else {
+ // `function_name` can be a nullptr, which throws an error when assigned to
+ // an `std::string`.
+ if (const char *name = frame.GetDisplayFunctionName())
+ frame_name = name;
+ }
+
if (frame_name.empty()) {
// If the function name is unavailable, display the pc address as a 16-digit
// hex string, e.g. "0x0000000000012345"
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index e103aabb870207f..3b8aa7851274e1d 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -653,6 +653,8 @@ void request_attach(const llvm::json::Object &request) {
GetBoolean(arguments, "enableSyntheticChildDebugging", false);
g_dap.command_escape_prefix =
GetString(arguments, "commandEscapePrefix", "`");
+ g_dap.show_frames_with_function_args =
+ GetBoolean(arguments, "showFramesWithFunctionArgs", false);
// This is a hack for loading DWARF in .o files on Mac where the .o files
// in the debug map of the main executable have relative paths which require
@@ -1805,6 +1807,8 @@ void request_launch(const llvm::json::Object &request) {
GetBoolean(arguments, "enableSyntheticChildDebugging", false);
g_dap.command_escape_prefix =
GetString(arguments, "commandEscapePrefix", "`");
+ g_dap.show_frames_with_function_args =
+ GetBoolean(arguments, "showFramesWithFunctionArgs", false);
// This is a hack for loading DWARF in .o files on Mac where the .o files
// in the debug map of the main executable have relative paths which require
diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json
index a0ae7ac834939d5..753887a8ef0161c 100644
--- a/lldb/tools/lldb-dap/package.json
+++ b/lldb/tools/lldb-dap/package.json
@@ -255,6 +255,11 @@
"type": "string",
"description": "The escape prefix to use for executing regular LLDB commands in the Debug Console, instead of printing variables. Defaults to a back-tick (`). If it's an empty string, then all expression in the Debug Console are treated as regular LLDB commands.",
"default": "`"
+ },
+ "showFramesWithFunctionArgs": {
+ "type": "boolean",
+ "description": "When enabled, show function arguments along with their corresponding values in the stack frames. This comes with a performance cost because debug information needs to be processed to generate such values.",
+ "default": "false"
}
}
},
@@ -349,6 +354,11 @@
"type": "string",
"description": "The escape prefix character to use for executing regular LLDB commands in the Debug Console, instead of printing variables. Defaults to a back-tick (`). If empty, then all expression in the Debug Console are treated as regular LLDB commands.",
"default": "`"
+ },
+ "showFramesWithFunctionArgs": {
+ "type": "boolean",
+ "description": "When enabled, show function arguments along with their corresponding values in the stack frames. This comes with a performance cost because debug information needs to be processed to generate such values.",
+ "default": "false"
}
}
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this hardcodes the frame display to only be the function name, or function name with args. Do we want to allow users to specify a frame format in the lldb-vscode settings? We could have two settings: one to enable the feature with something like showFramesWithCustomFormat
and on that contains the format string like customFrameFormat
which would default to "${function.name-with-args}"
. Then users have complete control over how and what gets displayed. Some people might want to show the pc value and then the function name like:
"customFrameFormat": "${frame.pc} ${function.name-with-args}"
If we want this then we will want to modify the SBFrame changes to be something like:
SBError SBFrame::ApplyFormat(SBStream &output, const char *format_string);
We might even consider makding a lldb::SBFormat
class that allows us to compile a format string into an object and then use it. If we do this then the API above will be:
SBError SBFrame::ApplyFormat(SBStream &output, lldb::SBFormat format);
lldb/source/API/SBFrame.cpp
Outdated
if (stop_locker.TryLock(&process->GetRunLock())) { | ||
frame = exe_ctx.GetFramePtr(); | ||
if (frame) { | ||
FormatEntity::Entry format; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this static and add a llvm::once call around the intiailization so we don't need to parse the format each time we call this function. If we make this static, rename to g_format
to indicate it is a global.
7b9b589
to
2f3841d
Compare
@clayborg , I did pretty much what you asked but with a few changes:
|
2f3841d
to
8ad22d8
Compare
f7e9a2a
to
650690b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just need to not use llvm::errs()
or llvm::outs()
in lldb DAP, use DAP::SendOutput(OutputType::Console, ...)` to ensure that the users sees this.
650690b
to
96d3b9e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last request: test the SBFormat API in a stand alone test in lldb/test/api/sbformat
so we can see if the errors work and the "operator bool" works as expected:
format = lldb.SBFormat()
self.assertFalse(format)
err = lldb.SBError()
format = lldb.SBFormat("${bad}", error)
self.assertFalse(format) # We expect an invalid object back if we have an error
self.assertTrue(error.Fail())
format = lldb.SBFormat("${frame.index}", error)
# Check the error string here as well
self.assertTrue(format)
self.assertTrue(error.Success())
When this option is enabled, display names of stack frames are generated using the `${function.name-with-args}` formatter instead of simply calling `SBFrame::GetDisplayFunctionName`. This makes lldb-dap show an output similar to the one in the CLI. This option is disabled by default because of its performance cost. It's a good option for non-gigantic programs.
96d3b9e
to
9dab558
Compare
✅ With the latest revision this PR passed the Python code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all of the changes. Looks great!
When this option gets enabled, descriptions of threads will be generated using the format provided in the launch configuration instead of generating it manually in the dap code. This allows lldb-dap to show an output similar to the one in the CLI. This is very similar to #71843
…1843) When this option gets enabled, descriptions of stack frames will be generated using the format provided in the launch configuration instead of simply calling `SBFrame::GetDisplayFunctionName`. This allows lldb-dap to show an output similar to the one in the CLI.
When this option gets enabled, descriptions of threads will be generated using the format provided in the launch configuration instead of generating it manually in the dap code. This allows lldb-dap to show an output similar to the one in the CLI. This is very similar to llvm#71843
…1843) When this option gets enabled, descriptions of stack frames will be generated using the format provided in the launch configuration instead of simply calling `SBFrame::GetDisplayFunctionName`. This allows lldb-dap to show an output similar to the one in the CLI. (cherry picked from commit d9ec4b2)
When this option gets enabled, descriptions of threads will be generated using the format provided in the launch configuration instead of generating it manually in the dap code. This allows lldb-dap to show an output similar to the one in the CLI. This is very similar to llvm#71843 (cherry picked from commit 1654d7d)
When this option gets enabled, descriptions of stack frames will be generated using the format provided in the launch configuration instead of simply calling
SBFrame::GetDisplayFunctionName
. This allows lldb-dap to show an output similar to the one in the CLI.