[cfe-commits] r163397 - in /cfe/trunk: lib/Analysis/ThreadSafety.cpp lib/Sema/SemaDeclAttr.cpp test/SemaCXX/warn-thread-safety-analysis.cpp
DeLesley Hutchins
delesley at google.com
Fri Sep 7 10:34:53 PDT 2012
Author: delesley
Date: Fri Sep 7 12:34:53 2012
New Revision: 163397
URL: http://llvm.org/viewvc/llvm-project?rev=163397&view=rev
Log:
Thread-safety analysis: Add support for selectively turning off warnings
within part of a particular method.
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=163397&r1=163396&r2=163397&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Fri Sep 7 12:34:53 2012
@@ -70,18 +70,19 @@
class SExpr {
private:
enum ExprOp {
- EOP_Nop, ///< No-op
- EOP_Wildcard, ///< Matches anything.
- EOP_This, ///< This keyword.
- EOP_NVar, ///< Named variable.
- EOP_LVar, ///< Local variable.
- EOP_Dot, ///< Field access
- EOP_Call, ///< Function call
- EOP_MCall, ///< Method call
- EOP_Index, ///< Array index
- EOP_Unary, ///< Unary operation
- EOP_Binary, ///< Binary operation
- EOP_Unknown ///< Catchall for everything else
+ EOP_Nop, ///< No-op
+ EOP_Wildcard, ///< Matches anything.
+ EOP_Universal, ///< Universal lock.
+ EOP_This, ///< This keyword.
+ EOP_NVar, ///< Named variable.
+ EOP_LVar, ///< Local variable.
+ EOP_Dot, ///< Field access
+ EOP_Call, ///< Function call
+ EOP_MCall, ///< Method call
+ EOP_Index, ///< Array index
+ EOP_Unary, ///< Unary operation
+ EOP_Binary, ///< Binary operation
+ EOP_Unknown ///< Catchall for everything else
};
@@ -118,18 +119,19 @@
unsigned arity() const {
switch (Op) {
- case EOP_Nop: return 0;
- case EOP_Wildcard: return 0;
- case EOP_NVar: return 0;
- case EOP_LVar: return 0;
- case EOP_This: return 0;
- case EOP_Dot: return 1;
- case EOP_Call: return Flags+1; // First arg is function.
- case EOP_MCall: return Flags+1; // First arg is implicit obj.
- case EOP_Index: return 2;
- case EOP_Unary: return 1;
- case EOP_Binary: return 2;
- case EOP_Unknown: return Flags;
+ case EOP_Nop: return 0;
+ case EOP_Wildcard: return 0;
+ case EOP_Universal: return 0;
+ case EOP_NVar: return 0;
+ case EOP_LVar: return 0;
+ case EOP_This: return 0;
+ case EOP_Dot: return 1;
+ case EOP_Call: return Flags+1; // First arg is function.
+ case EOP_MCall: return Flags+1; // First arg is implicit obj.
+ case EOP_Index: return 2;
+ case EOP_Unary: return 1;
+ case EOP_Binary: return 2;
+ case EOP_Unknown: return Flags;
}
return 0;
}
@@ -194,6 +196,11 @@
return NodeVec.size()-1;
}
+ unsigned makeUniversal() {
+ NodeVec.push_back(SExprNode(EOP_Universal, 0, 0));
+ return NodeVec.size()-1;
+ }
+
unsigned makeNamedVar(const NamedDecl *D) {
NodeVec.push_back(SExprNode(EOP_NVar, 0, D));
return NodeVec.size()-1;
@@ -447,10 +454,18 @@
void buildSExprFromExpr(Expr *MutexExp, Expr *DeclExp, const NamedDecl *D) {
CallingContext CallCtx(D);
- // Ignore string literals
- if (MutexExp && isa<StringLiteral>(MutexExp)) {
- makeNop();
- return;
+
+ if (MutexExp) {
+ if (StringLiteral* SLit = dyn_cast<StringLiteral>(MutexExp)) {
+ if (SLit->getString() == StringRef("*"))
+ // The "*" expr is a universal lock, which essentially turns off
+ // checks until it is removed from the lockset.
+ makeUniversal();
+ else
+ // Ignore other string literals for now.
+ makeNop();
+ return;
+ }
}
// If we are processing a raw attribute expression, with no substitutions.
@@ -520,6 +535,11 @@
return NodeVec[0].kind() == EOP_Nop;
}
+ bool isUniversal() const {
+ assert(NodeVec.size() > 0 && "Invalid Mutex");
+ return NodeVec[0].kind() == EOP_Universal;
+ }
+
/// Issue a warning about an invalid lock expression
static void warnInvalidLock(ThreadSafetyHandler &Handler, Expr* MutexExp,
Expr *DeclExp, const NamedDecl* D) {
@@ -567,6 +587,8 @@
return "_";
case EOP_Wildcard:
return "(?)";
+ case EOP_Universal:
+ return "*";
case EOP_This:
return "this";
case EOP_NVar:
@@ -709,6 +731,10 @@
ID.AddInteger(AcquireLoc.getRawEncoding());
ID.AddInteger(LKind);
}
+
+ bool isAtLeast(LockKind LK) {
+ return (LK == LK_Shared) || (LKind == LK_Exclusive);
+ }
};
@@ -796,7 +822,16 @@
LockData* findLock(FactManager& FM, const SExpr& M) const {
for (const_iterator I=begin(), E=end(); I != E; ++I) {
- if (FM[*I].MutID.matches(M)) return &FM[*I].LDat;
+ const SExpr& E = FM[*I].MutID;
+ if (E.matches(M)) return &FM[*I].LDat;
+ }
+ return 0;
+ }
+
+ LockData* findLockUniv(FactManager& FM, const SExpr& M) const {
+ for (const_iterator I=begin(), E=end(); I != E; ++I) {
+ const SExpr& E = FM[*I].MutID;
+ if (E.matches(M) || E.isUniversal()) return &FM[*I].LDat;
}
return 0;
}
@@ -1654,39 +1689,12 @@
void warnIfMutexNotHeld(const NamedDecl *D, Expr *Exp, AccessKind AK,
Expr *MutexExp, ProtectedOperationKind POK);
+ void warnIfMutexHeld(const NamedDecl *D, Expr *Exp, Expr *MutexExp);
void checkAccess(Expr *Exp, AccessKind AK);
void checkDereference(Expr *Exp, AccessKind AK);
void handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD = 0);
- /// \brief Returns true if the lockset contains a lock, regardless of whether
- /// the lock is held exclusively or shared.
- bool locksetContains(const SExpr &Mu) const {
- return FSet.findLock(Analyzer->FactMan, Mu);
- }
-
- /// \brief Returns true if the lockset contains a lock with the passed in
- /// locktype.
- bool locksetContains(const SExpr &Mu, LockKind KindRequested) const {
- const LockData *LockHeld = FSet.findLock(Analyzer->FactMan, Mu);
- return (LockHeld && KindRequested == LockHeld->LKind);
- }
-
- /// \brief Returns true if the lockset contains a lock with at least the
- /// passed in locktype. So for example, if we pass in LK_Shared, this function
- /// returns true if the lock is held LK_Shared or LK_Exclusive. If we pass in
- /// LK_Exclusive, this function returns true if the lock is held LK_Exclusive.
- bool locksetContainsAtLeast(const SExpr &Lock,
- LockKind KindRequested) const {
- switch (KindRequested) {
- case LK_Shared:
- return locksetContains(Lock);
- case LK_Exclusive:
- return locksetContains(Lock, KindRequested);
- }
- llvm_unreachable("Unknown LockKind");
- }
-
public:
BuildLockset(ThreadSafetyAnalyzer *Anlzr, CFGBlockInfo &Info)
: StmtVisitor<BuildLockset>(),
@@ -1724,15 +1732,35 @@
LockKind LK = getLockKindFromAccessKind(AK);
SExpr Mutex(MutexExp, Exp, D);
- if (!Mutex.isValid())
+ if (!Mutex.isValid()) {
SExpr::warnInvalidLock(Analyzer->Handler, MutexExp, Exp, D);
- else if (Mutex.shouldIgnore())
- return; // A Nop is an invalid mutex that we've decided to ignore.
- else if (!locksetContainsAtLeast(Mutex, LK))
+ return;
+ } else if (Mutex.shouldIgnore()) {
+ return;
+ }
+
+ LockData* LDat = FSet.findLockUniv(Analyzer->FactMan, Mutex);
+ if (!LDat || !LDat->isAtLeast(LK))
Analyzer->Handler.handleMutexNotHeld(D, POK, Mutex.toString(), LK,
Exp->getExprLoc());
}
+/// \brief Warn if the LSet contains the given lock.
+void BuildLockset::warnIfMutexHeld(const NamedDecl *D, Expr* Exp,
+ Expr *MutexExp) {
+ SExpr Mutex(MutexExp, Exp, D);
+ if (!Mutex.isValid()) {
+ SExpr::warnInvalidLock(Analyzer->Handler, MutexExp, Exp, D);
+ return;
+ }
+
+ LockData* LDat = FSet.findLock(Analyzer->FactMan, Mutex);
+ if (LDat)
+ Analyzer->Handler.handleFunExcludesLock(D->getName(), Mutex.toString(),
+ Exp->getExprLoc());
+}
+
+
/// \brief This method identifies variable dereferences and checks pt_guarded_by
/// and pt_guarded_var annotations. Note that we only check these annotations
/// at the time a pointer is dereferenced.
@@ -1841,15 +1869,10 @@
case attr::LocksExcluded: {
LocksExcludedAttr *A = cast<LocksExcludedAttr>(At);
+
for (LocksExcludedAttr::args_iterator I = A->args_begin(),
E = A->args_end(); I != E; ++I) {
- SExpr Mutex(*I, Exp, D);
- if (!Mutex.isValid())
- SExpr::warnInvalidLock(Analyzer->Handler, *I, Exp, D);
- else if (locksetContains(Mutex))
- Analyzer->Handler.handleFunExcludesLock(D->getName(),
- Mutex.toString(),
- Exp->getExprLoc());
+ warnIfMutexHeld(D, Exp, *I);
}
break;
}
@@ -2037,7 +2060,7 @@
JoinLoc, LEK1);
}
}
- else if (!LDat2.Managed)
+ else if (!LDat2.Managed && !FSet2Mutex.isUniversal())
Handler.handleMutexHeldEndOfScope(FSet2Mutex.toString(),
LDat2.AcquireLoc,
JoinLoc, LEK1);
@@ -2060,7 +2083,7 @@
JoinLoc, LEK1);
}
}
- else if (!LDat1.Managed)
+ else if (!LDat1.Managed && !FSet1Mutex.isUniversal())
Handler.handleMutexHeldEndOfScope(FSet1Mutex.toString(),
LDat1.AcquireLoc,
JoinLoc, LEK2);
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=163397&r1=163396&r2=163397&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Sep 7 12:34:53 2012
@@ -415,8 +415,10 @@
}
if (StringLiteral *StrLit = dyn_cast<StringLiteral>(ArgExp)) {
- if (StrLit->getLength() == 0) {
+ if (StrLit->getLength() == 0 ||
+ StrLit->getString() == StringRef("*")) {
// Pass empty strings to the analyzer without warnings.
+ // Treat "*" as the universal lock.
Args.push_back(ArgExp);
continue;
}
Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=163397&r1=163396&r2=163397&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Fri Sep 7 12:34:53 2012
@@ -24,10 +24,6 @@
__attribute__ ((shared_locks_required(__VA_ARGS__)))
#define NO_THREAD_SAFETY_ANALYSIS __attribute__ ((no_thread_safety_analysis))
-//-----------------------------------------//
-// Helper fields
-//-----------------------------------------//
-
class __attribute__((lockable)) Mutex {
public:
@@ -60,6 +56,14 @@
};
+// The universal lock, written "*", allows checking to be selectively turned
+// off for a particular piece of code.
+void beginNoWarnOnReads() SHARED_LOCK_FUNCTION("*");
+void endNoWarnOnReads() UNLOCK_FUNCTION("*");
+void beginNoWarnOnWrites() EXCLUSIVE_LOCK_FUNCTION("*");
+void endNoWarnOnWrites() UNLOCK_FUNCTION("*");
+
+
template<class T>
class SmartPtr {
public:
@@ -3217,3 +3221,79 @@
}
+namespace UniversalLock {
+
+class Foo {
+ Mutex mu_;
+ bool c;
+
+ int a GUARDED_BY(mu_);
+ void r_foo() SHARED_LOCKS_REQUIRED(mu_);
+ void w_foo() EXCLUSIVE_LOCKS_REQUIRED(mu_);
+
+ void test1() {
+ int b;
+
+ beginNoWarnOnReads();
+ b = a;
+ r_foo();
+ endNoWarnOnReads();
+
+ beginNoWarnOnWrites();
+ a = 0;
+ w_foo();
+ endNoWarnOnWrites();
+ }
+
+ // don't warn on joins with universal lock
+ void test2() {
+ if (c) {
+ beginNoWarnOnWrites();
+ }
+ a = 0; // \
+ // expected-warning {{writing variable 'a' requires locking 'mu_' exclusively}}
+ endNoWarnOnWrites(); // \
+ // expected-warning {{unlocking '*' that was not locked}}
+ }
+
+
+ // make sure the universal lock joins properly
+ void test3() {
+ if (c) {
+ mu_.Lock();
+ beginNoWarnOnWrites();
+ }
+ else {
+ beginNoWarnOnWrites();
+ mu_.Lock();
+ }
+ a = 0;
+ endNoWarnOnWrites();
+ mu_.Unlock();
+ }
+
+
+ // combine universal lock with other locks
+ void test4() {
+ beginNoWarnOnWrites();
+ mu_.Lock();
+ mu_.Unlock();
+ endNoWarnOnWrites();
+
+ mu_.Lock();
+ beginNoWarnOnWrites();
+ endNoWarnOnWrites();
+ mu_.Unlock();
+
+ mu_.Lock();
+ beginNoWarnOnWrites();
+ mu_.Unlock();
+ endNoWarnOnWrites();
+ }
+};
+
+}
+
+
+
+
More information about the cfe-commits
mailing list