[lldb/Target] Rework the way the inferior environment is created

[friss]

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/ 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.

[friss]

This needs some tests, but I wanted to put it out there for comments.

[labath]

I think this is a good change, and is in line with what we have discussed previously. I have just one question about the exact variable interactions here.

Since the raison d'être of the new setting is to be able to "un-inherit" a particular environment variable, I'm wondering if it should only take effect if inherit-env is true (as otherwise you can achieve the same effect by deleting stuff from env-vars)? So instead of the current flow, the sequence would be:

Environment env;
if (inherit-env) {
  env = platform.GetEnvironment();
  for (var : unset-env-vars)
    env.erase(var);
}
for (kv : env-vars)
  env[kv.first] = kv.second;

I'm not insisting on anything, I just want to see which flow would be least surprising.

[friss]

Add tests and update m_launch_info when undet-vars change.
Improve docs.

[friss]

In D76470#1933062, @labath wrote:

I think this is a good change, and is in line with what we have discussed previously. I have just one question about the exact variable interactions here.

Since the raison d'être of the new setting is to be able to "un-inherit" a particular environment variable, I'm wondering if it should only take effect if inherit-env is true (as otherwise you can achieve the same effect by deleting stuff from env-vars)? So instead of the current flow, the sequence would be:

I find the logic easier to explain if unset-env-vars applies generally. I don't see a reason other the the host environment to use it, but I also don't see the harm in letting it override env-vars.

[jingham]

Is there any command-based way to see the entire environment that a process will get when it launches? By populating target.env-vars with the inherited environment there was a way to mostly do that, but I don't see that anymore. It seems a shame not to be able to see that...

Also, it does seem weird to me that unset-env-vars would override setting a target.env-var explicitly. What I'm likely to do is say: environment variable 'PUT_HERE_TO_ANNOY_JIM' is always getting set, and I don't want it to be when I'm debugging, so I put:

settings set target.unset-env-vars PUT_HERE_TO_ANNOY_JIM

in my .lldbinit and then after a couple month of debugging happiness, I forget that I put it there.

Then for some reason in a debugging session I want to set it. So I do:

(lldb) env PUT_HERE_TO_ANNOY_JIM="Okay, This One Time..."

But then when I run my process, it doesn't get set. Now I have to go back and remember that I had done the unset thing...

It really seems to me the automatic unsets should happen before the explicit sets. And given the only way you get anything before the automatic sets is the inherited environment, it doesn't make sense to me to apply the unsets if inherit is off.

[friss]

In D76470#1933910, @jingham wrote:

Is there any command-based way to see the entire environment that a process will get when it launches? By populating target.env-vars with the inherited environment there was a way to mostly do that, but I don't see that anymore. It seems a shame not to be able to see that...

No, there is no way to do this, but it's not really a regression. If you do settings show target.env-vars today before running then you won't see what is going to be passed. Only after the first run will it be populated. This was a surprise to me and I find it highly inconsistent. Maybe I can add another property that would get updated with the computed environment. Do we have anything like read-only properties?

Also, it does seem weird to me that unset-env-vars would override setting a target.env-var explicitly. What I'm likely to do is say: environment variable 'PUT_HERE_TO_ANNOY_JIM' is always getting set, and I don't want it to be when I'm debugging, so I put:

settings set target.unset-env-vars PUT_HERE_TO_ANNOY_JIM

in my .lldbinit and then after a couple month of debugging happiness, I forget that I put it there.

Then for some reason in a debugging session I want to set it. So I do:

(lldb) env PUT_HERE_TO_ANNOY_JIM="Okay, This One Time..."

But then when I run my process, it doesn't get set. Now I have to go back and remember that I had done the unset thing...

It really seems to me the automatic unsets should happen before the explicit sets. And given the only way you get anything before the automatic sets is the inherited environment, it doesn't make sense to me to apply the unsets if inherit is off.

This is pretty far-fetched to me, but I also don't really care. I'm fine with having env-vars override unset-env-vars.

[jingham]

In D76470#1934146, @friss wrote:

In D76470#1933910, @jingham wrote:

Is there any command-based way to see the entire environment that a process will get when it launches? By populating target.env-vars with the inherited environment there was a way to mostly do that, but I don't see that anymore. It seems a shame not to be able to see that...

No, there is no way to do this, but it's not really a regression. If you do settings show target.env-vars today before running then you won't see what is going to be passed. Only after the first run will it be populated. This was a surprise to me and I find it highly inconsistent. Maybe I can add another property that would get updated with the computed environment. Do we have anything like read-only properties?

