Thread-safety analysis: fix handling of string constants in mutex

expressions, which should be ignored right now.

llvm-svn: 163026

clang/lib/Analysis/ThreadSafety.cpp

@@ -445,6 +445,12 @@ private:
   void buildSExprFromExpr(Expr *MutexExp, Expr *DeclExp, const NamedDecl *D) {
     CallingContext CallCtx(D);
 
+    // Ignore string literals
+    if (MutexExp && isa<StringLiteral>(MutexExp)) {
+      makeNop();
+      return;
+    }
+
     // If we are processing a raw attribute expression, with no substitutions.
     if (DeclExp == 0) {
       buildSExpr(MutexExp, 0);
@@ -506,6 +512,12 @@ public:
     return !NodeVec.empty();
   }
 
+  bool shouldIgnore() const {
+    // Nop is a mutex that we have decided to deliberately ignore.
+    assert(NodeVec.size() > 0 && "Invalid Mutex");
+    return NodeVec[0].kind() == EOP_Nop;
+  }
+
   /// Issue a warning about an invalid lock expression
   static void warnInvalidLock(ThreadSafetyHandler &Handler, Expr* MutexExp,
                               Expr *DeclExp, const NamedDecl* D) {
@@ -1376,6 +1388,9 @@ void ThreadSafetyAnalyzer::addLock(FactSet &FSet, const SExpr &Mutex,
                                    const LockData &LDat) {
   // FIXME: deal with acquired before/after annotations.
   // FIXME: Don't always warn when we have support for reentrant locks.
+  if (Mutex.shouldIgnore())
+    return;
+
   if (FSet.findLock(FactMan, Mutex)) {
     Handler.handleDoubleLock(Mutex.toString(), LDat.AcquireLoc);
   } else {
@@ -1391,6 +1406,9 @@ void ThreadSafetyAnalyzer::removeLock(FactSet &FSet,
                                       const SExpr &Mutex,
                                       SourceLocation UnlockLoc,
                                       bool FullyRemove) {
+  if (Mutex.shouldIgnore())
+    return;
+
   const LockData *LDat = FSet.findLock(FactMan, Mutex);
   if (!LDat) {
     Handler.handleUnmatchedUnlock(Mutex.toString(), UnlockLoc);
@@ -1703,6 +1721,8 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, Expr *Exp,
   SExpr Mutex(MutexExp, Exp, D);
   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))
     Analyzer->Handler.handleMutexNotHeld(D, POK, Mutex.toString(), LK,
                                          Exp->getExprLoc());

clang/lib/Sema/SemaDeclAttr.cpp

@@ -415,14 +415,17 @@ static void checkAttrArgsAreLockableObjs(Sema &S, Decl *D,
     }
 
     if (StringLiteral *StrLit = dyn_cast<StringLiteral>(ArgExp)) {
-      // Ignore empty strings without warnings
-      if (StrLit->getLength() == 0)
+      if (StrLit->getLength() == 0) {
+        // Pass empty strings to the analyzer without warnings.
+        Args.push_back(ArgExp);
         continue;
+      }
 
       // We allow constant strings to be used as a placeholder for expressions
       // that are not valid C++ syntax, but warn that they are ignored.
       S.Diag(Attr.getLoc(), diag::warn_thread_attribute_ignored) <<
         Attr.getName();
+      Args.push_back(ArgExp);
       continue;
     }
 

clang/test/SemaCXX/warn-thread-safety-analysis.cpp

@@ -3119,3 +3119,29 @@ void test() {
 
 } // end namespace ExistentialPatternMatching
 
+
+namespace StringIgnoreTest {
+
+class Foo {
+public:
+  Mutex mu_;
+  void lock()   EXCLUSIVE_LOCK_FUNCTION("");
+  void unlock() UNLOCK_FUNCTION("");
+  void goober() EXCLUSIVE_LOCKS_REQUIRED("");
+  void roober() SHARED_LOCKS_REQUIRED("");
+};
+
+
+class Bar : public Foo {
+public:
+  void bar(Foo* f) {
+    f->unlock();
+    f->goober();
+    f->roober();
+    f->lock();
+  };
+};
+
+} // end namespace StringIgnoreTest
+
+