[Lldb-commits] [lldb] b4a6e63 - [lldb/Target] Rework the way the inferior environment is created
Fred Riss via lldb-commits
lldb-commits at lists.llvm.org
Mon Mar 23 07:59:08 PDT 2020
Author: Fred Riss
Date: 2020-03-23T07:58:34-07:00
New Revision: b4a6e63ea12309bf667d1569a20ec5b081cbf2a4
URL: https://github.com/llvm/llvm-project/commit/b4a6e63ea12309bf667d1569a20ec5b081cbf2a4
DIFF: https://github.com/llvm/llvm-project/commit/b4a6e63ea12309bf667d1569a20ec5b081cbf2a4.diff
LOG: [lldb/Target] Rework the way the inferior environment is created
Summary:
The interactions between the environment settings (`target.env-vars`,
`target.inherit-env`) and the inferior life-cycle are non-obvious
today. For example, if `target.inherit-env` is set, the `target.env-vars`
setting will be augmented with the contents of the host environment
the first time the launch environment is queried (usually at
launch). After that point, toggling `target.inherit-env` will have no
effect as there's no tracking of what comes from the host and what is
a user setting.
This patch computes the environment every time it is queried rather
than updating the contents of the `target.env-vars` property. This
means that toggling the `target.inherit-env` property later will now
have the intended effect.
This patch also adds a `target.unset-env-vars` settings that one can
use to remove variables from the launch environment. Using this, you
can inherit all but a few of the host environment.
The way the launch environment is constructed is:
1/ if `target.inherit-env` is set, then read the host environment
into the launch environment.
2/ Remove for the environment the variables listed in
`target.unset-env`.
3/ Augment the launch environment with the contents of
`target.env-vars`. This overrides any common values with the host
environment.
The one functional difference here that could be seen as a regression
is that `target.env-vars` will not contain the inferior environment
after launch. The patch implements a better alternative in the
`target show-launch-environment` command which will return the
environment computed through the above rules.
Reviewers: labath, jingham
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76470
Added:
Modified:
lldb/include/lldb/Target/Target.h
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td
lldb/test/API/commands/settings/TestSettings.py
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 2e7932f49e6f..77cda4998192 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -225,9 +225,12 @@ class TargetProperties : public Properties {
void DisableASLRValueChangedCallback();
void DisableSTDIOValueChangedCallback();
+ Environment ComputeEnvironment() const;
+
// Member variables.
ProcessLaunchInfo m_launch_info;
std::unique_ptr<TargetExperimentalProperties> m_experimental_properties_up;
+ Target *m_target;
};
class EvaluateExpressionOptions {
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index c70117c7a80a..95f81fc6cd54 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -682,6 +682,41 @@ class CommandObjectTargetDelete : public CommandObjectParsed {
OptionGroupBoolean m_cleanup_option;
};
+class CommandObjectTargetShowLaunchEnvironment : public CommandObjectParsed {
+public:
+ CommandObjectTargetShowLaunchEnvironment(CommandInterpreter &interpreter)
+ : CommandObjectParsed(
+ interpreter, "target show-launch-environment",
+ "Shows the environment being passed to the process when launched, "
+ "taking info account 3 settings: target.env-vars, "
+ "target.inherit-env and target.unset-env-vars.",
+ nullptr, eCommandRequiresTarget) {}
+
+ ~CommandObjectTargetShowLaunchEnvironment() override = default;
+
+protected:
+ bool DoExecute(Args &args, CommandReturnObject &result) override {
+ Target *target = m_exe_ctx.GetTargetPtr();
+ Environment env = target->GetEnvironment();
+
+ std::vector<Environment::value_type *> env_vector;
+ env_vector.reserve(env.size());
+ for (auto &KV : env)
+ env_vector.push_back(&KV);
+ std::sort(env_vector.begin(), env_vector.end(),
+ [](Environment::value_type *a, Environment::value_type *b) {
+ return a->first() < b->first();
+ });
+
+ auto &strm = result.GetOutputStream();
+ for (auto &KV : env_vector)
+ strm.Format("{0}={1}\n", KV->first(), KV->second);
+
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+ return result.Succeeded();
+ }
+};
+
#pragma mark CommandObjectTargetVariable
// "target variable"
@@ -4876,6 +4911,9 @@ CommandObjectMultiwordTarget::CommandObjectMultiwordTarget(
CommandObjectSP(new CommandObjectTargetList(interpreter)));
LoadSubCommand("select",
CommandObjectSP(new CommandObjectTargetSelect(interpreter)));
+ LoadSubCommand("show-launch-environment",
+ CommandObjectSP(new CommandObjectTargetShowLaunchEnvironment(
+ interpreter)));
LoadSubCommand(
"stop-hook",
CommandObjectSP(new CommandObjectMultiwordTargetStopHooks(interpreter)));
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 0162ca838e78..e2c808120877 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3344,16 +3344,13 @@ enum {
class TargetOptionValueProperties : public OptionValueProperties {
public:
- TargetOptionValueProperties(ConstString name)
- : OptionValueProperties(name), m_target(nullptr), m_got_host_env(false) {}
+ TargetOptionValueProperties(ConstString name) : OptionValueProperties(name) {}
// This constructor is used when creating TargetOptionValueProperties when it
// is part of a new lldb_private::Target instance. It will copy all current
// global property values as needed
- TargetOptionValueProperties(Target *target,
- const TargetPropertiesSP &target_properties_sp)
- : OptionValueProperties(*target_properties_sp->GetValueProperties()),
- m_target(target), m_got_host_env(false) {}
+ TargetOptionValueProperties(const TargetPropertiesSP &target_properties_sp)
+ : OptionValueProperties(*target_properties_sp->GetValueProperties()) {}
const Property *GetPropertyAtIndex(const ExecutionContext *exe_ctx,
bool will_modify,
@@ -3361,9 +3358,6 @@ class TargetOptionValueProperties : public OptionValueProperties {
// When getting the value for a key from the target options, we will always
// try and grab the setting from the current target if there is one. Else
// we just use the one from this instance.
- if (idx == ePropertyEnvVars)
- GetHostEnvironmentIfNeeded();
-
if (exe_ctx) {
Target *target = exe_ctx->GetTargetPtr();
if (target) {
@@ -3376,41 +3370,6 @@ class TargetOptionValueProperties : public OptionValueProperties {
}
return ProtectedGetPropertyAtIndex(idx);
}
-
- lldb::TargetSP GetTargetSP() { return m_target->shared_from_this(); }
-
-protected:
- void GetHostEnvironmentIfNeeded() const {
- if (!m_got_host_env) {
- if (m_target) {
- m_got_host_env = true;
- const uint32_t idx = ePropertyInheritEnv;
- if (GetPropertyAtIndexAsBoolean(
- nullptr, idx, g_target_properties[idx].default_uint_value != 0)) {
- PlatformSP platform_sp(m_target->GetPlatform());
- if (platform_sp) {
- Environment env = platform_sp->GetEnvironment();
- OptionValueDictionary *env_dict =
- GetPropertyAtIndexAsOptionValueDictionary(nullptr,
- ePropertyEnvVars);
- if (env_dict) {
- const bool can_replace = false;
- for (const auto &KV : env) {
- // Don't allow existing keys to be replaced with ones we get
- // from the platform environment
- env_dict->SetValueForKey(
- ConstString(KV.first()),
- OptionValueSP(new OptionValueString(KV.second.c_str())),
- can_replace);
- }
- }
- }
- }
- }
- }
- }
- Target *m_target;
- mutable bool m_got_host_env;
};
// TargetProperties
@@ -3437,10 +3396,10 @@ TargetExperimentalProperties::TargetExperimentalProperties()
// TargetProperties
TargetProperties::TargetProperties(Target *target)
- : Properties(), m_launch_info() {
+ : Properties(), m_launch_info(), m_target(target) {
if (target) {
m_collection_sp = std::make_shared<TargetOptionValueProperties>(
- target, Target::GetGlobalProperties());
+ Target::GetGlobalProperties());
// Set callbacks to update launch_info whenever "settins set" updated any
// of these properties
@@ -3450,6 +3409,10 @@ TargetProperties::TargetProperties(Target *target)
ePropertyRunArgs, [this] { RunArgsValueChangedCallback(); });
m_collection_sp->SetValueChangedCallback(
ePropertyEnvVars, [this] { EnvVarsValueChangedCallback(); });
+ m_collection_sp->SetValueChangedCallback(
+ ePropertyUnsetEnvVars, [this] { EnvVarsValueChangedCallback(); });
+ m_collection_sp->SetValueChangedCallback(
+ ePropertyInheritEnv, [this] { EnvVarsValueChangedCallback(); });
m_collection_sp->SetValueChangedCallback(
ePropertyInputPath, [this] { InputPathValueChangedCallback(); });
m_collection_sp->SetValueChangedCallback(
@@ -3641,19 +3604,43 @@ void TargetProperties::SetRunArguments(const Args &args) {
m_launch_info.GetArguments() = args;
}
+Environment TargetProperties::ComputeEnvironment() const {
+ Environment env;
+
+ if (m_target &&
+ m_collection_sp->GetPropertyAtIndexAsBoolean(
+ nullptr, ePropertyInheritEnv,
+ g_target_properties[ePropertyInheritEnv].default_uint_value != 0)) {
+ if (auto platform_sp = m_target->GetPlatform()) {
+ Environment platform_env = platform_sp->GetEnvironment();
+ for (const auto &KV : platform_env)
+ env[KV.first()] = KV.second;
+ }
+ }
+
+ Args property_unset_env;
+ m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, ePropertyUnsetEnvVars,
+ property_unset_env);
+ for (const auto &var : property_unset_env)
+ env.erase(var.ref());
+
+ Args property_env;
+ m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, ePropertyEnvVars,
+ property_env);
+ for (const auto &KV : Environment(property_env))
+ env[KV.first()] = KV.second;
+
+ return env;
+}
+
Environment TargetProperties::GetEnvironment() const {
- // TODO: Get rid of the Args intermediate step
- Args env;
- const uint32_t idx = ePropertyEnvVars;
- m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, idx, env);
- return Environment(env);
+ return ComputeEnvironment();
}
void TargetProperties::SetEnvironment(Environment env) {
// TODO: Get rid of the Args intermediate step
const uint32_t idx = ePropertyEnvVars;
m_collection_sp->SetPropertyAtIndexFromArgs(nullptr, idx, Args(env));
- m_launch_info.GetEnvironment() = std::move(env);
}
bool TargetProperties::GetSkipPrologue() const {
@@ -3971,7 +3958,7 @@ void TargetProperties::RunArgsValueChangedCallback() {
}
void TargetProperties::EnvVarsValueChangedCallback() {
- m_launch_info.GetEnvironment() = GetEnvironment();
+ m_launch_info.GetEnvironment() = ComputeEnvironment();
}
void TargetProperties::InputPathValueChangedCallback() {
diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td
index 77579c66ccb0..c8dd0a12315e 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -80,7 +80,10 @@ let Definition = "target" in {
Desc<"A list containing all the arguments to be passed to the executable when it is run. Note that this does NOT include the argv[0] which is in target.arg0.">;
def EnvVars: Property<"env-vars", "Dictionary">,
ElementType<"String">,
- Desc<"A list of all the environment variables to be passed to the executable's environment, and their values.">;
+ Desc<"A list of user provided environment variables to be passed to the executable's environment, and their values.">;
+ def UnsetEnvVars: Property<"unset-env-vars", "Array">,
+ ElementType<"String">,
+ Desc<"A list of environment variable names to be unset in the inferior's environment. This is most useful to unset some host environment variables when target.inherit-env is true. target.env-vars takes precedence over target.unset-env-vars.">;
def InheritEnv: Property<"inherit-env", "Boolean">,
DefaultTrue,
Desc<"Inherit the environment from the process that is running LLDB.">;
diff --git a/lldb/test/API/commands/settings/TestSettings.py b/lldb/test/API/commands/settings/TestSettings.py
index b203f85de9fb..29360856a735 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -236,6 +236,10 @@ def do_test_run_args_and_env_vars(self, use_launchsimple):
self.assertTrue(found_env_var,
"MY_ENV_VAR was not set in LunchInfo object")
+ self.expect(
+ 'target show-launch-environment',
+ substrs=["MY_ENV_VAR=YES"])
+
wd = self.get_process_working_directory()
if use_launchsimple:
process = target.LaunchSimple(None, None, wd)
@@ -256,6 +260,31 @@ def do_test_run_args_and_env_vars(self, use_launchsimple):
"argv[3] matches",
"Environment variable 'MY_ENV_VAR' successfully passed."])
+ # Check that env-vars overrides unset-env-vars.
+ self.runCmd('settings set target.unset-env-vars MY_ENV_VAR')
+
+ self.expect(
+ 'target show-launch-environment',
+ 'env-vars overrides unset-env-vars',
+ substrs=["MY_ENV_VAR=YES"])
+
+ wd = self.get_process_working_directory()
+ if use_launchsimple:
+ process = target.LaunchSimple(None, None, wd)
+ self.assertTrue(process)
+ else:
+ self.runCmd("process launch --working-dir '{0}'".format(wd),
+ RUN_SUCCEEDED)
+
+ # Read the output file produced by running the program.
+ output = lldbutil.read_file_from_process_wd(self, "output2.txt")
+
+ self.expect(
+ output,
+ exe=False,
+ substrs=[
+ "Environment variable 'MY_ENV_VAR' successfully passed."])
+
@skipIfRemote # it doesn't make sense to send host env to remote target
def test_pass_host_env_vars(self):
"""Test that the host env vars are passed to the launched process."""
@@ -269,6 +298,7 @@ def test_pass_host_env_vars(self):
def unset_env_variables():
os.environ.pop("MY_HOST_ENV_VAR1")
os.environ.pop("MY_HOST_ENV_VAR2")
+ self.addTearDownHook(unset_env_variables)
exe = self.getBuildArtifact("a.out")
self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
@@ -279,7 +309,33 @@ def unset_env_variables():
"Default inherit-env is 'true'",
startstr="target.inherit-env (boolean) = true")
- self.addTearDownHook(unset_env_variables)
+ self.expect(
+ 'target show-launch-environment',
+ 'Host environment is passed correctly',
+ substrs=['MY_HOST_ENV_VAR1=VAR1', 'MY_HOST_ENV_VAR2=VAR2'])
+ self.runCmd("process launch --working-dir '{0}'".format(self.get_process_working_directory()),
+ RUN_SUCCEEDED)
+
+ # Read the output file produced by running the program.
+ output = lldbutil.read_file_from_process_wd(self, "output1.txt")
+
+ self.expect(
+ output,
+ exe=False,
+ substrs=[
+ "The host environment variable 'MY_HOST_ENV_VAR1' successfully passed.",
+ "The host environment variable 'MY_HOST_ENV_VAR2' successfully passed."])
+
+ # Now test that we can prevent the inferior from inheriting the
+ # environment.
+ self.runCmd('settings set target.inherit-env false')
+
+ self.expect(
+ 'target show-launch-environment',
+ 'target.inherit-env affects `target show-launch-environment`',
+ matching=False,
+ substrs = ['MY_HOST_ENV_VAR1=VAR1', 'MY_HOST_ENV_VAR2=VAR2'])
+
self.runCmd("process launch --working-dir '{0}'".format(self.get_process_working_directory()),
RUN_SUCCEEDED)
@@ -289,10 +345,42 @@ def unset_env_variables():
self.expect(
output,
exe=False,
+ matching=False,
substrs=[
"The host environment variable 'MY_HOST_ENV_VAR1' successfully passed.",
"The host environment variable 'MY_HOST_ENV_VAR2' successfully passed."])
+ # Now test that we can unset variables from the inherited environment.
+ self.runCmd('settings set target.inherit-env true')
+ self.runCmd('settings set target.unset-env-vars MY_HOST_ENV_VAR1')
+ self.runCmd("process launch --working-dir '{0}'".format(self.get_process_working_directory()),
+ RUN_SUCCEEDED)
+
+ # Read the output file produced by running the program.
+ output = lldbutil.read_file_from_process_wd(self, "output1.txt")
+
+ self.expect(
+ 'target show-launch-environment',
+ 'MY_HOST_ENV_VAR1 is unset, it shouldn\'t be in `target show-launch-environment`',
+ matching=False,
+ substrs = ['MY_HOST_ENV_VAR1=VAR1'])
+ self.expect(
+ 'target show-launch-environment',
+ 'MY_HOST_ENV_VAR2 shouldn be in `target show-launch-environment`',
+ substrs = ['MY_HOST_ENV_VAR2=VAR2'])
+
+ self.expect(
+ output,
+ exe=False,
+ matching=False,
+ substrs=[
+ "The host environment variable 'MY_HOST_ENV_VAR1' successfully passed."])
+ self.expect(
+ output,
+ exe=False,
+ substrs=[
+ "The host environment variable 'MY_HOST_ENV_VAR2' successfully passed."])
+
@skipIfDarwinEmbedded # <rdar://problem/34446098> debugserver on ios etc can't write files
def test_set_error_output_path(self):
"""Test that setting target.error/output-path for the launched process works."""
More information about the lldb-commits
mailing list