[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
ASDenysPetrov added a comment. A gentle ping :-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85984/new/ https://reviews.llvm.org/D85984 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
ASDenysPetrov updated this revision to Diff 308781.
ASDenysPetrov added a comment.
@NoQ , considered your suggestions about the case of recursive mutexes.
Implemented this feature. Now this works like below:
recursive_mutex rm;
rm.lock();
rm.lock(); // no-warning
recursive_timed_mutex rtm;
rtm.lock();
rtm.lock(); // no-warning
Please look.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85984/new/
https://reviews.llvm.org/D85984
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
Index: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
@@ -0,0 +1, @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.CPlusPlus11Lock -verify %s
+
+namespace std {
+
+namespace chrono {
+using duration = int;
+using time_point = int;
+} // namespace chrono
+
+struct mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct timed_mutex {
+ void lock();
+ bool try_lock();
+ bool try_lock_for(std::chrono::duration);
+ bool try_lock_until(std::chrono::time_point);
+ void unlock();
+};
+
+struct recursive_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct recursive_timed_mutex {
+ void lock();
+ bool try_lock();
+ bool try_lock_for(std::chrono::duration);
+ bool try_lock_until(std::chrono::time_point);
+ void unlock();
+};
+
+struct shared_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+ void lock_shared();
+ bool try_lock_shared();
+ void unlock_shared();
+};
+
+struct shared_timed_mutex {
+ void lock();
+ bool try_lock();
+ bool try_lock_for(std::chrono::duration);
+ bool try_lock_until(std::chrono::time_point);
+ void unlock();
+ void lock_shared();
+ bool try_lock_shared();
+ bool try_lock_shared_for(std::chrono::duration);
+ bool try_lock_shared_until(std::chrono::time_point);
+ void unlock_shared();
+};
+
+template
+struct lock_guard {
+ T
+ lock_guard(T ) : t(m) {
+t.lock();
+ }
+ ~lock_guard() {
+t.unlock();
+ }
+};
+
+template
+struct shared_lock {
+ T
+ shared_lock(T ) : t(m) {
+t.lock_shared();
+ }
+ ~shared_lock() {
+t.unlock_shared();
+ }
+};
+} // namespace std
+
+std::mutex m1;
+std::mutex m2;
+
+// mutex ok
+
+void m_ok1() {
+ m1.lock(); // no-warning
+}
+
+void m_ok2() {
+ m1.unlock(); // no-warning
+}
+
+void m_ok3() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok4() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok5() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+}
+
+void m_ok6(void) {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok7(void) {
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok8(void) {
+ m1.unlock(); // no-warning
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok9(void) {
+ if (!m1.try_lock()) // no-warning
+m1.lock();// no-warning
+ m1.unlock();// no-warning
+}
+
+void m_ok10() {
+ std::lock_guard gl(m1); // no-warning
+}
+
+// mutex bad
+
+void m_bad1() {
+ m1.lock(); // no-warning
+ m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad2() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad3() {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+ m2.unlock(); // no-warning
+}
+
+void m_bad5() {
+ while (true)
+m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad6() {
+ while (true)
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad7() {
+ if (m1.try_lock()) // no-warning
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+std::timed_mutex tm1;
+std::timed_mutex tm2;
+
+// timed_mutex ok
+
+void tm_ok1() {
+ tm1.lock(); // no-warning
+}
+
+void tm_ok2() {
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok3() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok4() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok5() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+ tm2.lock(); // no-warning
+ tm2.unlock(); // no-warning
+}
+
+void tm_ok6(void) {
+ tm1.lock(); // no-warning
+
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
ASDenysPetrov added a comment.
@NoQ
Comment at: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp:379-382
+void rm_bad1() {
+ rm1.lock(); // no-warning
+ rm1.lock(); // expected-warning{{This lock has already been acquired}}
+}
NoQ wrote:
> ASDenysPetrov wrote:
> > NoQ wrote:
> > > I repeat, this is a false positive. Recursive mutexes can be locked
> > > twice, that's the whole point.
> > Yes, I remember. I think you've mixed up with my previous attempts of
> > introducting a new checks for recursive mutexes.
> > This is not this case. This kind of checks already exists in
> > PthreadLockChecker before.
> > I've just reused this check for all kind of STL mutexes.
> >
> > If you look at `void bad1()` in clang\test\Analysis\pthreadlock.c you can
> > find the same case.
> > I've just reused this check for all kind of STL mutexes.
>
> You're taking code for mushing potatoes and applying it to differential
> calculus.
>
> Different kinds of mutexes behave differently. This is not the same case.
> That test is a true positive because that mutex is non-recursive, your test
> is a false positive because your mutex is recursive. In that test the program
> immediately hangs and there's no valid way to unhang it, in your test the
> programmer simply has to unlock the mutex twice and they're good to go.
@Noq, sorry. This might be a simple misunderstanging. Previously there was a
question in the summary of what if we will consider a twice lock for recursive
mutex as a correct case:
```
recursive_mutex rm;
m.lock();
m.lock(); // OK
```
You've answered that recursive locks are much more complicated than that
(https://reviews.llvm.org/D85984#2237242) and offered to make this separately.
So I decided not to make any changes and handle recursive_mutexes as all the
rest. But if you suggest to do this in a single patch, so I will do.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85984/new/
https://reviews.llvm.org/D85984
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
NoQ requested changes to this revision.
NoQ added inline comments.
This revision now requires changes to proceed.
Comment at: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp:379-382
+void rm_bad1() {
+ rm1.lock(); // no-warning
+ rm1.lock(); // expected-warning{{This lock has already been acquired}}
+}
ASDenysPetrov wrote:
> NoQ wrote:
> > I repeat, this is a false positive. Recursive mutexes can be locked twice,
> > that's the whole point.
> Yes, I remember. I think you've mixed up with my previous attempts of
> introducting a new checks for recursive mutexes.
> This is not this case. This kind of checks already exists in
> PthreadLockChecker before.
> I've just reused this check for all kind of STL mutexes.
>
> If you look at `void bad1()` in clang\test\Analysis\pthreadlock.c you can
> find the same case.
> I've just reused this check for all kind of STL mutexes.
You're taking code for mushing potatoes and applying it to differential
calculus.
Different kinds of mutexes behave differently. This is not the same case. That
test is a true positive because that mutex is non-recursive, your test is a
false positive because your mutex is recursive. In that test the program
immediately hangs and there's no valid way to unhang it, in your test the
programmer simply has to unlock the mutex twice and they're good to go.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85984/new/
https://reviews.llvm.org/D85984
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
ASDenysPetrov added a comment.
@steakhal
> the readability of the reports should be improved.
Absolutely agree. Let's do this in the next patches :)
@NoQ
You've recommended to extend PthreadLockChecker with STL mutexes. I think I've
done. Could you make a quick look, please?
Comment at: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp:379-382
+void rm_bad1() {
+ rm1.lock(); // no-warning
+ rm1.lock(); // expected-warning{{This lock has already been acquired}}
+}
NoQ wrote:
> I repeat, this is a false positive. Recursive mutexes can be locked twice,
> that's the whole point.
Yes, I remember. I think you've mixed up with my previous attempts of
introducting a new checks for recursive mutexes.
This is not this case. This kind of checks already exists in PthreadLockChecker
before.
I've just reused this check for all kind of STL mutexes.
If you look at `void bad1()` in clang\test\Analysis\pthreadlock.c you can find
the same case.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85984/new/
https://reviews.llvm.org/D85984
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
NoQ added inline comments.
Comment at: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp:379-382
+void rm_bad1() {
+ rm1.lock(); // no-warning
+ rm1.lock(); // expected-warning{{This lock has already been acquired}}
+}
I repeat, this is a false positive. Recursive mutexes can be locked twice,
that's the whole point.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85984/new/
https://reviews.llvm.org/D85984
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
steakhal added a comment.
I have checked only your test, but the readability of the reports should be
improved.
You frequently refer to previous events, such as `This lock has already been
unlocked`, `This lock has already been acquired`, etc.
It isn't clear to the reader where do you refer to. IMO you should put a
//NoteTag// at the interesting locations to achieve more readable diagnostics.
Such as:
void stms_bad2() {
stm1.lock();// expected-note {{Previously locked here}}
stm1.lock_shared(); // expected-warning {{This lock has already been
acquired}}
}
void stm_bad3() {
stm1.lock(); // hmm, might be a good idea to put one note here too
stm2.lock(); // expected-note {{Previously locked mutex}}
stm1.unlock(); // expected-warning {{This was not the most recently
acquired lock. Possible lock order reversal}}
stm2.unlock(); // no-warning
}
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85984/new/
https://reviews.llvm.org/D85984
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
ASDenysPetrov updated this revision to Diff 290135.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85984/new/
https://reviews.llvm.org/D85984
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
Index: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
@@ -0,0 +1, @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.CPlusPlus11Lock -verify %s
+
+namespace std {
+
+namespace chrono {
+using duration = int;
+using time_point = int;
+} // namespace chrono
+
+struct mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct timed_mutex {
+ void lock();
+ bool try_lock();
+ bool try_lock_for(std::chrono::duration);
+ bool try_lock_until(std::chrono::time_point);
+ void unlock();
+};
+
+struct recursive_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct recursive_timed_mutex {
+ void lock();
+ bool try_lock();
+ bool try_lock_for(std::chrono::duration);
+ bool try_lock_until(std::chrono::time_point);
+ void unlock();
+};
+
+struct shared_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+ void lock_shared();
+ bool try_lock_shared();
+ void unlock_shared();
+};
+
+struct shared_timed_mutex {
+ void lock();
+ bool try_lock();
+ bool try_lock_for(std::chrono::duration);
+ bool try_lock_until(std::chrono::time_point);
+ void unlock();
+ void lock_shared();
+ bool try_lock_shared();
+ bool try_lock_shared_for(std::chrono::duration);
+ bool try_lock_shared_until(std::chrono::time_point);
+ void unlock_shared();
+};
+
+template
+struct lock_guard {
+ T
+ lock_guard(T ) : t(m) {
+t.lock();
+ }
+ ~lock_guard() {
+t.unlock();
+ }
+};
+
+template
+struct shared_lock {
+ T
+ shared_lock(T ) : t(m) {
+t.lock_shared();
+ }
+ ~shared_lock() {
+t.unlock_shared();
+ }
+};
+} // namespace std
+
+std::mutex m1;
+std::mutex m2;
+
+// mutex ok
+
+void m_ok1() {
+ m1.lock(); // no-warning
+}
+
+void m_ok2() {
+ m1.unlock(); // no-warning
+}
+
+void m_ok3() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok4() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok5() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+}
+
+void m_ok6(void) {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok7(void) {
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok8(void) {
+ m1.unlock(); // no-warning
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok9(void) {
+ if (!m1.try_lock()) // no-warning
+m1.lock();// no-warning
+ m1.unlock();// no-warning
+}
+
+void m_ok10() {
+ std::lock_guard gl(m1); // no-warning
+}
+
+// mutex bad
+
+void m_bad1() {
+ m1.lock(); // no-warning
+ m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad2() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad3() {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+ m2.unlock(); // no-warning
+}
+
+void m_bad5() {
+ while (true)
+m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad6() {
+ while (true)
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad7() {
+ if (m1.try_lock()) // no-warning
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+std::timed_mutex tm1;
+std::timed_mutex tm2;
+
+// timed_mutex ok
+
+void tm_ok1() {
+ tm1.lock(); // no-warning
+}
+
+void tm_ok2() {
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok3() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok4() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok5() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+ tm2.lock(); // no-warning
+ tm2.unlock(); // no-warning
+}
+
+void tm_ok6(void) {
+ tm1.lock(); // no-warning
+ tm2.lock(); // no-warning
+ tm2.unlock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok7(void) {
+ if (tm1.try_lock()) // no-warning
+tm1.unlock(); // no-warning
+}
+
+void tm_ok8(void) {
+ tm1.unlock(); // no-warning
+ if (tm1.try_lock()) // no-warning
+
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
ASDenysPetrov updated this revision to Diff 289190.
ASDenysPetrov added a comment.
Added //timed// functions support and tests for them.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85984/new/
https://reviews.llvm.org/D85984
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
Index: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
@@ -0,0 +1, @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.CPlusPlus11Lock -verify %s
+
+namespace std {
+
+namespace chrono {
+using duration = int;
+using time_point = int;
+} // namespace chrono
+
+struct mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct timed_mutex {
+ void lock();
+ bool try_lock();
+ bool try_lock_for(std::chrono::duration);
+ bool try_lock_until(std::chrono::time_point);
+ void unlock();
+};
+
+struct recursive_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct recursive_timed_mutex {
+ void lock();
+ bool try_lock();
+ bool try_lock_for(std::chrono::duration);
+ bool try_lock_until(std::chrono::time_point);
+ void unlock();
+};
+
+struct shared_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+ void lock_shared();
+ bool try_lock_shared();
+ void unlock_shared();
+};
+
+struct shared_timed_mutex {
+ void lock();
+ bool try_lock();
+ bool try_lock_for(std::chrono::duration);
+ bool try_lock_until(std::chrono::time_point);
+ void unlock();
+ void lock_shared();
+ bool try_lock_shared();
+ bool try_lock_shared_for(std::chrono::duration);
+ bool try_lock_shared_until(std::chrono::time_point);
+ void unlock_shared();
+};
+
+template
+struct lock_guard {
+ T
+ lock_guard(T ) : t(m) {
+t.lock();
+ }
+ ~lock_guard() {
+t.unlock();
+ }
+};
+
+template
+struct shared_lock {
+ T
+ shared_lock(T ) : t(m) {
+t.lock_shared();
+ }
+ ~shared_lock() {
+t.unlock_shared();
+ }
+};
+} // namespace std
+
+std::mutex m1;
+std::mutex m2;
+
+// mutex ok
+
+void m_ok1() {
+ m1.lock(); // no-warning
+}
+
+void m_ok2() {
+ m1.unlock(); // no-warning
+}
+
+void m_ok3() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok4() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok5() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+}
+
+void m_ok6(void) {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok7(void) {
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok8(void) {
+ m1.unlock(); // no-warning
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok9(void) {
+ if (!m1.try_lock()) // no-warning
+m1.lock();// no-warning
+ m1.unlock();// no-warning
+}
+
+void m_ok10() {
+ std::lock_guard gl(m1); // no-warning
+}
+
+// mutex bad
+
+void m_bad1() {
+ m1.lock(); // no-warning
+ m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad2() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad3() {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+ m2.unlock(); // no-warning
+}
+
+void m_bad5() {
+ while (true)
+m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad6() {
+ while (true)
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad7() {
+ if (m1.try_lock()) // no-warning
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+std::timed_mutex tm1;
+std::timed_mutex tm2;
+
+// timed_mutex ok
+
+void tm_ok1() {
+ tm1.lock(); // no-warning
+}
+
+void tm_ok2() {
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok3() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok4() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok5() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+ tm2.lock(); // no-warning
+ tm2.unlock(); // no-warning
+}
+
+void tm_ok6(void) {
+ tm1.lock(); // no-warning
+ tm2.lock(); // no-warning
+ tm2.unlock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok7(void) {
+ if (tm1.try_lock()) // no-warning
+tm1.unlock(); // no-warning
+}
+
+void tm_ok8(void) {
+
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
ASDenysPetrov updated this revision to Diff 288930.
ASDenysPetrov added a comment.
Added //shared// semantics checks and correspondent tests.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85984/new/
https://reviews.llvm.org/D85984
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
Index: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
@@ -0,0 +1,902 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.CPlusPlus11Lock -verify %s
+
+namespace std {
+
+struct mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct timed_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct recursive_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct recursive_timed_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct shared_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+ void lock_shared();
+ bool try_lock_shared();
+ void unlock_shared();
+};
+
+struct shared_timed_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+ void lock_shared();
+ bool try_lock_shared();
+ void unlock_shared();
+};
+
+template
+struct lock_guard {
+ T
+ lock_guard(T ) : t(m) {
+t.lock();
+ }
+ ~lock_guard() {
+t.unlock();
+ }
+};
+
+template
+struct shared_lock {
+ T
+ shared_lock(T ) : t(m) {
+t.lock_shared();
+ }
+ ~shared_lock() {
+t.unlock_shared();
+ }
+};
+} // namespace std
+
+std::mutex m1;
+std::mutex m2;
+
+// mutex ok
+
+void m_ok1() {
+ m1.lock(); // no-warning
+}
+
+void m_ok2() {
+ m1.unlock(); // no-warning
+}
+
+void m_ok3() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok4() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok5() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+}
+
+void m_ok6(void) {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok7(void) {
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok8(void) {
+ m1.unlock(); // no-warning
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok9(void) {
+ if (!m1.try_lock()) // no-warning
+m1.lock();// no-warning
+ m1.unlock();// no-warning
+}
+
+void m_ok10() {
+ std::lock_guard gl(m1); // no-warning
+}
+
+// mutex bad
+
+void m_bad1() {
+ m1.lock(); // no-warning
+ m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad2() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad3() {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+ m2.unlock(); // no-warning
+}
+
+void m_bad5() {
+ while (true)
+m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad6() {
+ while (true)
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad7() {
+ if (m1.try_lock()) // no-warning
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+std::timed_mutex tm1;
+std::timed_mutex tm2;
+
+// timed_mutex ok
+
+void tm_ok1() {
+ tm1.lock(); // no-warning
+}
+
+void tm_ok2() {
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok3() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok4() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok5() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+ tm2.lock(); // no-warning
+ tm2.unlock(); // no-warning
+}
+
+void tm_ok6(void) {
+ tm1.lock(); // no-warning
+ tm2.lock(); // no-warning
+ tm2.unlock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok7(void) {
+ if (tm1.try_lock()) // no-warning
+tm1.unlock(); // no-warning
+}
+
+void tm_ok8(void) {
+ tm1.unlock(); // no-warning
+ if (tm1.try_lock()) // no-warning
+tm1.unlock(); // no-warning
+}
+
+void tm_ok9(void) {
+ if (!tm1.try_lock()) // no-warning
+tm1.lock();// no-warning
+ tm1.unlock();// no-warning
+}
+
+void tm_ok10() {
+ std::lock_guard gl(tm1); // no-warning
+}
+
+// timed_mutex bad
+
+void tm_bad1() {
+ tm1.lock(); // no-warning
+ tm1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
ASDenysPetrov updated this revision to Diff 287997.
ASDenysPetrov edited the summary of this revision.
ASDenysPetrov added a comment.
Added support of:
- std::timed_mutex
- std::recursive_timed_mutex
- std::shared_mutex
- std::shared_timed_mutex
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85984/new/
https://reviews.llvm.org/D85984
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
Index: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
@@ -0,0 +1,621 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.CPlusPlus11Lock -verify %s
+
+namespace std {
+
+struct mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct timed_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct recursive_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct recursive_timed_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct shared_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct shared_timed_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+template
+struct lock_guard {
+ T
+ lock_guard(T ) : t(m) {
+t.lock();
+ }
+ ~lock_guard() {
+t.unlock();
+ }
+};
+} // namespace std
+
+std::mutex m1;
+std::mutex m2;
+
+// mutex ok
+
+void m_ok1() {
+ m1.lock(); // no-warning
+}
+
+void m_ok2() {
+ m1.unlock(); // no-warning
+}
+
+void m_ok3() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok4() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok5() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+}
+
+void m_ok6(void) {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok7(void) {
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok8(void) {
+ m1.unlock(); // no-warning
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok9(void) {
+ if (!m1.try_lock()) // no-warning
+m1.lock();// no-warning
+ m1.unlock();// no-warning
+}
+
+void m_ok10() {
+ std::lock_guard gl1(m1); // no-warning
+}
+
+// mutex bad
+
+void m_bad1() {
+ m1.lock(); // no-warning
+ m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad2() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad3() {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+ m2.unlock(); // no-warning
+}
+
+void m_bad5() {
+ while (true)
+m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad6() {
+ while (true)
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad7() {
+ if (m1.try_lock()) // no-warning
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+std::timed_mutex tm1;
+std::timed_mutex tm2;
+
+// timed_mutex ok
+
+void tm_ok1() {
+ tm1.lock(); // no-warning
+}
+
+void tm_ok2() {
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok3() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok4() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok5() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+ tm2.lock(); // no-warning
+ tm2.unlock(); // no-warning
+}
+
+void tm_ok6(void) {
+ tm1.lock(); // no-warning
+ tm2.lock(); // no-warning
+ tm2.unlock(); // no-warning
+ tm1.unlock(); // no-warning
+}
+
+void tm_ok7(void) {
+ if (tm1.try_lock()) // no-warning
+tm1.unlock(); // no-warning
+}
+
+void tm_ok8(void) {
+ tm1.unlock(); // no-warning
+ if (tm1.try_lock()) // no-warning
+tm1.unlock(); // no-warning
+}
+
+void tm_ok9(void) {
+ if (!tm1.try_lock()) // no-warning
+tm1.lock();// no-warning
+ tm1.unlock();// no-warning
+}
+
+void tm_ok10() {
+ std::lock_guard gl(tm1); // no-warning
+}
+
+// timed_mutex bad
+
+void tm_bad1() {
+ tm1.lock(); // no-warning
+ tm1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void tm_bad2() {
+ tm1.lock(); // no-warning
+ tm1.unlock(); // no-warning
+ tm1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void tm_bad3() {
+ tm1.lock(); //
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
ASDenysPetrov added a comment.
@NoQ
> That's completely different checker logic.
I think, I got the message. The real recursive logic can be caught here:
std::recursive_mutex rm;
void recur1() {
recur2();
}
void recur2() {
rm.lock();
recur1(); // here we can ignore the meet of twice lock, can't we?
}
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85984/new/
https://reviews.llvm.org/D85984
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
NoQ added a comment. No-no, recursive locks are much more complicated than that, please do them separately. You'll have to discriminate between a real double lock / double unlock bug and an actual valid use of the recursive mutex in order to emit any warnings at all. That's completely different checker logic. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85984/new/ https://reviews.llvm.org/D85984 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
ASDenysPetrov updated this revision to Diff 287669.
ASDenysPetrov added a comment.
Added recursive_mutex support.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85984/new/
https://reviews.llvm.org/D85984
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
Index: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
@@ -0,0 +1,207 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.CPlusPlus11Lock -verify %s
+
+namespace std {
+
+struct mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct recursive_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+template
+struct guard_lock {
+ T
+ guard_lock(T ) : t(m) {
+t.lock();
+ }
+ ~guard_lock() {
+t.unlock();
+ }
+};
+} // namespace std
+
+std::mutex m1;
+std::mutex m2;
+
+// mutex ok
+
+void m_ok1() {
+ m1.lock(); // no-warning
+}
+
+void m_ok2() {
+ m1.unlock(); // no-warning
+}
+
+void m_ok3() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok4() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok5() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+}
+
+void m_ok6(void) {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void m_ok7(void) {
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok8(void) {
+ m1.unlock(); // no-warning
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok9(void) {
+ if (!m1.try_lock()) // no-warning
+m1.lock();// no-warning
+ m1.unlock();// no-warning
+}
+
+void m_ok10() {
+ std::guard_lock gl1(m1); // no-warning
+}
+
+// mutex bad
+
+void m_bad1() {
+ m1.lock(); // no-warning
+ m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad2() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad3() {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+ m2.unlock(); // no-warning
+}
+
+void m_bad5() {
+ while (true)
+m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad6() {
+ while (true)
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+std::recursive_mutex rm1;
+std::recursive_mutex rm2;
+
+// recursive_mutex ok
+
+void rm_ok1() {
+ rm1.lock(); // no-warning
+}
+
+void rm_ok2() {
+ rm1.unlock(); // no-warning
+}
+
+void rm_ok3() {
+ rm1.lock(); // no-warning
+ rm1.unlock(); // no-warning
+}
+
+void rm_ok4() {
+ rm1.lock(); // no-warning
+ rm1.unlock(); // no-warning
+ rm1.lock(); // no-warning
+ rm1.unlock(); // no-warning
+}
+
+void rm_ok5() {
+ rm1.lock(); // no-warning
+ rm1.unlock(); // no-warning
+ rm2.lock(); // no-warning
+ rm2.unlock(); // no-warning
+}
+
+void rm_ok6(void) {
+ rm1.lock(); // no-warning
+ rm2.lock(); // no-warning
+ rm2.unlock(); // no-warning
+ rm1.unlock(); // no-warning
+}
+
+void rm_ok7(void) {
+ if (rm1.try_lock()) // no-warning
+rm1.unlock(); // no-warning
+}
+
+void rm_ok8(void) {
+ rm1.unlock(); // no-warning
+ if (rm1.try_lock()) // no-warning
+rm1.unlock(); // no-warning
+}
+
+void rm_ok9(void) {
+ if (!rm1.try_lock()) // no-warning
+rm1.lock();// no-warning
+ rm1.unlock();// no-warning
+}
+
+void rm_ok10() {
+ std::guard_lock gl2(rm1); // no-warning
+}
+
+void rm_ok11() {
+ rm1.lock(); // no-warning
+ rm1.lock(); // no-warning
+}
+
+void rm_ok12() {
+ while (true)
+rm1.lock(); // no-warning
+}
+
+// recursive_mutex bad
+
+void rm_bad1() {
+ rm1.lock(); // no-warning
+ rm1.unlock(); // no-warning
+ rm1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void rm_bad2() {
+ rm1.lock(); // no-warning
+ rm2.lock(); // no-warning
+ rm1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+ rm2.unlock(); // no-warning
+}
+
+void rm_bad3() {
+ while (true)
+rm1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -70,11 +70,17
[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock
ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: NoQ, vsavchenko, xazax.hun, dcoughlin.
ASDenysPetrov added a project: clang.
Herald added subscribers: cfe-commits, steakhal, martong, Charusso, dkrupp,
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet,
baloghadamsoftware.
ASDenysPetrov requested review of this revision.
This checker is made above PthreadLockChecker and works the same. It is adapted
for **std::mutex** primitive.
Next I want to add support **std::recursive_mutex**.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D85984
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
Index: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
@@ -0,0 +1,119 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.CPlusPlus11Lock -verify %s
+
+namespace std {
+
+struct mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+struct recursive_mutex {
+ void lock();
+ bool try_lock();
+ void unlock();
+};
+
+template
+struct guard_lock {
+ T
+ guard_lock(T ) : t(m) {
+t.lock();
+ }
+ ~guard_lock() {
+t.unlock();
+ }
+};
+} // namespace std
+
+std::mutex m1;
+std::mutex m2;
+std::recursive_mutex rm1;
+std::recursive_mutex rm2;
+
+// ok
+
+void ok1() {
+ m1.lock(); // no-warning
+}
+
+void ok2() {
+ m1.unlock(); // no-warning
+}
+
+void ok3() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void ok4() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void ok5() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+}
+
+void ok6(void) {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m2.unlock(); // no-warning
+ m1.unlock(); // no-warning
+}
+
+void ok7(void) {
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void ok8(void) {
+ m1.unlock(); // no-warning
+ if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void ok9(void) {
+ if (!m1.try_lock()) // no-warning
+m1.lock();// no-warning
+ m1.unlock();// no-warning
+}
+
+void ok10() {
+ std::guard_lock gl(m1);
+}
+
+// bad
+
+void bad1() {
+ m1.lock(); // no-warning
+ m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void bad2() {
+ m1.lock(); // no-warning
+ m1.unlock(); // no-warning
+ m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void bad3() {
+ m1.lock(); // no-warning
+ m2.lock(); // no-warning
+ m1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+ m2.unlock(); // no-warning
+}
+
+void bad5() {
+ while (true)
+m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void bad6() {
+ while (true)
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -70,11 +70,17 @@
class PthreadLockChecker : public Checker {
public:
- enum LockingSemantics { NotApplicable = 0, PthreadSemantics, XNUSemantics };
+ enum LockingSemantics {
+NotApplicable = 0,
+PthreadSemantics,
+XNUSemantics,
+CPlusPlusSemantics,
+ };
enum CheckerKind {
CK_PthreadLockChecker,
CK_FuchsiaLockChecker,
CK_C11LockChecker,
+CK_CPlusPlus11LockChecker,
CK_NumCheckKinds
};
DefaultBool ChecksEnabled[CK_NumCheckKinds];
@@ -83,7 +89,7 @@
private:
typedef void (PthreadLockChecker::*FnCheck)(const CallEvent ,
CheckerContext ,
- CheckerKind checkkind) const;
+ CheckerKind CheckKind) const;
CallDescriptionMap PThreadCallbacks = {
// Init.
{{"pthread_mutex_init", 2}, ::InitAnyLock},
@@ -164,49 +170,82 @@
{{"mtx_destroy", 1}, ::DestroyPthreadLock},
};
+ CallDescriptionMap CPlusPlus11Callbacks = {
+
+ // Acquire.
+ {{{"std", "mutex", "lock"}, 0, 0},
+ ::AcquireCPlusPlus11Lock},
+ // TODO {{{"std", "recursive_mutex", "lock"}, 0, 0},
+ // ::AcquireCPlusPlus11RecursiveLock},
+
+ // Try.
+ {{{"std", "mutex", "try_lock"}, 0, 0},
+ ::TryCPlusPlus11Lock},
+ // TODO {{{"std", "recursive_mutex", "try_lock"}, 0, 0},
+ // ::TryCPlusPlus11RecursiveLock},
+
+ // Release.
+ {{{"std",
