caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Thread Safety Analysis: Differentiate between lock sets at real join points and expected/actual sets at function end (#105526)

Browse Source 
This fixes false positives related to returning a scoped lockable
object. At the end of a function, we check managed locks instead of
scoped locks.

At real join points, we skip checking managed locks because we assume
that the scope keeps track of its underlying mutexes and will release
them at its destruction. So, checking for the scopes is sufficient.
However, at the end of a function, we aim at comparing the expected and
the actual lock sets. There, we skip checking scoped locks to prevent to
get duplicate warnings for the same lock.
This commit is contained in:
Malek Ben Slimane
2024-09-04 16:18:11 +02:00
committed by GitHub
parent 30f1cfb4d0
commit e64ef634bb
2 changed files with 14 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
clang/lib/Analysis/ThreadSafety.cpp
View File

@@ -922,6 +922,9 @@ public:
handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
SourceLocation JoinLoc, LockErrorKind LEK,
ThreadSafetyHandler &Handler) const override {
if (LEK == LEK_LockedAtEndOfFunction || LEK == LEK_NotLockedAtEndOfFunction)
return;
for (const auto &UnderlyingMutex : UnderlyingMutexes) {
const auto *Entry = FSet.findLock(FactMan, UnderlyingMutex.Cap);
if ((UnderlyingMutex.Kind == UCK_Acquired && Entry) ||
@@ -2224,7 +2227,7 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &EntrySet,
if (join(FactMan[*EntryIt], ExitFact,
EntryLEK != LEK_LockedSomeLoopIterations))
*EntryIt = Fact;
} else if (!ExitFact.managed()) {
} else if (!ExitFact.managed() || EntryLEK == LEK_LockedAtEndOfFunction) {
ExitFact.handleRemovalFromIntersection(ExitSet, FactMan, JoinLoc,
EntryLEK, Handler);
}
@@ -2236,7 +2239,8 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &EntrySet,
const FactEntry *ExitFact = ExitSet.findLock(FactMan, *EntryFact);
if (!ExitFact) {
if (!EntryFact->managed() || ExitLEK == LEK_LockedSomeLoopIterations)
if (!EntryFact->managed() || ExitLEK == LEK_LockedSomeLoopIterations ||
ExitLEK == LEK_NotLockedAtEndOfFunction)
EntryFact->handleRemovalFromIntersection(EntrySetOrig, FactMan, JoinLoc,
ExitLEK, Handler);
if (ExitLEK == LEK_LockedSomePredecessors)

20
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
View File

@@ -6077,24 +6077,20 @@ namespace ReturnScopedLockable {
class Object {
public:
MutexLock lock() EXCLUSIVE_LOCK_FUNCTION(mutex) {
// TODO: False positive because scoped lock isn't destructed.
return MutexLock(&mutex); // expected-note {{mutex acquired here}}
} // expected-warning {{mutex 'mutex' is still held at the end of function}}
return MutexLock(&mutex);
}
ReaderMutexLock lockShared() SHARED_LOCK_FUNCTION(mutex) {
// TODO: False positive because scoped lock isn't destructed.
return ReaderMutexLock(&mutex); // expected-note {{mutex acquired here}}
} // expected-warning {{mutex 'mutex' is still held at the end of function}}
return ReaderMutexLock(&mutex);
}
MutexLock adopt() EXCLUSIVE_LOCKS_REQUIRED(mutex) {
// TODO: False positive because scoped lock isn't destructed.
return MutexLock(&mutex, true); // expected-note {{mutex acquired here}}
} // expected-warning {{mutex 'mutex' is still held at the end of function}}
return MutexLock(&mutex, true);
}
ReaderMutexLock adoptShared() SHARED_LOCKS_REQUIRED(mutex) {
// TODO: False positive because scoped lock isn't destructed.
return ReaderMutexLock(&mutex, true); // expected-note {{mutex acquired here}}
} // expected-warning {{mutex 'mutex' is still held at the end of function}}
return ReaderMutexLock(&mutex, true);
}
int x GUARDED_BY(mutex);
void needsLock() EXCLUSIVE_LOCKS_REQUIRED(mutex);