[clang] [analyzer] Disable lock order reversal check by default in PthreadLoc… (PR #202452)
Endre Fülöp via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 11 07:24:52 PDT 2026
https://github.com/gamesh411 updated https://github.com/llvm/llvm-project/pull/202452
>From 4352fd64c32319600ee8d7cefd170a460a2ff8ea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 8 Jun 2026 20:54:34 +0200
Subject: [PATCH 1/2] [analyzer] Disable lock order reversal check by default
in PthreadLockChecker
Lock order reversal is a real source of deadlocks, but the current
single-path intraprocedural analysis is a single-path analysis, and it
cannot reason about potentially overlapping executions.This makes this
part of the checker too imprecise for default-on.
Add a WarnOnLockOrderReversal option (default: false) for the previous
behavior.
---
.../clang/StaticAnalyzer/Checkers/Checkers.td | 7 ++++
.../Checkers/PthreadLockChecker.cpp | 27 ++++++++++---
clang/test/Analysis/c11lock.c | 11 +++++-
clang/test/Analysis/fuchsia_lock.c | 13 +++++--
clang/test/Analysis/pthreadlock.c | 38 +++++++++++++++----
5 files changed, 79 insertions(+), 17 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index eca2afbe340a9..a81a19063b168 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -594,6 +594,13 @@ let ParentPackage = UnixAlpha in {
def PthreadLockChecker : Checker<"PthreadLock">,
HelpText<"Simple lock -> unlock checker">,
+ CheckerOptions<[
+ CmdLineOption<Boolean,
+ "WarnOnLockOrderReversal",
+ "Warn when locks are not released in LIFO order.",
+ "false",
+ Released>
+ ]>,
Dependencies<[PthreadLockBase]>,
Documentation<HasDocumentation>;
diff --git a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
index 1731cb28aa794..41abd9b985589 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -80,6 +80,7 @@ class PthreadLockChecker : public Checker<check::PostCall, check::DeadSymbols,
};
bool ChecksEnabled[CK_NumCheckKinds] = {false};
CheckerNameRef CheckNames[CK_NumCheckKinds];
+ bool WarnOnLockOrderReversal = false;
private:
typedef void (PthreadLockChecker::*FnCheck)(const CallEvent &Call,
@@ -532,15 +533,18 @@ void PthreadLockChecker::ReleaseLockAux(const CallEvent &Call,
LockSetTy LS = state->get<LockSet>();
if (!LS.isEmpty()) {
- const MemRegion *firstLockR = LS.getHead();
- if (firstLockR != lockR) {
+ if (WarnOnLockOrderReversal && LS.getHead() != lockR) {
reportBug(C, BT_lor, MtxExpr, CheckKind,
"This was not the most recently acquired lock. Possible lock "
"order reversal");
return;
}
- // Record that the lock was released.
- state = state->set<LockSet>(LS.getTail());
+ auto &Factory = state->get_context<LockSet>();
+ llvm::ImmutableList<const MemRegion *> NewLS = Factory.getEmptyList();
+ for (auto I = LS.begin(), E = LS.end(); I != E; ++I)
+ if (*I != lockR)
+ NewLS = Factory.add(*I, NewLS);
+ state = state->set<LockSet>(NewLS);
}
state = state->set<LockMap>(lockR, LockState::getUnlocked());
@@ -742,8 +746,21 @@ bool ento::shouldRegisterPthreadLockBase(const CheckerManager &mgr) { return tru
\
bool ento::shouldRegister##name(const CheckerManager &mgr) { return true; }
-REGISTER_CHECKER(PthreadLockChecker)
REGISTER_CHECKER(FuchsiaLockChecker)
REGISTER_CHECKER(C11LockChecker)
#undef REGISTER_CHECKER
+
+void ento::registerPthreadLockChecker(CheckerManager &mgr) {
+ PthreadLockChecker *checker = mgr.getChecker<PthreadLockChecker>();
+ checker->ChecksEnabled[PthreadLockChecker::CK_PthreadLockChecker] = true;
+ checker->CheckNames[PthreadLockChecker::CK_PthreadLockChecker] =
+ mgr.getCurrentCheckerName();
+ checker->WarnOnLockOrderReversal =
+ mgr.getAnalyzerOptions().getCheckerBooleanOption(
+ mgr.getCurrentCheckerName(), "WarnOnLockOrderReversal");
+}
+
+bool ento::shouldRegisterPthreadLockChecker(const CheckerManager &mgr) {
+ return true;
+}
diff --git a/clang/test/Analysis/c11lock.c b/clang/test/Analysis/c11lock.c
index 0e867e9dada36..29ebcd5f37c57 100644
--- a/clang/test/Analysis/c11lock.c
+++ b/clang/test/Analysis/c11lock.c
@@ -1,4 +1,11 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.C11Lock -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=alpha.core.C11Lock \
+// RUN: -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=alpha.core.C11Lock \
+// RUN: -analyzer-checker=alpha.unix.PthreadLock \
+// RUN: -analyzer-config alpha.unix.PthreadLock:WarnOnLockOrderReversal=true \
+// RUN: -verify=expected,lor %s
typedef int mtx_t;
struct timespec;
@@ -61,7 +68,7 @@ void bad6(void) {
void bad7(void) {
mtx_lock(&mtx1);
mtx_lock(&mtx2);
- mtx_unlock(&mtx1); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+ mtx_unlock(&mtx1); // lor-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
mtx_unlock(&mtx2);
}
diff --git a/clang/test/Analysis/fuchsia_lock.c b/clang/test/Analysis/fuchsia_lock.c
index 2067b606f7517..1210ca6942150 100644
--- a/clang/test/Analysis/fuchsia_lock.c
+++ b/clang/test/Analysis/fuchsia_lock.c
@@ -1,4 +1,11 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.fuchsia.Lock -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=alpha.fuchsia.Lock \
+// RUN: -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=alpha.fuchsia.Lock \
+// RUN: -analyzer-checker=alpha.unix.PthreadLock \
+// RUN: -analyzer-config alpha.unix.PthreadLock:WarnOnLockOrderReversal=true \
+// RUN: -verify=expected,lor %s
typedef int spin_lock_t;
typedef int zx_status_t;
@@ -38,7 +45,7 @@ void bad3(void) {
void bad4(void) {
spin_lock(&mtx1);
spin_lock(&mtx2);
- spin_unlock(&mtx1); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+ spin_unlock(&mtx1); // lor-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
spin_unlock(&mtx2);
}
@@ -87,7 +94,7 @@ void bad13(void) {
void bad14(void) {
sync_mutex_lock(&smtx1);
sync_mutex_lock(&smtx2);
- sync_mutex_unlock(&smtx1); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+ sync_mutex_unlock(&smtx1); // lor-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
sync_mutex_unlock(&smtx2);
}
diff --git a/clang/test/Analysis/pthreadlock.c b/clang/test/Analysis/pthreadlock.c
index 85b34cbed918d..e931569c45ab8 100644
--- a/clang/test/Analysis/pthreadlock.c
+++ b/clang/test/Analysis/pthreadlock.c
@@ -1,4 +1,10 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.PthreadLock -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=alpha.unix.PthreadLock \
+// RUN: -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=alpha.unix.PthreadLock \
+// RUN: -analyzer-config alpha.unix.PthreadLock:WarnOnLockOrderReversal=true \
+// RUN: -verify=expected,lor %s
// Tests performing normal locking patterns and wrong locking orders
@@ -263,8 +269,8 @@ bad3(void)
{
pthread_mutex_lock(&mtx1); // no-warning
pthread_mutex_lock(&mtx2); // no-warning
- pthread_mutex_unlock(&mtx1); // expected-warning{{This was not the most recently acquired lock}}
- pthread_mutex_unlock(&mtx2);
+ pthread_mutex_unlock(&mtx1); // lor-warning{{This was not the most recently acquired lock. Possible lock order reversal}}
+ pthread_mutex_unlock(&mtx2); // no-warning
}
void
@@ -273,7 +279,7 @@ bad4(void)
if (pthread_mutex_trylock(&mtx1)) // no-warning
return;
pthread_mutex_lock(&mtx2); // no-warning
- pthread_mutex_unlock(&mtx1); // expected-warning{{This was not the most recently acquired lock}}
+ pthread_mutex_unlock(&mtx1); // lor-warning{{This was not the most recently acquired lock. Possible lock order reversal}}
}
void
@@ -297,8 +303,8 @@ bad7(void)
{
lck_mtx_lock(&lck1); // no-warning
lck_mtx_lock(&lck2); // no-warning
- lck_mtx_unlock(&lck1); // expected-warning{{This was not the most recently acquired lock}}
- lck_mtx_unlock(&lck2);
+ lck_mtx_unlock(&lck1); // lor-warning{{This was not the most recently acquired lock. Possible lock order reversal}}
+ lck_mtx_unlock(&lck2); // no-warning
}
void
@@ -307,7 +313,7 @@ bad8(void)
if (lck_mtx_try_lock(&lck1) == 0) // no-warning
return;
lck_mtx_lock(&lck2); // no-warning
- lck_mtx_unlock(&lck1); // expected-warning{{This was not the most recently acquired lock}}
+ lck_mtx_unlock(&lck1); // lor-warning{{This was not the most recently acquired lock. Possible lock order reversal}}
}
void
@@ -519,3 +525,21 @@ void nocrash1(pthread_mutex_t *mutex) {
if (ret == 0) // no crash
;
}
+
+pthread_mutex_t mtx3;
+
+void ok_non_lifo_unlock_three(void) {
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_lock(&mtx2); // no-warning
+ pthread_mutex_lock(&mtx3); // no-warning
+ pthread_mutex_unlock(&mtx2); // lor-warning{{This was not the most recently acquired lock. Possible lock order reversal}}
+ pthread_mutex_unlock(&mtx3); // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+}
+
+void ok_conditional_lock(int started) {
+ if (started)
+ pthread_mutex_lock(&mtx1);
+ // On the !started path, no lock was acquired.
+ pthread_mutex_unlock(&mtx1); // no-warning
+}
>From b35bdf70446fccea8ff087643018c46b280a7525 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Tue, 9 Jun 2026 01:08:05 +0200
Subject: [PATCH 2/2] add docs and tests for WarnOnLockOrderReversal option
---
clang/docs/ReleaseNotes.rst | 7 +++++++
clang/docs/analyzer/checkers.rst | 8 ++++++++
clang/test/Analysis/analyzer-config.c | 1 +
3 files changed, 16 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index aebd60e1646d6..a69203bb0d7c1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -937,6 +937,13 @@ Crash and bug fixes
- Fixed ``security.VAList`` checker producing false positives when analyzing
C23 code where ``va_start`` expands to ``__builtin_c23_va_start``.
+Improvements
+^^^^^^^^^^^^
+
+- The lock-order-reversal check in ``alpha.unix.PthreadLock`` is now disabled
+ by default. It can be re-enabled with
+ ``-analyzer-config alpha.unix.PthreadLock:WarnOnLockOrderReversal=true``.
+
.. comment:
This is for the Static Analyzer.
Using the caret `^^^` underlining for subsections:
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 61f591916018e..ee700b71eb3f8 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -3609,6 +3609,12 @@ Applies to: ``pthread_mutex_lock, pthread_rwlock_rdlock, pthread_rwlock_wrlock,
``lck_rw_lock_shared, pthread_mutex_trylock, pthread_rwlock_tryrdlock, pthread_rwlock_tryrwlock, lck_mtx_try_lock,
lck_rw_try_lock_exclusive, lck_rw_try_lock_shared, pthread_mutex_unlock, pthread_rwlock_unlock, lck_mtx_unlock, lck_rw_done``.
+**Options**
+
+* ``WarnOnLockOrderReversal`` (boolean). If set to true, the checker will warn
+ on non-LIFO unlock order (possible lock order reversal). Defaults to false
+ because detecting real lock order violations requires cross-path analysis of
+ acquisition order, which the analyzer's single-path engine does not support.
.. code-block:: c
@@ -3620,6 +3626,8 @@ lck_rw_try_lock_exclusive, lck_rw_try_lock_shared, pthread_mutex_unlock, pthread
// warn: this lock has already been acquired
}
+ // The following warnings require WarnOnLockOrderReversal=true:
+
lck_mtx_t lck1, lck2;
void test() {
diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index 04dc8c24421bc..8b11d1e74e020 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -9,6 +9,7 @@
// CHECK-NEXT: alpha.clone.CloneChecker:ReportNormalClones = true
// CHECK-NEXT: alpha.cplusplus.STLAlgorithmModeling:AggressiveStdFindModeling = false
// CHECK-NEXT: alpha.osx.cocoa.DirectIvarAssignment:AnnotatedFunctions = false
+// CHECK-NEXT: alpha.unix.PthreadLock:WarnOnLockOrderReversal = false
// CHECK-NEXT: apply-fixits = false
// CHECK-NEXT: assume-at-least-one-iteration = false
// CHECK-NEXT: assume-controlled-environment = false
More information about the cfe-commits
mailing list