[ASan] Prevent ASan/LSan deadlock by preloading modules before error reporting #131756
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…reporting This fixes a circular lock dependency between ASan's thread registry lock (B) and libdl's dl_iterate_phdr lock (A) that occurs when: 1. ASan error reporting (holding B) calls dl_iterate_phdr (needs A) 2. LSan leak check (holding A) acquires ASan thread registry lock (B) The fix proactively initializes module information before acquiring ASan's thread registry lock during error reporting. This eliminates the need to call `dl_iterate_phdr` while already holding lock B, breaking the dangerous A→B/B→A cross-thread locking pattern. Add test case (compiler-rt/test/asan/TestCases/asan_lsan_deadlock.cpp) that reproduces the deadlock without this patch.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
compiler-rt
compiler-rt:asan
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: None (Camsyn) ChangesDescriptionThis PR resolves a deadlock between AddressSanitizer (ASan) and LeakSanitizer (LSan) Issue DetailsReproducer // Thread 1: ASan error path
int arr[1] = {0};
std::thread t([&]() {
arr[1] = 1; // Triggers ASan OOB error
});
// Thread 2: LSan check path
__lsan_do_leak_check(); Lock Order Conflict:
This creates a circular wait condition (A -> B -> A) meeting all four Coffman deadlock criteria. Fix StrategyThe root cause lies in ASan's error reporting path needing
Key code change: // Before acquiring ASan's thread registry lock:
Symbolizer::GetOrInit()->GetRefreshedListOfModules(); This guarantees module information is cached before lock acquisition, eliminating TestingAdded asan_lsan_deadlock.cpp test case:
Note: Due to the inherent non-determinism of thread scheduling and lock acquisition timing, Impact
Relevant Buggy Code
explicit ScopedInErrorReport(bool fatal = false)
: halt_on_error_(fatal || flags()->halt_on_error) {
// Acquire lock B
asanThreadRegistry().Lock();
}
~ScopedInErrorReport() {
...
// Try to acquire lock A under holding lock B via the following path
// #<!-- -->4 0x000071a353d83e93 in __GI___dl_iterate_phdr (
// callback=0x5d1a07a39580 <__sanitizer::dl_iterate_phdr_cb(dl_phdr_info*, unsigned long, void*)>,
// data=0x6da3510fd3f0) at ./elf/dl-iteratephdr.c:39
// #<!-- -->5 0x00005d1a07a39574 in __sanitizer::ListOfModules::init (this=0x71a353ebc080)
// at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp:784
// #<!-- -->6 0x00005d1a07a429e3 in __sanitizer::Symbolizer::RefreshModules (this=0x71a353ebc058)
// at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp:188
// #<!-- -->7 __sanitizer::Symbolizer::FindModuleForAddress (this=this@<!-- -->entry=0x71a353ebc058,
// address=address@<!-- -->entry=102366378805727)
// at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp:214
// #<!-- -->8 0x00005d1a07a4291b in __sanitizer::Symbolizer::SymbolizePC (this=0x71a353ebc058, addr=102366378805727)
// at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp:88
// #<!-- -->9 0x00005d1a07a40df7 in __sanitizer::(anonymous namespace)::StackTraceTextPrinter::ProcessAddressFrames (
// this=this@<!-- -->entry=0x6da3510fd520, pc=102366378805727)
// at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp:37
// #<!-- -->10 0x00005d1a07a40d27 in __sanitizer::StackTrace::PrintTo (this=this@<!-- -->entry=0x6da3510fd5e8,
// output=output@<!-- -->entry=0x6da3510fd588)
// at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp:110
// #<!-- -->11 0x00005d1a07a410a1 in __sanitizer::StackTrace::Print (this=0x6da3510fd5e8)
// at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp:133
// #<!-- -->12 0x00005d1a0798758d in __asan::ErrorGeneric::Print (
// this=0x5d1a07aa4e08 <__asan::ScopedInErrorReport::current_error_+8>)
// at llvm-project/compiler-rt/lib/asan/asan_errors.cpp:617
current_error_.Print();
...
}
void LockStuffAndStopTheWorld(StopTheWorldCallback callback,
CheckForLeaksParam *argument) {
// Acquire lock A
dl_iterate_phdr(LockStuffAndStopTheWorldCallback, &param);
}
static int LockStuffAndStopTheWorldCallback(struct dl_phdr_info *info,
size_t size, void *data) {
// Try to acquire lock B under holding lock A via the following path
// #<!-- -->3 0x000055555562b34a in __sanitizer::ThreadRegistry::Lock (this=<optimized out>)
// at llvm-project/compiler-rt/lib/asan/../sanitizer_common/sanitizer_thread_registry.h:99
// #<!-- -->4 __lsan::LockThreads () at llvm-project/compiler-rt/lib/asan/asan_thread.cpp:484
// #<!-- -->5 0x0000555555652629 in __lsan::ScopedStopTheWorldLock::ScopedStopTheWorldLock (this=<optimized out>)
// at llvm-project/compiler-rt/lib/lsan/lsan_common.h:164
// #<!-- -->6 __lsan::LockStuffAndStopTheWorldCallback (info=<optimized out>, size=<optimized out>, data=0x0,
// data@<!-- -->entry=0x7fffffffd158) at llvm-project/compiler-rt/lib/lsan/lsan_common_linux.cpp:120
ScopedStopTheWorldLock lock;
DoStopTheWorldParam *param = reinterpret_cast<DoStopTheWorldParam *>(data);
StopTheWorld(param->callback, param->argument);
return 1;
}Full diff: https://github.com/llvm/llvm-project/pull/131756.diff 2 Files Affected:
diff --git a/compiler-rt/lib/asan/asan_report.cpp b/compiler-rt/lib/asan/asan_report.cpp
index 45aa607dcda07..8aeb938b2ee3d 100644
--- a/compiler-rt/lib/asan/asan_report.cpp
+++ b/compiler-rt/lib/asan/asan_report.cpp
@@ -126,6 +126,27 @@ class ScopedInErrorReport {
public:
explicit ScopedInErrorReport(bool fatal = false)
: halt_on_error_(fatal || flags()->halt_on_error) {
+ /*
+ * Deadlock Prevention Between ASan and LSan
+ *
+ * Background:
+ * - The `dl_iterate_phdr` function requires holding libdl's internal lock (Lock A).
+ * - LSan acquires the ASan thread registry lock (Lock B) *after* calling `dl_iterate_phdr`.
+ *
+ * Problem Scenario:
+ * When ASan attempts to call `dl_iterate_phdr` while holding Lock B (e.g., during
+ * error reporting via `ErrorDescription::Print`), a circular lock dependency may occur:
+ * 1. Thread 1: Holds Lock B → Requests Lock A (via dl_iterate_phdr)
+ * 2. Thread 2: Holds Lock A → Requests Lock B (via LSan operations)
+ *
+ * Solution:
+ * Proactively load all required modules before acquiring Lock B. This ensures:
+ * 1. Any `dl_iterate_phdr` calls during module loading complete before locking
+ * 2. Subsequent error reporting avoids nested lock acquisition patterns
+ * 3. Eliminates the lock order inversion risk between libdl and ASan's thread registry
+ */
+ Symbolizer::GetOrInit()->GetRefreshedListOfModules();
+
// Make sure the registry and sanitizer report mutexes are locked while
// we're printing an error report.
// We can lock them only here to avoid self-deadlock in case of
diff --git a/compiler-rt/test/asan/TestCases/asan_lsan_deadlock.cpp b/compiler-rt/test/asan/TestCases/asan_lsan_deadlock.cpp
new file mode 100644
index 0000000000000..6d4c91eb49241
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/asan_lsan_deadlock.cpp
@@ -0,0 +1,73 @@
+// Test for potential deadlock in LeakSanitizer+AddressSanitizer.
+// REQUIRES: leak-detection
+//
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: %env_asan_opts=detect_leaks=1 not %run %t 2>&1 | FileCheck %s
+
+/*
+ * Purpose: Verify deadlock prevention between ASan error reporting and LSan leak checking.
+ *
+ * Test Design:
+ * 1. Creates contention scenario between:
+ * - ASan's error reporting (requires lock B -> lock A ordering)
+ * - LSan's leak check (requires lock A -> lock B ordering)
+ * 2. Thread timing:
+ * - Main thread: Holds 'in' mutex -> Triggers LSan check (lock A then B)
+ * - Worker thread: Triggers ASan OOB error (lock B then A via symbolization)
+ *
+ * Deadlock Condition (if unfixed):
+ * Circular lock dependency forms when:
+ * [Main Thread] LSan: lock A -> requests lock B
+ * [Worker Thread] ASan: lock B -> requests lock A
+ *
+ * Success Criteria:
+ * With proper lock ordering enforcement, watchdog should NOT trigger - test exits normally.
+ * If deadlock occurs, watchdog terminates via _exit(1) after 10s timeout.
+ */
+
+#include <mutex>
+#include <sanitizer/lsan_interface.h>
+#include <stdio.h>
+#include <thread>
+#include <unistd.h>
+
+std::mutex in;
+
+void Watchdog() {
+ // Safety mechanism: Turn infinite deadlock into finite test failure
+ usleep(10000000);
+ // CHECK-NOT: Timeout! Deadlock detected.
+ puts("Timeout! Deadlock detected.");
+ fflush(stdout);
+ _exit(1);
+}
+
+int main(int argc, char **argv) {
+ int arr[1] = {0};
+ in.lock();
+
+ std::thread w(Watchdog);
+ w.detach();
+
+ std::thread t([&]() {
+ in.unlock();
+ /*
+ * Provoke ASan error: ASan's error reporting acquires:
+ * 1. ASan's thread registry lock (B) during the reporting
+ * 2. dl_iterate_phdr lock (A) during symbolization
+ */
+ // CHECK: SUMMARY: AddressSanitizer: stack-buffer-overflow
+ arr[argc] = 1; // Deliberate OOB access
+ });
+
+ in.lock();
+ /*
+ * Critical section: LSan's check acquires:
+ * 1. dl_iterate_phdr lock (A)
+ * 2. ASan's thread registry lock (B)
+ * before Stop The World.
+ */
+ __lsan_do_leak_check();
+ t.join();
+ return 0;
+}
\ No newline at end of file
|
|
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
fmayer
reviewed
Mar 20, 2025
vitalybuka
reviewed
Mar 20, 2025
Proposed Fixes for ASan-LSan Deadlock IssueAfter analyzing the ASan/LSan code and commit history, I confirm that concurrent error reporting from different threads should not be considered (it has already been handled by 1. Maintain Current Patch (Incomplete)
2. Revert LSan Lock Order (Incomplete)
3. Align ASan with LSan (Complete)
4. Leverage
|
| Solution | Completeness | Risk | Code Impact |
|---|---|---|---|
| 1 | Partial | Regression risk | Low |
| 1 (with supplement) | Full | Code changes in sanitizer_common |
Moderate |
| 2 | Partial | New deadlock scenarios | Moderate |
| 3 | Full | High maintenance cost | High |
| 4 | Full | Logical coupling concerns | Low |
Which fix should we choose or are there any other suggestions?
fmayer
approved these changes
Apr 8, 2025
|
@vitalybuka does this LGTY? @Camsyn do you want me to merge if it looks good to Vitaly? |
vitalybuka
reviewed
Apr 11, 2025
Sure, happy for you to merge if it looks good to both of you. Also, I’d love to hear your thoughts on the other potential fixes I mentioned above — do you think they’re worth pursuing for a more complete resolution of the deadlock issue? If so, I’d be happy to open a separate PR for those. |
vitalybuka
approved these changes
Apr 16, 2025
|
@Camsyn Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
|
This is failing on the bots: https://green.lab.llvm.org/job/llvm.org/job/clang-stage1-cmake-RA-expensive/3965/ |
|
The newly added testcase only failed on AddressSanitizer-x86_64-darwin. The test failed due to a timeout, and it seems that in that environment, the deadlock problem of the test case has not been resolved, and the built-in WatchDog thread did not kill the test process normally. I'm looking for a Darwin machine to determine what is happening. |
|
This is causing a build failure on the Fuchsia CI bots: There could be something wrong with the CMake plumbing; unsure why this symbol is undefined. Do you have an intuition for what's going wrong? |
|
I am going to try to fix/disable the test on those platforms |
vitalybuka
added a commit
that referenced
this pull request
Apr 16, 2025
Does not link after #131756
vitalybuka
added a commit
that referenced
this pull request
Apr 16, 2025
@Camsyn Will this work? |
Then we will break libdl_deadlock.cpp :( Lets keep as is for now. |
Commit f1b6bd moved the alloca lock (as well as the thread lock) into As for moving out the thread lock, I have analyzed this fix schema; see the second point of the above analysis for details :(. If LSan only advances the thread lock to |
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
…reporting (llvm#131756) ### Description This PR resolves a deadlock between AddressSanitizer (ASan) and LeakSanitizer (LSan) that occurs when both sanitizers attempt to acquire locks in conflicting orders across threads. The fix ensures safe lock acquisition ordering by preloading module information before error reporting. --- ### Issue Details **Reproducer** ```cpp // Thread 1: ASan error path int arr[1] = {0}; std::thread t([&]() { arr[1] = 1; // Triggers ASan OOB error }); // Thread 2: LSan check path __lsan_do_leak_check(); ``` **Lock Order Conflict**: - Thread 1 (ASan error reporting): 1. Acquires ASan thread registry lock (B) 1. Attempts to acquire libdl lock (A) via `dl_iterate_phdr` - Thread 2 (LSan leak check): 1. Acquires libdl lock (A) via `dl_iterate_phdr` 1. Attempts to acquire ASan thread registry lock (B) This creates a circular wait condition (A -> B -> A) meeting all four Coffman deadlock criteria. --- ### Fix Strategy The root cause lies in ASan's error reporting path needing `dl_iterate_phdr` (requiring lock A) while already holding its thread registry lock (B). The solution: 1. **Preload Modules Early**: Force module list initialization _before_ acquiring ASan's thread lock 2. **Avoid Nested Locking**: Ensure symbolization (via dl_iterate_phdr) completes before error reporting locks Key code change: ```cpp // Before acquiring ASan's thread registry lock: Symbolizer::GetOrInit()->GetRefreshedListOfModules(); ``` This guarantees module information is cached before lock acquisition, eliminating the need for `dl_iterate_phdr` calls during error reporting. --- ### Testing Added **asan_lsan_deadlock.cpp** test case: - Reproduces deadlock reliably without fix **under idle system conditions** - Uses watchdog thread to detect hangs - Verifies ASan error reports correctly without deadlock **Note**: Due to the inherent non-determinism of thread scheduling and lock acquisition timing, this test may not reliably reproduce the deadlock on busy systems (e.g., during parallel `ninja check-asan` runs). --- ### Impact - Fixes rare but severe deadlocks in mixed ASan+LSan environments - Maintains thread safety guarantees for both sanitizers - No user-visible behavior changes except deadlock elimination --- ### Relevant Buggy Code - Code in ASan's asan_report.cpp ```cpp explicit ScopedInErrorReport(bool fatal = false) : halt_on_error_(fatal || flags()->halt_on_error) { // Acquire lock B asanThreadRegistry().Lock(); } ~ScopedInErrorReport() { ... // Try to acquire lock A under holding lock B via the following path // #4 0x000071a353d83e93 in __GI___dl_iterate_phdr ( // callback=0x5d1a07a39580 <__sanitizer::dl_iterate_phdr_cb(dl_phdr_info*, unsigned long, void*)>, // data=0x6da3510fd3f0) at ./elf/dl-iteratephdr.c:39 // #5 0x00005d1a07a39574 in __sanitizer::ListOfModules::init (this=0x71a353ebc080) // at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp:784 // llvm#6 0x00005d1a07a429e3 in __sanitizer::Symbolizer::RefreshModules (this=0x71a353ebc058) // at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp:188 // llvm#7 __sanitizer::Symbolizer::FindModuleForAddress (this=this@entry=0x71a353ebc058, // address=address@entry=102366378805727) // at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp:214 // llvm#8 0x00005d1a07a4291b in __sanitizer::Symbolizer::SymbolizePC (this=0x71a353ebc058, addr=102366378805727) // at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp:88 // llvm#9 0x00005d1a07a40df7 in __sanitizer::(anonymous namespace)::StackTraceTextPrinter::ProcessAddressFrames ( // this=this@entry=0x6da3510fd520, pc=102366378805727) // at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp:37 // llvm#10 0x00005d1a07a40d27 in __sanitizer::StackTrace::PrintTo (this=this@entry=0x6da3510fd5e8, // output=output@entry=0x6da3510fd588) // at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp:110 // llvm#11 0x00005d1a07a410a1 in __sanitizer::StackTrace::Print (this=0x6da3510fd5e8) // at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp:133 // llvm#12 0x00005d1a0798758d in __asan::ErrorGeneric::Print ( // this=0x5d1a07aa4e08 <__asan::ScopedInErrorReport::current_error_+8>) // at llvm-project/compiler-rt/lib/asan/asan_errors.cpp:617 current_error_.Print(); ... } ``` - Code in LSan's lsan_common_linux.cpp ```cpp void LockStuffAndStopTheWorld(StopTheWorldCallback callback, CheckForLeaksParam *argument) { // Acquire lock A dl_iterate_phdr(LockStuffAndStopTheWorldCallback, ¶m); } static int LockStuffAndStopTheWorldCallback(struct dl_phdr_info *info, size_t size, void *data) { // Try to acquire lock B under holding lock A via the following path // #3 0x000055555562b34a in __sanitizer::ThreadRegistry::Lock (this=<optimized out>) // at llvm-project/compiler-rt/lib/asan/../sanitizer_common/sanitizer_thread_registry.h:99 // #4 __lsan::LockThreads () at llvm-project/compiler-rt/lib/asan/asan_thread.cpp:484 // #5 0x0000555555652629 in __lsan::ScopedStopTheWorldLock::ScopedStopTheWorldLock (this=<optimized out>) // at llvm-project/compiler-rt/lib/lsan/lsan_common.h:164 // llvm#6 __lsan::LockStuffAndStopTheWorldCallback (info=<optimized out>, size=<optimized out>, data=0x0, // data@entry=0x7fffffffd158) at llvm-project/compiler-rt/lib/lsan/lsan_common_linux.cpp:120 ScopedStopTheWorldLock lock; DoStopTheWorldParam *param = reinterpret_cast<DoStopTheWorldParam *>(data); StopTheWorld(param->callback, param->argument); return 1; } ```
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
Does not link after llvm#131756
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
|
Independently from Darwin, I've noticed this is a source of flakiness on our Linux x64 builders. It failed the 5 most recent runs, passed the proceeding 4, but failed the proceeding 2. Failure message: Example failure: Builder history: |
Deadlock ModelA->B conflicts with B->A. Before Fixbool module_fresh_ = false;
Mutex A, B;
void Thread1() {
{
Lock L1(B);
if (!module_fresh_) {
Lock L2(A);
}
}
}
void Thread2() {
{
Lock L1(A);
{
Lock L2(B);
}
}
}After Fixbool module_fresh_ = false;
Mutex A, B;
void Thread1() {
{
Lock L0(A);
}
module_fresh_ = true;
{
Lock L1(B);
if (!module_fresh_) {
Lock L2(A);
}
}
}
void Thread2() {
{
Lock L1(A);
{
Lock L2(B);
}
}
}I used TSan to compile the simplified model (after fix) and repeated it multiple times. TSan did not report the risk of deadlock. In my opinion, there might be THREE reasons:
I managed to replicate the build locally, but unfortunately couldn't reproduce the flakiness (run I will still actively look for the cause, and it would be better if there could be more information about the test failure. Perhaps we need to temporarily disable the test case before fixing the flakiness? @vitalybuka |
|
Just an FYI - on msvc's ASan, which is based off the llvm codebase, we're seeing the fuzzer test That test expects ASan to report stack overflow error, but after this change, the program crashes prior to being able to report the stack overflow. This is because we're adding more work (and thus more stack frames) in the I haven't had the chance to test this with clang on windows, but I wanted to flag this behavior in case it's reproducible with clang. In any case, it would be great to find a fix for this deadlock that meshes will with trying to report a stack overflow. |
In fact, this deadlock only occurs when LSan is enabled and Previously, I didn’t do this because I thought the patch was quite general and shouldn’t cause any significant negative side effects. And indeed, I haven’t encountered any issues with it in my Linux environment :(. |
Camsyn
added a commit
to Camsyn/llvm-project
that referenced
this pull request
Apr 24, 2025
PR llvm#131756 introduced a patch to fix a deadlock between LSan and ASan. The relevant deadlock only occurs when LSan is enabled and `dl_iterate_phdr` is used for Stop-the-World, i.e., under the condition `CAN_SANITIZE_LEAKS && (SANITIZER_LINUX || SANITIZER_NETBSD)`. Therefore, this commit also sets the effective condition of this patch to the above conditions, avoiding unnecessary problems in other environments.
vitalybuka
pushed a commit
that referenced
this pull request
Apr 24, 2025
PR #131756 introduced a patch to fix a deadlock between LSan and ASan. The relevant deadlock only occurs when LSan is enabled and `dl_iterate_phdr` is used for Stop-the-World, i.e., under the condition `CAN_SANITIZE_LEAKS && (SANITIZER_LINUX || SANITIZER_NETBSD)`. Therefore, this commit also sets the effective condition of this patch to the above condition, avoiding unnecessary problems in other environments, e.g., stack overflow on MSVC/Windows.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR resolves a deadlock between AddressSanitizer (ASan) and LeakSanitizer (LSan)
that occurs when both sanitizers attempt to acquire locks in conflicting orders across
threads. The fix ensures safe lock acquisition ordering by preloading module information
before error reporting.
Issue Details
Reproducer
Lock Order Conflict:
Thread 1 (ASan error reporting):
dl_iterate_phdrThread 2 (LSan leak check):
dl_iterate_phdrThis creates a circular wait condition (A -> B -> A) meeting all four Coffman deadlock criteria.
Fix Strategy
The root cause lies in ASan's error reporting path needing
dl_iterate_phdr(requiring lock A)while already holding its thread registry lock (B). The solution:
Key code change:
This guarantees module information is cached before lock acquisition, eliminating
the need for
dl_iterate_phdrcalls during error reporting.Testing
Added asan_lsan_deadlock.cpp test case:
Note: Due to the inherent non-determinism of thread scheduling and lock acquisition timing,
this test may not reliably reproduce the deadlock on busy systems (e.g., during parallel
ninja check-asanruns).Impact
Relevant Buggy Code