No, but I don't think I'd do this with a property anyway. You are asking the target to compute the result of the various settings that affect the environment of a process it might launch. That sounds more like the result of a command ("target show-environment" or something like that.) If we knew how to get the environment from a running process, the same command could show the current environment in a running process, which would sometimes be handy.

I agree the behavior before was pretty unhelpful, so I don't think you need to add a new command to access this in this change set. But we should put it on our list of things to do.

Also, it does seem weird to me that unset-env-vars would override setting a target.env-var explicitly. What I'm likely to do is say: environment variable 'PUT_HERE_TO_ANNOY_JIM' is always getting set, and I don't want it to be when I'm debugging, so I put:

settings set target.unset-env-vars PUT_HERE_TO_ANNOY_JIM

in my .lldbinit and then after a couple month of debugging happiness, I forget that I put it there.

Then for some reason in a debugging session I want to set it. So I do:

(lldb) env PUT_HERE_TO_ANNOY_JIM="Okay, This One Time..."

But then when I run my process, it doesn't get set. Now I have to go back and remember that I had done the unset thing...

It really seems to me the automatic unsets should happen before the explicit sets. And given the only way you get anything before the automatic sets is the inherited environment, it doesn't make sense to me to apply the unsets if inherit is off.

This is pretty far-fetched to me, but I also don't really care. I'm fine with having env-vars override unset-env-vars.

If a variable FOO exists in lldb's environment with the value BAR, and then I have done env FOO=BAZ, when the process launches, FOO will be BAZ, not BAR. So it would be inconsistent to have env-vars override the values of lldb environment variables, but not the entries in unset-env-vars. So I do think that way makes more sense.

[friss]

Add a new target command to dump the launch environment
Make env-vars override unset-env-vars

[friss]

In D76470#1934201, @jingham wrote:

In D76470#1934146, @friss wrote:

In D76470#1933910, @jingham wrote:

Is there any command-based way to see the entire environment that a process will get when it launches? By populating target.env-vars with the inherited environment there was a way to mostly do that, but I don't see that anymore. It seems a shame not to be able to see that...

No, there is no way to do this, but it's not really a regression. If you do settings show target.env-vars today before running then you won't see what is going to be passed. Only after the first run will it be populated. This was a surprise to me and I find it highly inconsistent. Maybe I can add another property that would get updated with the computed environment. Do we have anything like read-only properties?

No, but I don't think I'd do this with a property anyway. You are asking the target to compute the result of the various settings that affect the environment of a process it might launch. That sounds more like the result of a command ("target show-environment" or something like that.) If we knew how to get the environment from a running process, the same command could show the current environment in a running process, which would sometimes be handy.

I agree the behavior before was pretty unhelpful, so I don't think you need to add a new command to access this in this change set. But we should put it on our list of things to do.

The command was not hard to add, so I just did it. I used "target show-launch-environment" because I thought "show-environment" would imply that it fetches the inferior environment. I also made env-vars take precedence over unset-env-vars

[jingham]

Except for the suggestion to use eCommandRequiresTarget for the new command, this looks good.

lldb/source/Commands/CommandObjectTarget.cpp

690

"passed to the process when launched"

might be clearer.

693

If you pass eCommandRequiresTarget to the optional last argument here, then you don't have to check in the DoExecute, and you will get consistent error reporting.

701

Pass eCommandRequiresTarget when you make the command and you can omit this checking.

[friss]

Simplify command definition

[labath]

looks good to me.

lldb/include/lldb/Target/Target.h

private:
   void EnvVarsValueChangedCallback();
   void InputPathValueChangedCallback();
   void OutputPathValueChangedCallback();
   void ErrorPathValueChangedCallback();
   void DetachOnErrorValueChangedCallback();
   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 {
 public:
 // MSVC has a bug here that reports C4268: 'const' static/global data
 // initialized with compiler generated default constructor fills the object
 // with zeros. Confirmed that MSVC is *not* zero-initializing, it's just a
 // bogus warning.

lldb/source/Commands/CommandObjectTarget.cpp

bool DoExecute(Args &args, CommandReturnObject &result) override {
     return true;
   }
 
   OptionGroupOptions m_option_group;
   OptionGroupBoolean m_all_option;
   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"
 
 class CommandObjectTargetVariable : public CommandObjectParsed {
   static const uint32_t SHORT_OPTION_FILE = 0x66696c65; // 'file'
   static const uint32_t SHORT_OPTION_SHLB = 0x73686c62; // 'shlb'
 
CommandObjectMultiwordTarget::CommandObjectMultiwordTarget(
   LoadSubCommand("create",
                  CommandObjectSP(new CommandObjectTargetCreate(interpreter)));
   LoadSubCommand("delete",
                  CommandObjectSP(new CommandObjectTargetDelete(interpreter)));
   LoadSubCommand("list",
                  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)));
   LoadSubCommand("modules",
                  CommandObjectSP(new CommandObjectTargetModules(interpreter)));
   LoadSubCommand("symbols",
                  CommandObjectSP(new CommandObjectTargetSymbols(interpreter)));
   LoadSubCommand("variable",
                  CommandObjectSP(new CommandObjectTargetVariable(interpreter)));
 }
 
 CommandObjectMultiwordTarget::~CommandObjectMultiwordTarget() = default;

