[Lldb-commits] [lldb] [lldb] Fix data race on shared global ProcessProperties callback (PR #197980)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

<details>
<summary>Changes</summary>

ProcessProperties::ProcessProperties was installing a per-process value-changed callback on ePropertyDisableLangRuntimeUnwindPlans via CreateLocalCopy. That property is declared Global in TargetProperties.td, which means its underlying OptionValue is shared across every ProcessProperties. Every Process constructor therefore wrote into the same std::function slot, racing with concurrent constructors and silently clobbering any earlier Process's callback.

Install the callback once on the global and have it walk all live Debuggers/Targets/Processes. That matches the original intent of the global setting.

Found by ThreadSanitizer as part of #<!-- -->197792.

---
Full diff: https://github.com/llvm/llvm-project/pull/197980.diff


2 Files Affected:

- (modified) lldb/include/lldb/Target/Process.h (-1) 
- (modified) lldb/source/Target/Process.cpp (+19-12) 


``````````diff
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index e2d3d54b8bb0f..38414388aca94 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -101,7 +101,6 @@ class ProcessProperties : public Properties {
   void SetStopOnSharedLibraryEvents(bool stop);
   bool GetDisableLangRuntimeUnwindPlans() const;
   void SetDisableLangRuntimeUnwindPlans(bool disable);
-  void DisableLanguageRuntimeUnwindPlansCallback();
   bool GetDetachKeepsStopped() const;
   void SetDetachKeepsStopped(bool keep_stopped);
   bool GetWarningsOptimization() const;
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index ae0f4fbcbb683..33adb7c62be36 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -174,15 +174,31 @@ ProcessProperties::ProcessProperties(lldb_private::Process *process)
     m_collection_sp->AppendProperty(
         "thread", "Settings specific to threads.", true,
         Thread::GetGlobalProperties().GetValueProperties());
+    // DisableLangRuntimeUnwindPlans is a global property. Install the
+    // setting-change callback once, rather than per-Process which would race.
+    m_collection_sp->SetValueChangedCallback(
+        ePropertyDisableLangRuntimeUnwindPlans, [] {
+          const size_t num_debuggers = Debugger::GetNumDebuggers();
+          for (size_t i = 0; i < num_debuggers; ++i) {
+            if (DebuggerSP debugger_sp = Debugger::GetDebuggerAtIndex(i)) {
+              for (TargetSP target_sp :
+                   debugger_sp->GetTargetList().Targets()) {
+                if (ProcessSP process_sp = target_sp->GetProcessSP()) {
+                  for (ThreadSP thread_sp : process_sp->Threads()) {
+                    thread_sp->ClearStackFrames();
+                    thread_sp->DiscardThreadPlans(/*force=*/true);
+                  }
+                }
+              }
+            }
+          }
+        });
   } else {
     m_collection_sp =
         OptionValueProperties::CreateLocalCopy(Process::GetGlobalProperties());
     m_collection_sp->SetValueChangedCallback(
         ePropertyPythonOSPluginPath,
         [this] { m_process->LoadOperatingSystemPlugin(true); });
-    m_collection_sp->SetValueChangedCallback(
-        ePropertyDisableLangRuntimeUnwindPlans,
-        [this] { DisableLanguageRuntimeUnwindPlansCallback(); });
   }
 
   m_experimental_properties_up =
@@ -297,15 +313,6 @@ void ProcessProperties::SetDisableLangRuntimeUnwindPlans(bool disable) {
   m_process->Flush();
 }
 
-void ProcessProperties::DisableLanguageRuntimeUnwindPlansCallback() {
-  if (!m_process)
-    return;
-  for (auto thread_sp : m_process->Threads()) {
-    thread_sp->ClearStackFrames();
-    thread_sp->DiscardThreadPlans(/*force*/ true);
-  }
-}
-
 bool ProcessProperties::GetDetachKeepsStopped() const {
   const uint32_t idx = ePropertyDetachKeepsStopped;
   return GetPropertyAtIndexAs<bool>(

``````````

</details>


https://github.com/llvm/llvm-project/pull/197980