ajtowns added a comment.
In D87629#2272676 <https://reviews.llvm.org/D87629#2272676>, @aaronpuchert
wrote:
> (The existing `LockAssertion` class that the change removed looks like it
> should use `ASSERT_CAPABILITY` instead—please don't use `ACQUIRE` when the
> capability is assumed to be held previously.)
Not sure the `try { AssertHeld } catch (...) { a = 0; }` example reveals very
much: it seems like thread safety annotations aren't checked within a catch
block at all?
Mutex m;
int i GUARDED_BY(m);
static void thrower() { throw std::exception(); }
void f() { i = 1; }
void g() {
try {
thrower();
} catch (...) {
i = 5;
}
i = 6;
}
gives me warnings for `i=1` and `i=6` but not `i=5` ?
Extending the above with:
void AssertHeld() ASSERT_CAPABILITY(m) { return; }
void h(bool b) {
try {
if (b) thrower();
i = 7;
AssertHeld();
i = 8;
} catch (...) { }
i = 9;
}
only gives me a warning for `i = 7`, even though when `b` is true `i = 9` is
just as bad as the `i = 6` assignment that did generate a warning.
Using a dummy RAII class with SCOPED_CAPABILITY that does ACQUIRE/RELEASE
without modifying the actual mutex seems like it behaves more sensibly to me...
class SCOPED_LOCKABLE assumer
{
public:
assumer(Mutex& m_) EXCLUSIVE_LOCK_FUNCTION(m_) { }
~assumer() UNLOCK_FUNCTION() { }
};
void h(bool b) {
try {
if (b) thrower();
i = 7;
assumer a(m);
i = 8;
} catch (...) { }
i = 9;
}
gives me warnings for both `i=7` and `i=9` but allows `i=8`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87629/new/
https://reviews.llvm.org/D87629
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
