[clang] [analyzer] Add path notes to PthreadLockChecker (PR #202473)
Endre Fülöp via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 8 17:50:39 PDT 2026
https://github.com/gamesh411 created https://github.com/llvm/llvm-project/pull/202473
Bug reports from this checker lack context about where mutexes were locked or unlocked, making it hard to understand the diagnostic without reading surrounding code.
Add NoteTag on lock, unlock, destroy, and init events. Notes name the mutex when possible and are filtered to only appear when the mutex is relevant to the reported bug.
>From 60f2ea6875021060ebe0e3a8beac55271b250f04 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 21:16:12 +0200
Subject: [PATCH] [analyzer] Add path notes to PthreadLockChecker
Bug reports from this checker lack context about where mutexes were
locked or unlocked, making it hard to understand the diagnostic without
reading surrounding code.
Add NoteTag on lock, unlock, destroy, and init events. Notes name the
mutex when possible and are filtered to only appear when the mutex is
relevant to the reported bug.
---
clang/docs/ReleaseNotes.rst | 6 +
.../Checkers/PthreadLockChecker.cpp | 111 ++++++++++++++----
clang/test/Analysis/pthreadlock-notes.c | 42 +++++++
3 files changed, 136 insertions(+), 23 deletions(-)
create mode 100644 clang/test/Analysis/pthreadlock-notes.c
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index aebd60e1646d6..76223e6f0bb32 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -937,6 +937,12 @@ 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
+^^^^^^^^^^^^
+
+- ``alpha.unix.PthreadLock`` now emits path notes on lock, unlock, destroy,
+ and init operations.
+
.. comment:
This is for the Static Analyzer.
Using the caret `^^^` underlining for subsections:
diff --git a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
index 1731cb28aa794..7f4a8479a1a15 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -13,8 +13,6 @@
// * FuchsiaLocksChecker, which is also rather similar.
// * C11LockChecker which also closely follows Pthread semantics.
//
-// TODO: Path notes.
-//
//===----------------------------------------------------------------------===//
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
@@ -202,8 +200,8 @@ class PthreadLockChecker : public Checker<check::PostCall, check::DeadSymbols,
const MemRegion *lockR,
const SymbolRef *sym) const;
void reportBug(CheckerContext &C, std::unique_ptr<BugType> BT[],
- const Expr *MtxExpr, CheckerKind CheckKind,
- StringRef Desc) const;
+ const Expr *MtxExpr, const MemRegion *MtxRegion,
+ CheckerKind CheckKind, StringRef Desc) const;
// Init.
void InitAnyLock(const CallEvent &Call, CheckerContext &C,
@@ -441,11 +439,11 @@ void PthreadLockChecker::AcquireLockAux(const CallEvent &Call,
if (const LockState *LState = state->get<LockMap>(lockR)) {
if (LState->isLocked()) {
- reportBug(C, BT_doublelock, MtxExpr, CheckKind,
+ reportBug(C, BT_doublelock, MtxExpr, lockR, CheckKind,
"This lock has already been acquired");
return;
} else if (LState->isDestroyed()) {
- reportBug(C, BT_destroylock, MtxExpr, CheckKind,
+ reportBug(C, BT_destroylock, MtxExpr, lockR, CheckKind,
"This lock has already been destroyed");
return;
}
@@ -492,7 +490,19 @@ void PthreadLockChecker::AcquireLockAux(const CallEvent &Call,
// Record that the lock was acquired.
lockSucc = lockSucc->add<LockSet>(lockR);
lockSucc = lockSucc->set<LockMap>(lockR, LockState::getLocked());
- C.addTransition(lockSucc);
+ const NoteTag *Note =
+ C.getNoteTag([lockR](PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+ if (BR.getBugType().getCategory() != "Lock checker")
+ return;
+ if (!BR.isInteresting(lockR))
+ return;
+ std::string Name = lockR->getDescriptiveName();
+ if (Name.empty())
+ OS << "Mutex acquired here";
+ else
+ OS << "Locking " << Name << " here";
+ });
+ C.addTransition(lockSucc, Note);
}
void PthreadLockChecker::ReleaseAnyLock(const CallEvent &Call,
@@ -519,11 +529,11 @@ void PthreadLockChecker::ReleaseLockAux(const CallEvent &Call,
if (const LockState *LState = state->get<LockMap>(lockR)) {
if (LState->isUnlocked()) {
- reportBug(C, BT_doubleunlock, MtxExpr, CheckKind,
+ reportBug(C, BT_doubleunlock, MtxExpr, lockR, CheckKind,
"This lock has already been unlocked");
return;
} else if (LState->isDestroyed()) {
- reportBug(C, BT_destroylock, MtxExpr, CheckKind,
+ reportBug(C, BT_destroylock, MtxExpr, lockR, CheckKind,
"This lock has already been destroyed");
return;
}
@@ -534,9 +544,19 @@ void PthreadLockChecker::ReleaseLockAux(const CallEvent &Call,
if (!LS.isEmpty()) {
const MemRegion *firstLockR = LS.getHead();
if (firstLockR != lockR) {
- reportBug(C, BT_lor, MtxExpr, CheckKind,
- "This was not the most recently acquired lock. Possible lock "
- "order reversal");
+ ExplodedNode *N = C.generateErrorNode();
+ if (!N)
+ return;
+ initBugType(CheckKind);
+ auto Report = std::make_unique<PathSensitiveBugReport>(
+ *BT_lor[CheckKind],
+ "This was not the most recently acquired lock. Possible lock "
+ "order reversal",
+ N);
+ Report->addRange(MtxExpr->getSourceRange());
+ Report->markInteresting(lockR);
+ Report->markInteresting(firstLockR);
+ C.emitReport(std::move(Report));
return;
}
// Record that the lock was released.
@@ -544,7 +564,19 @@ void PthreadLockChecker::ReleaseLockAux(const CallEvent &Call,
}
state = state->set<LockMap>(lockR, LockState::getUnlocked());
- C.addTransition(state);
+ const NoteTag *Note =
+ C.getNoteTag([lockR](PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+ if (BR.getBugType().getCategory() != "Lock checker")
+ return;
+ if (!BR.isInteresting(lockR))
+ return;
+ std::string Name = lockR->getDescriptiveName();
+ if (Name.empty())
+ OS << "Mutex released here";
+ else
+ OS << "Unlocking " << Name << " here";
+ });
+ C.addTransition(state, Note);
}
void PthreadLockChecker::DestroyPthreadLock(const CallEvent &Call,
@@ -587,7 +619,15 @@ void PthreadLockChecker::DestroyLockAux(const CallEvent &Call,
SymbolRef sym = Call.getReturnValue().getAsSymbol();
if (!sym) {
State = State->remove<LockMap>(LockR);
- C.addTransition(State);
+ const NoteTag *Note = C.getNoteTag(
+ [LockR](PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+ if (BR.getBugType().getCategory() != "Lock checker")
+ return;
+ if (!BR.isInteresting(LockR))
+ return;
+ OS << "Destroying mutex here";
+ });
+ C.addTransition(State, Note);
return;
}
State = State->set<DestroyRetVal>(LockR, sym);
@@ -597,13 +637,29 @@ void PthreadLockChecker::DestroyLockAux(const CallEvent &Call,
else
State = State->set<LockMap>(
LockR, LockState::getUntouchedAndPossiblyDestroyed());
- C.addTransition(State);
+ const NoteTag *Note = C.getNoteTag(
+ [LockR](PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+ if (BR.getBugType().getCategory() != "Lock checker")
+ return;
+ if (!BR.isInteresting(LockR))
+ return;
+ OS << "Destroying mutex here";
+ });
+ C.addTransition(State, Note);
return;
}
} else {
if (!LState || LState->isUnlocked()) {
State = State->set<LockMap>(LockR, LockState::getDestroyed());
- C.addTransition(State);
+ const NoteTag *Note = C.getNoteTag(
+ [LockR](PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+ if (BR.getBugType().getCategory() != "Lock checker")
+ return;
+ if (!BR.isInteresting(LockR))
+ return;
+ OS << "Destroying mutex here";
+ });
+ C.addTransition(State, Note);
return;
}
}
@@ -612,7 +668,7 @@ void PthreadLockChecker::DestroyLockAux(const CallEvent &Call,
? "This lock is still locked"
: "This lock has already been destroyed";
- reportBug(C, BT_destroylock, MtxExpr, CheckKind, Message);
+ reportBug(C, BT_destroylock, MtxExpr, LockR, CheckKind, Message);
}
void PthreadLockChecker::InitAnyLock(const CallEvent &Call, CheckerContext &C,
@@ -639,7 +695,15 @@ void PthreadLockChecker::InitLockAux(const CallEvent &Call, CheckerContext &C,
const struct LockState *LState = State->get<LockMap>(LockR);
if (!LState || LState->isDestroyed()) {
State = State->set<LockMap>(LockR, LockState::getUnlocked());
- C.addTransition(State);
+ const NoteTag *Note = C.getNoteTag(
+ [LockR](PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+ if (BR.getBugType().getCategory() != "Lock checker")
+ return;
+ if (!BR.isInteresting(LockR))
+ return;
+ OS << "Initializing mutex here";
+ });
+ C.addTransition(State, Note);
return;
}
@@ -647,13 +711,12 @@ void PthreadLockChecker::InitLockAux(const CallEvent &Call, CheckerContext &C,
? "This lock is still being held"
: "This lock has already been initialized";
- reportBug(C, BT_initlock, MtxExpr, CheckKind, Message);
+ reportBug(C, BT_initlock, MtxExpr, LockR, CheckKind, Message);
}
-void PthreadLockChecker::reportBug(CheckerContext &C,
- std::unique_ptr<BugType> BT[],
- const Expr *MtxExpr, CheckerKind CheckKind,
- StringRef Desc) const {
+void PthreadLockChecker::reportBug(
+ CheckerContext &C, std::unique_ptr<BugType> BT[], const Expr *MtxExpr,
+ const MemRegion *MtxRegion, CheckerKind CheckKind, StringRef Desc) const {
ExplodedNode *N = C.generateErrorNode();
if (!N)
return;
@@ -661,6 +724,8 @@ void PthreadLockChecker::reportBug(CheckerContext &C,
auto Report =
std::make_unique<PathSensitiveBugReport>(*BT[CheckKind], Desc, N);
Report->addRange(MtxExpr->getSourceRange());
+ if (MtxRegion)
+ Report->markInteresting(MtxRegion);
C.emitReport(std::move(Report));
}
diff --git a/clang/test/Analysis/pthreadlock-notes.c b/clang/test/Analysis/pthreadlock-notes.c
new file mode 100644
index 0000000000000..3536457fabe5a
--- /dev/null
+++ b/clang/test/Analysis/pthreadlock-notes.c
@@ -0,0 +1,42 @@
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=alpha.unix.PthreadLock \
+// RUN: -analyzer-output text \
+// RUN: -verify %s
+
+#include "Inputs/system-header-simulator-for-pthread-lock.h"
+
+pthread_mutex_t mtx;
+
+void double_lock(void) {
+ pthread_mutex_lock(&mtx); // expected-note{{Locking 'mtx' here}}
+ pthread_mutex_lock(&mtx); // expected-warning{{This lock has already been acquired}}
+ // expected-note at -1{{This lock has already been acquired}}
+}
+
+void double_unlock(void) {
+ pthread_mutex_lock(&mtx); // expected-note{{Locking 'mtx' here}}
+ pthread_mutex_unlock(&mtx); // expected-note{{Unlocking 'mtx' here}}
+ pthread_mutex_unlock(&mtx); // expected-warning{{This lock has already been unlocked}}
+ // expected-note at -1{{This lock has already been unlocked}}
+}
+
+void use_after_destroy(void) {
+ pthread_mutex_destroy(&mtx); // expected-note{{Destroying mutex here}}
+ pthread_mutex_lock(&mtx); // expected-warning{{This lock has already been destroyed}}
+ // expected-note at -1{{This lock has already been destroyed}}
+}
+
+void double_init(void) {
+ pthread_mutex_init(&mtx, 0); // expected-note{{Initializing mutex here}}
+ pthread_mutex_init(&mtx, 0); // expected-warning{{This lock has already been initialized}}
+ // expected-note at -1{{This lock has already been initialized}}
+}
+
+pthread_mutex_t mtx2;
+
+void lock_order_reversal(void) {
+ pthread_mutex_lock(&mtx); // expected-note{{Locking 'mtx' here}}
+ pthread_mutex_lock(&mtx2); // expected-note{{Locking 'mtx2' here}}
+ pthread_mutex_unlock(&mtx); // expected-warning{{This was not the most recently acquired lock}}
+ // expected-note at -1{{This was not the most recently acquired lock}}
+}
More information about the cfe-commits
mailing list