r215677 - Thread safety analysis: add -Wthread-safety-verbose flag, which adds additional notes that are helpful when compiling statistics on thread safety warnings.
DeLesley Hutchins
delesley at google.com
Thu Aug 14 14:40:15 PDT 2014
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.
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();
+ }
+};
+
More information about the cfe-commits
mailing list