Index: include/clang/Analysis/Analyses/ThreadSafety.h =================================================================== --- include/clang/Analysis/Analyses/ThreadSafety.h +++ include/clang/Analysis/Analyses/ThreadSafety.h @@ -128,9 +128,10 @@ /// \param Kind -- the capability's name parameter (role, mutex, etc). /// \param LockName -- A StringRef name for the lock expression, to be printed /// in the error message. + /// \param LocLocked -- The location of the first lock expression. /// \param Loc -- The location of the second lock expression. virtual void handleDoubleLock(StringRef Kind, Name LockName, - SourceLocation Loc) {} + SourceLocation LocLocked, SourceLocation Loc) {} /// Warn about situations where a mutex is sometimes held and sometimes not. /// The three situations are: Index: lib/Analysis/ThreadSafety.cpp =================================================================== --- lib/Analysis/ThreadSafety.cpp +++ lib/Analysis/ThreadSafety.cpp @@ -873,7 +873,7 @@ void handleLock(FactSet &FSet, FactManager &FactMan, const FactEntry &entry, ThreadSafetyHandler &Handler, StringRef DiagKind) const override { - Handler.handleDoubleLock(DiagKind, entry.toString(), entry.loc()); + Handler.handleDoubleLock(DiagKind, entry.toString(), loc(), entry.loc()); } void handleUnlock(FactSet &FSet, FactManager &FactMan, @@ -981,12 +981,13 @@ void lock(FactSet &FSet, FactManager &FactMan, const CapabilityExpr &Cp, LockKind kind, SourceLocation loc, ThreadSafetyHandler *Handler, StringRef DiagKind) const { - if (!FSet.findLock(FactMan, Cp)) { + if (const FactEntry *fact = FSet.findLock(FactMan, Cp)) { + if (Handler) + Handler->handleDoubleLock(DiagKind, Cp.toString(), fact->loc(), loc); + } else { FSet.removeLock(FactMan, !Cp); FSet.addLock(FactMan, llvm::make_unique(Cp, kind, loc)); - } else if (Handler) { - Handler->handleDoubleLock(DiagKind, Cp.toString(), loc); } } Index: lib/Sema/AnalysisBasedWarnings.cpp =================================================================== --- lib/Sema/AnalysisBasedWarnings.cpp +++ lib/Sema/AnalysisBasedWarnings.cpp @@ -1638,17 +1638,6 @@ return ONS; } - // Helper functions - void warnLockMismatch(unsigned DiagID, StringRef Kind, Name LockName, - SourceLocation Loc) { - // Gracefully handle rare cases when the analysis can't get a more - // precise source location. - if (!Loc.isValid()) - Loc = FunLocation; - PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) << Kind << LockName); - Warnings.emplace_back(std::move(Warning), getNotes()); - } - public: ThreadSafetyReporter(Sema &S, SourceLocation FL, SourceLocation FEL) : S(S), FunLocation(FL), FunEndLocation(FEL), @@ -1677,7 +1666,11 @@ void handleUnmatchedUnlock(StringRef Kind, Name LockName, SourceLocation Loc) override { - warnLockMismatch(diag::warn_unlock_but_no_lock, Kind, LockName, Loc); + if (Loc.isInvalid()) + Loc = FunLocation; + PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_unlock_but_no_lock) + << Kind << LockName); + Warnings.emplace_back(std::move(Warning), getNotes()); } void handleIncorrectUnlockKind(StringRef Kind, Name LockName, @@ -1691,8 +1684,18 @@ Warnings.emplace_back(std::move(Warning), getNotes()); } - void handleDoubleLock(StringRef Kind, Name LockName, SourceLocation Loc) override { - warnLockMismatch(diag::warn_double_lock, Kind, LockName, Loc); + void handleDoubleLock(StringRef Kind, Name LockName, SourceLocation LocLocked, + SourceLocation Loc) override { + if (Loc.isInvalid()) + Loc = FunLocation; + PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_double_lock) + << Kind << LockName); + OptionalNotes notes = + LocLocked.isValid() + ? getNotes(PartialDiagnosticAt( + LocLocked, S.PDiag(diag::note_locked_here) << Kind)) + : getNotes(); + Warnings.emplace_back(std::move(Warning), std::move(notes)); } void handleMutexHeldEndOfScope(StringRef Kind, Name LockName, Index: test/SemaCXX/warn-thread-safety-analysis.cpp =================================================================== --- test/SemaCXX/warn-thread-safety-analysis.cpp +++ test/SemaCXX/warn-thread-safety-analysis.cpp @@ -239,7 +239,7 @@ } void sls_fun_bad_2() { - sls_mu.Lock(); + sls_mu.Lock(); // expected-note{{mutex acquired here}} sls_mu.Lock(); // \ // expected-warning{{acquiring mutex 'sls_mu' that is already held}} sls_mu.Unlock(); @@ -365,7 +365,7 @@ } void aa_fun_bad_2() { - glock.globalLock(); + glock.globalLock(); // expected-note{{mutex acquired here}} glock.globalLock(); // \ // expected-warning{{acquiring mutex 'aa_mu' that is already held}} glock.globalUnlock(); @@ -1691,7 +1691,7 @@ } void foo3() { - MutexLock mulock_a(&mu1); + MutexLock mulock_a(&mu1); // expected-note{{mutex acquired here}} MutexLock mulock_b(&mu1); // \ // expected-warning {{acquiring mutex 'mu1' that is already held}} } @@ -2710,14 +2710,14 @@ } void doubleLock1() { - RelockableExclusiveMutexLock scope(&mu); + RelockableExclusiveMutexLock scope(&mu); // expected-note{{mutex acquired here}} scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already held}} } void doubleLock2() { RelockableExclusiveMutexLock scope(&mu); scope.Unlock(); - scope.Lock(); + scope.Lock(); // expected-note{{mutex acquired here}} scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already held}} } @@ -2754,7 +2754,7 @@ }; void relockShared2() { - MemberLock lock; + MemberLock lock; // expected-note{{mutex acquired here}} lock.Lock(); // expected-warning {{acquiring mutex 'lock.mutex' that is already held}} } @@ -2861,7 +2861,7 @@ void doubleLock() EXCLUSIVE_LOCKS_REQUIRED(mu) { MutexUnlock scope(&mu); - scope.Lock(); + scope.Lock(); // expected-note{{mutex acquired here}} scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already held}} } @@ -3164,7 +3164,7 @@ void Foo::test8() { - mu_->Lock(); + mu_->Lock(); // expected-note 2 {{mutex acquired here}} mu_.get()->Lock(); // expected-warning {{acquiring mutex 'mu_' that is already held}} (*mu_).Lock(); // expected-warning {{acquiring mutex 'mu_' that is already held}} mu_.get()->Unlock(); @@ -3298,7 +3298,7 @@ foo.lock(); foo.unlock(); - foo.lock(); + foo.lock(); // expected-note{{mutex acquired here}} foo.lock(); // expected-warning {{acquiring mutex 'foo' that is already held}} foo.unlock(); foo.unlock(); // expected-warning {{releasing mutex 'foo' that was not held}} @@ -3311,7 +3311,7 @@ foo.a = 0; foo.unlock1(); - foo.lock1(); + foo.lock1(); // expected-note{{mutex acquired here}} foo.lock1(); // expected-warning {{acquiring mutex 'foo.mu1_' that is already held}} foo.a = 0; foo.unlock1(); @@ -3325,7 +3325,7 @@ int d1 = foo.a; foo.unlock1(); - foo.slock1(); + foo.slock1(); // expected-note{{mutex acquired here}} foo.slock1(); // expected-warning {{acquiring mutex 'foo.mu1_' that is already held}} int d2 = foo.a; foo.unlock1(); @@ -3342,7 +3342,7 @@ foo.c = 0; foo.unlock3(); - foo.lock3(); + foo.lock3(); // expected-note 3 {{mutex acquired here}} foo.lock3(); // \ // expected-warning {{acquiring mutex 'foo.mu1_' that is already held}} \ // expected-warning {{acquiring mutex 'foo.mu2_' that is already held}} \ @@ -3366,7 +3366,7 @@ foo.c = 0; foo.unlocklots(); - foo.locklots(); + foo.locklots(); // expected-note 3 {{mutex acquired here}} foo.locklots(); // \ // expected-warning {{acquiring mutex 'foo.mu1_' that is already held}} \ // expected-warning {{acquiring mutex 'foo.mu2_' that is already held}} \ @@ -3524,7 +3524,7 @@ LockAllGraphs(); g2.mu_.Unlock(); - LockAllGraphs(); + LockAllGraphs(); // expected-note{{mutex acquired here}} g1.mu_.Lock(); // expected-warning {{acquiring mutex 'g1.mu_' that is already held}} g1.mu_.Unlock(); }