friss created this revision.
friss added reviewers: labath, jingham.
Herald added a project: LLDB.
friss added a comment.
This needs some tests, but I wanted to put it out there for comments.
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/ Augment the launch environment with the contents of
`target.env-vars`. This overrides any common values with the host
environment.
3/ Remove for the environment the variables listed in
`target.unset-env`.
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. It's unclear that this is what any user would have
expected anyway, and we can make it clear(er) in the help that this
setting only contains the values specified by the user.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D76470
Files:
lldb/include/lldb/Target/Target.h
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td
Index: lldb/source/Target/TargetProperties.td
===================================================================
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -81,6 +81,9 @@
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.">;
+ def UnsetEnvVars: Property<"unset-env-vars", "Array">,
+ DefaultUnsignedValue<16>,
+ Desc<"A list of environment variable names to be unset in the inferior's environment.">;
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,8 @@
ePropertyRunArgs, [this] { RunArgsValueChangedCallback(); });
m_collection_sp->SetValueChangedCallback(
ePropertyEnvVars, [this] { EnvVarsValueChangedCallback(); });
+ m_collection_sp->SetValueChangedCallback(
+ ePropertyInheritEnv, [this] { EnvVarsValueChangedCallback(); });
m_collection_sp->SetValueChangedCallback(
ePropertyInputPath, [this] { InputPathValueChangedCallback(); });
m_collection_sp->SetValueChangedCallback(
@@ -3642,19 +3603,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_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 +3957,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
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
