aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
We allow branches to join where one holds a managed lock but the other
doesn't, but we can't do so for back edges: because there we can't drop
them from the lockset, as we have already analyzed the loop with the
larger lockset. So we can't allow dropping managed locks on back edges.
We move the managed() check from handleRemovalFromIntersection up to
intersectAndWarn, where we additionally check if we're on a back edge if
we're removing from the first lock set (the entry set of the next block)
but not if we're removing from the second lock set (the exit set of the
previous block). Now that the order of arguments matters, I had to swap
them in one invocation, which also causes some minor differences in the
tests.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D104261
Files:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===================================================================
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -636,11 +636,11 @@
void shared_fun_1() {
sls_mu.ReaderLock(); // \
- // expected-warning {{mutex 'sls_mu' is acquired exclusively and shared in the same scope}}
+ // expected-note {{the other acquisition of mutex 'sls_mu' is here}}
do {
sls_mu.Unlock();
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());
sls_mu.Unlock();
}
@@ -695,11 +695,11 @@
void shared_bad_0() {
sls_mu.Lock(); // \
- // expected-warning {{mutex 'sls_mu' is acquired exclusively and shared in the same scope}}
+ // expected-note {{the other acquisition of mutex 'sls_mu' is here}}
do {
sls_mu.Unlock();
sls_mu.ReaderLock(); // \
- // 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());
sls_mu.Unlock();
}
@@ -2773,6 +2773,48 @@
x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
}
+void loopAcquire() {
+ RelockableMutexLock scope(&mu, DeferTraits{});
+ for (unsigned i = 1; i < 10; ++i) {
+ x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+ if (i == 5)
+ scope.Lock();
+ }
+}
+
+void loopRelease() {
+ RelockableMutexLock scope(&mu, ExclusiveTraits{}); // expected-note {{mutex acquired here}}
+ // 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.
+ if (i == 5)
+ scope.Unlock();
+ }
+}
+
+void loopAcquireContinue() {
+ RelockableMutexLock scope(&mu, DeferTraits{});
+ for (unsigned i = 1; i < 10; ++i) {
+ x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+ if (i == 5) {
+ scope.Lock();
+ continue;
+ }
+ }
+}
+
+void loopReleaseContinue() {
+ RelockableMutexLock scope(&mu, ExclusiveTraits{}); // expected-note {{mutex acquired here}}
+ // We have to warn on this join point despite the lock being managed ...
+ for (unsigned i = 1; i < 10; ++i) {
+ x = 1; // ... because we might miss that this doesn't always happen under lock.
+ if (i == 5) {
+ scope.Unlock();
+ continue; // expected-warning {{expecting mutex 'mu' to be held at start of each loop}}
+ }
+ }
+}
+
void exclusiveSharedJoin() {
RelockableMutexLock scope(&mu, DeferTraits{});
if (b)
Index: clang/lib/Analysis/ThreadSafety.cpp
===================================================================
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -865,7 +865,7 @@
handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
SourceLocation JoinLoc, LockErrorKind LEK,
ThreadSafetyHandler &Handler) const override {
- if (!managed() && !asserted() && !negative() && !isUniversal()) {
+ if (!asserted() && !negative() && !isUniversal()) {
Handler.handleMutexHeldEndOfScope("mutex", toString(), loc(), JoinLoc,
LEK);
}
@@ -2239,7 +2239,7 @@
if (Iter1 != FSet1.end()) {
if (join(FactMan[*Iter1], LDat2) && LEK1 == LEK_LockedSomePredecessors)
*Iter1 = Fact;
- } else {
+ } else if (!LDat2.managed()) {
LDat2.handleRemovalFromIntersection(FSet2, FactMan, JoinLoc, LEK1,
Handler);
}
@@ -2251,8 +2251,9 @@
const FactEntry *LDat2 = FSet2.findLock(FactMan, *LDat1);
if (!LDat2) {
- LDat1->handleRemovalFromIntersection(FSet1Orig, FactMan, JoinLoc, LEK2,
- Handler);
+ if (!LDat1->managed() || LEK1 == LEK_LockedSomeLoopIterations)
+ LDat1->handleRemovalFromIntersection(FSet1Orig, FactMan, JoinLoc, LEK2,
+ Handler);
if (LEK2 == LEK_LockedSomePredecessors)
FSet1.removeLock(FactMan, *LDat1);
}
@@ -2528,7 +2529,7 @@
CFGBlock *FirstLoopBlock = *SI;
CFGBlockInfo *PreLoop = &BlockInfo[FirstLoopBlock->getBlockID()];
CFGBlockInfo *LoopEnd = &BlockInfo[CurrBlockID];
- intersectAndWarn(LoopEnd->ExitSet, PreLoop->EntrySet, PreLoop->EntryLoc,
+ intersectAndWarn(PreLoop->EntrySet, LoopEnd->ExitSet, PreLoop->EntryLoc,
LEK_LockedSomeLoopIterations);
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
