[Lldb-commits] [lldb] Revert "[lldb-dap] Validate utf8 protocol messages." (PR #181930)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 17 14:49:48 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: John Harrison (ashgti)
<details>
<summary>Changes</summary>
Reverts llvm/llvm-project#<!-- -->181261
Breaking builds on linux, reverting while I investigate.
---
Patch is 68.42 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/181930.diff
23 Files Affected:
- (modified) lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py (+13-33)
- (modified) lldb/test/API/tools/lldb-dap/variables/main.cpp (-4)
- (modified) lldb/tools/lldb-dap/DAP.cpp (+20-17)
- (modified) lldb/tools/lldb-dap/DAP.h (+4-6)
- (modified) lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp (+1-1)
- (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+8-6)
- (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+5-5)
- (modified) lldb/tools/lldb-dap/JSONUtils.h (+3-3)
- (modified) lldb/tools/lldb-dap/LLDBUtils.cpp (+2-2)
- (modified) lldb/tools/lldb-dap/LLDBUtils.h (+2-3)
- (modified) lldb/tools/lldb-dap/Protocol/DAPTypes.h (+4-4)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp (+13-42)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolBase.h (+9-99)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+4-6)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+50-50)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+67-67)
- (modified) lldb/tools/lldb-dap/ProtocolUtils.cpp (+3-2)
- (modified) lldb/tools/lldb-dap/tool/lldb-dap.cpp (+2-2)
- (modified) lldb/unittests/DAP/CMakeLists.txt (-1)
- (removed) lldb/unittests/DAP/ProtocolBaseTest.cpp (-29)
- (modified) lldb/unittests/DAP/ProtocolTypesTest.cpp (+1-1)
- (modified) lldb/unittests/DAP/TestBase.cpp (+1-1)
- (modified) lldb/unittests/DAP/VariablesTest.cpp (+1-2)
``````````diff
diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
index 9de3e1af2dd20..10c67a94407e6 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -193,8 +193,6 @@ def do_test_scopes_variables_setVariable_evaluate(
},
"readOnly": True,
},
- "valid_str": {},
- "malformed_str": {},
"x": {"equals": {"type": "int"}},
}
@@ -347,9 +345,9 @@ def do_test_scopes_variables_setVariable_evaluate(
verify_locals["argc"]["equals"]["value"] = "123"
verify_locals["pt"]["children"]["x"]["equals"]["value"] = "111"
- verify_locals["x @ main.cpp:23"] = {"equals": {"type": "int", "value": "89"}}
- verify_locals["x @ main.cpp:25"] = {"equals": {"type": "int", "value": "42"}}
- verify_locals["x @ main.cpp:27"] = {"equals": {"type": "int", "value": "72"}}
+ verify_locals["x @ main.cpp:19"] = {"equals": {"type": "int", "value": "89"}}
+ verify_locals["x @ main.cpp:21"] = {"equals": {"type": "int", "value": "42"}}
+ verify_locals["x @ main.cpp:23"] = {"equals": {"type": "int", "value": "72"}}
self.verify_variables(verify_locals, self.dap_server.get_local_variables())
@@ -357,22 +355,22 @@ def do_test_scopes_variables_setVariable_evaluate(
self.assertFalse(self.set_local("x2", 9)["success"])
self.assertFalse(self.set_local("x @ main.cpp:0", 9)["success"])
- self.assertTrue(self.set_local("x @ main.cpp:23", 19)["success"])
- self.assertTrue(self.set_local("x @ main.cpp:25", 21)["success"])
- self.assertTrue(self.set_local("x @ main.cpp:27", 23)["success"])
+ self.assertTrue(self.set_local("x @ main.cpp:19", 19)["success"])
+ self.assertTrue(self.set_local("x @ main.cpp:21", 21)["success"])
+ self.assertTrue(self.set_local("x @ main.cpp:23", 23)["success"])
# The following should have no effect
- self.assertFalse(self.set_local("x @ main.cpp:27", "invalid")["success"])
+ self.assertFalse(self.set_local("x @ main.cpp:23", "invalid")["success"])
- verify_locals["x @ main.cpp:23"]["equals"]["value"] = "19"
- verify_locals["x @ main.cpp:25"]["equals"]["value"] = "21"
- verify_locals["x @ main.cpp:27"]["equals"]["value"] = "23"
+ verify_locals["x @ main.cpp:19"]["equals"]["value"] = "19"
+ verify_locals["x @ main.cpp:21"]["equals"]["value"] = "21"
+ verify_locals["x @ main.cpp:23"]["equals"]["value"] = "23"
self.verify_variables(verify_locals, self.dap_server.get_local_variables())
# The plain x variable shold refer to the innermost x
self.assertTrue(self.set_local("x", 22)["success"])
- verify_locals["x @ main.cpp:27"]["equals"]["value"] = "22"
+ verify_locals["x @ main.cpp:23"]["equals"]["value"] = "22"
self.verify_variables(verify_locals, self.dap_server.get_local_variables())
@@ -389,9 +387,9 @@ def do_test_scopes_variables_setVariable_evaluate(
names = [var["name"] for var in locals]
# The first shadowed x shouldn't have a suffix anymore
verify_locals["x"] = {"equals": {"type": "int", "value": "19"}}
+ self.assertNotIn("x @ main.cpp:19", names)
+ self.assertNotIn("x @ main.cpp:21", names)
self.assertNotIn("x @ main.cpp:23", names)
- self.assertNotIn("x @ main.cpp:25", names)
- self.assertNotIn("x @ main.cpp:27", names)
self.verify_variables(verify_locals, locals)
@@ -462,22 +460,6 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
},
"readOnly": True,
},
- "valid_str": {
- "equals": {
- "type": "const char *",
- },
- "matches": {
- "value": r'0x\w+ "πΆπ°LπΎπ CππΌπ΄π"',
- },
- },
- "malformed_str": {
- "equals": {
- "type": "const char *",
- },
- "matches": {
- "value": r'0x\w+ "lone trailing \\x81\\x82 bytes"',
- },
- },
"x": {
"equals": {"type": "int"},
"missing": ["indexedVariables"],
@@ -718,8 +700,6 @@ def test_return_variables(self):
"argc": {},
"argv": {},
"pt": {"readOnly": True},
- "valid_str": {},
- "malformed_str": {},
"x": {},
"return_result": {"equals": {"type": "int"}},
}
diff --git a/lldb/test/API/tools/lldb-dap/variables/main.cpp b/lldb/test/API/tools/lldb-dap/variables/main.cpp
index 04fc62f02c22f..0e363001f2f42 100644
--- a/lldb/test/API/tools/lldb-dap/variables/main.cpp
+++ b/lldb/test/API/tools/lldb-dap/variables/main.cpp
@@ -5,7 +5,6 @@ struct PointType {
int y;
int buffer[BUFFER_SIZE];
};
-#include <cstdio>
#include <vector>
int g_global = 123;
static int s_global = 234;
@@ -17,9 +16,6 @@ int main(int argc, char const *argv[]) {
PointType pt = {11, 22, {0}};
for (int i = 0; i < BUFFER_SIZE; ++i)
pt.buffer[i] = i;
- const char *valid_str = "πΆπ°LπΎπ CππΌπ΄π";
- const char *malformed_str = "lone trailing \x81\x82 bytes";
- printf("print malformed utf8 %s %s\n", valid_str, malformed_str);
int x = s_global - g_global - pt.y; // breakpoint 1
{
int x = 42;
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index fcfd1da3878e8..6d5e175ae29e0 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -124,7 +124,7 @@ static std::string capitalize(llvm::StringRef str) {
llvm::StringRef DAP::debug_adapter_path = "";
DAP::DAP(Log &log, const ReplMode default_repl_mode,
- const std::vector<String> &pre_init_commands, bool no_lldbinit,
+ std::vector<std::string> pre_init_commands, bool no_lldbinit,
llvm::StringRef client_name, DAPTransport &transport, MainLoop &loop)
: log(log), transport(transport), reference_storage(log),
broadcaster("lldb-dap"),
@@ -132,7 +132,7 @@ DAP::DAP(Log &log, const ReplMode default_repl_mode,
[&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }),
repl_mode(default_repl_mode), no_lldbinit(no_lldbinit),
m_client_name(client_name), m_loop(loop) {
- configuration.preInitCommands = pre_init_commands;
+ configuration.preInitCommands = std::move(pre_init_commands);
RegisterRequests();
}
@@ -413,10 +413,9 @@ void DAP::SendOutput(OutputType o, const llvm::StringRef output) {
if (end == llvm::StringRef::npos)
end = output.size() - 1;
llvm::json::Object event(CreateEventObject("output"));
- llvm::json::Object body{
- {"category", category},
- {"output", protocol::String(output.slice(idx, end + 1))},
- };
+ llvm::json::Object body;
+ body.try_emplace("category", category);
+ EmplaceSafeString(body, "output", output.slice(idx, end + 1).str());
event.try_emplace("body", std::move(body));
SendJSON(llvm::json::Value(std::move(event)));
idx = end + 1;
@@ -724,7 +723,7 @@ DAP::ResolveAssemblySource(lldb::SBAddress address) {
}
bool DAP::RunLLDBCommands(llvm::StringRef prefix,
- llvm::ArrayRef<String> commands) {
+ llvm::ArrayRef<std::string> commands) {
bool required_command_failed = false;
std::string output = ::RunLLDBCommands(
debugger, prefix, commands, required_command_failed,
@@ -743,13 +742,15 @@ static llvm::Error createRunLLDBCommandsErrorMessage(llvm::StringRef category) {
.c_str());
}
-llvm::Error DAP::RunAttachCommands(llvm::ArrayRef<String> attach_commands) {
+llvm::Error
+DAP::RunAttachCommands(llvm::ArrayRef<std::string> attach_commands) {
if (!RunLLDBCommands("Running attachCommands:", attach_commands))
return createRunLLDBCommandsErrorMessage("attach");
return llvm::Error::success();
}
-llvm::Error DAP::RunLaunchCommands(llvm::ArrayRef<String> launch_commands) {
+llvm::Error
+DAP::RunLaunchCommands(llvm::ArrayRef<std::string> launch_commands) {
if (!RunLLDBCommands("Running launchCommands:", launch_commands))
return createRunLLDBCommandsErrorMessage("launch");
return llvm::Error::success();
@@ -801,9 +802,9 @@ lldb::SBTarget DAP::CreateTarget(lldb::SBError &error) {
// omitted at all), so it is good to leave the user an opportunity to specify
// those. Any of those three can be left empty.
auto target = this->debugger.CreateTarget(
- /*filename=*/configuration.program.c_str(),
- /*target_triple=*/configuration.targetTriple.c_str(),
- /*platform_name=*/configuration.platformName.c_str(),
+ /*filename=*/configuration.program.data(),
+ /*target_triple=*/configuration.targetTriple.data(),
+ /*platform_name=*/configuration.platformName.data(),
/*add_dependent_modules=*/true, // Add dependent modules.
error);
@@ -848,7 +849,8 @@ bool DAP::HandleObject(const Message &M) {
});
auto handler_pos = request_handlers.find(req->command);
- dispatcher.Set("client_data", "request_command:" + req->command);
+ dispatcher.Set("client_data",
+ llvm::Twine("request_command:", req->command).str());
if (handler_pos != request_handlers.end()) {
handler_pos->second->Run(*req);
} else {
@@ -876,13 +878,14 @@ bool DAP::HandleObject(const Message &M) {
// Result should be given, use null if not.
if (resp->success) {
(*response_handler)(resp->body);
- dispatcher.Set("client_data", "response_command:" + resp->command);
+ dispatcher.Set("client_data",
+ llvm::Twine("response_command:", resp->command).str());
} else {
llvm::StringRef message = "Unknown error, response failed";
if (resp->message) {
message =
std::visit(llvm::makeVisitor(
- [](const String &message) -> llvm::StringRef {
+ [](const std::string &message) -> llvm::StringRef {
return message;
},
[](const protocol::ResponseMessage &message)
@@ -966,7 +969,7 @@ void DAP::ClearCancelRequest(const CancelArguments &args) {
template <typename T>
static std::optional<T> getArgumentsIfRequest(const Request &req,
- const protocol::String &command) {
+ llvm::StringLiteral command) {
if (req.command != command)
return std::nullopt;
@@ -1264,7 +1267,7 @@ protocol::Capabilities DAP::GetCapabilities() {
capabilities.exceptionBreakpointFilters = std::move(filters);
// FIXME: This should be registered based on the supported languages?
- std::vector<String> completion_characters;
+ std::vector<std::string> completion_characters;
completion_characters.emplace_back(".");
// FIXME: I wonder if we should remove this key... its very aggressive
// triggering and accepting completions.
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 9457628aa0f42..a164cc484f4be 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -194,7 +194,7 @@ struct DAP final : public DAPTransport::MessageHandler {
/// \param[in] loop
/// Main loop associated with this instance.
DAP(Log &log, const ReplMode default_repl_mode,
- const std::vector<protocol::String> &pre_init_commands, bool no_lldbinit,
+ std::vector<std::string> pre_init_commands, bool no_lldbinit,
llvm::StringRef client_name, DAPTransport &transport,
lldb_private::MainLoop &loop);
@@ -314,12 +314,10 @@ struct DAP final : public DAPTransport::MessageHandler {
/// \b false if a fatal error was found while executing these commands,
/// according to the rules of \a LLDBUtils::RunLLDBCommands.
bool RunLLDBCommands(llvm::StringRef prefix,
- llvm::ArrayRef<protocol::String> commands);
+ llvm::ArrayRef<std::string> commands);
- llvm::Error
- RunAttachCommands(llvm::ArrayRef<protocol::String> attach_commands);
- llvm::Error
- RunLaunchCommands(llvm::ArrayRef<protocol::String> launch_commands);
+ llvm::Error RunAttachCommands(llvm::ArrayRef<std::string> attach_commands);
+ llvm::Error RunLaunchCommands(llvm::ArrayRef<std::string> launch_commands);
llvm::Error RunPreInitCommands();
llvm::Error RunInitCommands();
llvm::Error RunPreRunCommands();
diff --git a/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp
index d24072c8cc05d..e1a3b15b4697b 100644
--- a/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp
@@ -30,7 +30,7 @@ llvm::Expected<CompileUnitsResponseBody> CompileUnitsRequestHandler::Run(
int num_modules = dap.target.GetNumModules();
for (int i = 0; i < num_modules; i++) {
auto curr_module = dap.target.GetModuleAtIndex(i);
- if (args->moduleId == curr_module.GetUUIDString()) {
+ if (args->moduleId == llvm::StringRef(curr_module.GetUUIDString())) {
int num_units = curr_module.GetNumCompileUnits();
for (int j = 0; j < num_units; j++) {
auto curr_unit = curr_module.GetCompileUnitAtIndex(j);
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
index 5e8c2163c838f..47ae9a7195a7d 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
@@ -37,7 +37,8 @@ using namespace lldb_dap::protocol;
namespace lldb_dap {
-static std::vector<const char *> MakeArgv(const llvm::ArrayRef<String> &strs) {
+static std::vector<const char *>
+MakeArgv(const llvm::ArrayRef<std::string> &strs) {
// Create and return an array of "const char *", one for each C string in
// "strs" and terminate the list with a NULL. This can be used for argument
// vectors (argv) or environment vectors (envp) like those passed to the
@@ -59,8 +60,9 @@ static uint32_t SetLaunchFlag(uint32_t flags, bool flag,
return flags;
}
-static void SetupIORedirection(const std::vector<std::optional<String>> &stdio,
- lldb::SBLaunchInfo &launch_info) {
+static void
+SetupIORedirection(const std::vector<std::optional<std::string>> &stdio,
+ lldb::SBLaunchInfo &launch_info) {
for (const auto &[idx, value_opt] : llvm::enumerate(stdio)) {
if (!value_opt)
continue;
@@ -189,7 +191,7 @@ void BaseRequestHandler::Run(const Request &request) {
llvm::Error BaseRequestHandler::LaunchProcess(
const protocol::LaunchRequestArguments &arguments) const {
- const std::vector<String> &launchCommands = arguments.launchCommands;
+ const std::vector<std::string> &launchCommands = arguments.launchCommands;
// Instantiate a launch info instance for the target.
auto launch_info = dap.target.GetLaunchInfo();
@@ -338,8 +340,8 @@ void BaseRequestHandler::BuildErrorResponse(
error_message.format = err.getMessage();
error_message.showUser = err.getShowUser();
error_message.id = err.convertToErrorCode().value();
- error_message.url = err.getURL().value_or("");
- error_message.urlLabel = err.getURLLabel().value_or("");
+ error_message.url = err.getURL();
+ error_message.urlLabel = err.getURLLabel();
protocol::ErrorResponseBody body;
body.error = error_message;
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 460d2a46049c6..5bcc2f9c71c2d 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -462,10 +462,10 @@ std::pair<int64_t, bool> UnpackLocation(int64_t location_id) {
/// See
/// https://microsoft.github.io/debug-adapter-protocol/specification#Reverse_Requests_RunInTerminal
llvm::json::Object CreateRunInTerminalReverseRequest(
- llvm::StringRef program, const std::vector<protocol::String> &args,
- const llvm::StringMap<protocol::String> &env, llvm::StringRef cwd,
+ llvm::StringRef program, const std::vector<std::string> &args,
+ const llvm::StringMap<std::string> &env, llvm::StringRef cwd,
llvm::StringRef comm_file, lldb::pid_t debugger_pid,
- const std::vector<std::optional<protocol::String>> &stdio, bool external) {
+ const std::vector<std::optional<std::string>> &stdio, bool external) {
llvm::json::Object run_in_terminal_args;
if (external) {
// This indicates the IDE to open an external terminal window.
@@ -488,10 +488,10 @@ llvm::json::Object CreateRunInTerminalReverseRequest(
std::stringstream ss;
std::string_view delimiter;
- for (const std::optional<protocol::String> &file : stdio) {
+ for (const std::optional<std::string> &file : stdio) {
ss << std::exchange(delimiter, ":");
if (file)
- ss << file->str();
+ ss << *file;
}
req_args.push_back(ss.str());
}
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index e12fb5b937bac..232b1810a3cf4 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -322,10 +322,10 @@ std::pair<int64_t, bool> UnpackLocation(int64_t location_id);
/// A "runInTerminal" JSON object that follows the specification outlined by
/// Microsoft.
llvm::json::Object CreateRunInTerminalReverseRequest(
- llvm::StringRef program, const std::vector<protocol::String> &args,
- const llvm::StringMap<protocol::String> &env, llvm::StringRef cwd,
+ llvm::StringRef program, const std::vector<std::string> &args,
+ const llvm::StringMap<std::string> &env, llvm::StringRef cwd,
llvm::StringRef comm_file, lldb::pid_t debugger_pid,
- const std::vector<std::optional<protocol::String>> &stdio, bool external);
+ const std::vector<std::optional<std::string>> &stdio, bool external);
/// Create a "Terminated" JSON object that contains statistics
///
diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp b/lldb/tools/lldb-dap/LLDBUtils.cpp
index 7e69813477b29..e7407e9da9efb 100644
--- a/lldb/tools/lldb-dap/LLDBUtils.cpp
+++ b/lldb/tools/lldb-dap/LLDBUtils.cpp
@@ -32,7 +32,7 @@
namespace lldb_dap {
bool RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix,
- const llvm::ArrayRef<protocol::String> &commands,
+ const llvm::ArrayRef<std::string> &commands,
llvm::raw_ostream &strm, bool parse_command_directives,
bool echo_commands) {
if (commands.empty())
@@ -115,7 +115,7 @@ bool RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix,
}
std::string RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix,
- const llvm::ArrayRef<protocol::String> &commands,
+ const llvm::ArrayRef<std::string> &commands,
bool &required_command_failed,
bool parse_command_directives, bool echo_commands) {
required_command_failed = false;
diff --git a/lldb/tools/lldb-dap/LLDBUtils.h b/lldb/tools/lldb-dap/LLDBUtils.h
index 37757ada7ab4d..19174ba59654d 100644
--- a/lldb/tools/lldb-dap/LLDBUtils.h
+++ b/lldb/tools/lldb-dap/LLDBUtils.h
@@ -10,7 +10,6 @@
#define LLDB_TOOLS_LLDB_DAP_LLDBUTILS_H
#include "DAPForward.h"
-#include "Protocol/ProtocolBase.h"
#include "lldb/API/SBDebugger.h"
#include "lldb/API/SBEnvironment.h"
#include "lldb/API/SBError.h"
@@ -64,7 +63,7 @@ namespace lldb_dap {
/// \b true, unless a command prefixed with \b ! fails and parsing of
/// command directives is enabled.
bool RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix,
- const llvm::ArrayRef<protocol::String> &commands,
+ const llvm::ArrayRef<std::string> &commands,
llvm::raw_ostream &strm, bool parse_command_directives,
bool echo_commands);
@@ -98,7 +97,7 @@ bool RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix,
/// A std::string tha...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/181930
More information about the lldb-commits
mailing list