Author: Da-Viper Date: 2024-10-07T12:38:36-04:00 New Revision: d4c17891126a79ae49237a7de0f9948aeedcd177
URL: https://github.com/llvm/llvm-project/commit/d4c17891126a79ae49237a7de0f9948aeedcd177 DIFF: https://github.com/llvm/llvm-project/commit/d4c17891126a79ae49237a7de0f9948aeedcd177.diff LOG: Make env and source map dictionaries #95137 (#106919) Fixes #95137 Added: Modified: lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py lldb/tools/lldb-dap/JSONUtils.cpp lldb/tools/lldb-dap/JSONUtils.h lldb/tools/lldb-dap/LLDBUtils.cpp lldb/tools/lldb-dap/LLDBUtils.h lldb/tools/lldb-dap/README.md lldb/tools/lldb-dap/lldb-dap.cpp lldb/tools/lldb-dap/package.json Removed: ################################################################################ diff --git a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py index b85b6048439639..5189435185607f 100644 --- a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py +++ b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py @@ -58,7 +58,7 @@ def test_core_file(self): self.assertEqual(self.get_stackFrames(), expected_frames) @skipIfLLVMTargetMissing("X86") - def test_core_file_source_mapping(self): + def test_core_file_source_mapping_array(self): """Test that sourceMap property is correctly applied when loading a core""" current_dir = os.path.dirname(__file__) exe_file = os.path.join(current_dir, "linux-x86_64.out") @@ -70,3 +70,17 @@ def test_core_file_source_mapping(self): self.attach(exe_file, coreFile=core_file, sourceMap=source_map) self.assertIn(current_dir, self.get_stackFrames()[0]["source"]["path"]) + + @skipIfLLVMTargetMissing("X86") + def test_core_file_source_mapping_object(self): + """Test that sourceMap property is correctly applied when loading a core""" + current_dir = os.path.dirname(__file__) + exe_file = os.path.join(current_dir, "linux-x86_64.out") + core_file = os.path.join(current_dir, "linux-x86_64.core") + + self.create_debug_adaptor() + + source_map = {"/home/labath/test": current_dir} + self.attach(exe_file, coreFile=core_file, sourceMap=source_map) + + self.assertIn(current_dir, self.get_stackFrames()[0]["source"]["path"]) diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py index a16f2da3c4df71..7898d01457afc4 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -224,12 +224,47 @@ def test_args(self): 'arg[%i] "%s" not in "%s"' % (i + 1, quoted_arg, lines[i]), ) - def test_environment(self): + def test_environment_with_object(self): + """ + Tests launch of a simple program with environment variables + """ + program = self.getBuildArtifact("a.out") + env = { + "NO_VALUE": "", + "WITH_VALUE": "BAR", + "EMPTY_VALUE": "", + "SPACE": "Hello World", + } + + self.build_and_launch(program, env=env) + self.continue_to_exit() + + # Now get the STDOUT and verify our arguments got passed correctly + output = self.get_stdout() + self.assertTrue(output and len(output) > 0, "expect program output") + lines = output.splitlines() + # Skip the all arguments so we have only environment vars left + while len(lines) and lines[0].startswith("arg["): + lines.pop(0) + # Make sure each environment variable in "env" is actually set in the + # program environment that was printed to STDOUT + for var in env: + found = False + for program_var in lines: + if var in program_var: + found = True + break + self.assertTrue( + found, '"%s" must exist in program environment (%s)' % (var, lines) + ) + + def test_environment_with_array(self): """ Tests launch of a simple program with environment variables """ program = self.getBuildArtifact("a.out") env = ["NO_VALUE", "WITH_VALUE=BAR", "EMPTY_VALUE=", "SPACE=Hello World"] + self.build_and_launch(program, env=env) self.continue_to_exit() diff --git a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py index b214b512c0de34..e866787c4d9d53 100644 --- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py +++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py @@ -90,6 +90,28 @@ def test_runInTerminal(self): env = self.dap_server.request_evaluate("foo")["body"]["result"] self.assertIn("bar", env) + def test_runInTerminalWithObjectEnv(self): + if not self.isTestSupported(): + return + """ + Tests the "runInTerminal" reverse request. It makes sure that the IDE can + launch the inferior with the correct environment variables using an object. + """ + program = self.getBuildArtifact("a.out") + self.build_and_launch(program, runInTerminal=True, env={"FOO": "BAR"}) + + self.assertEqual( + len(self.dap_server.reverse_requests), + 1, + "make sure we got a reverse request", + ) + + request = self.dap_server.reverse_requests[0] + request_envs = request["arguments"]["env"] + + self.assertIn("FOO", request_envs) + self.assertEqual("BAR", request_envs["FOO"]) + @skipIfWindows @skipIf(archs=no_match(["x86_64"])) def test_runInTerminalInvalidTarget(self): diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 4f9c9c01cf4b6b..558f889c4b7f23 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -152,6 +152,31 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj, return strs; } +std::unordered_map<std::string, std::string> +GetStringMap(const llvm::json::Object &obj, llvm::StringRef key) { + std::unordered_map<std::string, std::string> strs; + const auto *const json_object = obj.getObject(key); + if (!json_object) + return strs; + + for (const auto &[key, value] : *json_object) { + switch (value.kind()) { + case llvm::json::Value::String: + strs.emplace(key.str(), value.getAsString()->str()); + break; + case llvm::json::Value::Number: + case llvm::json::Value::Boolean: + strs.emplace(key.str(), llvm::to_string(value)); + break; + case llvm::json::Value::Null: + case llvm::json::Value::Object: + case llvm::json::Value::Array: + break; + } + } + return strs; +} + static bool IsClassStructOrUnionType(lldb::SBType t) { return (t.GetTypeClass() & (lldb::eTypeClassUnion | lldb::eTypeClassStruct | lldb::eTypeClassArray)) != 0; @@ -1439,16 +1464,22 @@ CreateRunInTerminalReverseRequest(const llvm::json::Object &launch_request, if (!cwd.empty()) run_in_terminal_args.try_emplace("cwd", cwd); - // We need to convert the input list of environments variables into a - // dictionary - std::vector<std::string> envs = GetStrings(launch_request_arguments, "env"); - llvm::json::Object environment; - for (const std::string &env : envs) { - size_t index = env.find('='); - environment.try_emplace(env.substr(0, index), env.substr(index + 1)); + auto envs = GetEnvironmentFromArguments(*launch_request_arguments); + llvm::json::Object env_json; + for (size_t index = 0, env_count = envs.GetNumValues(); index < env_count; + index++) { + llvm::StringRef key = envs.GetNameAtIndex(index); + llvm::StringRef value = envs.GetValueAtIndex(index); + + if (key.empty()) + g_dap.SendOutput(OutputType::Stderr, + "empty environment variable for value: \"" + + value.str() + '\"'); + else + env_json.try_emplace(key, value); } run_in_terminal_args.try_emplace("env", - llvm::json::Value(std::move(environment))); + llvm::json::Value(std::move(env_json))); return run_in_terminal_args; } diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index 13018458ffe0ad..18cfb4081fece1 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -16,6 +16,7 @@ #include "llvm/Support/JSON.h" #include <cstdint> #include <optional> +#include <unordered_map> namespace lldb_dap { @@ -159,6 +160,27 @@ DecodeMemoryReference(llvm::StringRef memoryReference); std::vector<std::string> GetStrings(const llvm::json::Object *obj, llvm::StringRef key); +/// Extract an object of key value strings for the specified key from an object. +/// +/// String values in the object will be extracted without any quotes +/// around them. Numbers and Booleans will be converted into +/// strings. Any NULL, array or objects values in the array will be +/// ignored. +/// +/// \param[in] obj +/// A JSON object that we will attempt to extract the array from +/// +/// \param[in] key +/// The key to use when extracting the value +/// +/// \return +/// An object of key value strings for the specified \a key, or +/// \a fail_value if there is no key that matches or if the +/// value is not an object or key and values in the object are not +/// strings, numbers or booleans. +std::unordered_map<std::string, std::string> +GetStringMap(const llvm::json::Object &obj, llvm::StringRef key); + /// Fill a response object given the request object. /// /// The \a response object will get its "type" set to "response", diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp b/lldb/tools/lldb-dap/LLDBUtils.cpp index a74b32609a167b..b38833c0fdb6b6 100644 --- a/lldb/tools/lldb-dap/LLDBUtils.cpp +++ b/lldb/tools/lldb-dap/LLDBUtils.cpp @@ -135,4 +135,29 @@ int64_t MakeDAPFrameID(lldb::SBFrame &frame) { frame.GetFrameID(); } +lldb::SBEnvironment +GetEnvironmentFromArguments(const llvm::json::Object &arguments) { + lldb::SBEnvironment envs{}; + constexpr llvm::StringRef env_key = "env"; + const llvm::json::Value *raw_json_env = arguments.get(env_key); + + if (!raw_json_env) + return envs; + + if (raw_json_env->kind() == llvm::json::Value::Object) { + auto env_map = GetStringMap(arguments, env_key); + for (const auto &[key, value] : env_map) + envs.Set(key.c_str(), value.c_str(), true); + + } else if (raw_json_env->kind() == llvm::json::Value::Array) { + const auto envs_strings = GetStrings(&arguments, env_key); + lldb::SBStringList entries{}; + for (const auto &env : envs_strings) + entries.AppendString(env.c_str()); + + envs.SetEntries(entries, true); + } + return envs; +} + } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/LLDBUtils.h b/lldb/tools/lldb-dap/LLDBUtils.h index ee701da2230fe0..d5072d19029a1e 100644 --- a/lldb/tools/lldb-dap/LLDBUtils.h +++ b/lldb/tools/lldb-dap/LLDBUtils.h @@ -10,11 +10,12 @@ #define LLDB_TOOLS_LLDB_DAP_LLDBUTILS_H #include "DAPForward.h" +#include "lldb/API/SBEnvironment.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/JSON.h" #include "llvm/Support/raw_ostream.h" #include <string> -#include <vector> namespace lldb_dap { @@ -135,6 +136,17 @@ uint32_t GetLLDBThreadIndexID(uint64_t dap_frame_id); /// The LLDB frame index ID. uint32_t GetLLDBFrameID(uint64_t dap_frame_id); +/// Gets all the environment variables from the json object depending on if the +/// kind is an object or an array. +/// +/// \param[in] arguments +/// The json object with the launch options +/// +/// \return +/// The environment variables stored in the env key +lldb::SBEnvironment +GetEnvironmentFromArguments(const llvm::json::Object &arguments); + } // namespace lldb_dap #endif diff --git a/lldb/tools/lldb-dap/README.md b/lldb/tools/lldb-dap/README.md index 7248df310c14a9..3a7d82e887cca3 100644 --- a/lldb/tools/lldb-dap/README.md +++ b/lldb/tools/lldb-dap/README.md @@ -36,7 +36,10 @@ adds `FOO=1` and `bar` to the environment: "name": "Debug", "program": "/tmp/a.out", "args": [ "one", "two", "three" ], - "env": [ "FOO=1", "BAR" ], + "env": { + "FOO": "1" + "BAR": "" + } } ``` diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 0b6d6402410654..ac18e8f24a4e39 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -53,6 +53,7 @@ #include <thread> #include <vector> +#include "lldb/API/SBEnvironment.h" #include "lldb/API/SBStream.h" #include "lldb/Host/Config.h" #include "llvm/ADT/ArrayRef.h" @@ -610,25 +611,32 @@ void SetSourceMapFromArguments(const llvm::json::Object &arguments) { std::string sourceMapCommand; llvm::raw_string_ostream strm(sourceMapCommand); strm << "settings set target.source-map "; - auto sourcePath = GetString(arguments, "sourcePath"); + const auto sourcePath = GetString(arguments, "sourcePath"); // sourceMap is the new, more general form of sourcePath and overrides it. - auto sourceMap = arguments.getArray("sourceMap"); - if (sourceMap) { - for (const auto &value : *sourceMap) { - auto mapping = value.getAsArray(); + constexpr llvm::StringRef sourceMapKey = "sourceMap"; + + if (const auto *sourceMapArray = arguments.getArray(sourceMapKey)) { + for (const auto &value : *sourceMapArray) { + const auto *mapping = value.getAsArray(); if (mapping == nullptr || mapping->size() != 2 || (*mapping)[0].kind() != llvm::json::Value::String || (*mapping)[1].kind() != llvm::json::Value::String) { g_dap.SendOutput(OutputType::Console, llvm::StringRef(sourceMapHelp)); return; } - auto mapFrom = GetAsString((*mapping)[0]); - auto mapTo = GetAsString((*mapping)[1]); + const auto mapFrom = GetAsString((*mapping)[0]); + const auto mapTo = GetAsString((*mapping)[1]); strm << "\"" << mapFrom << "\" \"" << mapTo << "\" "; } + } else if (const auto *sourceMapObj = arguments.getObject(sourceMapKey)) { + for (const auto &[key, value] : *sourceMapObj) { + if (value.kind() == llvm::json::Value::String) { + strm << "\"" << key.str() << "\" \"" << GetAsString(value) << "\" "; + } + } } else { - if (ObjectContainsKey(arguments, "sourceMap")) { + if (ObjectContainsKey(arguments, sourceMapKey)) { g_dap.SendOutput(OutputType::Console, llvm::StringRef(sourceMapHelp)); return; } @@ -2069,9 +2077,8 @@ lldb::SBError LaunchProcess(const llvm::json::Object &request) { launch_info.SetArguments(MakeArgv(args).data(), true); // Pass any environment variables along that the user specified. - auto envs = GetStrings(arguments, "env"); - if (!envs.empty()) - launch_info.SetEnvironmentEntries(MakeArgv(envs).data(), true); + const auto envs = GetEnvironmentFromArguments(*arguments); + launch_info.SetEnvironment(envs, true); auto flags = launch_info.GetLaunchFlags(); diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index 480929703e4b56..9155163c65ba5c 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -170,9 +170,27 @@ "default": "${workspaceRoot}" }, "env": { - "type": "array", - "description": "Additional environment variables to set when launching the program. This is an array of strings that contains the variable name followed by an optional '=' character and the environment variable's value.", - "default": [] + "anyOf": [ + { + "type": "object", + "description": "Additional environment variables to set when launching the program. E.g. `{ \"FOO\": \"1\" }`", + "patternProperties": { + ".*": { + "type": "string" + } + }, + "default": {} + }, + { + "type": "array", + "description": "Additional environment variables to set when launching the program. E.g. `[\"FOO=1\", \"BAR\"]`", + "items": { + "type": "string", + "pattern": "^((\\w+=.*)|^\\w+)$" + }, + "default": [] + } + ] }, "stopOnEntry": { "type": "boolean", @@ -204,9 +222,31 @@ "description": "Specify a source path to remap \"./\" to allow full paths to be used when setting breakpoints in binaries that have relative source paths." }, "sourceMap": { - "type": "array", - "description": "Specify an array of path remappings; each element must itself be a two element array containing a source and destination path name. Overrides sourcePath.", - "default": [] + "anyOf": [ + { + "type": "object", + "description": "Specify an object of path remappings; each entry has a key containing the source path and a value containing the destination path. E.g `{ \"/the/source/path\": \"/the/destination/path\" }`. Overrides sourcePath.", + "patternProperties": { + ".*": { + "type": "string" + } + }, + "default": {} + }, + { + "type": "array", + "description": "Specify an array of path remappings; each element must itself be a two element array containing a source and destination path name. Overrides sourcePath.", + "items": { + "type": "array", + "minItems": 2, + "maxItems": 2, + "items": { + "type": "string" + } + }, + "default": [] + } + ] }, "debuggerRoot": { "type": "string", @@ -319,9 +359,31 @@ "description": "Specify a source path to remap \"./\" to allow full paths to be used when setting breakpoints in binaries that have relative source paths." }, "sourceMap": { - "type": "array", - "description": "Specify an array of path remappings; each element must itself be a two element array containing a source and destination path name. Overrides sourcePath.", - "default": [] + "anyOf": [ + { + "type": "object", + "description": "Specify an object of path remappings; each entry has a key containing the source path and a value containing the destination path. E.g `{ \"/the/source/path\": \"/the/destination/path\" }`. Overrides sourcePath.", + "patternProperties": { + ".*": { + "type": "string" + } + }, + "default": {} + }, + { + "type": "array", + "description": "Specify an array of path remappings; each element must itself be a two element array containing a source and destination path name. Overrides sourcePath.", + "items": { + "type": "array", + "minItems": 2, + "maxItems": 2, + "items": { + "type": "string" + } + }, + "default": [] + } + ] }, "debuggerRoot": { "type": "string", @@ -451,4 +513,4 @@ } ] } -} +} \ No newline at end of file _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
