r215677 - Thread safety analysis: add -Wthread-safety-verbose flag, which adds additional notes that are helpful when compiling statistics on thread safety warnings.
David Blaikie
dblaikie at gmail.com
Thu Aug 14 16:47:07 PDT 2014
On Thu, Aug 14, 2014 at 2:40 PM, DeLesley Hutchins <delesley at google.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list