From: DeLesley Hutchins <delesley@google.com>
Date: Tue, 3 Jul 2012 19:47:18 +0000 (+0000)
Subject: Thread safety analysis: improve handling of smart pointers.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=96fac6a7fe89deff7860e536febbd4ae17bb57f3;p=clang

Thread safety analysis: improve handling of smart pointers.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@159679 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp
index 238a888194..fb53d076a4 100644
--- a/lib/Analysis/ThreadSafety.cpp
+++ b/lib/Analysis/ThreadSafety.cpp
@@ -26,6 +26,7 @@
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/OperatorKinds.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableMap.h"
@@ -162,6 +163,13 @@ class MutexID {
         buildMutexID(At->getArg(), &LRCallCtx);
         return;
       }
+      // Hack to treat smart pointers and iterators as pointers;
+      // ignore any method named get().
+      if (CMCE->getMethodDecl()->getNameAsString() == "get" &&
+          CMCE->getNumArgs() == 0) {
+        buildMutexID(CMCE->getImplicitObjectArgument(), CallCtx);
+        return;
+      }
       DeclSeq.push_back(CMCE->getMethodDecl()->getCanonicalDecl());
       buildMutexID(CMCE->getImplicitObjectArgument(), CallCtx);
       unsigned NumCallArgs = CMCE->getNumArgs();
@@ -179,6 +187,15 @@ class MutexID {
         buildMutexID(At->getArg(), &LRCallCtx);
         return;
       }
+      // Treat smart pointers and iterators as pointers;
+      // ignore the * and -> operators.
+      if (CXXOperatorCallExpr *OE = dyn_cast<CXXOperatorCallExpr>(CE)) {
+        OverloadedOperatorKind k = OE->getOperator();
+        if (k == OO_Arrow || k == OO_Star) {
+          buildMutexID(OE->getArg(0), CallCtx);
+          return;
+        }
+      }
       buildMutexID(CE->getCallee(), CallCtx);
       unsigned NumCallArgs = CE->getNumArgs();
       Expr** CallArgs = CE->getArgs();
@@ -208,6 +225,8 @@ class MutexID {
       buildMutexID(PE->getSubExpr(), CallCtx);
     } else if (ExprWithCleanups *EWC = dyn_cast<ExprWithCleanups>(Exp)) {
       buildMutexID(EWC->getSubExpr(), CallCtx);
+    } else if (CXXBindTemporaryExpr *E = dyn_cast<CXXBindTemporaryExpr>(Exp)) {
+      buildMutexID(E->getSubExpr(), CallCtx);
     } else if (isa<CharacterLiteral>(Exp) ||
                isa<CXXNullPtrLiteralExpr>(Exp) ||
                isa<GNUNullExpr>(Exp) ||
diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp
index de3f321379..9f01223f03 100644
--- a/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -67,7 +67,7 @@ public:
 
   T* get()        const { return ptr_; }
   T* operator->() const { return ptr_; }
-  T& operator*()  const { return ptr_; }
+  T& operator*()  const { return *ptr_; }
 
 private:
   T* ptr_;
@@ -2568,10 +2568,175 @@ void Foo::test() {
   {
     ReaderMutexLock lock(getMutexPtr().get());
     int b = a;
-  }           // FIXME: handle smart pointers better.
-  int b = a;  // expected-warning {{reading variable 'a' requires locking 'get'}}
+  }
+  int b = a;  // expected-warning {{reading variable 'a' requires locking 'getMutexPtr'}}
 }
 
 } // end namespace TemporaryCleanupExpr
 
 
+
+namespace SmartPointerTests {
+
+class Foo {
+public:
+  SmartPtr<Mutex> mu_;
+  int a GUARDED_BY(mu_);
+  int b GUARDED_BY(mu_.get());
+  int c GUARDED_BY(*mu_);
+
+  void Lock()   EXCLUSIVE_LOCK_FUNCTION(mu_);
+  void Unlock() UNLOCK_FUNCTION(mu_);
+
+  void test0();
+  void test1();
+  void test2();
+  void test3();
+  void test4();
+  void test5();
+  void test6();
+  void test7();
+  void test8();
+};
+
+void Foo::test0() {
+  a = 0;  // expected-warning {{writing variable 'a' requires locking 'mu_' exclusively}}
+  b = 0;  // expected-warning {{writing variable 'b' requires locking 'mu_' exclusively}}
+  c = 0;  // expected-warning {{writing variable 'c' requires locking 'mu_' exclusively}}
+}
+
+void Foo::test1() {
+  mu_->Lock();
+  a = 0;
+  b = 0;
+  c = 0;
+  mu_->Unlock();
+}
+
+void Foo::test2() {
+  (*mu_).Lock();
+  a = 0;
+  b = 0;
+  c = 0;
+  (*mu_).Unlock();
+}
+
+
+void Foo::test3() {
+  mu_.get()->Lock();
+  a = 0;
+  b = 0;
+  c = 0;
+  mu_.get()->Unlock();
+}
+
+
+void Foo::test4() {
+  MutexLock lock(mu_.get());
+  a = 0;
+  b = 0;
+  c = 0;
+}
+
+
+void Foo::test5() {
+  MutexLock lock(&(*mu_));
+  a = 0;
+  b = 0;
+  c = 0;
+}
+
+
+void Foo::test6() {
+  Lock();
+  a = 0;
+  b = 0;
+  c = 0;
+  Unlock();
+}
+
+
+void Foo::test7() {
+  {
+    Lock();
+    mu_->Unlock();
+  }
+  {
+    mu_->Lock();
+    Unlock();
+  }
+  {
+    mu_.get()->Lock();
+    mu_->Unlock();
+  }
+  {
+    mu_->Lock();
+    mu_.get()->Unlock();
+  }
+  {
+    mu_.get()->Lock();
+    (*mu_).Unlock();
+  }
+  {
+    (*mu_).Lock();
+    mu_->Unlock();
+  }
+}
+
+
+void Foo::test8() {
+  mu_->Lock();
+  mu_.get()->Lock();    // expected-warning {{locking 'mu_' that is already locked}}
+  (*mu_).Lock();        // expected-warning {{locking 'mu_' that is already locked}}
+  mu_.get()->Unlock();
+  Unlock();             // expected-warning {{unlocking 'mu_' that was not locked}}
+}
+
+
+class Bar {
+  SmartPtr<Foo> foo;
+
+  void test0();
+  void test1();
+  void test2();
+  void test3();
+};
+
+
+void Bar::test0() {
+  foo->a = 0;         // expected-warning {{writing variable 'a' requires locking 'mu_' exclusively}}
+  (*foo).b = 0;       // expected-warning {{writing variable 'b' requires locking 'mu_' exclusively}}
+  foo.get()->c = 0;   // expected-warning {{writing variable 'c' requires locking 'mu_' exclusively}}
+}
+
+
+void Bar::test1() {
+  foo->mu_->Lock();
+  foo->a = 0;
+  (*foo).b = 0;
+  foo.get()->c = 0;
+  foo->mu_->Unlock();
+}
+
+
+void Bar::test2() {
+  (*foo).mu_->Lock();
+  foo->a = 0;
+  (*foo).b = 0;
+  foo.get()->c = 0;
+  foo.get()->mu_->Unlock();
+}
+
+
+void Bar::test3() {
+  MutexLock lock(foo->mu_.get());
+  foo->a = 0;
+  (*foo).b = 0;
+  foo.get()->c = 0;
+}
+
+}  // end namespace SmartPointerTests
+
+
+
+