[Lldb-commits] [lldb] [lldb] fix step in AArch64 trampoline (PR #90783)
@@ -506,9 +506,29 @@
DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread ,
Target = thread.GetProcess()->GetTarget();
const ModuleList = target.GetImages();
- images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode, target_symbols);
- if (!target_symbols.GetSize())
-return thread_plan_sp;
+ llvm::StringRef target_name = sym_name.GetStringRef();
+ // On AArch64, the trampoline name has a prefix (__AArch64ADRPThunk_ or
+ // __AArch64AbsLongThunk_) added to the function name. If we detect a
+ // trampoline with the prefix, we need to remove the prefix to find the
+ // function symbol.
+ if (target_name.consume_front("__AArch64ADRPThunk_")) {
v-bulle wrote:
done
https://github.com/llvm/llvm-project/pull/90783
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] fix step in AArch64 trampoline (PR #90783)
https://github.com/v-bulle updated
https://github.com/llvm/llvm-project/pull/90783
>From 12464941c1b11ffad0ff2566642df3d30976a3f9 Mon Sep 17 00:00:00 2001
From: Vincent Belliard
Date: Thu, 18 Apr 2024 10:39:59 -0700
Subject: [PATCH 1/3] [lldb] fix step in AArch64 trampoline
---
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 26 ---
.../Plugins/ObjectFile/ELF/ObjectFileELF.cpp | 19 +-
.../StepIn/Inputs/aarch64_thunk.cc| 15 +++
.../StepIn/step_through-aarch64-thunk.test| 17
4 files changed, 73 insertions(+), 4 deletions(-)
create mode 100644 lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
create mode 100644
lldb/test/Shell/ExecControl/StepIn/step_through-aarch64-thunk.test
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 9fa245fc41d40c..232030268e42c8 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -506,9 +506,29 @@
DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread ,
Target = thread.GetProcess()->GetTarget();
const ModuleList = target.GetImages();
- images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode, target_symbols);
- if (!target_symbols.GetSize())
-return thread_plan_sp;
+ llvm::StringRef target_name = sym_name.GetStringRef();
+ // On AArch64, the trampoline name has a prefix (__AArch64ADRPThunk_ or
+ // __AArch64AbsLongThunk_) added to the function name. If we detect a
+ // trampoline with the prefix, we need to remove the prefix to find the
+ // function symbol.
+ if (target_name.consume_front("__AArch64ADRPThunk_")) {
+// An empty target name can happen when for trampolines generated for
+// section-referencing relocations.
+if (!target_name.empty()) {
+ images.FindSymbolsWithNameAndType(ConstString(target_name),
+eSymbolTypeCode, target_symbols);
+}
+ } else if (target_name.consume_front("__AArch64AbsLongThunk_")) {
+// An empty target name can happen when for trampolines generated for
+// section-referencing relocations.
+if (!target_name.empty()) {
+ images.FindSymbolsWithNameAndType(ConstString(target_name),
+eSymbolTypeCode, target_symbols);
+}
+ } else {
+images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode,
+ target_symbols);
+ }
typedef std::vector AddressVector;
AddressVector addrs;
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 16f6d2e884b577..1646ee9aa34a61 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2356,13 +2356,30 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab,
user_id_t start_id,
bool symbol_size_valid =
symbol.st_size != 0 || symbol.getType() != STT_FUNC;
+bool is_trampoline = false;
+if (arch.IsValid() && (arch.GetMachine() == llvm::Triple::aarch64)) {
+ // On AArch64, trampolines are registered as code.
+ // If we detect a trampoline (which starts with __AArch64ADRPThunk_ or
+ // __AArch64AbsLongThunk_) we register the symbol as a trampoline. This
+ // way we will be able to detect the trampoline when we step in a
function
+ // and step through the trampoline.
+ if (symbol_type == eSymbolTypeCode) {
+llvm::StringRef trampoline_name = mangled.GetName().GetStringRef();
+if (trampoline_name.starts_with("__AArch64ADRPThunk_") ||
+trampoline_name.starts_with("__AArch64AbsLongThunk_")) {
+ symbol_type = eSymbolTypeTrampoline;
+ is_trampoline = true;
+}
+ }
+}
+
Symbol dc_symbol(
i + start_id, // ID is the original symbol table index.
mangled,
symbol_type,// Type of this symbol
is_global, // Is this globally visible?
false, // Is this symbol debug info?
-false, // Is this symbol a trampoline?
+is_trampoline, // Is this symbol a trampoline?
false, // Is this symbol artificial?
AddressRange(symbol_section_sp, // Section in which this symbol is
// defined or null.
diff --git a/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
b/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
new file mode 100644
index 00..02f3bef32a59a3
--- /dev/null
+++ b/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
@@ -0,0 +1,15 @@
+extern "C" int __attribute__((naked)) __AArch64ADRPThunk_step_here()
[Lldb-commits] [lldb] [lldb] fix step in AArch64 trampoline (PR #90783)
https://github.com/v-bulle updated
https://github.com/llvm/llvm-project/pull/90783
>From 12464941c1b11ffad0ff2566642df3d30976a3f9 Mon Sep 17 00:00:00 2001
From: Vincent Belliard
Date: Thu, 18 Apr 2024 10:39:59 -0700
Subject: [PATCH 1/2] [lldb] fix step in AArch64 trampoline
---
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 26 ---
.../Plugins/ObjectFile/ELF/ObjectFileELF.cpp | 19 +-
.../StepIn/Inputs/aarch64_thunk.cc| 15 +++
.../StepIn/step_through-aarch64-thunk.test| 17
4 files changed, 73 insertions(+), 4 deletions(-)
create mode 100644 lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
create mode 100644
lldb/test/Shell/ExecControl/StepIn/step_through-aarch64-thunk.test
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 9fa245fc41d40c..232030268e42c8 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -506,9 +506,29 @@
DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread ,
Target = thread.GetProcess()->GetTarget();
const ModuleList = target.GetImages();
- images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode, target_symbols);
- if (!target_symbols.GetSize())
-return thread_plan_sp;
+ llvm::StringRef target_name = sym_name.GetStringRef();
+ // On AArch64, the trampoline name has a prefix (__AArch64ADRPThunk_ or
+ // __AArch64AbsLongThunk_) added to the function name. If we detect a
+ // trampoline with the prefix, we need to remove the prefix to find the
+ // function symbol.
+ if (target_name.consume_front("__AArch64ADRPThunk_")) {
+// An empty target name can happen when for trampolines generated for
+// section-referencing relocations.
+if (!target_name.empty()) {
+ images.FindSymbolsWithNameAndType(ConstString(target_name),
+eSymbolTypeCode, target_symbols);
+}
+ } else if (target_name.consume_front("__AArch64AbsLongThunk_")) {
+// An empty target name can happen when for trampolines generated for
+// section-referencing relocations.
+if (!target_name.empty()) {
+ images.FindSymbolsWithNameAndType(ConstString(target_name),
+eSymbolTypeCode, target_symbols);
+}
+ } else {
+images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode,
+ target_symbols);
+ }
typedef std::vector AddressVector;
AddressVector addrs;
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 16f6d2e884b577..1646ee9aa34a61 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2356,13 +2356,30 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab,
user_id_t start_id,
bool symbol_size_valid =
symbol.st_size != 0 || symbol.getType() != STT_FUNC;
+bool is_trampoline = false;
+if (arch.IsValid() && (arch.GetMachine() == llvm::Triple::aarch64)) {
+ // On AArch64, trampolines are registered as code.
+ // If we detect a trampoline (which starts with __AArch64ADRPThunk_ or
+ // __AArch64AbsLongThunk_) we register the symbol as a trampoline. This
+ // way we will be able to detect the trampoline when we step in a
function
+ // and step through the trampoline.
+ if (symbol_type == eSymbolTypeCode) {
+llvm::StringRef trampoline_name = mangled.GetName().GetStringRef();
+if (trampoline_name.starts_with("__AArch64ADRPThunk_") ||
+trampoline_name.starts_with("__AArch64AbsLongThunk_")) {
+ symbol_type = eSymbolTypeTrampoline;
+ is_trampoline = true;
+}
+ }
+}
+
Symbol dc_symbol(
i + start_id, // ID is the original symbol table index.
mangled,
symbol_type,// Type of this symbol
is_global, // Is this globally visible?
false, // Is this symbol debug info?
-false, // Is this symbol a trampoline?
+is_trampoline, // Is this symbol a trampoline?
false, // Is this symbol artificial?
AddressRange(symbol_section_sp, // Section in which this symbol is
// defined or null.
diff --git a/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
b/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
new file mode 100644
index 00..02f3bef32a59a3
--- /dev/null
+++ b/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
@@ -0,0 +1,15 @@
+extern "C" int __attribute__((naked)) __AArch64ADRPThunk_step_here()
[Lldb-commits] [lldb] [lldb] fix step in AArch64 trampoline (PR #90783)
https://github.com/v-bulle created
https://github.com/llvm/llvm-project/pull/90783
Detects AArch64 trampolines in order to be able to step in a function through a
trampoline on AArch64.
>From 12464941c1b11ffad0ff2566642df3d30976a3f9 Mon Sep 17 00:00:00 2001
From: Vincent Belliard
Date: Thu, 18 Apr 2024 10:39:59 -0700
Subject: [PATCH] [lldb] fix step in AArch64 trampoline
---
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 26 ---
.../Plugins/ObjectFile/ELF/ObjectFileELF.cpp | 19 +-
.../StepIn/Inputs/aarch64_thunk.cc| 15 +++
.../StepIn/step_through-aarch64-thunk.test| 17
4 files changed, 73 insertions(+), 4 deletions(-)
create mode 100644 lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
create mode 100644
lldb/test/Shell/ExecControl/StepIn/step_through-aarch64-thunk.test
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 9fa245fc41d40c..232030268e42c8 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -506,9 +506,29 @@
DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread ,
Target = thread.GetProcess()->GetTarget();
const ModuleList = target.GetImages();
- images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode, target_symbols);
- if (!target_symbols.GetSize())
-return thread_plan_sp;
+ llvm::StringRef target_name = sym_name.GetStringRef();
+ // On AArch64, the trampoline name has a prefix (__AArch64ADRPThunk_ or
+ // __AArch64AbsLongThunk_) added to the function name. If we detect a
+ // trampoline with the prefix, we need to remove the prefix to find the
+ // function symbol.
+ if (target_name.consume_front("__AArch64ADRPThunk_")) {
+// An empty target name can happen when for trampolines generated for
+// section-referencing relocations.
+if (!target_name.empty()) {
+ images.FindSymbolsWithNameAndType(ConstString(target_name),
+eSymbolTypeCode, target_symbols);
+}
+ } else if (target_name.consume_front("__AArch64AbsLongThunk_")) {
+// An empty target name can happen when for trampolines generated for
+// section-referencing relocations.
+if (!target_name.empty()) {
+ images.FindSymbolsWithNameAndType(ConstString(target_name),
+eSymbolTypeCode, target_symbols);
+}
+ } else {
+images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode,
+ target_symbols);
+ }
typedef std::vector AddressVector;
AddressVector addrs;
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 16f6d2e884b577..1646ee9aa34a61 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2356,13 +2356,30 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab,
user_id_t start_id,
bool symbol_size_valid =
symbol.st_size != 0 || symbol.getType() != STT_FUNC;
+bool is_trampoline = false;
+if (arch.IsValid() && (arch.GetMachine() == llvm::Triple::aarch64)) {
+ // On AArch64, trampolines are registered as code.
+ // If we detect a trampoline (which starts with __AArch64ADRPThunk_ or
+ // __AArch64AbsLongThunk_) we register the symbol as a trampoline. This
+ // way we will be able to detect the trampoline when we step in a
function
+ // and step through the trampoline.
+ if (symbol_type == eSymbolTypeCode) {
+llvm::StringRef trampoline_name = mangled.GetName().GetStringRef();
+if (trampoline_name.starts_with("__AArch64ADRPThunk_") ||
+trampoline_name.starts_with("__AArch64AbsLongThunk_")) {
+ symbol_type = eSymbolTypeTrampoline;
+ is_trampoline = true;
+}
+ }
+}
+
Symbol dc_symbol(
i + start_id, // ID is the original symbol table index.
mangled,
symbol_type,// Type of this symbol
is_global, // Is this globally visible?
false, // Is this symbol debug info?
-false, // Is this symbol a trampoline?
+is_trampoline, // Is this symbol a trampoline?
false, // Is this symbol artificial?
AddressRange(symbol_section_sp, // Section in which this symbol is
// defined or null.
diff --git a/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
b/lldb/test/Shell/ExecControl/StepIn/Inputs/aarch64_thunk.cc
new file mode 100644
index 00..02f3bef32a59a3
--- /dev/null
+++
[Lldb-commits] [lldb] [lldb] fix dead lock in TypeCategoryMap.cpp (PR #87540)
@@ -25,19 +25,25 @@ TypeCategoryMap::TypeCategoryMap(IFormatChangeListener *lst)
}
void TypeCategoryMap::Add(KeyType name, const TypeCategoryImplSP ) {
- std::lock_guard guard(m_map_mutex);
- m_map[name] = entry;
+ {
+std::lock_guard guard(m_map_mutex);
+m_map[name] = entry;
+ }
+ // The lock is now released for the eventual call to Changed.
v-bulle wrote:
done
https://github.com/llvm/llvm-project/pull/87540
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] fix dead lock in TypeCategoryMap.cpp (PR #87540)
https://github.com/v-bulle updated
https://github.com/llvm/llvm-project/pull/87540
>From 5234f6873d894dd80b2c1fef40fd18e4b722a2c9 Mon Sep 17 00:00:00 2001
From: Vincent Belliard
Date: Wed, 3 Apr 2024 11:31:06 -0700
Subject: [PATCH 1/3] [lldb] fix dead lock in TypeCategoryMap.cpp
FormatManager::GetCategoryForLanguage and
FormatManager::GetCategory(can_create = true) can be called concurently
and they both take the TypeCategory::m_map_mutex and the
FormatManager::m_language_categories_mutex but in reverse order.
Without the patch, we can have a dead lock.
---
.../source/DataFormatters/TypeCategoryMap.cpp | 28 +++
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/lldb/source/DataFormatters/TypeCategoryMap.cpp
b/lldb/source/DataFormatters/TypeCategoryMap.cpp
index fd76bab95826af..0d1f55fff473d1 100644
--- a/lldb/source/DataFormatters/TypeCategoryMap.cpp
+++ b/lldb/source/DataFormatters/TypeCategoryMap.cpp
@@ -25,19 +25,23 @@ TypeCategoryMap::TypeCategoryMap(IFormatChangeListener *lst)
}
void TypeCategoryMap::Add(KeyType name, const TypeCategoryImplSP ) {
- std::lock_guard guard(m_map_mutex);
- m_map[name] = entry;
+ {
+std::lock_guard guard(m_map_mutex);
+m_map[name] = entry;
+ }
if (listener)
listener->Changed();
}
bool TypeCategoryMap::Delete(KeyType name) {
- std::lock_guard guard(m_map_mutex);
- MapIterator iter = m_map.find(name);
- if (iter == m_map.end())
-return false;
- m_map.erase(name);
- Disable(name);
+ {
+std::lock_guard guard(m_map_mutex);
+MapIterator iter = m_map.find(name);
+if (iter == m_map.end())
+ return false;
+m_map.erase(name);
+Disable(name);
+ }
if (listener)
listener->Changed();
return true;
@@ -123,9 +127,11 @@ void TypeCategoryMap::DisableAllCategories() {
}
void TypeCategoryMap::Clear() {
- std::lock_guard guard(m_map_mutex);
- m_map.clear();
- m_active_categories.clear();
+ {
+std::lock_guard guard(m_map_mutex);
+m_map.clear();
+m_active_categories.clear();
+ }
if (listener)
listener->Changed();
}
>From 7305de06ab0425631fdf554cd1ad0498ac9fc7ed Mon Sep 17 00:00:00 2001
From: Vincent Belliard
Date: Thu, 4 Apr 2024 15:20:59 -0700
Subject: [PATCH 2/3] Adds comments
---
lldb/source/DataFormatters/TypeCategoryMap.cpp | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lldb/source/DataFormatters/TypeCategoryMap.cpp
b/lldb/source/DataFormatters/TypeCategoryMap.cpp
index 0d1f55fff473d1..ecbdcde98d07d3 100644
--- a/lldb/source/DataFormatters/TypeCategoryMap.cpp
+++ b/lldb/source/DataFormatters/TypeCategoryMap.cpp
@@ -29,6 +29,7 @@ void TypeCategoryMap::Add(KeyType name, const
TypeCategoryImplSP ) {
std::lock_guard guard(m_map_mutex);
m_map[name] = entry;
}
+ // The lock is now released for the eventual call to Changed.
if (listener)
listener->Changed();
}
@@ -42,6 +43,7 @@ bool TypeCategoryMap::Delete(KeyType name) {
m_map.erase(name);
Disable(name);
}
+ // The lock is now released for the eventual call to Changed.
if (listener)
listener->Changed();
return true;
@@ -132,6 +134,7 @@ void TypeCategoryMap::Clear() {
m_map.clear();
m_active_categories.clear();
}
+ // The lock is now released for the eventual call to Changed.
if (listener)
listener->Changed();
}
>From bb51c8944ccecac2af5d9527a61718e1d1e85d4b Mon Sep 17 00:00:00 2001
From: Vincent Belliard
Date: Tue, 9 Apr 2024 08:33:36 -0700
Subject: [PATCH 3/3] Modify comment
---
lldb/source/DataFormatters/TypeCategoryMap.cpp | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/lldb/source/DataFormatters/TypeCategoryMap.cpp
b/lldb/source/DataFormatters/TypeCategoryMap.cpp
index ecbdcde98d07d3..ce2cf369b5be53 100644
--- a/lldb/source/DataFormatters/TypeCategoryMap.cpp
+++ b/lldb/source/DataFormatters/TypeCategoryMap.cpp
@@ -29,7 +29,10 @@ void TypeCategoryMap::Add(KeyType name, const
TypeCategoryImplSP ) {
std::lock_guard guard(m_map_mutex);
m_map[name] = entry;
}
- // The lock is now released for the eventual call to Changed.
+ // Release the mutex to avoid a potential deadlock between
+ // TypeCategoryMap::m_map_mutex and
+ // FormatManager::m_language_categories_mutex which can be acquired in
+ // reverse order when calling FormatManager::Changed.
if (listener)
listener->Changed();
}
@@ -43,7 +46,10 @@ bool TypeCategoryMap::Delete(KeyType name) {
m_map.erase(name);
Disable(name);
}
- // The lock is now released for the eventual call to Changed.
+ // Release the mutex to avoid a potential deadlock between
+ // TypeCategoryMap::m_map_mutex and
+ // FormatManager::m_language_categories_mutex which can be acquired in
+ // reverse order when calling FormatManager::Changed.
if (listener)
listener->Changed();
return true;
@@ -134,7 +140,10 @@ void TypeCategoryMap::Clear() {
m_map.clear();
m_active_categories.clear();
}
-
[Lldb-commits] [lldb] [lldb] fix dead lock in TypeCategoryMap.cpp (PR #87540)
https://github.com/v-bulle updated
https://github.com/llvm/llvm-project/pull/87540
>From 5234f6873d894dd80b2c1fef40fd18e4b722a2c9 Mon Sep 17 00:00:00 2001
From: Vincent Belliard
Date: Wed, 3 Apr 2024 11:31:06 -0700
Subject: [PATCH 1/2] [lldb] fix dead lock in TypeCategoryMap.cpp
FormatManager::GetCategoryForLanguage and
FormatManager::GetCategory(can_create = true) can be called concurently
and they both take the TypeCategory::m_map_mutex and the
FormatManager::m_language_categories_mutex but in reverse order.
Without the patch, we can have a dead lock.
---
.../source/DataFormatters/TypeCategoryMap.cpp | 28 +++
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/lldb/source/DataFormatters/TypeCategoryMap.cpp
b/lldb/source/DataFormatters/TypeCategoryMap.cpp
index fd76bab95826af..0d1f55fff473d1 100644
--- a/lldb/source/DataFormatters/TypeCategoryMap.cpp
+++ b/lldb/source/DataFormatters/TypeCategoryMap.cpp
@@ -25,19 +25,23 @@ TypeCategoryMap::TypeCategoryMap(IFormatChangeListener *lst)
}
void TypeCategoryMap::Add(KeyType name, const TypeCategoryImplSP ) {
- std::lock_guard guard(m_map_mutex);
- m_map[name] = entry;
+ {
+std::lock_guard guard(m_map_mutex);
+m_map[name] = entry;
+ }
if (listener)
listener->Changed();
}
bool TypeCategoryMap::Delete(KeyType name) {
- std::lock_guard guard(m_map_mutex);
- MapIterator iter = m_map.find(name);
- if (iter == m_map.end())
-return false;
- m_map.erase(name);
- Disable(name);
+ {
+std::lock_guard guard(m_map_mutex);
+MapIterator iter = m_map.find(name);
+if (iter == m_map.end())
+ return false;
+m_map.erase(name);
+Disable(name);
+ }
if (listener)
listener->Changed();
return true;
@@ -123,9 +127,11 @@ void TypeCategoryMap::DisableAllCategories() {
}
void TypeCategoryMap::Clear() {
- std::lock_guard guard(m_map_mutex);
- m_map.clear();
- m_active_categories.clear();
+ {
+std::lock_guard guard(m_map_mutex);
+m_map.clear();
+m_active_categories.clear();
+ }
if (listener)
listener->Changed();
}
>From 7305de06ab0425631fdf554cd1ad0498ac9fc7ed Mon Sep 17 00:00:00 2001
From: Vincent Belliard
Date: Thu, 4 Apr 2024 15:20:59 -0700
Subject: [PATCH 2/2] Adds comments
---
lldb/source/DataFormatters/TypeCategoryMap.cpp | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lldb/source/DataFormatters/TypeCategoryMap.cpp
b/lldb/source/DataFormatters/TypeCategoryMap.cpp
index 0d1f55fff473d1..ecbdcde98d07d3 100644
--- a/lldb/source/DataFormatters/TypeCategoryMap.cpp
+++ b/lldb/source/DataFormatters/TypeCategoryMap.cpp
@@ -29,6 +29,7 @@ void TypeCategoryMap::Add(KeyType name, const
TypeCategoryImplSP ) {
std::lock_guard guard(m_map_mutex);
m_map[name] = entry;
}
+ // The lock is now released for the eventual call to Changed.
if (listener)
listener->Changed();
}
@@ -42,6 +43,7 @@ bool TypeCategoryMap::Delete(KeyType name) {
m_map.erase(name);
Disable(name);
}
+ // The lock is now released for the eventual call to Changed.
if (listener)
listener->Changed();
return true;
@@ -132,6 +134,7 @@ void TypeCategoryMap::Clear() {
m_map.clear();
m_active_categories.clear();
}
+ // The lock is now released for the eventual call to Changed.
if (listener)
listener->Changed();
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] fix dead lock in TypeCategoryMap.cpp (PR #87540)
https://github.com/v-bulle edited https://github.com/llvm/llvm-project/pull/87540 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
