[clang] [clang][analyzer] Add note tags to alpha.unix.BlockInCriticalSection (PR #80029)
Endre Fülöp via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 7 04:12:48 PST 2024
https://github.com/gamesh411 updated https://github.com/llvm/llvm-project/pull/80029
>From f7875a7f1ff20f3cf850ce1c23bec6d6c3d88d57 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Tue, 30 Jan 2024 11:33:30 +0100
Subject: [PATCH 1/2] [clang][analyzer] Add note tags to
alpha.unix.BlockInCriticalSection checker
On entering a critical section, a note tag is now placed along the
bugpath.
---
.../BlockInCriticalSectionChecker.cpp | 18 ++++-
.../Analysis/block-in-critical-section.cpp | 77 ++++++++++++++-----
2 files changed, 75 insertions(+), 20 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index 66e080adb1382b..1297ae96c8b644 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -57,6 +57,8 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
const CallEvent &call,
CheckerContext &C) const;
+ const NoteTag *createCriticalSectionNote(CheckerContext &C) const;
+
public:
bool isBlockingFunction(const CallEvent &Call) const;
bool isLockFunction(const CallEvent &Call) const;
@@ -126,8 +128,9 @@ void BlockInCriticalSectionChecker::checkPostCall(const CallEvent &Call,
State = State->set<MutexCounter>(--mutexCount);
C.addTransition(State);
} else if (isLockFunction(Call)) {
+ const NoteTag *Note = createCriticalSectionNote(C);
State = State->set<MutexCounter>(++mutexCount);
- C.addTransition(State);
+ C.addTransition(State, Note);
} else if (mutexCount > 0) {
SymbolRef BlockDesc = Call.getReturnValue().getAsSymbol();
reportBlockInCritSection(BlockDesc, Call, C);
@@ -151,10 +154,21 @@ void BlockInCriticalSectionChecker::reportBlockInCritSection(
C.emitReport(std::move(R));
}
+const NoteTag *BlockInCriticalSectionChecker::createCriticalSectionNote(
+ CheckerContext &C) const {
+ const BugType *BT = &this->BlockInCritSectionBugType;
+ return C.getNoteTag([BT](PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+ if (&BR.getBugType() != BT)
+ return;
+ OS << "Entering critical section here";
+ });
+}
+
void ento::registerBlockInCriticalSectionChecker(CheckerManager &mgr) {
mgr.registerChecker<BlockInCriticalSectionChecker>();
}
-bool ento::shouldRegisterBlockInCriticalSectionChecker(const CheckerManager &mgr) {
+bool ento::shouldRegisterBlockInCriticalSectionChecker(
+ const CheckerManager &mgr) {
return true;
}
diff --git a/clang/test/Analysis/block-in-critical-section.cpp b/clang/test/Analysis/block-in-critical-section.cpp
index fcf6188fc033ec..93d46c741e16ff 100644
--- a/clang/test/Analysis/block-in-critical-section.cpp
+++ b/clang/test/Analysis/block-in-critical-section.cpp
@@ -1,4 +1,8 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.BlockInCriticalSection -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=alpha.unix.BlockInCriticalSection \
+// RUN: -std=c++11 \
+// RUN: -analyzer-output text \
+// RUN: -verify %s
void sleep(int x) {}
@@ -21,7 +25,7 @@ template<typename T>
struct not_real_lock {
not_real_lock<T>(std::mutex) {}
};
-}
+} // namespace std
void getc() {}
void fgets() {}
@@ -39,81 +43,115 @@ void mtx_unlock() {}
void testBlockInCriticalSectionWithStdMutex() {
std::mutex m;
- m.lock();
+ m.lock(); // expected-note 5{{Entering critical section here}}
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'getc' inside of critical section}}
fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'fgets' inside of critical section}}
read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'read' inside of critical section}}
recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'recv' inside of critical section}}
m.unlock();
}
void testBlockInCriticalSectionWithPthreadMutex() {
- pthread_mutex_lock();
+ pthread_mutex_lock(); // expected-note 10{{Entering critical section here}}
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'getc' inside of critical section}}
fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'fgets' inside of critical section}}
read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'read' inside of critical section}}
recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'recv' inside of critical section}}
pthread_mutex_unlock();
- pthread_mutex_trylock();
+ pthread_mutex_trylock(); // expected-note 5{{Entering critical section here}}
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'getc' inside of critical section}}
fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'fgets' inside of critical section}}
read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'read' inside of critical section}}
recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'recv' inside of critical section}}
pthread_mutex_unlock();
}
void testBlockInCriticalSectionC11Locks() {
- mtx_lock();
+ mtx_lock(); // expected-note 15{{Entering critical section here}}
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'getc' inside of critical section}}
fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'fgets' inside of critical section}}
read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'read' inside of critical section}}
recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'recv' inside of critical section}}
mtx_unlock();
- mtx_timedlock();
+ mtx_timedlock(); // expected-note 10{{Entering critical section here}}
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'getc' inside of critical section}}
fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'fgets' inside of critical section}}
read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'read' inside of critical section}}
recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'recv' inside of critical section}}
mtx_unlock();
- mtx_trylock();
+ mtx_trylock(); // expected-note 5{{Entering critical section here}}
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'getc' inside of critical section}}
fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'fgets' inside of critical section}}
read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'read' inside of critical section}}
recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'recv' inside of critical section}}
mtx_unlock();
}
void testBlockInCriticalSectionWithNestedMutexes() {
std::mutex m, n, k;
- m.lock();
- n.lock();
- k.lock();
+ m.lock(); // expected-note 3{{Entering critical section here}}
+ n.lock(); // expected-note 3{{Entering critical section here}}
+ k.lock(); // expected-note 3{{Entering critical section here}}
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
k.unlock();
sleep(5); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
n.unlock();
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
m.unlock();
sleep(3); // no-warning
}
void f() {
sleep(1000); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
}
void testBlockInCriticalSectionInterProcedural() {
std::mutex m;
- m.lock();
- f();
+ m.lock(); // expected-note {{Entering critical section here}}
+ f(); // expected-note {{Calling 'f'}}
m.unlock();
}
@@ -121,8 +159,9 @@ void testBlockInCriticalSectionUnexpectedUnlock() {
std::mutex m;
m.unlock();
sleep(1); // no-warning
- m.lock();
+ m.lock(); // expected-note {{Entering critical section here}}
sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
}
void testBlockInCriticalSectionLockGuard() {
@@ -130,12 +169,13 @@ void testBlockInCriticalSectionLockGuard() {
std::not_real_lock<std::mutex> not_real_lock(g_mutex);
sleep(1); // no-warning
- std::lock_guard<std::mutex> lock(g_mutex);
+ std::lock_guard<std::mutex> lock(g_mutex); // expected-note {{Entering critical section here}}
sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
}
void testBlockInCriticalSectionLockGuardNested() {
- testBlockInCriticalSectionLockGuard();
+ testBlockInCriticalSectionLockGuard(); // expected-note {{Calling 'testBlockInCriticalSectionLockGuard'}}
sleep(1); // no-warning
}
@@ -144,11 +184,12 @@ void testBlockInCriticalSectionUniqueLock() {
std::not_real_lock<std::mutex> not_real_lock(g_mutex);
sleep(1); // no-warning
- std::unique_lock<std::mutex> lock(g_mutex);
+ std::unique_lock<std::mutex> lock(g_mutex); // expected-note {{Entering critical section here}}
sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
}
void testBlockInCriticalSectionUniqueLockNested() {
- testBlockInCriticalSectionUniqueLock();
+ testBlockInCriticalSectionUniqueLock(); // expected-note {{Calling 'testBlockInCriticalSectionUniqueLock'}}
sleep(1); // no-warning
}
>From a39f9e410499aa6f13e78e5957d3014e5fe7a9cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 5 Feb 2024 11:20:27 +0100
Subject: [PATCH 2/2] Only report active critical sections
---
.../BlockInCriticalSectionChecker.cpp | 35 ++++++++++++++++---
.../Analysis/block-in-critical-section.cpp | 16 +++++++--
2 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index 1297ae96c8b644..8e17530259cf46 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -26,6 +26,10 @@ using namespace ento;
namespace {
class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
+public:
+ using CritSectionMarker = const Expr *;
+
+private:
mutable IdentifierInfo *IILockGuard = nullptr;
mutable IdentifierInfo *IIUniqueLock = nullptr;
mutable bool IdentifierInfoInitialized = false;
@@ -57,7 +61,11 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
const CallEvent &call,
CheckerContext &C) const;
- const NoteTag *createCriticalSectionNote(CheckerContext &C) const;
+ CritSectionMarker getCriticalSectionMarker(const CallEvent &Call,
+ CheckerContext &C) const;
+ const NoteTag *
+ createCriticalSectionNote(const CritSectionMarker &CriticalSectionBegin,
+ CheckerContext &C) const;
public:
bool isBlockingFunction(const CallEvent &Call) const;
@@ -73,6 +81,8 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
} // end anonymous namespace
REGISTER_TRAIT_WITH_PROGRAMSTATE(MutexCounter, unsigned)
+REGISTER_SET_WITH_PROGRAMSTATE(CurrentCritSections,
+ BlockInCriticalSectionChecker::CritSectionMarker)
void BlockInCriticalSectionChecker::initIdentifierInfo(ASTContext &Ctx) const {
if (!IdentifierInfoInitialized) {
@@ -126,11 +136,15 @@ void BlockInCriticalSectionChecker::checkPostCall(const CallEvent &Call,
unsigned mutexCount = State->get<MutexCounter>();
if (isUnlockFunction(Call) && mutexCount > 0) {
State = State->set<MutexCounter>(--mutexCount);
+ if (mutexCount == 0) {
+ State = State->remove<CurrentCritSections>();
+ }
C.addTransition(State);
} else if (isLockFunction(Call)) {
- const NoteTag *Note = createCriticalSectionNote(C);
+ CritSectionMarker CritSectionBegin = getCriticalSectionMarker(Call, C);
+ State = State->add<CurrentCritSections>(CritSectionBegin);
State = State->set<MutexCounter>(++mutexCount);
- C.addTransition(State, Note);
+ C.addTransition(State, createCriticalSectionNote(CritSectionBegin, C));
} else if (mutexCount > 0) {
SymbolRef BlockDesc = Call.getReturnValue().getAsSymbol();
reportBlockInCritSection(BlockDesc, Call, C);
@@ -154,12 +168,23 @@ void BlockInCriticalSectionChecker::reportBlockInCritSection(
C.emitReport(std::move(R));
}
+BlockInCriticalSectionChecker::CritSectionMarker
+BlockInCriticalSectionChecker::getCriticalSectionMarker(
+ const CallEvent &Call, CheckerContext &C) const {
+ return Call.getOriginExpr();
+}
+
const NoteTag *BlockInCriticalSectionChecker::createCriticalSectionNote(
- CheckerContext &C) const {
+ const CritSectionMarker &CritSectionBegin, CheckerContext &C) const {
const BugType *BT = &this->BlockInCritSectionBugType;
- return C.getNoteTag([BT](PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+ return C.getNoteTag([CritSectionBegin, BT](PathSensitiveBugReport &BR,
+ llvm::raw_ostream &OS) {
if (&BR.getBugType() != BT)
return;
+ const auto &InterestingCritSections =
+ BR.getErrorNode()->getState()->get<CurrentCritSections>();
+ if (!InterestingCritSections.contains(CritSectionBegin))
+ return;
OS << "Entering critical section here";
});
}
diff --git a/clang/test/Analysis/block-in-critical-section.cpp b/clang/test/Analysis/block-in-critical-section.cpp
index 93d46c741e16ff..1e91b027b80921 100644
--- a/clang/test/Analysis/block-in-critical-section.cpp
+++ b/clang/test/Analysis/block-in-critical-section.cpp
@@ -58,7 +58,7 @@ void testBlockInCriticalSectionWithStdMutex() {
}
void testBlockInCriticalSectionWithPthreadMutex() {
- pthread_mutex_lock(); // expected-note 10{{Entering critical section here}}
+ pthread_mutex_lock(); // expected-note 5{{Entering critical section here}}
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
// expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
@@ -86,7 +86,7 @@ void testBlockInCriticalSectionWithPthreadMutex() {
}
void testBlockInCriticalSectionC11Locks() {
- mtx_lock(); // expected-note 15{{Entering critical section here}}
+ mtx_lock(); // expected-note 5{{Entering critical section here}}
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
// expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
@@ -99,7 +99,7 @@ void testBlockInCriticalSectionC11Locks() {
// expected-note at -1 {{Call to blocking function 'recv' inside of critical section}}
mtx_unlock();
- mtx_timedlock(); // expected-note 10{{Entering critical section here}}
+ mtx_timedlock(); // expected-note 5{{Entering critical section here}}
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
// expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
@@ -143,6 +143,16 @@ void testBlockInCriticalSectionWithNestedMutexes() {
sleep(3); // no-warning
}
+void testBlockInCriticalSectionNonOverlapping() {
+ std::mutex m;
+ m.lock(); // There should be no warning here
+ m.unlock();
+ m.lock(); // expected-note {{Entering critical section here}}
+ sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
+ // expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
+ m.unlock();
+}
+
void f() {
sleep(1000); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
// expected-note at -1 {{Call to blocking function 'sleep' inside of critical section}}
More information about the cfe-commits
mailing list