https://github.com/jpienaar updated 
https://github.com/llvm/llvm-project/pull/139252

>From c5ffbd84f8b68bae2112e8cec68803cefe571a72 Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jpien...@google.com>
Date: Fri, 9 May 2025 05:23:00 -0700
Subject: [PATCH 1/3] [lldb][plugin] Clear in same thread as set

Here we were initializing & locking a mutex in a thread, while releasing it in 
the parent which may/often turned out to be a different thread 
(shared_mutex::unlock_shared is undefined behavior if called from a thread that 
doesn't hold the lock).

I'm not quite sure what the expectation is here as the variable is never used, 
so instead I've just reset in same thread as which it was set to ensure its 
freed in thread holding lock.
---
 lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 523820874752a..0f0226ea9650c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -121,6 +121,7 @@ void ManualDWARFIndex::Index() {
       units_to_index.size());
   for_each_unit([&clear_cu_dies](size_t, size_t idx, DWARFUnit *unit) {
     clear_cu_dies[idx] = unit->ExtractDIEsScoped();
+    ckear_cu_duex[idx].reset();
   });
 
   // Now index all DWARF unit in parallel.

>From 5f5b8dc0deae4f63ddb83e0dfab96ab3a9e0cc80 Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jpien...@google.com>
Date: Fri, 9 May 2025 08:15:26 -0700
Subject: [PATCH 2/3] Fix typo

---
 lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 0f0226ea9650c..6139d005b4f2e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -121,7 +121,7 @@ void ManualDWARFIndex::Index() {
       units_to_index.size());
   for_each_unit([&clear_cu_dies](size_t, size_t idx, DWARFUnit *unit) {
     clear_cu_dies[idx] = unit->ExtractDIEsScoped();
-    ckear_cu_duex[idx].reset();
+    ckear_cu_dies[idx].reset();
   });
 
   // Now index all DWARF unit in parallel.

>From 6d8c69c480ce214772cb84a27da645b428916ecb Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jpien...@google.com>
Date: Mon, 12 May 2025 13:19:45 +0000
Subject: [PATCH 3/3] Use plain reader counter

---
 lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp   | 12 +++++++++---
 lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h     |  4 +++-
 .../Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp    |  1 -
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index 7d0afc04ac3b6..3a8409b1c3b66 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -189,17 +189,23 @@ DWARFUnit::ScopedExtractDIEs 
DWARFUnit::ExtractDIEsScoped() {
 }
 
 DWARFUnit::ScopedExtractDIEs::ScopedExtractDIEs(DWARFUnit &cu) : m_cu(&cu) {
-  m_cu->m_die_array_scoped_mutex.lock_shared();
+  llvm::sys::ScopedLock lock(m_cu->m_die_array_scoped_mutex);
+  ++m_cu->m_die_array_scoped_count;
 }
 
 DWARFUnit::ScopedExtractDIEs::~ScopedExtractDIEs() {
   if (!m_cu)
     return;
-  m_cu->m_die_array_scoped_mutex.unlock_shared();
+  {
+    llvm::sys::ScopedLock lock(m_cu->m_die_array_scoped_mutex);
+    --m_cu->m_die_array_scoped_count;
+    if (m_cu->m_die_array_scoped_count == 0)
+      return;
+  }
   if (!m_clear_dies || m_cu->m_cancel_scopes)
     return;
   // Be sure no other ScopedExtractDIEs is running anymore.
-  llvm::sys::ScopedWriter lock_scoped(m_cu->m_die_array_scoped_mutex);
+  llvm::sys::ScopedLock lock_scoped(m_cu->m_die_array_scoped_mutex);
   llvm::sys::ScopedWriter lock(m_cu->m_die_array_mutex);
   if (m_cu->m_cancel_scopes)
     return;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
index 75a003e0a663c..c05bba36ed74b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -17,6 +17,7 @@
 #include "llvm/DebugInfo/DWARF/DWARFAddressRange.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugRnglists.h"
+#include "llvm/Support/Mutex.h"
 #include "llvm/Support/RWMutex.h"
 #include <atomic>
 #include <optional>
@@ -328,7 +329,8 @@ class DWARFUnit : public DWARFExpression::Delegate, public 
UserID {
   DWARFDebugInfoEntry::collection m_die_array;
   mutable llvm::sys::RWMutex m_die_array_mutex;
   // It is used for tracking of ScopedExtractDIEs instances.
-  mutable llvm::sys::RWMutex m_die_array_scoped_mutex;
+  mutable llvm::sys::Mutex m_die_array_scoped_mutex;
+  mutable int m_die_array_scoped_count = 0;
   // ScopedExtractDIEs instances should not call ClearDIEsRWLocked()
   // as someone called ExtractDIEsIfNeeded().
   std::atomic<bool> m_cancel_scopes;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 6139d005b4f2e..523820874752a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -121,7 +121,6 @@ void ManualDWARFIndex::Index() {
       units_to_index.size());
   for_each_unit([&clear_cu_dies](size_t, size_t idx, DWARFUnit *unit) {
     clear_cu_dies[idx] = unit->ExtractDIEsScoped();
-    ckear_cu_dies[idx].reset();
   });
 
   // Now index all DWARF unit in parallel.

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to