diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp --- a/clang/lib/Analysis/ThreadSafety.cpp +++ b/clang/lib/Analysis/ThreadSafety.cpp @@ -1050,7 +1050,7 @@ const CFGBlock* PredBlock, const CFGBlock *CurrBlock); - bool join(const FactEntry &a, const FactEntry &b); + bool join(const FactEntry &a, const FactEntry &b, bool CanModify); void intersectAndWarn(FactSet &EntrySet, const FactSet &ExitSet, SourceLocation JoinLoc, LockErrorKind EntryLEK, @@ -2188,25 +2188,28 @@ } } -/// Given two facts merging on a join point, decide whether to warn and which -/// one to keep. +/// Given two facts merging on a join point, possibly warn and decide whether to +/// keep or replace. /// -/// \return false if we should keep \p A, true if we should keep \p B. -bool ThreadSafetyAnalyzer::join(const FactEntry &A, const FactEntry &B) { +/// \param CanModify Whether we can replace \p A by \p B. +/// \return false if we should keep \p A, true if we should take \p B. +bool ThreadSafetyAnalyzer::join(const FactEntry &A, const FactEntry &B, + bool CanModify) { if (A.kind() != B.kind()) { // For managed capabilities, the destructor should unlock in the right mode // anyway. For asserted capabilities no unlocking is needed. if ((A.managed() || A.asserted()) && (B.managed() || B.asserted())) { - // The shared capability subsumes the exclusive capability. - return B.kind() == LK_Shared; - } else { - Handler.handleExclusiveAndShared("mutex", B.toString(), B.loc(), A.loc()); - // Take the exclusive capability to reduce further warnings. - return B.kind() == LK_Exclusive; + // The shared capability subsumes the exclusive capability, if possible. + bool ShouldTakeB = B.kind() == LK_Shared; + if (CanModify || !ShouldTakeB) + return ShouldTakeB; } + Handler.handleExclusiveAndShared("mutex", B.toString(), B.loc(), A.loc()); + // Take the exclusive capability to reduce further warnings. + return CanModify && B.kind() == LK_Exclusive; } else { // The non-asserted capability is the one we want to track. - return A.asserted() && !B.asserted(); + return CanModify && A.asserted() && !B.asserted(); } } @@ -2237,8 +2240,8 @@ FactSet::iterator EntryIt = EntrySet.findLockIter(FactMan, ExitFact); if (EntryIt != EntrySet.end()) { - if (join(FactMan[*EntryIt], ExitFact) && - EntryLEK == LEK_LockedSomePredecessors) + if (join(FactMan[*EntryIt], ExitFact, + EntryLEK != LEK_LockedSomeLoopIterations)) *EntryIt = Fact; } else if (!ExitFact.managed()) { ExitFact.handleRemovalFromIntersection(ExitSet, FactMan, JoinLoc, diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -2789,6 +2789,25 @@ } } +void loopPromote() { + RelockableMutexLock scope(&mu, SharedTraits{}); + for (unsigned i = 1; i < 10; ++i) { + x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}} + if (i == 5) + scope.PromoteShared(); + } +} + +void loopDemote() { + RelockableMutexLock scope(&mu, ExclusiveTraits{}); // expected-note {{the other acquisition of mutex 'mu' is 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 exclusive lock. + if (i == 5) + scope.DemoteExclusive(); // expected-warning {{mutex 'mu' is acquired exclusively and shared in the same scope}} + } +} + void loopAcquireContinue() { RelockableMutexLock scope(&mu, DeferTraits{}); for (unsigned i = 1; i < 10; ++i) { @@ -2812,6 +2831,29 @@ } } +void loopPromoteContinue() { + RelockableMutexLock scope(&mu, SharedTraits{}); + for (unsigned i = 1; i < 10; ++i) { + x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}} + if (i == 5) { + scope.PromoteShared(); + continue; + } + } +} + +void loopDemoteContinue() { + RelockableMutexLock scope(&mu, ExclusiveTraits{}); // expected-note {{the other acquisition of mutex 'mu' is 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 exclusive lock. + if (i == 5) { + scope.DemoteExclusive(); // expected-warning {{mutex 'mu' is acquired exclusively and shared in the same scope}} + continue; + } + } +} + void exclusiveSharedJoin() { RelockableMutexLock scope(&mu, DeferTraits{}); if (b)