[cfe-commits] r159152 - in /cfe/trunk: lib/Analysis/ThreadSafety.cpp test/SemaCXX/warn-thread-safety-analysis.cpp
DeLesley Hutchins
delesley at google.com
Mon Jun 25 11:33:19 PDT 2012
Author: delesley
Date: Mon Jun 25 13:33:18 2012
New Revision: 159152
URL: http://llvm.org/viewvc/llvm-project?rev=159152&view=rev
Log:
Thread safety analysis: implement lock_returned attribute.
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.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=159152&r1=159151&r2=159152&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Mon Jun 25 13:33:18 2012
@@ -84,12 +84,33 @@
class MutexID {
SmallVector<NamedDecl*, 2> DeclSeq;
+ /// \brief Encapsulates the lexical context of a function call. The lexical
+ /// context includes the arguments to the call, including the implicit object
+ /// argument. When an attribute containing a mutex expression is attached to
+ /// a method, the expression may refer to formal parameters of the method.
+ /// Actual arguments must be substituted for formal parameters to derive
+ /// the appropriate mutex expression in the lexical context where the function
+ /// is called. PrevCtx holds the context in which the arguments themselves
+ /// should be evaluated; multiple calling contexts can be chained together
+ /// by the lock_returned attribute.
+ struct CallingContext {
+ const NamedDecl* AttrDecl; // The decl to which the attribute is attached.
+ Expr* SelfArg; // Implicit object argument -- e.g. 'this'
+ unsigned NumArgs; // Number of funArgs
+ Expr** FunArgs; // Function arguments
+ CallingContext* PrevCtx; // The previous context; or 0 if none.
+
+ CallingContext(const NamedDecl* D = 0, Expr* S = 0,
+ unsigned N = 0, Expr** A = 0, CallingContext* P = 0)
+ : AttrDecl(D), SelfArg(S), NumArgs(N), FunArgs(A), PrevCtx(P)
+ { }
+ };
+
/// Build a Decl sequence representing the lock from the given expression.
/// Recursive function that terminates on DeclRefExpr.
/// Note: this function merely creates a MutexID; it does not check to
/// ensure that the original expression is a valid mutex expression.
- void buildMutexID(Expr *Exp, const NamedDecl *D, Expr *Parent,
- unsigned NumArgs, Expr **FunArgs) {
+ void buildMutexID(Expr *Exp, CallingContext* CallCtx) {
if (!Exp) {
DeclSeq.clear();
return;
@@ -103,10 +124,11 @@
cast<FunctionDecl>(PV->getDeclContext())->getCanonicalDecl();
unsigned i = PV->getFunctionScopeIndex();
- if (FunArgs && FD == D->getCanonicalDecl()) {
+ if (CallCtx && CallCtx->FunArgs &&
+ FD == CallCtx->AttrDecl->getCanonicalDecl()) {
// Substitute call arguments for references to function parameters
- assert(i < NumArgs);
- buildMutexID(FunArgs[i], D, 0, 0, 0);
+ assert(i < CallCtx->NumArgs);
+ buildMutexID(CallCtx->FunArgs[i], CallCtx->PrevCtx);
return;
}
// Map the param back to the param of the original function declaration.
@@ -115,54 +137,75 @@
}
// Not a function parameter -- just store the reference.
DeclSeq.push_back(ND);
- } else if (MemberExpr *ME = dyn_cast<MemberExpr>(Exp)) {
- NamedDecl *ND = ME->getMemberDecl();
- DeclSeq.push_back(ND);
- buildMutexID(ME->getBase(), D, Parent, NumArgs, FunArgs);
} else if (isa<CXXThisExpr>(Exp)) {
- if (Parent)
- buildMutexID(Parent, D, 0, 0, 0);
+ // Substitute parent for 'this'
+ if (CallCtx && CallCtx->SelfArg)
+ buildMutexID(CallCtx->SelfArg, CallCtx->PrevCtx);
else {
DeclSeq.push_back(0); // Use 0 to represent 'this'.
return; // mutexID is still valid in this case
}
+ } else if (MemberExpr *ME = dyn_cast<MemberExpr>(Exp)) {
+ NamedDecl *ND = ME->getMemberDecl();
+ DeclSeq.push_back(ND);
+ buildMutexID(ME->getBase(), CallCtx);
} else if (CXXMemberCallExpr *CMCE = dyn_cast<CXXMemberCallExpr>(Exp)) {
+ // When calling a function with a lock_returned attribute, replace
+ // the function call with the expression in lock_returned.
+ if (LockReturnedAttr* At =
+ CMCE->getMethodDecl()->getAttr<LockReturnedAttr>()) {
+ CallingContext LRCallCtx(CMCE->getMethodDecl());
+ LRCallCtx.SelfArg = CMCE->getImplicitObjectArgument();
+ LRCallCtx.NumArgs = CMCE->getNumArgs();
+ LRCallCtx.FunArgs = CMCE->getArgs();
+ LRCallCtx.PrevCtx = CallCtx;
+ buildMutexID(At->getArg(), &LRCallCtx);
+ return;
+ }
DeclSeq.push_back(CMCE->getMethodDecl()->getCanonicalDecl());
- buildMutexID(CMCE->getImplicitObjectArgument(),
- D, Parent, NumArgs, FunArgs);
+ buildMutexID(CMCE->getImplicitObjectArgument(), CallCtx);
unsigned NumCallArgs = CMCE->getNumArgs();
Expr** CallArgs = CMCE->getArgs();
for (unsigned i = 0; i < NumCallArgs; ++i) {
- buildMutexID(CallArgs[i], D, Parent, NumArgs, FunArgs);
+ buildMutexID(CallArgs[i], CallCtx);
}
} else if (CallExpr *CE = dyn_cast<CallExpr>(Exp)) {
- buildMutexID(CE->getCallee(), D, Parent, NumArgs, FunArgs);
+ if (LockReturnedAttr* At =
+ CE->getDirectCallee()->getAttr<LockReturnedAttr>()) {
+ CallingContext LRCallCtx(CE->getDirectCallee());
+ LRCallCtx.NumArgs = CE->getNumArgs();
+ LRCallCtx.FunArgs = CE->getArgs();
+ LRCallCtx.PrevCtx = CallCtx;
+ buildMutexID(At->getArg(), &LRCallCtx);
+ return;
+ }
+ buildMutexID(CE->getCallee(), CallCtx);
unsigned NumCallArgs = CE->getNumArgs();
Expr** CallArgs = CE->getArgs();
for (unsigned i = 0; i < NumCallArgs; ++i) {
- buildMutexID(CallArgs[i], D, Parent, NumArgs, FunArgs);
+ buildMutexID(CallArgs[i], CallCtx);
}
} else if (BinaryOperator *BOE = dyn_cast<BinaryOperator>(Exp)) {
- buildMutexID(BOE->getLHS(), D, Parent, NumArgs, FunArgs);
- buildMutexID(BOE->getRHS(), D, Parent, NumArgs, FunArgs);
+ buildMutexID(BOE->getLHS(), CallCtx);
+ buildMutexID(BOE->getRHS(), CallCtx);
} else if (UnaryOperator *UOE = dyn_cast<UnaryOperator>(Exp)) {
- buildMutexID(UOE->getSubExpr(), D, Parent, NumArgs, FunArgs);
+ buildMutexID(UOE->getSubExpr(), CallCtx);
} else if (ArraySubscriptExpr *ASE = dyn_cast<ArraySubscriptExpr>(Exp)) {
- buildMutexID(ASE->getBase(), D, Parent, NumArgs, FunArgs);
- buildMutexID(ASE->getIdx(), D, Parent, NumArgs, FunArgs);
+ buildMutexID(ASE->getBase(), CallCtx);
+ buildMutexID(ASE->getIdx(), CallCtx);
} else if (AbstractConditionalOperator *CE =
dyn_cast<AbstractConditionalOperator>(Exp)) {
- buildMutexID(CE->getCond(), D, Parent, NumArgs, FunArgs);
- buildMutexID(CE->getTrueExpr(), D, Parent, NumArgs, FunArgs);
- buildMutexID(CE->getFalseExpr(), D, Parent, NumArgs, FunArgs);
+ buildMutexID(CE->getCond(), CallCtx);
+ buildMutexID(CE->getTrueExpr(), CallCtx);
+ buildMutexID(CE->getFalseExpr(), CallCtx);
} else if (ChooseExpr *CE = dyn_cast<ChooseExpr>(Exp)) {
- buildMutexID(CE->getCond(), D, Parent, NumArgs, FunArgs);
- buildMutexID(CE->getLHS(), D, Parent, NumArgs, FunArgs);
- buildMutexID(CE->getRHS(), D, Parent, NumArgs, FunArgs);
+ buildMutexID(CE->getCond(), CallCtx);
+ buildMutexID(CE->getLHS(), CallCtx);
+ buildMutexID(CE->getRHS(), CallCtx);
} else if (CastExpr *CE = dyn_cast<CastExpr>(Exp)) {
- buildMutexID(CE->getSubExpr(), D, Parent, NumArgs, FunArgs);
+ buildMutexID(CE->getSubExpr(), CallCtx);
} else if (ParenExpr *PE = dyn_cast<ParenExpr>(Exp)) {
- buildMutexID(PE->getSubExpr(), D, Parent, NumArgs, FunArgs);
+ buildMutexID(PE->getSubExpr(), CallCtx);
} else if (isa<CharacterLiteral>(Exp) ||
isa<CXXNullPtrLiteralExpr>(Exp) ||
isa<GNUNullExpr>(Exp) ||
@@ -184,43 +227,42 @@
/// occurs.
/// \param D The declaration to which the lock/unlock attribute is attached.
void buildMutexIDFromExp(Expr *MutexExp, Expr *DeclExp, const NamedDecl *D) {
- Expr *Parent = 0;
- unsigned NumArgs = 0;
- Expr **FunArgs = 0;
+ CallingContext CallCtx(D);
// If we are processing a raw attribute expression, with no substitutions.
if (DeclExp == 0) {
- buildMutexID(MutexExp, D, 0, 0, 0);
+ buildMutexID(MutexExp, 0);
return;
}
- // Examine DeclExp to find Parent and FunArgs, which are used to substitute
+ // Examine DeclExp to find SelfArg and FunArgs, which are used to substitute
// for formal parameters when we call buildMutexID later.
if (MemberExpr *ME = dyn_cast<MemberExpr>(DeclExp)) {
- Parent = ME->getBase();
+ CallCtx.SelfArg = ME->getBase();
} else if (CXXMemberCallExpr *CE = dyn_cast<CXXMemberCallExpr>(DeclExp)) {
- Parent = CE->getImplicitObjectArgument();
- NumArgs = CE->getNumArgs();
- FunArgs = CE->getArgs();
+ CallCtx.SelfArg = CE->getImplicitObjectArgument();
+ CallCtx.NumArgs = CE->getNumArgs();
+ CallCtx.FunArgs = CE->getArgs();
} else if (CallExpr *CE = dyn_cast<CallExpr>(DeclExp)) {
- NumArgs = CE->getNumArgs();
- FunArgs = CE->getArgs();
+ CallCtx.NumArgs = CE->getNumArgs();
+ CallCtx.FunArgs = CE->getArgs();
} else if (CXXConstructExpr *CE = dyn_cast<CXXConstructExpr>(DeclExp)) {
- Parent = 0; // FIXME -- get the parent from DeclStmt
- NumArgs = CE->getNumArgs();
- FunArgs = CE->getArgs();
+ CallCtx.SelfArg = 0; // FIXME -- get the parent from DeclStmt
+ CallCtx.NumArgs = CE->getNumArgs();
+ CallCtx.FunArgs = CE->getArgs();
} else if (D && isa<CXXDestructorDecl>(D)) {
// There's no such thing as a "destructor call" in the AST.
- Parent = DeclExp;
+ CallCtx.SelfArg = DeclExp;
}
// If the attribute has no arguments, then assume the argument is "this".
if (MutexExp == 0) {
- buildMutexID(Parent, D, 0, 0, 0);
+ buildMutexID(CallCtx.SelfArg, 0);
return;
}
- buildMutexID(MutexExp, D, Parent, NumArgs, FunArgs);
+ // For most attributes.
+ buildMutexID(MutexExp, &CallCtx);
}
public:
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=159152&r1=159151&r2=159152&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Mon Jun 25 13:33:18 2012
@@ -2258,3 +2258,106 @@
} // end namespace TrylockJoinPoint
+namespace LockReturned {
+
+class Foo {
+public:
+ int a GUARDED_BY(mu_);
+ void foo() EXCLUSIVE_LOCKS_REQUIRED(mu_);
+ void foo2(Foo* f) EXCLUSIVE_LOCKS_REQUIRED(mu_, f->mu_);
+
+ static void sfoo(Foo* f) EXCLUSIVE_LOCKS_REQUIRED(f->mu_);
+
+ Mutex* getMu() LOCK_RETURNED(mu_);
+
+ Mutex mu_;
+
+ static Mutex* getMu(Foo* f) LOCK_RETURNED(f->mu_);
+};
+
+
+// Calls getMu() directly to lock and unlock
+void test1(Foo* f1, Foo* f2) {
+ f1->a = 0; // expected-warning {{writing variable 'a' requires locking 'mu_' exclusively}}
+ f1->foo(); // expected-warning {{calling function 'foo' requires exclusive lock on 'mu_'}}
+
+ f1->foo2(f2); // expected-warning 2{{calling function 'foo2' requires exclusive lock on 'mu_'}}
+ Foo::sfoo(f1); // expected-warning {{calling function 'sfoo' requires exclusive lock on 'mu_'}}
+
+ f1->getMu()->Lock();
+
+ f1->a = 0;
+ f1->foo();
+ f1->foo2(f2); // expected-warning {{calling function 'foo2' requires exclusive lock on 'mu_'}}
+
+ Foo::getMu(f2)->Lock();
+ f1->foo2(f2);
+ Foo::getMu(f2)->Unlock();
+
+ Foo::sfoo(f1);
+
+ f1->getMu()->Unlock();
+}
+
+
+Mutex* getFooMu(Foo* f) LOCK_RETURNED(Foo::getMu(f));
+
+class Bar : public Foo {
+public:
+ int b GUARDED_BY(getMu());
+ void bar() EXCLUSIVE_LOCKS_REQUIRED(getMu());
+ void bar2(Bar* g) EXCLUSIVE_LOCKS_REQUIRED(getMu(this), g->getMu());
+
+ static void sbar(Bar* g) EXCLUSIVE_LOCKS_REQUIRED(g->getMu());
+ static void sbar2(Bar* g) EXCLUSIVE_LOCKS_REQUIRED(getFooMu(g));
+};
+
+
+
+// Use getMu() within other attributes.
+// This requires at lest levels of substitution, more in the case of
+void test2(Bar* b1, Bar* b2) {
+ b1->b = 0; // expected-warning {{writing variable 'b' requires locking 'mu_' exclusively}}
+ b1->bar(); // expected-warning {{calling function 'bar' requires exclusive lock on 'mu_'}}
+ b1->bar2(b2); // expected-warning 2{{calling function 'bar2' requires exclusive lock on 'mu_'}}
+ Bar::sbar(b1); // expected-warning {{calling function 'sbar' requires exclusive lock on 'mu_'}}
+ Bar::sbar2(b1); // expected-warning {{calling function 'sbar2' requires exclusive lock on 'mu_'}}
+
+ b1->getMu()->Lock();
+
+ b1->b = 0;
+ b1->bar();
+ b1->bar2(b2); // expected-warning {{calling function 'bar2' requires exclusive lock on 'mu_'}}
+
+ b2->getMu()->Lock();
+ b1->bar2(b2);
+
+ b2->getMu()->Unlock();
+
+ Bar::sbar(b1);
+ Bar::sbar2(b1);
+
+ b1->getMu()->Unlock();
+}
+
+
+// Sanity check -- lock the mutex directly, but use attributes that call getMu()
+// Also lock the mutex using getFooMu, which calls a lock_returned function.
+void test3(Bar* b1, Bar* b2) {
+ b1->mu_.Lock();
+ b1->b = 0;
+ b1->bar();
+
+ getFooMu(b2)->Lock();
+ b1->bar2(b2);
+ getFooMu(b2)->Unlock();
+
+ Bar::sbar(b1);
+ Bar::sbar2(b1);
+
+ b1->mu_.Unlock();
+}
+
+} // end namespace LockReturned
+
+
More information about the cfe-commits
mailing list