This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
aaronpuchert marked an inline comment as done.
Closed by commit rGf664e2ec371f: Thread safety analysis: Always warn when
dropping locks on back edges (authored by aaronpuchert).
Changed prior to commit:
https://reviews.llvm.org/D104261?vs=351972&id=355373#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104261/new/
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,45 @@
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)
+ scope.Lock(); // We could catch this double lock with negative capabilities.
+}
+
+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() || LEK2 == 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
