[cfe-commits] r148599 - in /cfe/trunk: lib/Analysis/ThreadSafety.cpp test/SemaCXX/warn-thread-safety-analysis.cpp

[DeLesley Hutchins]

Author: delesley
Date: Fri Jan 20 17:24:41 2012
New Revision: 148599

URL: http://llvm.org/viewvc/llvm-project?rev=148599&view=rev
Log:
Handle thread safety attributes on functions with separate definitions and declarations.

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=148599&r1=148598&r2=148599&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Fri Jan 20 17:24:41 2012
@@ -88,38 +88,46 @@
   /// 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, Expr *Parent, int NumArgs,
-                    const NamedDecl **FunArgDecls, Expr **FunArgs) {
+  void buildMutexID(Expr *Exp, const NamedDecl *D, Expr *Parent,
+                    unsigned NumArgs, Expr **FunArgs) {
     if (!Exp) {
       DeclSeq.clear();
       return;
     }
 
     if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Exp)) {
-      if (FunArgDecls) {
-        // Substitute call arguments for references to function parameters
-        for (int i = 0; i < NumArgs; ++i) {
-          if (DRE->getDecl() == FunArgDecls[i]) {
-            buildMutexID(FunArgs[i], 0, 0, 0, 0);
-            return;
-          }
+      NamedDecl *ND = cast<NamedDecl>(DRE->getDecl()->getCanonicalDecl());
+      ParmVarDecl *PV = dyn_cast_or_null<ParmVarDecl>(ND);
+      if (PV) {
+        FunctionDecl *FD =
+          cast<FunctionDecl>(PV->getDeclContext())->getCanonicalDecl();
+        unsigned i = PV->getFunctionScopeIndex();
+
+        if (FunArgs && FD == D->getCanonicalDecl()) {
+          // Substitute call arguments for references to function parameters
+          assert(i < NumArgs);
+          buildMutexID(FunArgs[i], D, 0, 0, 0);
+          return;
         }
+        // Map the param back to the param of the original function declaration.
+        DeclSeq.push_back(FD->getParamDecl(i));
+        return;
       }
-      NamedDecl *ND = cast<NamedDecl>(DRE->getDecl()->getCanonicalDecl());
+      // 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(), Parent, NumArgs, FunArgDecls, FunArgs);
+      buildMutexID(ME->getBase(), D, Parent, NumArgs, FunArgs);
     } else if (isa<CXXThisExpr>(Exp)) {
       if (Parent)
-        buildMutexID(Parent, 0, 0, 0, 0);
+        buildMutexID(Parent, D, 0, 0, 0);
       else
         return;  // mutexID is still valid in this case
     } else if (UnaryOperator *UOE = dyn_cast<UnaryOperator>(Exp))
-      buildMutexID(UOE->getSubExpr(), Parent, NumArgs, FunArgDecls, FunArgs);
+      buildMutexID(UOE->getSubExpr(), D, Parent, NumArgs, FunArgs);
     else if (CastExpr *CE = dyn_cast<CastExpr>(Exp))
-      buildMutexID(CE->getSubExpr(), Parent, NumArgs, FunArgDecls, FunArgs);
+      buildMutexID(CE->getSubExpr(), D, Parent, NumArgs, FunArgs);
     else
       DeclSeq.clear(); // Mark as invalid lock expression.
   }
@@ -133,11 +141,10 @@
     Expr *Parent = 0;
     unsigned NumArgs = 0;
     Expr **FunArgs = 0;
-    SmallVector<const NamedDecl*, 8> FunArgDecls;
 
     // If we are processing a raw attribute expression, with no substitutions.
     if (DeclExp == 0) {
-      buildMutexID(MutexExp, 0, 0, 0, 0);
+      buildMutexID(MutexExp, D, 0, 0, 0);
       return;
     }
 
@@ -163,17 +170,11 @@
 
     // If the attribute has no arguments, then assume the argument is "this".
     if (MutexExp == 0) {
-      buildMutexID(Parent, 0, 0, 0, 0);
+      buildMutexID(Parent, D, 0, 0, 0);
       return;
     }
 
-    // FIXME: handle default arguments
-    if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D)) {
-      for (unsigned i = 0, ni = FD->getNumParams(); i < ni && i < NumArgs; ++i) {
-        FunArgDecls.push_back(FD->getParamDecl(i));
-      }
-    }
-    buildMutexID(MutexExp, Parent, NumArgs, &FunArgDecls.front(), FunArgs);
+    buildMutexID(MutexExp, D, Parent, NumArgs, FunArgs);
   }
 
 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=148599&r1=148598&r2=148599&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Fri Jan 20 17:24:41 2012
@@ -1875,3 +1875,29 @@
 };  // end namespace TestTemplateAttributeInstantiation
 
 
+namespace FunctionDeclDefTest {
+
+class Foo {
+public:
+  Mutex mu_;
+  int a GUARDED_BY(mu_);
+
+  virtual void foo1(Foo *f_declared) EXCLUSIVE_LOCKS_REQUIRED(f_declared->mu_);
+};
+
+// EXCLUSIVE_LOCKS_REQUIRED should be applied, and rewritten to f_defined->mu_
+void Foo::foo1(Foo *f_defined) {
+  f_defined->a = 0;
+};
+
+void test() {
+  Foo myfoo;
+  myfoo.foo1(&myfoo);  // \
+    // expected-warning {{calling function 'foo1' requires exclusive lock on 'mu_'}}
+  myfoo.mu_.Lock();
+  myfoo.foo1(&myfoo);
+  myfoo.mu_.Unlock();
+}
+
+};
+