[Lldb-commits] [lldb] 9228a9e - [lldb/Target] Initialize new targets environment variables from target.env-vars

[Fred Riss via lldb-commits]

Author: Fred Riss
Date: 2020-03-23T07:58:33-07:00
New Revision: 9228a9efc6c57a932d936ebb214f6ff5bafe79ff

URL: https://github.com/llvm/llvm-project/commit/9228a9efc6c57a932d936ebb214f6ff5bafe79ff
DIFF: https://github.com/llvm/llvm-project/commit/9228a9efc6c57a932d936ebb214f6ff5bafe79ff.diff

LOG: [lldb/Target] Initialize new targets environment variables from target.env-vars

Summary:
The TargetProperties constructor invokes a series of callbacks to
prime the properties from the default ones. The one callback in
charge of updating the inferior environment was commented out
because it crashed.

The reason for the crash is that TargetProperties is a parent class
of Target and the callbacks were invoked using a Target that was
not fully initialized. This patch moves the initial callback
invocations to a separate function that can be called at the end
the Target constructor, thus preventing the crash.

One existing test had to be modified, because the initialization of
the environment properties now take place at the time the target is
created, not at the first use of the environment (usually launch
time).

The added test checks that the LaunchInfo object returned by
the target has been primed with the values from the settings.

Reviewers: jingham, labath

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D76009

Added: 
    

Modified: 
    lldb/include/lldb/Target/Target.h
    lldb/source/Target/Target.cpp
    lldb/test/API/commands/settings/TestSettings.py

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 3a8570c0d630..2e7932f49e6f 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -211,6 +211,8 @@ class TargetProperties : public Properties {
 
   bool GetAutoInstallMainExecutable() const;
 
+  void UpdateLaunchInfoFromProperties();
+
 private:
   // Callbacks for m_launch_info.
   void Arg0ValueChangedCallback();

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 2bb53bcd4230..0162ca838e78 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -113,6 +113,8 @@ Target::Target(Debugger &debugger, const ArchSpec &target_arch,
              target_arch.GetArchitectureName(),
              target_arch.GetTriple().getTriple().c_str());
   }
+
+  UpdateLaunchInfoFromProperties();
 }
 
 Target::~Target() {
@@ -3468,18 +3470,6 @@ TargetProperties::TargetProperties(Target *target)
         ConstString("Experimental settings - setting these won't produce "
                     "errors if the setting is not present."),
         true, m_experimental_properties_up->GetValueProperties());
-
-    // Update m_launch_info once it was created
-    Arg0ValueChangedCallback();
-    RunArgsValueChangedCallback();
-    // EnvVarsValueChangedCallback(); // FIXME: cause segfault in
-    // Target::GetPlatform()
-    InputPathValueChangedCallback();
-    OutputPathValueChangedCallback();
-    ErrorPathValueChangedCallback();
-    DetachOnErrorValueChangedCallback();
-    DisableASLRValueChangedCallback();
-    DisableSTDIOValueChangedCallback();
   } else {
     m_collection_sp =
         std::make_shared<TargetOptionValueProperties>(ConstString("target"));
@@ -3498,6 +3488,18 @@ TargetProperties::TargetProperties(Target *target)
 
 TargetProperties::~TargetProperties() = default;
 
+void TargetProperties::UpdateLaunchInfoFromProperties() {
+  Arg0ValueChangedCallback();
+  RunArgsValueChangedCallback();
+  EnvVarsValueChangedCallback();
+  InputPathValueChangedCallback();
+  OutputPathValueChangedCallback();
+  ErrorPathValueChangedCallback();
+  DetachOnErrorValueChangedCallback();
+  DisableASLRValueChangedCallback();
+  DisableSTDIOValueChangedCallback();
+}
+
 bool TargetProperties::GetInjectLocalVariables(
     ExecutionContext *exe_ctx) const {
   const Property *exp_property = m_collection_sp->GetPropertyAtIndex(

diff  --git a/lldb/test/API/commands/settings/TestSettings.py b/lldb/test/API/commands/settings/TestSettings.py
index ffb194fda808..6ec59bbe7a09 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -218,6 +218,15 @@ def test_run_args_and_env_vars(self):
         self.addTearDownHook(
             lambda: self.runCmd("settings clear target.env-vars"))
 
+        launch_info = self.dbg.GetTargetAtIndex(0).GetLaunchInfo()
+        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.runCmd("process launch --working-dir '{0}'".format(self.get_process_working_directory()),
                 RUN_SUCCEEDED)
 
@@ -238,15 +247,6 @@ def test_pass_host_env_vars(self):
         """Test that the host env vars are passed to the launched process."""
         self.build()
 
-        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")
-
         # Set some host environment variables now.
         os.environ["MY_HOST_ENV_VAR1"] = "VAR1"
         os.environ["MY_HOST_ENV_VAR2"] = "VAR2"
@@ -256,6 +256,15 @@ def unset_env_variables():
             os.environ.pop("MY_HOST_ENV_VAR1")
             os.environ.pop("MY_HOST_ENV_VAR2")
 
+        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.runCmd("process launch --working-dir '{0}'".format(self.get_process_working_directory()),
                 RUN_SUCCEEDED)