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/5] [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/5] 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/5] 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.
>From 180c82fa32c9a9ec3e03f92b14c76c1e736c7271 Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jpien...@google.com>
Date: Mon, 12 May 2025 18:17:57 +0000
Subject: [PATCH 4/5] Simplify critical region
---
.../Plugins/SymbolFile/DWARF/DWARFUnit.cpp | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index 3a8409b1c3b66..ffd6f1dd52aff 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -196,20 +196,13 @@ DWARFUnit::ScopedExtractDIEs::ScopedExtractDIEs(DWARFUnit
&cu) : m_cu(&cu) {
DWARFUnit::ScopedExtractDIEs::~ScopedExtractDIEs() {
if (!m_cu)
return;
- {
- 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;
+ 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 && m_clear_dies &&
+ !m_cu->m_cancel_scopes) {
+ llvm::sys::ScopedWriter lock(m_cu->m_die_array_mutex);
+ m_cu->ClearDIEsRWLocked();
}
- if (!m_clear_dies || m_cu->m_cancel_scopes)
- return;
- // Be sure no other ScopedExtractDIEs is running anymore.
- 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;
- m_cu->ClearDIEsRWLocked();
}
DWARFUnit::ScopedExtractDIEs::ScopedExtractDIEs(ScopedExtractDIEs &&rhs)
>From a6ecf88ec6ec2bc83e700cabd8555f1fe2716fb7 Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jpien...@google.com>
Date: Tue, 13 May 2025 07:34:00 +0000
Subject: [PATCH 5/5] Add timeout to diagnostics test
Was flakey before this (1 in 15 in one test) and timeout seems to
suffice in not returning none here.
---
lldb/test/API/tools/lldb-dap/console/TestDAP_console.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
index 8642e317f9b3a..9cdb978368cc1 100644
--- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
+++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
@@ -176,7 +176,7 @@ def test_diagnositcs(self):
f"target create --core {core}", context="repl"
)
- output = self.get_important()
+ output = self.get_important(timeout=2.0)
self.assertIn(
"warning: unable to retrieve process ID from minidump file",
output,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
