friss created this revision.
friss added reviewers: jingham, labath.
Herald added a project: LLDB.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76009

Files:
  lldb/include/lldb/Target/Target.h
  lldb/source/Target/Target.cpp
  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
@@ -218,6 +218,15 @@
         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 @@
         """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 @@
             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)
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -81,6 +81,19 @@
   return class_name;
 }
 
+void TargetProperties::RunCallbacks() {
+  // Update m_launch_info once it was created
+  Arg0ValueChangedCallback();
+  RunArgsValueChangedCallback();
+  EnvVarsValueChangedCallback();
+  InputPathValueChangedCallback();
+  OutputPathValueChangedCallback();
+  ErrorPathValueChangedCallback();
+  DetachOnErrorValueChangedCallback();
+  DisableASLRValueChangedCallback();
+  DisableSTDIOValueChangedCallback();
+}
+
 Target::Target(Debugger &debugger, const ArchSpec &target_arch,
                const lldb::PlatformSP &platform_sp, bool is_dummy_target)
     : TargetProperties(this),
@@ -113,6 +126,8 @@
              target_arch.GetArchitectureName(),
              target_arch.GetTriple().getTriple().c_str());
   }
+
+  RunCallbacks();
 }
 
 Target::~Target() {
@@ -3468,18 +3483,6 @@
         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"));
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -211,6 +211,8 @@
 
   bool GetAutoInstallMainExecutable() const;
 
+  void RunCallbacks();
+
 private:
   // Callbacks for m_launch_info.
   void Arg0ValueChangedCallback();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to