[PATCH] D104261: Thread safety analysis: Always warn when dropping locks on back edges
Aaron Puchert via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 14 13:15:27 PDT 2021
aaronpuchert added inline comments.
================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:868
ThreadSafetyHandler &Handler) const override {
- if (!managed() && !asserted() && !negative() && !isUniversal()) {
+ if (!asserted() && !negative() && !isUniversal()) {
Handler.handleMutexHeldEndOfScope("mutex", toString(), loc(), JoinLoc,
----------------
One might ask: what about asserted capabilities? I plan to introduce a warning when they are released, because that can't be consistent, and then they can't disappear on back edges without warning.
For negative capabilities we'd presumably see a warning for the "positive" capability instead.
Not sure how universal capabilities are typically used. Presumably one could release such a capability in a loop? Then on the other hand, code using such capabilities is probably not very interested in false negatives.
================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:2227-2228
/// \param LEK2 The error message to report if a mutex is missing from Lset2
void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1,
const FactSet &FSet2,
SourceLocation JoinLoc,
----------------
Presumably we should call these `EntrySet` and `ExitSet` instead? The second parameter is always the exit set of an existing block, the first parameter the entry set of a (sometimes new) block.
================
Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:643
sls_mu.Lock(); // \
- // expected-note {{the other acquisition of mutex 'sls_mu' is here}}
+ // expected-warning {{mutex 'sls_mu' is acquired exclusively and shared in the same scope}}
} while (getBool());
----------------
These are just swapped because I'm merging the branches in a different order now. I think that's Ok.
================
Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:2788
+ // We have to warn on this join point despite the lock being managed ...
+ for (unsigned i = 1; i < 10; ++i) { // expected-warning {{expecting mutex 'mu' to be held at start of each loop}}
+ x = 1; // ... because we might miss that this doesn't always happen under lock.
----------------
This warning is new.
================
Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:2813
+ scope.Unlock();
+ continue; // expected-warning {{expecting mutex 'mu' to be held at start of each loop}}
+ }
----------------
This is also new.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104261/new/
https://reviews.llvm.org/D104261
More information about the cfe-commits
mailing list