maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[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
This commit is contained in:
Fred Riss 2020-03-19 18:02:05 -07:00
parent cd7b45057c
commit b4a6e63ea1
5 changed files with 174 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
lldb/include/lldb/Target/Target.h
View File

@ -225,9 +225,12 @@ private:
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 {

38
lldb/source/Commands/CommandObjectTarget.cpp
View File

@ -682,6 +682,41 @@ protected:
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)));

93
lldb/source/Target/Target.cpp
View File

@ -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 @@ public:
// 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 @@ public:
}
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() {

5
lldb/source/Target/TargetProperties.td
View File

@ -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.">;

90
lldb/test/API/commands/settings/TestSettings.py
View File

@ -236,6 +236,10 @@ class SettingsCommandTestCase(TestBase):
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 @@ class SettingsCommandTestCase(TestBase):
"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 @@ class SettingsCommandTestCase(TestBase):
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,10 @@ class SettingsCommandTestCase(TestBase):
"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)
@ -293,6 +326,61 @@ class SettingsCommandTestCase(TestBase):
"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)
# Read the output file produced by running the program.
output = lldbutil.read_file_from_process_wd(self, "output1.txt")
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."""