[Lldb-commits] [lldb] d4c1789 - Make env and source map dictionaries #95137 (#106919)

via lldb-commits lldb-commits at lists.llvm.org
Mon Oct 7 09:38:40 PDT 2024


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


        


More information about the lldb-commits mailing list