friss updated this revision to Diff 251813.
friss added a comment.
Simplify command definition
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/Commands/CommandObjectTarget.cpp
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
@@ -236,6 +236,10 @@
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 @@
"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 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 @@
"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 @@
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."""
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.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.">;
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3344,16 +3344,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,
@@ -3361,9 +3358,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) {
@@ -3376,41 +3370,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
@@ -3437,10 +3396,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
@@ -3450,6 +3409,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(
@@ -3641,19 +3604,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_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::EnvVarsValueChangedCallback() {
- m_launch_info.GetEnvironment() = GetEnvironment();
+ m_launch_info.GetEnvironment() = ComputeEnvironment();
}
void TargetProperties::InputPathValueChangedCallback() {
Index: lldb/source/Commands/CommandObjectTarget.cpp
===================================================================
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -682,6 +682,41 @@
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 @@
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)));
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
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
