On Thu, Aug 14, 2014 at 2:40 PM, DeLesley Hutchins <[email protected]> wrote: > Author: delesley > Date: Thu Aug 14 16:40:15 2014 > New Revision: 215677 > > URL: http://llvm.org/viewvc/llvm-project?rev=215677&view=rev > Log: > Thread safety analysis: add -Wthread-safety-verbose flag, which adds > additional notes that are helpful when compiling statistics on thread safety > warnings.
Sounds like this could be a remark, rather than a warning... but I don't know too much about that, just that it's a new tool we've got for this sort of "investigating compiler behavior" sort of things :) > > Added: > cfe/trunk/test/SemaCXX/warn-thread-safety-verbose.cpp > Modified: > cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h > cfe/trunk/include/clang/Basic/DiagnosticGroups.td > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/lib/Analysis/ThreadSafety.cpp > cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp > > Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h?rev=215677&r1=215676&r2=215677&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h (original) > +++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h Thu Aug 14 > 16:40:15 2014 > @@ -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; } > > > Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=215677&r1=215676&r2=215677&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Aug 14 16:40:15 2014 > @@ -596,6 +596,7 @@ def ThreadSafety : DiagGroup<"thread-saf > ThreadSafetyAnalysis, > ThreadSafetyPrecise, > ThreadSafetyNegative]>; > +def ThreadSafetyVerbose : DiagGroup<"thread-safety-verbose">; > def ThreadSafetyBeta : DiagGroup<"thread-safety-beta">; > > // Uniqueness Analysis warnings > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=215677&r1=215676&r2=215677&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Aug 14 16:40:15 > 2014 > @@ -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< > > Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=215677&r1=215676&r2=215677&view=diff > ============================================================================== > --- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original) > +++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Aug 14 16:40:15 2014 > @@ -1810,6 +1810,7 @@ void ThreadSafetyAnalyzer::runAnalysis(A > > 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(A > 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(A > LEK_LockedAtEndOfFunction, > LEK_NotLockedAtEndOfFunction, > false); > + > + Handler.leaveFunction(CurrentFunction); > } > > > > Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=215677&r1=215676&r2=215677&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original) > +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Thu Aug 14 16:40:15 2014 > @@ -1451,6 +1451,30 @@ class ThreadSafetyReporter : public clan > 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 clan > 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 clan > 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 clan > 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 clan > 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 clan > << 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 clan > 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 clan > << 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 clan > 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 clan > 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 clan > 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(sem > 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(); > > Added: cfe/trunk/test/SemaCXX/warn-thread-safety-verbose.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-verbose.cpp?rev=215677&view=auto > ============================================================================== > --- cfe/trunk/test/SemaCXX/warn-thread-safety-verbose.cpp (added) > +++ cfe/trunk/test/SemaCXX/warn-thread-safety-verbose.cpp Thu Aug 14 16:40:15 > 2014 > @@ -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(); > + } > +}; > + > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
