From 45aa435661d80843554d425156f300b207d8a0a0 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Mon, 29 Nov 2021 11:30:23 +0100 Subject: [PATCH] [lldb/qemu] Separate host and target environments Qemu normally forwards its (host) environment variables to the emulated process. While this works fine for most variables, there are some (few, but fairly important) variables where this is not possible. LD_LIBRARY_PATH is the probably the most important of those -- we don't want the library search path for the emulated libraries to interfere with the libraries that the emulator itself needs. For this reason, qemu provides a mechanism (QEMU_SET_ENV, QEMU_UNSET_ENV) to set variables only for the emulated process. This patch makes use of that functionality to pass any user-provided variables to the emulated process. Since we're piggy-backing on the normal lldb environment-handling mechanism, all the usual mechanism to provide environment (target.env-vars setting, SBLaunchInfo, etc.) work out-of-the-box, and the only thing we need to do is to properly construct the qemu environment variables. This patch also adds a new setting -- target-env-vars, which represents environment variables which are added (on top of the host environment) to the default launch environments of all (qemu) targets. The reason for its existence is to enable the configuration (e.g., from a startup script) of the default launch environment, before any target is created. The idea is that this would contain the variables (like the aforementioned LD_LIBRARY_PATH) common to all targets being debugged on the given system. The user is, of course, free to customize the environment for a particular target in the usual manner. The reason I do not want to use/recommend the "global" version of the target.env-vars setting for this purpose is that the setting would apply to all targets, whereas the settings (their values) I have mentioned would be specific to the given platform. Differential Revision: https://reviews.llvm.org/D115246 --- .../Plugins/Platform/QemuUser/PlatformQemuUser.cpp | 52 +++++++++++++ .../Plugins/Platform/QemuUser/PlatformQemuUser.h | 2 +- .../QemuUser/PlatformQemuUserProperties.td | 3 + lldb/test/API/qemu/TestQemuLaunch.py | 85 +++++++++++++++++----- lldb/test/API/qemu/qemu.py | 5 +- 5 files changed, 126 insertions(+), 21 deletions(-) diff --git a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp index 82b9bc0..3bcb1e1 100644 --- a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp +++ b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp @@ -54,6 +54,13 @@ public: result); return result; } + + Environment GetTargetEnvVars() { + Args args; + m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, ePropertyTargetEnvVars, + args); + return Environment(args); + } }; static PluginProperties &GetGlobalProperties() { @@ -105,6 +112,42 @@ static auto get_arg_range(const Args &args) { args.GetArgumentArrayRef().end()); } +// Returns the emulator environment which result in the desired environment +// being presented to the emulated process. We want to be careful about +// preserving the host environment, as it may contain entries (LD_LIBRARY_PATH, +// for example) needed for the operation of the emulator itself. +static Environment ComputeLaunchEnvironment(Environment target, + Environment host) { + std::vector set_env; + for (const auto &KV : target) { + // If the host value differs from the target (or is unset), then set it + // through QEMU_SET_ENV. Identical entries will be forwarded automatically. + auto host_it = host.find(KV.first()); + if (host_it == host.end() || host_it->second != KV.second) + set_env.push_back(Environment::compose(KV)); + } + + std::vector unset_env; + for (const auto &KV : host) { + // If the target is missing some host entries, then unset them through + // QEMU_UNSET_ENV. + if (target.count(KV.first()) == 0) + unset_env.push_back(KV.first()); + } + + // The actual QEMU_(UN)SET_ENV variables should not be forwarded to the + // target. + if (!set_env.empty()) { + host["QEMU_SET_ENV"] = llvm::join(set_env, ","); + unset_env.push_back("QEMU_SET_ENV"); + } + if (!unset_env.empty()) { + unset_env.push_back("QEMU_UNSET_ENV"); + host["QEMU_UNSET_ENV"] = llvm::join(unset_env, ","); + } + return host; +} + lldb::ProcessSP PlatformQemuUser::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, Target &target, Status &error) { @@ -130,6 +173,8 @@ lldb::ProcessSP PlatformQemuUser::DebugProcess(ProcessLaunchInfo &launch_info, get_arg_range(args)); launch_info.SetArguments(args, true); + launch_info.GetEnvironment() = ComputeLaunchEnvironment( + std::move(launch_info.GetEnvironment()), Host::GetEnvironment()); launch_info.SetLaunchInSeparateProcessGroup(true); launch_info.GetFlags().Clear(eLaunchFlagDebug); launch_info.SetMonitorProcessCallback(ProcessLaunchInfo::NoOpMonitorCallback, @@ -166,3 +211,10 @@ lldb::ProcessSP PlatformQemuUser::DebugProcess(ProcessLaunchInfo &launch_info, process_sp->WaitForProcessToStop(llvm::None, nullptr, false, listener_sp); return process_sp; } + +Environment PlatformQemuUser::GetEnvironment() { + Environment env = Host::GetEnvironment(); + for (const auto &KV : GetGlobalProperties().GetTargetEnvVars()) + env[KV.first()] = KV.second; + return env; +} diff --git a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.h b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.h index f4f5d22..71df1b7 100644 --- a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.h +++ b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.h @@ -45,7 +45,7 @@ public: void CalculateTrapHandlerSymbolNames() override {} - Environment GetEnvironment() override { return Host::GetEnvironment(); } + Environment GetEnvironment() override; private: static lldb::PlatformSP CreateInstance(bool force, const ArchSpec *arch); diff --git a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td index 19de9c8..2792e2c 100644 --- a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td +++ b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUserProperties.td @@ -13,4 +13,7 @@ let Definition = "platformqemuuser" in { Global, DefaultStringValue<"">, Desc<"Extra arguments to pass to the emulator.">; + def TargetEnvVars: Property<"target-env-vars", "Dictionary">, + ElementType<"String">, + Desc<"Extra variables to add to emulated target environment.">; } diff --git a/lldb/test/API/qemu/TestQemuLaunch.py b/lldb/test/API/qemu/TestQemuLaunch.py index 259b381..5431249 100644 --- a/lldb/test/API/qemu/TestQemuLaunch.py +++ b/lldb/test/API/qemu/TestQemuLaunch.py @@ -43,7 +43,7 @@ class TestQemuLaunch(TestBase): self.set_emulator_setting("architecture", self.getArchitecture()) self.set_emulator_setting("emulator-path", emulator) - def _run_and_get_state(self): + def _create_target(self): self.build() exe = self.getBuildArtifact() @@ -52,11 +52,21 @@ class TestQemuLaunch(TestBase): target = self.dbg.CreateTarget(exe, '', 'qemu-user', False, error) self.assertSuccess(error) self.assertEqual(target.GetPlatform().GetName(), "qemu-user") + return target + + def _run_and_get_state(self, target=None, info=None): + if target is None: + target = self._create_target() + + if info is None: + info = target.GetLaunchInfo() # "Launch" the process. Our fake qemu implementation will pretend it # immediately exited. - process = target.LaunchSimple( - ["dump:" + self.getBuildArtifact("state.log")], None, None) + info.SetArguments(["dump:" + self.getBuildArtifact("state.log")], True) + error = lldb.SBError() + process = target.Launch(info, error) + self.assertSuccess(error) self.assertIsNotNone(process) self.assertEqual(process.GetState(), lldb.eStateExited) self.assertEqual(process.GetExitStatus(), 0x47) @@ -73,25 +83,21 @@ class TestQemuLaunch(TestBase): ["dump:" + self.getBuildArtifact("state.log")]) def test_stdio_pty(self): - self.build() - exe = self.getBuildArtifact() - - # Create a target using our platform - error = lldb.SBError() - target = self.dbg.CreateTarget(exe, '', 'qemu-user', False, error) - self.assertSuccess(error) + target = self._create_target() - info = lldb.SBLaunchInfo([ + info = target.GetLaunchInfo() + info.SetArguments([ "stdin:stdin", "stdout:STDOUT CONTENT\n", "stderr:STDERR CONTENT\n", "dump:" + self.getBuildArtifact("state.log"), - ]) + ], False) listener = lldb.SBListener("test_stdio") info.SetListener(listener) self.dbg.SetAsync(True) + error = lldb.SBError() process = target.Launch(info, error) self.assertSuccess(error) lldbutil.expect_state_changes(self, listener, process, @@ -152,14 +158,9 @@ class TestQemuLaunch(TestBase): self.set_emulator_setting("emulator-path", self.getBuildArtifact("nonexistent.file")) - self.build() - exe = self.getBuildArtifact() - - error = lldb.SBError() - target = self.dbg.CreateTarget(exe, '', 'qemu-user', False, error) - self.assertEqual(target.GetPlatform().GetName(), "qemu-user") - self.assertSuccess(error) + target = self._create_target() info = lldb.SBLaunchInfo([]) + error = lldb.SBError() target.Launch(info, error) self.assertTrue(error.Fail()) self.assertIn("doesn't exist", error.GetCString()) @@ -169,3 +170,49 @@ class TestQemuLaunch(TestBase): state = self._run_and_get_state() self.assertEqual(state["fake-arg"], "fake-value") + + def test_target_env_vars(self): + # First clear any global environment to have a clean slate for this test + self.runCmd("settings clear target.env-vars") + self.runCmd("settings clear target.unset-env-vars") + + def var(i): + return "LLDB_TEST_QEMU_VAR%d" % i + + # Set some variables in the host environment. + for i in range(4): + os.environ[var(i)]="from host" + def cleanup(): + for i in range(4): + del os.environ[var(i)] + self.addTearDownHook(cleanup) + + # And through the platform setting. + self.set_emulator_setting("target-env-vars", + "%s='from platform' %s='from platform'" % (var(1), var(2))) + + target = self._create_target() + info = target.GetLaunchInfo() + env = info.GetEnvironment() + + # Platform settings should trump host values. + self.assertEqual(env.Get(var(0)), "from host") + self.assertEqual(env.Get(var(1)), "from platform") + self.assertEqual(env.Get(var(2)), "from platform") + self.assertEqual(env.Get(var(3)), "from host") + + # Finally, make some launch_info specific changes. + env.Set(var(2), "from target", overwrite=True) + env.Unset(var(3)) + info.SetEnvironment(env, append=False) + + # Now check everything. Launch info changes should trump everything, but + # only for the target environment -- the emulator should still get the + # host values. + state = self._run_and_get_state(target, info) + for i in range(4): + self.assertEqual(state["environ"][var(i)], "from host") + self.assertEqual(state["environ"]["QEMU_SET_ENV"], + "%s=from platform,%s=from target" % (var(1), var(2))) + self.assertEqual(state["environ"]["QEMU_UNSET_ENV"], + "%s,QEMU_SET_ENV,QEMU_UNSET_ENV" % var(3)) diff --git a/lldb/test/API/qemu/qemu.py b/lldb/test/API/qemu/qemu.py index 4b94c76..a749768 100755 --- a/lldb/test/API/qemu/qemu.py +++ b/lldb/test/API/qemu/qemu.py @@ -1,6 +1,7 @@ import argparse import socket import json +import os import sys import use_lldb_suite @@ -60,7 +61,9 @@ def main(): parser.add_argument("args", nargs=argparse.REMAINDER) args = parser.parse_args() - emulator = FakeEmulator(args.g, vars(args)) + state = vars(args) + state["environ"] = dict(os.environ) + emulator = FakeEmulator(args.g, state) emulator.run() if __name__ == "__main__": -- 2.7.4