This patch expands the set of legal lock expressions to include
function calls, binary operators, and literals.  The current
implementation is still temporary, pending a larger fix that
implements something more robust.

  http://codereview.appspot.com/5695049/

  -DeLesley

-- 
DeLesley Hutchins | Software Engineer | [email protected] | 505-206-0315
diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp
index 40ad79b..5556f70 100644
--- a/lib/Analysis/ThreadSafety.cpp
+++ b/lib/Analysis/ThreadSafety.cpp
@@ -126,11 +126,40 @@ class MutexID {
         DeclSeq.push_back(0);  // Use 0 to represent 'this'.
         return;  // mutexID is still valid in this case
       }
+    } else if (CXXMemberCallExpr *CMCE = dyn_cast<CXXMemberCallExpr>(Exp)) {
+      DeclSeq.push_back(CMCE->getMethodDecl()->getCanonicalDecl());
+      buildMutexID(CMCE->getImplicitObjectArgument(),
+                   D, Parent, NumArgs, FunArgs);
+      unsigned NumCallArgs = CMCE->getNumArgs();
+      Expr** CallArgs = CMCE->getArgs();
+      for (unsigned i = 0; i < NumCallArgs; ++i) {
+        buildMutexID(CallArgs[i], D, Parent, NumArgs, FunArgs);
+      }
+    } else if (CallExpr *CE = dyn_cast<CallExpr>(Exp)) {
+      buildMutexID(CE->getCallee(), D, Parent, NumArgs, FunArgs);
+      unsigned NumCallArgs = CE->getNumArgs();
+      Expr** CallArgs = CE->getArgs();
+      for (unsigned i = 0; i < NumCallArgs; ++i) {
+        buildMutexID(CallArgs[i], D, Parent, NumArgs, FunArgs);
+      }
+    } else if (BinaryOperator *BOE = dyn_cast<BinaryOperator>(Exp)) {
+      buildMutexID(BOE->getLHS(), D, Parent, NumArgs, FunArgs);
+      buildMutexID(BOE->getRHS(), D, Parent, NumArgs, FunArgs);
     } else if (UnaryOperator *UOE = dyn_cast<UnaryOperator>(Exp))
       buildMutexID(UOE->getSubExpr(), D, Parent, NumArgs, FunArgs);
     else if (CastExpr *CE = dyn_cast<CastExpr>(Exp))
       buildMutexID(CE->getSubExpr(), D, Parent, NumArgs, FunArgs);
-    else
+    else if (isa<CharacterLiteral>(Exp) ||
+             isa<CXXNullPtrLiteralExpr>(Exp) ||
+             isa<GNUNullExpr>(Exp) ||
+             isa<CXXBoolLiteralExpr>(Exp) ||
+             isa<FloatingLiteral>(Exp) ||
+             isa<ImaginaryLiteral>(Exp) ||
+             isa<IntegerLiteral>(Exp) ||
+             isa<StringLiteral>(Exp) ||
+             isa<ObjCStringLiteral>(Exp)) {
+      return;  // FIXME: Ignore literals for now
+    } else
       DeclSeq.clear(); // Mark as invalid lock expression.
   }
 
@@ -233,11 +262,11 @@ public:
   /// The caret will point unambiguously to the lock expression, so using this
   /// name in diagnostics is a way to get simple, and consistent, mutex names.
   /// We do not want to output the entire expression text for security reasons.
-  StringRef getName() const {
+  std::string getName() const {
     assert(isValid());
     if (!DeclSeq.front())
       return "this";  // Use 0 to represent 'this'.
-    return DeclSeq.front()->getName();
+    return DeclSeq.front()->getNameAsString();
   }
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp
index 9c48b8d..f237c7e 100644
--- a/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -775,8 +775,9 @@ int y __attribute__((guarded_by(mua[0])));
 
 
 void testUnparse() {
-  x = 5; // \
-    // expected-warning{{cannot resolve lock expression}}
+  // FIXME -- derive new tests for unhandled expressions
+  // x = 5; // \
+    // EXPECTED-WARNING{{cannot resolve lock expression}}
   y = 5; // \
     // expected-warning{{cannot resolve lock expression}}
 }
@@ -2167,3 +2168,82 @@ class Foo {
 
 }
 
+
+namespace MoreLockExpressions {
+
+class Foo {
+public:
+  Mutex mu_;
+  int a GUARDED_BY(mu_);
+};
+
+class Bar {
+public:
+  int b;
+  Foo* f;
+
+  Foo& getFoo()              { return *f; }
+  Foo& getFoo2(int c)        { return *f; }
+  Foo& getFoo3(int c, int d) { return *f; }
+
+  Foo& getFooey() { return *f; }
+};
+
+Foo& getBarFoo(Bar &bar, int c) { return bar.getFoo2(c); }
+
+void test() {
+  Foo foo;
+  Bar bar;
+  int a;
+  int b;
+  int c;
+
+  bar.getFoo().mu_.Lock();
+  bar.getFoo().a = 0;
+  bar.getFoo().mu_.Unlock();
+
+  bar.getFoo2(a).mu_.Lock();
+  bar.getFoo2(a).a = 0;
+  bar.getFoo2(a).mu_.Unlock();
+
+  bar.getFoo3(a, b).mu_.Lock();
+  bar.getFoo3(a, b).a = 0;
+  bar.getFoo3(a, b).mu_.Unlock();
+
+  getBarFoo(bar, a).mu_.Lock();
+  getBarFoo(bar, a).a = 0;
+  getBarFoo(bar, a).mu_.Unlock();
+
+  bar.getFoo2(10).mu_.Lock();
+  bar.getFoo2(10).a = 0;
+  bar.getFoo2(10).mu_.Unlock();
+
+  bar.getFoo2(a + 1).mu_.Lock();
+  bar.getFoo2(a + 1).a = 0;
+  bar.getFoo2(a + 1).mu_.Unlock();
+
+  bar.getFoo().mu_.Lock();
+  bar.getFooey().a = 0; // \
+    // expected-warning {{writing variable 'a' requires locking 'mu_' exclusively}}
+  bar.getFoo().mu_.Unlock();
+
+  bar.getFoo2(a).mu_.Lock();
+  bar.getFoo2(b).a = 0; // \
+    // expected-warning {{writing variable 'a' requires locking 'mu_' exclusively}}
+  bar.getFoo2(a).mu_.Unlock();
+
+  bar.getFoo3(a, b).mu_.Lock();
+  bar.getFoo3(a, c).a = 0;  // \
+    // expected-warning {{writing variable 'a' requires locking 'mu_' exclusively}}
+  bar.getFoo3(a, b).mu_.Unlock();
+
+  getBarFoo(bar, a).mu_.Lock();
+  getBarFoo(bar, b).a = 0;  // \
+    // expected-warning {{writing variable 'a' requires locking 'mu_' exclusively}}
+  getBarFoo(bar, a).mu_.Unlock();
+}
+
+
+} // end namespace
+
+
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to