friss updated this revision to Diff 251670.
friss added a comment.
Add tests and update m_launch_info when undet-vars change.
Improve docs.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76470/new/
https://reviews.llvm.org/D76470
Files:
lldb/include/lldb/Target/Target.h
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td
lldb/test/API/commands/settings/TestSettings.py
Index: lldb/test/API/commands/settings/TestSettings.py
===================================================================
--- lldb/test/API/commands/settings/TestSettings.py
+++ lldb/test/API/commands/settings/TestSettings.py
@@ -256,6 +256,26 @@
"argv[3] matches",
"Environment variable 'MY_ENV_VAR' successfully passed."])
+ # Check that unset-env-vars overrides env-vars.
+ self.runCmd('settings set target.unset-env-vars MY_ENV_VAR')
+ 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,
+ matching=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 +289,7 @@
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 +300,6 @@
"Default inherit-env is 'true'",
startstr="target.inherit-env (boolean) = true")
- self.addTearDownHook(unset_env_variables)
self.runCmd("process launch --working-dir '{0}'".format(self.get_process_working_directory()),
RUN_SUCCEEDED)
@@ -293,6 +313,44 @@
"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.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(
+ 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."""
Index: lldb/source/Target/TargetProperties.td
===================================================================
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -80,7 +80,10 @@
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">,
DefaultUnsignedValue<16>,
- 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">,
+ DefaultUnsignedValue<16>,
+ 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.unset-env-vars takes precedence over target.env-vars.">;
def InheritEnv: Property<"inherit-env", "Boolean">,
DefaultTrue,
Desc<"Inherit the environment from the process that is running LLDB.">;
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3357,16 +3357,13 @@
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,
@@ -3374,9 +3371,6 @@
// 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) {
@@ -3389,41 +3383,6 @@
}
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
@@ -3450,10 +3409,10 @@
// 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
@@ -3463,6 +3422,10 @@
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(
@@ -3642,19 +3605,43 @@
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_env;
+ m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, ePropertyEnvVars,
+ property_env);
+ for (const auto &KV : Environment(property_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());
+
+ 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 {
@@ -3972,7 +3959,7 @@
}
void TargetProperties::EnvVarsValueChangedCallback() {
- m_launch_info.GetEnvironment() = GetEnvironment();
+ m_launch_info.GetEnvironment() = ComputeEnvironment();
}
void TargetProperties::InputPathValueChangedCallback() {
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -225,9 +225,12 @@
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 {
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
