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
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
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:
>
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
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?
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.
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.
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
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:
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:
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
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
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
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
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,
15 matches
Mail list logo
