Thread-safety analysis: Add support for selectively turning off warnings

within part of a particular method.

llvm-svn: 163397
This commit is contained in:
DeLesley Hutchins 2012-09-07 17:34:53 +00:00
parent e45e22b20f
commit a5a00e830a
3 changed files with 180 additions and 75 deletions

View File

@ -70,18 +70,19 @@ namespace {
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 @@ private:
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 @@ private:
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 @@ private:
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 @@ public:
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 @@ public:
return "_";
case EOP_Wildcard:
return "(?)";
case EOP_Universal:
return "*";
case EOP_This:
return "this";
case EOP_NVar:
@ -709,6 +731,10 @@ struct LockData {
ID.AddInteger(AcquireLoc.getRawEncoding());
ID.AddInteger(LKind);
}
bool isAtLeast(LockKind LK) {
return (LK == LK_Shared) || (LKind == LK_Exclusive);
}
};
@ -796,7 +822,16 @@ public:
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 @@ class BuildLockset : public StmtVisitor<BuildLockset> {
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 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, Expr *Exp,
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 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
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 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1,
JoinLoc, LEK1);
}
}
else if (!LDat2.Managed)
else if (!LDat2.Managed && !FSet2Mutex.isUniversal())
Handler.handleMutexHeldEndOfScope(FSet2Mutex.toString(),
LDat2.AcquireLoc,
JoinLoc, LEK1);
@ -2060,7 +2083,7 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1,
JoinLoc, LEK1);
}
}
else if (!LDat1.Managed)
else if (!LDat1.Managed && !FSet1Mutex.isUniversal())
Handler.handleMutexHeldEndOfScope(FSet1Mutex.toString(),
LDat1.AcquireLoc,
JoinLoc, LEK2);

View File

@ -415,8 +415,10 @@ static void checkAttrArgsAreLockableObjs(Sema &S, Decl *D,
}
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;
}

View File

@ -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 @@ class SCOPED_LOCKABLE ReleasableMutexLock {
};
// 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 @@ static void test() {
}
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();
}
};
}