diff --git a/include/clang/Analysis/Analyses/ThreadSafety.h b/include/clang/Analysis/Analyses/ThreadSafety.h index 08ba97545391f5d20a6cafaf9ff73a549683a7bd..38600912dae8d15728fc52ddd032197237da3369 100644 --- a/include/clang/Analysis/Analyses/ThreadSafety.h +++ b/include/clang/Analysis/Analyses/ThreadSafety.h @@ -181,6 +181,16 @@ public: virtual void handleFunExcludesLock(StringRef Kind, Name FunName, Name LockName, SourceLocation Loc) {} + /// Called by the analysis when starting analysis of a function. + /// Used to issue suggestions for changes to annotations. + virtual void enterFunction(const FunctionDecl *FD) {} + + /// Called by the analysis when finishing analysis of a function. + virtual void leaveFunction(const FunctionDecl *FD) {} + + /// Return the number of errors found within this function so far. + virtual int numErrors() { return 0; } + bool issueBetaWarnings() { return IssueBetaWarnings; } void setIssueBetaWarnings(bool b) { IssueBetaWarnings = b; } diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index c737270cae3f7a655df700fdbd1eeacefdd117a3..13981a545dadbc8404a4e64cf1a6bcc775331136 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -596,6 +596,7 @@ def ThreadSafety : DiagGroup<"thread-safety", ThreadSafetyAnalysis, ThreadSafetyPrecise, ThreadSafetyNegative]>; +def ThreadSafetyVerbose : DiagGroup<"thread-safety-verbose">; def ThreadSafetyBeta : DiagGroup<"thread-safety-beta">; // Uniqueness Analysis warnings diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index ca37b076fe56c3a4c54190d9ab75bdbb850e1316..a1b2bc6389c79c7401fe77c8b44355f66fc6ed1d 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2349,9 +2349,15 @@ def warn_fun_requires_lock_precise : InGroup<ThreadSafetyPrecise>, DefaultIgnore; def note_found_mutex_near_match : Note<"found near match '%0'">; +// Verbose thread safety warnings +def warn_thread_safety_verbose : Warning<"Thread safety verbose warning.">, + InGroup<ThreadSafetyVerbose>, DefaultIgnore; +def note_thread_warning_in_fun : Note<"Thread warning in function '%0'">; +def note_guarded_by_declared_here : Note<"Guarded_by declared here.">; + // Dummy warning that will trigger "beta" warnings from the analysis if enabled. -def warn_thread_safety_beta : Warning< - "Thread safety beta warning.">, InGroup<ThreadSafetyBeta>, DefaultIgnore; +def warn_thread_safety_beta : Warning<"Thread safety beta warning.">, + InGroup<ThreadSafetyBeta>, DefaultIgnore; // Consumed warnings def warn_use_in_invalid_state : Warning< diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp index c49e6e7049bf8c7646bc71a5b81a4fd82fd3fe48..469e79be7242f494e9a6c3d8167c2bb04bc0e8a7 100644 --- a/lib/Analysis/ThreadSafety.cpp +++ b/lib/Analysis/ThreadSafety.cpp @@ -1810,6 +1810,7 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { CFG *CFGraph = walker.getGraph(); const NamedDecl *D = walker.getDecl(); + const FunctionDecl *CurrentFunction = dyn_cast<FunctionDecl>(D); CurrentMethod = dyn_cast<CXXMethodDecl>(D); if (D->hasAttr<NoThreadSafetyAnalysisAttr>()) @@ -1824,6 +1825,8 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { if (isa<CXXDestructorDecl>(D)) return; // Don't check inside destructors. + Handler.enterFunction(CurrentFunction); + BlockInfo.resize(CFGraph->getNumBlockIDs(), CFGBlockInfo::getEmptyBlockInfo(LocalVarMap)); @@ -2079,6 +2082,8 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { LEK_LockedAtEndOfFunction, LEK_NotLockedAtEndOfFunction, false); + + Handler.leaveFunction(CurrentFunction); } diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp index 59d13de8309aefbc8e1eae2f44153c81cd387d08..93ee16b4c52df428c4f78279d7e56b76c2a0e7cf 100644 --- a/lib/Sema/AnalysisBasedWarnings.cpp +++ b/lib/Sema/AnalysisBasedWarnings.cpp @@ -1451,6 +1451,30 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { DiagList Warnings; SourceLocation FunLocation, FunEndLocation; + const FunctionDecl *CurrentFunction; + bool Verbose; + + OptionalNotes getNotes() { + if (Verbose && CurrentFunction) { + PartialDiagnosticAt FNote(CurrentFunction->getBody()->getLocStart(), + S.PDiag(diag::note_thread_warning_in_fun) + << CurrentFunction->getNameAsString()); + return OptionalNotes(1, FNote); + } + else return OptionalNotes(); + } + + OptionalNotes getNotes(const PartialDiagnosticAt &Note) { + OptionalNotes ONS(1, Note); + if (Verbose && CurrentFunction) { + PartialDiagnosticAt FNote(CurrentFunction->getBody()->getLocStart(), + S.PDiag(diag::note_thread_warning_in_fun) + << CurrentFunction->getNameAsString()); + ONS.push_back(FNote); + } + return ONS; + } + // Helper functions void warnLockMismatch(unsigned DiagID, StringRef Kind, Name LockName, SourceLocation Loc) { @@ -1459,12 +1483,15 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { if (!Loc.isValid()) Loc = FunLocation; PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) << Kind << LockName); - Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); + Warnings.push_back(DelayedDiag(Warning, getNotes())); } public: ThreadSafetyReporter(Sema &S, SourceLocation FL, SourceLocation FEL) - : S(S), FunLocation(FL), FunEndLocation(FEL) {} + : S(S), FunLocation(FL), FunEndLocation(FEL), + CurrentFunction(nullptr), Verbose(false) {} + + void setVerbose(bool b) { Verbose = b; } /// \brief Emit all buffered diagnostics in order of sourcelocation. /// We need to output diagnostics produced while iterating through @@ -1482,12 +1509,14 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { void handleInvalidLockExp(StringRef Kind, SourceLocation Loc) override { PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_cannot_resolve_lock) << Loc); - Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); + Warnings.push_back(DelayedDiag(Warning, getNotes())); } + void handleUnmatchedUnlock(StringRef Kind, Name LockName, SourceLocation Loc) override { warnLockMismatch(diag::warn_unlock_but_no_lock, Kind, LockName, Loc); } + void handleIncorrectUnlockKind(StringRef Kind, Name LockName, LockKind Expected, LockKind Received, SourceLocation Loc) override { @@ -1496,8 +1525,9 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_unlock_kind_mismatch) << Kind << LockName << Received << Expected); - Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); + Warnings.push_back(DelayedDiag(Warning, getNotes())); } + void handleDoubleLock(StringRef Kind, Name LockName, SourceLocation Loc) override { warnLockMismatch(diag::warn_double_lock, Kind, LockName, Loc); } @@ -1529,10 +1559,10 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { if (LocLocked.isValid()) { PartialDiagnosticAt Note(LocLocked, S.PDiag(diag::note_locked_here) << Kind); - Warnings.push_back(DelayedDiag(Warning, OptionalNotes(1, Note))); + Warnings.push_back(DelayedDiag(Warning, getNotes(Note))); return; } - Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); + Warnings.push_back(DelayedDiag(Warning, getNotes())); } void handleExclusiveAndShared(StringRef Kind, Name LockName, @@ -1543,7 +1573,7 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { << Kind << LockName); PartialDiagnosticAt Note(Loc2, S.PDiag(diag::note_lock_exclusive_and_shared) << Kind << LockName); - Warnings.push_back(DelayedDiag(Warning, OptionalNotes(1, Note))); + Warnings.push_back(DelayedDiag(Warning, getNotes(Note))); } void handleNoMutexHeld(StringRef Kind, const NamedDecl *D, @@ -1556,7 +1586,7 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { diag::warn_var_deref_requires_any_lock; PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) << D->getNameAsString() << getLockKindFromAccessKind(AK)); - Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); + Warnings.push_back(DelayedDiag(Warning, getNotes())); } void handleMutexNotHeld(StringRef Kind, const NamedDecl *D, @@ -1581,7 +1611,7 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { << LockName << LK); PartialDiagnosticAt Note(Loc, S.PDiag(diag::note_found_mutex_near_match) << *PossibleMatch); - Warnings.push_back(DelayedDiag(Warning, OptionalNotes(1, Note))); + Warnings.push_back(DelayedDiag(Warning, getNotes(Note))); } else { switch (POK) { case POK_VarAccess: @@ -1597,7 +1627,15 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) << Kind << D->getNameAsString() << LockName << LK); - Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); + if (Verbose && POK == POK_VarAccess) { + PartialDiagnosticAt Note(D->getLocation(), + S.PDiag(diag::note_guarded_by_declared_here) << + D->getNameAsString()); + Warnings.push_back(DelayedDiag(Warning, getNotes(Note))); + } + else { + Warnings.push_back(DelayedDiag(Warning, getNotes())); + } } } @@ -1606,7 +1644,7 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_acquire_requires_negative_cap) << Kind << LockName << Neg); - Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); + Warnings.push_back(DelayedDiag(Warning, getNotes())); } @@ -1614,7 +1652,15 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { SourceLocation Loc) override { PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_fun_excludes_mutex) << Kind << FunName << LockName); - Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); + Warnings.push_back(DelayedDiag(Warning, getNotes())); + } + + void enterFunction(const FunctionDecl* FD) override { + CurrentFunction = FD; + } + + void leaveFunction(const FunctionDecl* FD) override { + CurrentFunction = 0; } }; @@ -1908,6 +1954,8 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P, threadSafety::ThreadSafetyReporter Reporter(S, FL, FEL); if (!Diags.isIgnored(diag::warn_thread_safety_beta, D->getLocStart())) Reporter.setIssueBetaWarnings(true); + if (!Diags.isIgnored(diag::warn_thread_safety_verbose, D->getLocStart())) + Reporter.setVerbose(true); threadSafety::runThreadSafetyAnalysis(AC, Reporter); Reporter.emitDiagnostics(); diff --git a/test/SemaCXX/warn-thread-safety-verbose.cpp b/test/SemaCXX/warn-thread-safety-verbose.cpp new file mode 100644 index 0000000000000000000000000000000000000000..4e19f30bf2c3efad880d089a2c96f887f1ca476f --- /dev/null +++ b/test/SemaCXX/warn-thread-safety-verbose.cpp @@ -0,0 +1,86 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wthread-safety-verbose -Wno-thread-safety-negative -fcxx-exceptions %s + +#define LOCKABLE __attribute__ ((lockable)) +#define SCOPED_LOCKABLE __attribute__ ((scoped_lockable)) +#define GUARDED_BY(x) __attribute__ ((guarded_by(x))) +#define GUARDED_VAR __attribute__ ((guarded_var)) +#define PT_GUARDED_BY(x) __attribute__ ((pt_guarded_by(x))) +#define PT_GUARDED_VAR __attribute__ ((pt_guarded_var)) +#define ACQUIRED_AFTER(...) __attribute__ ((acquired_after(__VA_ARGS__))) +#define ACQUIRED_BEFORE(...) __attribute__ ((acquired_before(__VA_ARGS__))) +#define EXCLUSIVE_LOCK_FUNCTION(...) __attribute__ ((exclusive_lock_function(__VA_ARGS__))) +#define SHARED_LOCK_FUNCTION(...) __attribute__ ((shared_lock_function(__VA_ARGS__))) +#define ASSERT_EXCLUSIVE_LOCK(...) __attribute__ ((assert_exclusive_lock(__VA_ARGS__))) +#define ASSERT_SHARED_LOCK(...) __attribute__ ((assert_shared_lock(__VA_ARGS__))) +#define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__ ((exclusive_trylock_function(__VA_ARGS__))) +#define SHARED_TRYLOCK_FUNCTION(...) __attribute__ ((shared_trylock_function(__VA_ARGS__))) +#define UNLOCK_FUNCTION(...) __attribute__ ((unlock_function(__VA_ARGS__))) +#define LOCK_RETURNED(x) __attribute__ ((lock_returned(x))) +#define LOCKS_EXCLUDED(...) __attribute__ ((locks_excluded(__VA_ARGS__))) +#define EXCLUSIVE_LOCKS_REQUIRED(...) \ + __attribute__ ((exclusive_locks_required(__VA_ARGS__))) +#define SHARED_LOCKS_REQUIRED(...) \ + __attribute__ ((shared_locks_required(__VA_ARGS__))) +#define NO_THREAD_SAFETY_ANALYSIS __attribute__ ((no_thread_safety_analysis)) + + +class __attribute__((lockable)) Mutex { + public: + void Lock() __attribute__((exclusive_lock_function)); + void ReaderLock() __attribute__((shared_lock_function)); + void Unlock() __attribute__((unlock_function)); + bool TryLock() __attribute__((exclusive_trylock_function(true))); + bool ReaderTryLock() __attribute__((shared_trylock_function(true))); + void LockWhen(const int &cond) __attribute__((exclusive_lock_function)); + + // for negative capabilities + const Mutex& operator!() const { return *this; } + + void AssertHeld() ASSERT_EXCLUSIVE_LOCK(); + void AssertReaderHeld() ASSERT_SHARED_LOCK(); +}; + + +class Test { + Mutex mu; + int a GUARDED_BY(mu); // expected-note3 {{Guarded_by declared here.}} + + void foo1() EXCLUSIVE_LOCKS_REQUIRED(mu); + void foo2() SHARED_LOCKS_REQUIRED(mu); + void foo3() LOCKS_EXCLUDED(mu); + + void test1() { // expected-note {{Thread warning in function 'test1'}} + a = 0; // expected-warning {{writing variable 'a' requires holding mutex 'mu' exclusively}} + } + + void test2() { // expected-note {{Thread warning in function 'test2'}} + int b = a; // expected-warning {{reading variable 'a' requires holding mutex 'mu'}} + } + + void test3() { // expected-note {{Thread warning in function 'test3'}} + foo1(); // expected-warning {{calling function 'foo1' requires holding mutex 'mu' exclusively}} + } + + void test4() { // expected-note {{Thread warning in function 'test4'}} + foo2(); // expected-warning {{calling function 'foo2' requires holding mutex 'mu'}} + } + + void test5() { // expected-note {{Thread warning in function 'test5'}} + mu.ReaderLock(); + foo1(); // expected-warning {{calling function 'foo1' requires holding mutex 'mu' exclusively}} + mu.Unlock(); + } + + void test6() { // expected-note {{Thread warning in function 'test6'}} + mu.ReaderLock(); + a = 0; // expected-warning {{writing variable 'a' requires holding mutex 'mu' exclusively}} + mu.Unlock(); + } + + void test7() { // expected-note {{Thread warning in function 'test7'}} + mu.Lock(); + foo3(); // expected-warning {{cannot call function 'foo3' while mutex 'mu' is held}} + mu.Unlock(); + } +}; +