lldb/source/Target/Target.cpp

 enum {
 #define LLDB_PROPERTIES_target
 #include "TargetPropertiesEnum.inc"
   ePropertyExperimental,
 };
 
 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,
                                      uint32_t idx) const override {
     // 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) {
         TargetOptionValueProperties *target_properties =
             static_cast<TargetOptionValueProperties *>(
                 target->GetValueProperties().get());
         if (this != target_properties)
           return target_properties->ProtectedGetPropertyAtIndex(idx);
       }
     }
     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
 #define LLDB_PROPERTIES_experimental
 #include "TargetProperties.inc"
 
 enum {
 #define LLDB_PROPERTIES_experimental

 TargetExperimentalProperties::TargetExperimentalProperties()
     : Properties(OptionValuePropertiesSP(
           new TargetExperimentalOptionValueProperties())) {
   m_collection_sp->Initialize(g_experimental_properties);
 }
 
 // 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
     m_collection_sp->SetValueChangedCallback(
         ePropertyArg0, [this] { Arg0ValueChangedCallback(); });
     m_collection_sp->SetValueChangedCallback(
         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(
         ePropertyOutputPath, [this] { OutputPathValueChangedCallback(); });
     m_collection_sp->SetValueChangedCallback(
         ePropertyErrorPath, [this] { ErrorPathValueChangedCallback(); });
     m_collection_sp->SetValueChangedCallback(ePropertyDetachOnError, [this] {
       DetachOnErrorValueChangedCallback();
     });

 }
 
 void TargetProperties::SetRunArguments(const Args &args) {
   const uint32_t idx = ePropertyRunArgs;
   m_collection_sp->SetPropertyAtIndexFromArgs(nullptr, idx, 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 {
   const uint32_t idx = ePropertySkipPrologue;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
       nullptr, idx, g_target_properties[idx].default_uint_value != 0);
 }
 

 
 void TargetProperties::RunArgsValueChangedCallback() {
   Args args;
   if (GetRunArguments(args))
     m_launch_info.GetArguments() = args;
 }
 
 void TargetProperties::EnvVarsValueChangedCallback() {
-  m_launch_info.GetEnvironment() = GetEnvironment();
+  m_launch_info.GetEnvironment() = ComputeEnvironment();
 }
 
 void TargetProperties::InputPathValueChangedCallback() {
   m_launch_info.AppendOpenFileAction(STDIN_FILENO, GetStandardInputPath(), true,
                                      false);
 }
 
 void TargetProperties::OutputPathValueChangedCallback() {

lldb/source/Target/TargetProperties.td

let Definition = "target" in {
   def Arg0: Property<"arg0", "String">,
     DefaultStringValue<"">,
     Desc<"The first argument passed to the program in the argument array which can be different from the executable itself.">;
   def RunArgs: Property<"run-args", "Args">,
     DefaultStringValue<"">,
     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.">;
   def InputPath: Property<"input-path", "FileSpec">,
     DefaultStringValue<"">,
     Desc<"The file/path to be used by the executable program for reading its standard input.">;
   def OutputPath: Property<"output-path", "FileSpec">,
     DefaultStringValue<"">,

lldb/test/API/commands/settings/TestSettings.py

def do_test_run_args_and_env_vars(self, use_launchsimple):
         found_env_var = False
         for i in range(0, launch_info.GetNumEnvironmentEntries()):
             if launch_info.GetEnvironmentEntryAtIndex(i) == "MY_ENV_VAR=YES":
                 found_env_var = True
                 break
         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)
             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=[
                 "argv[1] matches",
                 "argv[2] matches",
                 "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."""
         self.build()
 
         # Set some host environment variables now.
         os.environ["MY_HOST_ENV_VAR1"] = "VAR1"
         os.environ["MY_HOST_ENV_VAR2"] = "VAR2"
 
         # This is the function to unset the two env variables set above.
         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)
 
         # By default, inherit-env is 'true'.
         self.expect(
             'settings show target.inherit-env',
             "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)
+
+        # 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."""
         self.build()
 
         exe = self.getBuildArtifact("a.out")
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)