[Lldb-commits] [lldb] [lldb-dap] Introduce the new privateConfiguration setting (PR #74748)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Dec 7 11:03:43 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Walter Erquinigo (walter-erquinigo)
<details>
<summary>Changes</summary>
There are some typescript vscode extensions that are effectively wrappers of lldb-dap, and they commonly configure the debug adapter in particular ways using initCommands, among other settings. An unfortunate side effect is that, at least for initCommands, their output is printed on the Debug Console, which doesn't look clean and might confuse some users because.
As a way to improve the experience, I'm definining a new `privateConfiguration` setting, which can be used by adapter wrappers for private initialization. I'm starting with private initCommands, whose output can also be controlled via a `printMode` setting. By default, the output is only printed upon errors.
This setting helps distinguish things that the adapter wrapper does privately from what the user might want to do using public settings in JSON.
---
Full diff: https://github.com/llvm/llvm-project/pull/74748.diff
11 Files Affected:
- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+3)
- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+4)
- (added) lldb/test/API/tools/lldb-dap/privateConfiguration/Makefile (+2)
- (added) lldb/test/API/tools/lldb-dap/privateConfiguration/TestDAP_privateConfiguration.py (+77)
- (added) lldb/test/API/tools/lldb-dap/privateConfiguration/main.cpp (+1)
- (modified) lldb/tools/lldb-dap/DAP.cpp (+30-3)
- (modified) lldb/tools/lldb-dap/DAP.h (+27-2)
- (modified) lldb/tools/lldb-dap/LLDBUtils.cpp (+11-6)
- (modified) lldb/tools/lldb-dap/LLDBUtils.h (+10-5)
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+6-1)
- (modified) lldb/tools/lldb-dap/package.json (+34)
``````````diff
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 bb863bb8719176..4f2bff7ad5101d 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
@@ -734,6 +734,7 @@ def request_launch(
commandEscapePrefix=None,
customFrameFormat=None,
customThreadFormat=None,
+ privateConfiguration=None,
):
args_dict = {"program": program}
if args:
@@ -779,6 +780,8 @@ def request_launch(
args_dict["customFrameFormat"] = customFrameFormat
if customThreadFormat:
args_dict["customThreadFormat"] = customThreadFormat
+ if privateConfiguration:
+ args_dict["privateConfiguration"] = privateConfiguration
args_dict["enableAutoVariableSummaries"] = enableAutoVariableSummaries
args_dict["enableSyntheticChildDebugging"] = enableSyntheticChildDebugging
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 4ccd6014e54be6..f1bf4decbe0779 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
@@ -357,6 +357,7 @@ def launch(
commandEscapePrefix=None,
customFrameFormat=None,
customThreadFormat=None,
+ privateConfiguration=None,
):
"""Sending launch request to dap"""
@@ -398,6 +399,7 @@ def cleanup():
commandEscapePrefix=commandEscapePrefix,
customFrameFormat=customFrameFormat,
customThreadFormat=customThreadFormat,
+ privateConfiguration=privateConfiguration,
)
if expectFailure:
@@ -437,6 +439,7 @@ def build_and_launch(
commandEscapePrefix=None,
customFrameFormat=None,
customThreadFormat=None,
+ privateConfiguration=None,
):
"""Build the default Makefile target, create the DAP debug adaptor,
and launch the process.
@@ -470,4 +473,5 @@ def build_and_launch(
commandEscapePrefix=commandEscapePrefix,
customFrameFormat=customFrameFormat,
customThreadFormat=customThreadFormat,
+ privateConfiguration=privateConfiguration,
)
diff --git a/lldb/test/API/tools/lldb-dap/privateConfiguration/Makefile b/lldb/test/API/tools/lldb-dap/privateConfiguration/Makefile
new file mode 100644
index 00000000000000..3d0b98f13f3d7b
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/privateConfiguration/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/privateConfiguration/TestDAP_privateConfiguration.py b/lldb/test/API/tools/lldb-dap/privateConfiguration/TestDAP_privateConfiguration.py
new file mode 100644
index 00000000000000..0d930dcbc80e38
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/privateConfiguration/TestDAP_privateConfiguration.py
@@ -0,0 +1,77 @@
+"""
+Test lldb-dap stack trace response
+"""
+
+
+import os
+
+import dap_server
+import lldbdap_testcase
+from lldbsuite.test import lldbtest, lldbutil
+from lldbsuite.test.decorators import *
+
+
+class TestDAP_privateConfiguration(lldbdap_testcase.DAPTestCaseBase):
+ def do_test_initCommands(self, printMode, includeError=False):
+ """
+ Test the initCommands property of the privateConfiguration setting and
+ its various print modes.
+ """
+ commands = [
+ "settings set target.show-hex-variable-values-with-leading-zeroes false"
+ ]
+ if includeError:
+ commands.append(
+ "settings set target.show-hex-variable-values-with-leading-zeroes fooooo"
+ )
+
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(
+ program,
+ privateConfiguration={
+ "initCommands": {
+ "commands": commands,
+ "printMode": printMode,
+ }
+ },
+ )
+ full_output = self.collect_console(duration=1.0)
+ expected_output = """Running privateInitCommands:
+(lldb) settings set target.show-hex-variable-values-with-leading-zeroes false"""
+ expected_error_output = "error: invalid boolean string value: 'fooooo'"
+
+ if printMode == "always":
+ self.assertIn(expected_output, full_output)
+ if includeError:
+ self.assertIn(expected_error_output, full_output)
+ elif printMode == "never":
+ self.assertNotIn(expected_output, full_output)
+ if includeError:
+ self.assertNotIn(expected_error_output, full_output)
+ else:
+ if includeError:
+ self.assertIn(expected_output, full_output)
+ self.assertIn(expected_error_output, full_output)
+ else:
+ self.assertNotIn(expected_output, full_output)
+ self.assertNotIn(expected_error_output, full_output)
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_initCommands_with_print_mode_always(self):
+ self.do_test_initCommands("always")
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_initCommands_with_print_mode_never(self):
+ self.do_test_initCommands("never", includeError=True)
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_initCommands_with_print_mode_onError_with_failure(self):
+ self.do_test_initCommands("onError", includeError=True)
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_initCommands_with_print_mode_onError_no_actual_failures(self):
+ self.do_test_initCommands("onError", includeError=False)
diff --git a/lldb/test/API/tools/lldb-dap/privateConfiguration/main.cpp b/lldb/test/API/tools/lldb-dap/privateConfiguration/main.cpp
new file mode 100644
index 00000000000000..76e8197013aabc
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/privateConfiguration/main.cpp
@@ -0,0 +1 @@
+int main() { return 0; }
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 49266209739765..75549ff3a11c09 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -435,9 +435,19 @@ ExpressionContext DAP::DetectExpressionContext(lldb::SBFrame &frame,
}
void DAP::RunLLDBCommands(llvm::StringRef prefix,
- const std::vector<std::string> &commands) {
- SendOutput(OutputType::Console,
- llvm::StringRef(::RunLLDBCommands(prefix, commands)));
+ llvm::ArrayRef<std::string> commands,
+ CommandsPrintMode print_mode) {
+ auto [output, success] = ::RunLLDBCommands(prefix, commands);
+ if (print_mode == CommandsPrintMode::Always ||
+ (!success && print_mode == CommandsPrintMode::OnError)) {
+ SendOutput(OutputType::Console, output);
+ }
+}
+
+void DAP::RunPrivateInitCommands() {
+ RunLLDBCommands("Running privateInitCommands:",
+ private_configuration.init_commands.commands,
+ private_configuration.init_commands.print_mode);
}
void DAP::RunInitCommands() {
@@ -854,4 +864,21 @@ void DAP::SetThreadFormat(llvm::StringRef format) {
}
}
+void DAP::ParsePrivateConfiguration(
+ const llvm::json::Object *private_configuration) {
+ if (!private_configuration)
+ return;
+ if (auto *init_commands = private_configuration->getObject("initCommands")) {
+ if (std::optional<llvm::StringRef> print_mode =
+ init_commands->getString("printMode")) {
+ this->private_configuration.init_commands.print_mode =
+ print_mode == "onError" ? CommandsPrintMode::OnError
+ : print_mode == "never" ? CommandsPrintMode::Never
+ : CommandsPrintMode::Always;
+ }
+ this->private_configuration.init_commands.commands =
+ GetStrings(init_commands, "commands");
+ }
+}
+
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index c7d56a06bfa1fd..f75500d4362bd5 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -143,6 +143,16 @@ struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface {
};
struct DAP {
+ /// Enum that controls when to print the output of a series of LLDB commands.
+ enum CommandsPrintMode {
+ /// The output is always printed to the user.
+ Always,
+ /// The output is only printed if one of the commands fails.
+ OnError,
+ /// The output is never printed.
+ Never,
+ };
+
std::string debug_adaptor_path;
InputStream input;
OutputStream output;
@@ -161,6 +171,14 @@ struct DAP {
std::vector<std::string> exit_commands;
std::vector<std::string> stop_commands;
std::vector<std::string> terminate_commands;
+
+ struct PrivateConfiguration {
+ struct {
+ std::vector<std::string> commands;
+ CommandsPrintMode print_mode = CommandsPrintMode::OnError;
+ } init_commands;
+ } private_configuration;
+
// A copy of the last LaunchRequest or AttachRequest so we can reuse its
// arguments if we get a RestartRequest.
std::optional<llvm::json::Object> last_launch_or_attach_request;
@@ -227,9 +245,11 @@ struct DAP {
ExpressionContext DetectExpressionContext(lldb::SBFrame &frame,
std::string &text);
- void RunLLDBCommands(llvm::StringRef prefix,
- const std::vector<std::string> &commands);
+ void
+ RunLLDBCommands(llvm::StringRef prefix, llvm::ArrayRef<std::string> commands,
+ CommandsPrintMode print_mode = CommandsPrintMode::Always);
+ void RunPrivateInitCommands();
void RunInitCommands();
void RunPreRunCommands();
void RunStopCommands();
@@ -312,6 +332,11 @@ struct DAP {
void SetThreadFormat(llvm::StringRef format);
+ /// Parse the `privateConfiguration` JSON object. If \b nullptr, this method
+ /// does nothing.
+ void
+ ParsePrivateConfiguration(const llvm::json::Object *private_configuration);
+
private:
// Send the JSON in "json_str" to the "out" stream. Correctly send the
// "Content-Length:" field followed by the length, followed by the raw
diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp b/lldb/tools/lldb-dap/LLDBUtils.cpp
index 955c11f636895b..25989f404de875 100644
--- a/lldb/tools/lldb-dap/LLDBUtils.cpp
+++ b/lldb/tools/lldb-dap/LLDBUtils.cpp
@@ -11,11 +11,13 @@
namespace lldb_dap {
-void RunLLDBCommands(llvm::StringRef prefix,
+bool RunLLDBCommands(llvm::StringRef prefix,
const llvm::ArrayRef<std::string> &commands,
llvm::raw_ostream &strm) {
if (commands.empty())
- return;
+ return true;
+ bool success = true;
+
lldb::SBCommandInterpreter interp = g_dap.debugger.GetCommandInterpreter();
if (!prefix.empty())
strm << prefix << "\n";
@@ -32,17 +34,20 @@ void RunLLDBCommands(llvm::StringRef prefix,
if (error_len) {
const char *error = result.GetError();
strm << error;
+ success = false;
}
}
+ return success;
}
-std::string RunLLDBCommands(llvm::StringRef prefix,
- const llvm::ArrayRef<std::string> &commands) {
+std::pair<std::string, bool>
+RunLLDBCommands(llvm::StringRef prefix,
+ const llvm::ArrayRef<std::string> &commands) {
std::string s;
llvm::raw_string_ostream strm(s);
- RunLLDBCommands(prefix, commands, strm);
+ bool success = RunLLDBCommands(prefix, commands, strm);
strm.flush();
- return s;
+ return {s, success};
}
bool ThreadHasStopReason(lldb::SBThread &thread) {
diff --git a/lldb/tools/lldb-dap/LLDBUtils.h b/lldb/tools/lldb-dap/LLDBUtils.h
index a99f798835370d..e329a30107adbb 100644
--- a/lldb/tools/lldb-dap/LLDBUtils.h
+++ b/lldb/tools/lldb-dap/LLDBUtils.h
@@ -33,7 +33,10 @@ namespace lldb_dap {
/// \param[in] strm
/// The stream that will receive the prefix, prompt + command and
/// all command output.
-void RunLLDBCommands(llvm::StringRef prefix,
+///
+/// \return
+/// \b true if and only if all the commands executed successfully.
+bool RunLLDBCommands(llvm::StringRef prefix,
const llvm::ArrayRef<std::string> &commands,
llvm::raw_ostream &strm);
@@ -50,10 +53,12 @@ void RunLLDBCommands(llvm::StringRef prefix,
/// An array of LLDB commands to execute.
///
/// \return
-/// A std::string that contains the prefix and all commands and
-/// command output
-std::string RunLLDBCommands(llvm::StringRef prefix,
- const llvm::ArrayRef<std::string> &commands);
+/// A \a std::string that contains the prefix and all commands and
+/// command output, along with a \a bool that signals whether the
+/// entire execution of commands succeeded or not.
+std::pair<std::string, bool>
+RunLLDBCommands(llvm::StringRef prefix,
+ const llvm::ArrayRef<std::string> &commands);
/// Check if a thread has a stop reason.
///
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index d6b593eba93eca..c082289e16e2f4 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -634,6 +634,7 @@ void request_attach(const llvm::json::Object &request) {
attach_info.SetProcessID(pid);
const auto wait_for = GetBoolean(arguments, "waitFor", false);
attach_info.SetWaitForLaunch(wait_for, false /*async*/);
+ g_dap.ParsePrivateConfiguration(arguments->getObject("privateConfiguration"));
g_dap.init_commands = GetStrings(arguments, "initCommands");
g_dap.pre_run_commands = GetStrings(arguments, "preRunCommands");
g_dap.stop_commands = GetStrings(arguments, "stopCommands");
@@ -664,6 +665,7 @@ void request_attach(const llvm::json::Object &request) {
llvm::sys::fs::set_current_path(debuggerRoot);
// Run any initialize LLDB commands the user specified in the launch.json
+ g_dap.RunPrivateInitCommands();
g_dap.RunInitCommands();
SetSourceMapFromArguments(*arguments);
@@ -1270,7 +1272,8 @@ void request_evaluate(const llvm::json::Object &request) {
if (frame.IsValid()) {
g_dap.focus_tid = frame.GetThread().GetThreadID();
}
- auto result = RunLLDBCommands(llvm::StringRef(), {std::string(expression)});
+ auto result =
+ RunLLDBCommands(llvm::StringRef(), {std::string(expression)}).first;
EmplaceSafeString(body, "result", result);
body.try_emplace("variablesReference", (int64_t)0);
} else {
@@ -1792,6 +1795,7 @@ void request_launch(const llvm::json::Object &request) {
llvm::json::Object response;
FillResponse(request, response);
auto arguments = request.getObject("arguments");
+ g_dap.ParsePrivateConfiguration(arguments->getObject("privateConfiguration"));
g_dap.init_commands = GetStrings(arguments, "initCommands");
g_dap.pre_run_commands = GetStrings(arguments, "preRunCommands");
g_dap.stop_commands = GetStrings(arguments, "stopCommands");
@@ -1820,6 +1824,7 @@ void request_launch(const llvm::json::Object &request) {
// Run any initialize LLDB commands the user specified in the launch.json.
// This is run before target is created, so commands can't do anything with
// the targets - preRunCommands are run with the target.
+ g_dap.RunPrivateInitCommands();
g_dap.RunInitCommands();
SetSourceMapFromArguments(*arguments);
diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json
index ebb1103d695e17..a7210d8356112b 100644
--- a/lldb/tools/lldb-dap/package.json
+++ b/lldb/tools/lldb-dap/package.json
@@ -265,6 +265,23 @@
"type": "string",
"description": "If non-empty, threads will have descriptions generated based on the provided format. See https://lldb.llvm.org/use/formatting.html for an explanation on format strings for threads. If the format string contains errors, an error message will be displayed on the Debug Console and the default thread names will be used. This might come with a performance cost because debug information might need to be processed to generate the description.",
"default": ""
+ },
+ "privateConfiguration": {
+ "description": "Additional settings that extensions that wrap `lldb-dap` can use to configure the debugger without interfering with the other settings, which are meant to be used directly by the end user.",
+ "properties": {
+ "initCommands": {
+ "properties": {
+ "commands": {
+ "type": "array",
+ "description": "Initialization commands executed upon debugger startup. It is executed before the public `initCommands` and its output can be controlled by the `printMode` property."
+ },
+ "printMode": {
+ "type": "string",
+ "description": "If \"always\", then the output of the private `initCommands` is always printed. If \"onError\", then the output is only printed if any of the commands fails. Otherwise, if \"never\", then the output is never printed. Defaults to \"onError\"."
+ }
+ }
+ }
+ }
}
}
},
@@ -369,6 +386,23 @@
"type": "string",
"description": "If non-empty, threads will have descriptions generated based on the provided format. See https://lldb.llvm.org/use/formatting.html for an explanation on format strings for threads. If the format string contains errors, an error message will be displayed on the Debug Console and the default thread names will be used. This might come with a performance cost because debug information might need to be processed to generate the description.",
"default": ""
+ },
+ "privateConfiguration": {
+ "description": "Additional settings that extensions that wrap `lldb-dap` can use to configure the debugger without interfering with the other settings, which are meant to be used directly by the end user.",
+ "properties": {
+ "initCommands": {
+ "properties": {
+ "commands": {
+ "type": "array",
+ "description": "Initialization commands executed upon debugger startup. It is executed before the public `initCommands` and its output can be controlled by the `printMode` property."
+ },
+ "printMode": {
+ "type": "string",
+ "description": "If \"always\", then the output of the private `initCommands` is always printed. If \"onError\", then the output is only printed if any of the commands fails. Otherwise, if \"never\", then the output is never printed. Defaults to \"onError\"."
+ }
+ }
+ }
+ }
}
}
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/74748
More information about the lldb-commits
mailing list