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
