This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6de19ea4b626: Thread safety analysis: Drop special block
handling (authored by aaronpuchert).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106715/new/
https://reviews.llvm.org/D106715
Files:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/PCH/thread-safety-attrs.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
@@ -280,12 +280,12 @@
void sls_fun_bad_7() {
sls_mu.Lock();
- while (getBool()) {
+ while (getBool()) { // \
+ expected-warning{{expecting mutex 'sls_mu' to be held at start of each loop}}
sls_mu.Unlock();
if (getBool()) {
if (getBool()) {
- continue; // \
- expected-warning{{expecting mutex 'sls_mu' to be held at start of each loop}}
+ continue;
}
}
sls_mu.Lock(); // expected-note {{mutex acquired here}}
@@ -332,13 +332,14 @@
sls_mu.Unlock();
if (getBool()) {
if (getBool()) {
- break; // expected-warning{{mutex 'sls_mu' is not held on every path through here}}
+ break;
}
}
sls_mu.Lock();
}
sls_mu.Unlock(); // \
- // expected-warning{{releasing mutex 'sls_mu' that was not held}}
+ expected-warning{{mutex 'sls_mu' is not held on every path through here}} \
+ expected-warning{{releasing mutex 'sls_mu' that was not held}}
}
//-----------------------------------------//
@@ -2086,13 +2087,13 @@
mutex m;
void test() {
m.lock();
- while (foo()) {
+ while (foo()) { // expected-warning {{expecting mutex 'm' to be held at start of each loop}}
m.unlock();
// ...
if (bar()) {
// ...
if (foo())
- continue; // expected-warning {{expecting mutex 'm' to be held at start of each loop}}
+ continue;
//...
}
// ...
@@ -2822,11 +2823,11 @@
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) {
+ 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();
- continue; // expected-warning {{expecting mutex 'mu' to be held at start of each loop}}
+ continue;
}
}
}
Index: clang/test/PCH/thread-safety-attrs.cpp
===================================================================
--- clang/test/PCH/thread-safety-attrs.cpp
+++ clang/test/PCH/thread-safety-attrs.cpp
@@ -254,12 +254,12 @@
void sls_fun_bad_7() {
sls_mu.Lock();
- while (getBool()) {
+ while (getBool()) { // \
+ expected-warning{{expecting mutex 'sls_mu' to be held at start of each loop}}
sls_mu.Unlock();
if (getBool()) {
if (getBool()) {
- continue; // \
- expected-warning{{expecting mutex 'sls_mu' to be held at start of each loop}}
+ continue;
}
}
sls_mu.Lock(); // expected-note {{mutex acquired here}}
@@ -306,13 +306,14 @@
sls_mu.Unlock();
if (getBool()) {
if (getBool()) {
- break; // expected-warning{{mutex 'sls_mu' is not held on every path through here}}
+ break;
}
}
sls_mu.Lock();
}
sls_mu.Unlock(); // \
- // expected-warning{{releasing mutex 'sls_mu' that was not held}}
+ expected-warning{{mutex 'sls_mu' is not held on every path through here}} \
+ expected-warning{{releasing mutex 'sls_mu' that was not held}}
}
#endif
Index: clang/lib/Analysis/ThreadSafety.cpp
===================================================================
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -849,6 +849,11 @@
// location.
CurrBlockInfo->EntryLoc = CurrBlockInfo->ExitLoc =
BlockInfo[(*CurrBlock->pred_begin())->getBlockID()].ExitLoc;
+ } else if (CurrBlock->succ_size() == 1 && *CurrBlock->succ_begin()) {
+ // The block is empty, and has a single successor. Use its entry
+ // location.
+ CurrBlockInfo->EntryLoc = CurrBlockInfo->ExitLoc =
+ BlockInfo[(*CurrBlock->succ_begin())->getBlockID()].EntryLoc;
}
}
}
@@ -2415,7 +2420,6 @@
// union because the real error is probably that we forgot to unlock M on
// all code paths.
bool LocksetInitialized = false;
- SmallVector<CFGBlock *, 8> SpecialBlocks;
for (CFGBlock::const_pred_iterator PI = CurrBlock->pred_begin(),
PE = CurrBlock->pred_end(); PI != PE; ++PI) {
// if *PI -> CurrBlock is a back edge
@@ -2432,17 +2436,6 @@
// Okay, we can reach this block from the entry.
CurrBlockInfo->Reachable = true;
- // If the previous block ended in a 'continue' or 'break' statement, then
- // a difference in locksets is probably due to a bug in that block, rather
- // than in some other predecessor. In that case, keep the other
- // predecessor's lockset.
- if (const Stmt *Terminator = (*PI)->getTerminatorStmt()) {
- if (isa<ContinueStmt>(Terminator) || isa<BreakStmt>(Terminator)) {
- SpecialBlocks.push_back(*PI);
- continue;
- }
- }
-
FactSet PrevLockset;
getEdgeLockset(PrevLockset, PrevBlockInfo->ExitSet, *PI, CurrBlock);
@@ -2450,9 +2443,14 @@
CurrBlockInfo->EntrySet = PrevLockset;
LocksetInitialized = true;
} else {
- intersectAndWarn(CurrBlockInfo->EntrySet, PrevLockset,
- CurrBlockInfo->EntryLoc,
- LEK_LockedSomePredecessors);
+ // Surprisingly 'continue' doesn't always produce back edges, because
+ // the CFG has empty "transition" blocks where they meet with the end
+ // of the regular loop body. We still want to diagnose them as loop.
+ intersectAndWarn(
+ CurrBlockInfo->EntrySet, PrevLockset, CurrBlockInfo->EntryLoc,
+ isa_and_nonnull<ContinueStmt>((*PI)->getTerminatorStmt())
+ ? LEK_LockedSomeLoopIterations
+ : LEK_LockedSomePredecessors);
}
}
@@ -2460,35 +2458,6 @@
if (!CurrBlockInfo->Reachable)
continue;
- // Process continue and break blocks. Assume that the lockset for the
- // resulting block is unaffected by any discrepancies in them.
- for (const auto *PrevBlock : SpecialBlocks) {
- unsigned PrevBlockID = PrevBlock->getBlockID();
- CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID];
-
- if (!LocksetInitialized) {
- CurrBlockInfo->EntrySet = PrevBlockInfo->ExitSet;
- LocksetInitialized = true;
- } else {
- // Determine whether this edge is a loop terminator for diagnostic
- // purposes. FIXME: A 'break' statement might be a loop terminator, but
- // it might also be part of a switch. Also, a subsequent destructor
- // might add to the lockset, in which case the real issue might be a
- // double lock on the other path.
- const Stmt *Terminator = PrevBlock->getTerminatorStmt();
- bool IsLoop = Terminator && isa<ContinueStmt>(Terminator);
-
- FactSet PrevLockset;
- getEdgeLockset(PrevLockset, PrevBlockInfo->ExitSet,
- PrevBlock, CurrBlock);
-
- // Do not update EntrySet.
- intersectAndWarn(
- CurrBlockInfo->EntrySet, PrevLockset, PrevBlockInfo->ExitLoc,
- IsLoop ? LEK_LockedSomeLoopIterations : LEK_LockedSomePredecessors);
- }
- }
-
BuildLockset LocksetBuilder(this, *CurrBlockInfo);
// Visit all the statements in the basic block.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
