Skip to content

Commit 18ad020

Browse files
NagyDonatsteakhal
authored andcommitted
[analyzer] Restore recognition of mutex methods (#101511)
Before commit 705788c the checker alpha.unix.BlockInCriticalSection "recognized" the methods `std::mutex::lock` and `std::mutex::unlock` with an extremely trivial check that accepted any function (or method) named lock/unlock. To avoid matching unrelated user-defined function, this was refined to a check that also requires the presence of "std" and "mutex" as distinct parts of the qualified name. However, as #99628 reported, there are standard library implementations where some methods of `std::mutex` are inherited from an implementation detail base class and the new code wasn't able to recognize these methods, which led to emitting false positive reports. As a workaround, this commit partially restores the old behavior by omitting the check for the class name. In the future, it would be good to replace this hack with a solution which ensures that `CallDescription` understands inherited methods. (cherry picked from commit 99ae2ed)
1 parent 6b52570 commit 18ad020

File tree

2 files changed

+43
-4
lines changed

2 files changed

+43
-4
lines changed

clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,18 @@ using MutexDescriptor =
147147
class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
148148
private:
149149
const std::array<MutexDescriptor, 8> MutexDescriptors{
150-
MemberMutexDescriptor({/*MatchAs=*/CDM::CXXMethod,
151-
/*QualifiedName=*/{"std", "mutex", "lock"},
152-
/*RequiredArgs=*/0},
153-
{CDM::CXXMethod, {"std", "mutex", "unlock"}, 0}),
150+
// NOTE: There are standard library implementations where some methods
151+
// of `std::mutex` are inherited from an implementation detail base
152+
// class, and those aren't matched by the name specification {"std",
153+
// "mutex", "lock"}.
154+
// As a workaround here we omit the class name and only require the
155+
// presence of the name parts "std" and "lock"/"unlock".
156+
// TODO: Ensure that CallDescription understands inherited methods.
157+
MemberMutexDescriptor(
158+
{/*MatchAs=*/CDM::CXXMethod,
159+
/*QualifiedName=*/{"std", /*"mutex",*/ "lock"},
160+
/*RequiredArgs=*/0},
161+
{CDM::CXXMethod, {"std", /*"mutex",*/ "unlock"}, 0}),
154162
FirstArgMutexDescriptor({CDM::CLibrary, {"pthread_mutex_lock"}, 1},
155163
{CDM::CLibrary, {"pthread_mutex_unlock"}, 1}),
156164
FirstArgMutexDescriptor({CDM::CLibrary, {"mtx_lock"}, 1},
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: %clang_analyze_cc1 \
2+
// RUN: -analyzer-checker=unix.BlockInCriticalSection \
3+
// RUN: -std=c++11 \
4+
// RUN: -analyzer-output text \
5+
// RUN: -verify %s
6+
7+
unsigned int sleep(unsigned int seconds) {return 0;}
8+
namespace std {
9+
// There are some standard library implementations where some mutex methods
10+
// come from an implementation detail base class. We need to ensure that these
11+
// are matched correctly.
12+
class __mutex_base {
13+
public:
14+
void lock();
15+
};
16+
class mutex : public __mutex_base{
17+
public:
18+
void unlock();
19+
bool try_lock();
20+
};
21+
} // namespace std
22+
23+
void gh_99628() {
24+
std::mutex m;
25+
m.lock();
26+
// expected-note@-1 {{Entering critical section here}}
27+
sleep(10);
28+
// expected-warning@-1 {{Call to blocking function 'sleep' inside of critical section}}
29+
// expected-note@-2 {{Call to blocking function 'sleep' inside of critical section}}
30+
m.unlock();
31+
}

0 commit comments

Comments
 (0)