Skip to content

Commit 829c6bc

Browse files
committedMar 10, 2017
[analyzer] Extend block in critical section check with C11 and Pthread APIs.
Patch by Zoltan Daniel Torok! Differential Revision: https://reviews.llvm.org/D29567 llvm-svn: 297461
1 parent e9313ba commit 829c6bc

File tree

3 files changed

+150
-20
lines changed

3 files changed

+150
-20
lines changed
 

‎clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp

+57-13
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ namespace {
2929
class BlockInCriticalSectionChecker : public Checker<check::PostCall,
3030
check::PreCall> {
3131

32-
CallDescription LockFn, UnlockFn, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn;
32+
CallDescription LockFn, UnlockFn, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn,
33+
PthreadLockFn, PthreadTryLockFn, PthreadUnlockFn,
34+
MtxLock, MtxTimedLock, MtxTryLock, MtxUnlock;
3335

3436
std::unique_ptr<BugType> BlockInCritSectionBugType;
3537

@@ -40,6 +42,10 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall,
4042
public:
4143
BlockInCriticalSectionChecker();
4244

45+
bool isBlockingFunction(const CallEvent &Call) const;
46+
bool isLockFunction(const CallEvent &Call) const;
47+
bool isUnlockFunction(const CallEvent &Call) const;
48+
4349
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
4450

4551
/// Process unlock.
@@ -55,34 +61,69 @@ REGISTER_TRAIT_WITH_PROGRAMSTATE(MutexCounter, unsigned)
5561

5662
BlockInCriticalSectionChecker::BlockInCriticalSectionChecker()
5763
: LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"),
58-
FgetsFn("fgets"), ReadFn("read"), RecvFn("recv") {
64+
FgetsFn("fgets"), ReadFn("read"), RecvFn("recv"),
65+
PthreadLockFn("pthread_mutex_lock"),
66+
PthreadTryLockFn("pthread_mutex_trylock"),
67+
PthreadUnlockFn("pthread_mutex_unlock"),
68+
MtxLock("mtx_lock"),
69+
MtxTimedLock("mtx_timedlock"),
70+
MtxTryLock("mtx_trylock"),
71+
MtxUnlock("mtx_unlock") {
5972
// Initialize the bug type.
6073
BlockInCritSectionBugType.reset(
6174
new BugType(this, "Call to blocking function in critical section",
6275
"Blocking Error"));
6376
}
6477

78+
bool BlockInCriticalSectionChecker::isBlockingFunction(const CallEvent &Call) const {
79+
if (Call.isCalled(SleepFn)
80+
|| Call.isCalled(GetcFn)
81+
|| Call.isCalled(FgetsFn)
82+
|| Call.isCalled(ReadFn)
83+
|| Call.isCalled(RecvFn)) {
84+
return true;
85+
}
86+
return false;
87+
}
88+
89+
bool BlockInCriticalSectionChecker::isLockFunction(const CallEvent &Call) const {
90+
if (Call.isCalled(LockFn)
91+
|| Call.isCalled(PthreadLockFn)
92+
|| Call.isCalled(PthreadTryLockFn)
93+
|| Call.isCalled(MtxLock)
94+
|| Call.isCalled(MtxTimedLock)
95+
|| Call.isCalled(MtxTryLock)) {
96+
return true;
97+
}
98+
return false;
99+
}
100+
101+
bool BlockInCriticalSectionChecker::isUnlockFunction(const CallEvent &Call) const {
102+
if (Call.isCalled(UnlockFn)
103+
|| Call.isCalled(PthreadUnlockFn)
104+
|| Call.isCalled(MtxUnlock)) {
105+
return true;
106+
}
107+
return false;
108+
}
109+
65110
void BlockInCriticalSectionChecker::checkPreCall(const CallEvent &Call,
66111
CheckerContext &C) const {
67112
}
68113

69114
void BlockInCriticalSectionChecker::checkPostCall(const CallEvent &Call,
70115
CheckerContext &C) const {
71-
if (!Call.isCalled(LockFn)
72-
&& !Call.isCalled(SleepFn)
73-
&& !Call.isCalled(GetcFn)
74-
&& !Call.isCalled(FgetsFn)
75-
&& !Call.isCalled(ReadFn)
76-
&& !Call.isCalled(RecvFn)
77-
&& !Call.isCalled(UnlockFn))
116+
if (!isBlockingFunction(Call)
117+
&& !isLockFunction(Call)
118+
&& !isUnlockFunction(Call))
78119
return;
79120

80121
ProgramStateRef State = C.getState();
81122
unsigned mutexCount = State->get<MutexCounter>();
82-
if (Call.isCalled(UnlockFn) && mutexCount > 0) {
123+
if (isUnlockFunction(Call) && mutexCount > 0) {
83124
State = State->set<MutexCounter>(--mutexCount);
84125
C.addTransition(State);
85-
} else if (Call.isCalled(LockFn)) {
126+
} else if (isLockFunction(Call)) {
86127
State = State->set<MutexCounter>(++mutexCount);
87128
C.addTransition(State);
88129
} else if (mutexCount > 0) {
@@ -97,8 +138,11 @@ void BlockInCriticalSectionChecker::reportBlockInCritSection(
97138
if (!ErrNode)
98139
return;
99140

100-
auto R = llvm::make_unique<BugReport>(*BlockInCritSectionBugType,
101-
"A blocking function %s is called inside a critical section.", ErrNode);
141+
std::string msg;
142+
llvm::raw_string_ostream os(msg);
143+
os << "Call to blocking function '" << Call.getCalleeIdentifier()->getName()
144+
<< "' inside of critical section";
145+
auto R = llvm::make_unique<BugReport>(*BlockInCritSectionBugType, os.str(), ErrNode);
102146
R->addRange(Call.getSourceRange());
103147
R->markInteresting(BlockDescSym);
104148
C.emitReport(std::move(R));

‎clang/test/Analysis/block-in-critical-section.cpp

+69-7
Original file line numberDiff line numberDiff line change
@@ -9,29 +9,91 @@ struct mutex {
99
};
1010
}
1111

12-
void testBlockInCriticalSection() {
12+
void getc() {}
13+
void fgets() {}
14+
void read() {}
15+
void recv() {}
16+
17+
void pthread_mutex_lock() {}
18+
void pthread_mutex_trylock() {}
19+
void pthread_mutex_unlock() {}
20+
21+
void mtx_lock() {}
22+
void mtx_timedlock() {}
23+
void mtx_trylock() {}
24+
void mtx_unlock() {}
25+
26+
void testBlockInCriticalSectionWithStdMutex() {
1327
std::mutex m;
1428
m.lock();
15-
sleep(3); // expected-warning {{A blocking function %s is called inside a critical section}}
29+
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
30+
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
31+
fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
32+
read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
33+
recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
1634
m.unlock();
1735
}
1836

37+
void testBlockInCriticalSectionWithPthreadMutex() {
38+
pthread_mutex_lock();
39+
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
40+
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
41+
fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
42+
read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
43+
recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
44+
pthread_mutex_unlock();
45+
46+
pthread_mutex_trylock();
47+
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
48+
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
49+
fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
50+
read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
51+
recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
52+
pthread_mutex_unlock();
53+
}
54+
55+
void testBlockInCriticalSectionC11Locks() {
56+
mtx_lock();
57+
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
58+
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
59+
fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
60+
read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
61+
recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
62+
mtx_unlock();
63+
64+
mtx_timedlock();
65+
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
66+
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
67+
fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
68+
read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
69+
recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
70+
mtx_unlock();
71+
72+
mtx_trylock();
73+
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
74+
getc(); // expected-warning {{Call to blocking function 'getc' inside of critical section}}
75+
fgets(); // expected-warning {{Call to blocking function 'fgets' inside of critical section}}
76+
read(); // expected-warning {{Call to blocking function 'read' inside of critical section}}
77+
recv(); // expected-warning {{Call to blocking function 'recv' inside of critical section}}
78+
mtx_unlock();
79+
}
80+
1981
void testBlockInCriticalSectionWithNestedMutexes() {
2082
std::mutex m, n, k;
2183
m.lock();
2284
n.lock();
2385
k.lock();
24-
sleep(3); // expected-warning {{A blocking function %s is called inside a critical section}}
86+
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
2587
k.unlock();
26-
sleep(5); // expected-warning {{A blocking function %s is called inside a critical section}}
88+
sleep(5); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
2789
n.unlock();
28-
sleep(3); // expected-warning {{A blocking function %s is called inside a critical section}}
90+
sleep(3); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
2991
m.unlock();
3092
sleep(3); // no-warning
3193
}
3294

3395
void f() {
34-
sleep(1000); // expected-warning {{A blocking function %s is called inside a critical section}}
96+
sleep(1000); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
3597
}
3698

3799
void testBlockInCriticalSectionInterProcedural() {
@@ -46,5 +108,5 @@ void testBlockInCriticalSectionUnexpectedUnlock() {
46108
m.unlock();
47109
sleep(1); // no-warning
48110
m.lock();
49-
sleep(1); // expected-warning {{A blocking function %s is called inside a critical section}}
111+
sleep(1); // expected-warning {{Call to blocking function 'sleep' inside of critical section}}
50112
}

‎clang/www/analyzer/alpha_checks.html

+24
Original file line numberDiff line numberDiff line change
@@ -910,6 +910,30 @@ <h3 id="unix_alpha_checkers">Unix Alpha Checkers</h3>
910910
}
911911
</pre></div></div></td></tr>
912912

913+
914+
<tr><td><div class="namedescr expandable"><span class="name">
915+
alpha.unix.cstring.BlockInCriticalSection</span><span class="lang">
916+
(C)</span><div class="descr">
917+
Check for calls to blocking functions inside a critical section; applies
918+
to:<div class=functions>
919+
lock, unlock<br>
920+
sleep<br>
921+
getc<br>
922+
fgets<br>
923+
read<br>
924+
recv<br>
925+
pthread_mutex_lock, pthread_mutex_trylock, pthread_mutex_unlock<br>
926+
mtx_lock, mtx_timedlock, mtx_trylock, mtx_unlock<br>
927+
</div></div></div></td>
928+
<td><div class="exampleContainer expandable">
929+
<div class="example"><pre>
930+
void testBlockInCriticalSection() {
931+
std::mutex m;
932+
m.lock();
933+
sleep(3); // warn
934+
m.unlock();
935+
}
936+
</pre></div></div></td></tr>
913937
</tbody></table>
914938

915939
</div> <!-- page -->

0 commit comments

Comments
 (0)
Please sign in to comment.