Reland "Thread Safety Analysis: fix assert_capability.", add warnings

[jmgao]

Summary

Reland "Thread Safety Analysis: fix assert_capability."

Delete the test that was broken by rL309725, and add it back in a
follow up commit. Also, improve the tests a bit.

---

Thread Safety Analysis: warn on nonsensical attributes.

Add warnings in cases where an implicit this argument is expected to
attributes because either this doesn't exist because the attribute is
on a free function, or because this is on a type that doesn't have a
corresponding capability/lockable/scoped_lockable attribute.

[jmgao]

Remove accidental trailing backslash.

[jmgao]

Fix commit messages.

[delesley]

Overall looks good. However, I would change the wording on the warning to the following. The terms "free function" and "instance method" may be confusing to some people. Also, warn_thread_attribute_noargs_static_method should not mention the capability attribute, which is checked separately in warn_thread_attribute_noargs_not_lockable.

def warn_thread_attribute_noargs_not_method: ... "%0 attribute without arguments can only be applied to a method of a class." ...
def warn_thread_attribute_noargs_static_method: ... "%0 attribute without arguments cannot be applied a static method." ...

[jmgao]

Reword warnings.

[jmgao]

Thanks for the review!

cfe/trunk/include/clang/Basic/Attr.td

let Spellings = [GNU<"assert_capability">,
                    CXX11<"clang", "assert_capability">,
                    GNU<"assert_shared_capability">,
                    CXX11<"clang", "assert_shared_capability">];
   let Subjects = SubjectList<[Function]>;
   let LateParsed = 1;
   let TemplateDependent = 1;
   let ParseArgumentsAsUnevaluated = 1;
   let DuplicatesAllowedWhileMerging = 1;
-  let Args = [ExprArgument<"Expr">];
+  let Args = [VariadicExprArgument<"Args">];
   let Accessors = [Accessor<"isShared",
                     [GNU<"assert_shared_capability">,
                      CXX11<"clang", "assert_shared_capability">]>];
   let Documentation = [AssertCapabilityDocs];
 }
 
 def AcquireCapability : InheritableAttr {
   let Spellings = [GNU<"acquire_capability">,

cfe/trunk/lib/Analysis/ThreadSafety.cpp

switch (At->getKind()) {
         break;
       }
       case attr::AssertSharedLock: {
         AssertSharedLockAttr *A = cast<AssertSharedLockAttr>(At);
 
         CapExprSet AssertLocks;
         Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
         for (const auto &AssertLock : AssertLocks)
-          Analyzer->addLock(FSet, llvm::make_unique<LockableFactEntry>(
-                                      AssertLock, LK_Shared, Loc, false, true),
+          Analyzer->addLock(FSet,
+                            llvm::make_unique<LockableFactEntry>(
                             ClassifyDiagnostic(A));
         break;
       }
 
+      case attr::AssertCapability: {
+        AssertCapabilityAttr *A = cast<AssertCapabilityAttr>(At);
+        CapExprSet AssertLocks;
+        Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
+        for (const auto &AssertLock : AssertLocks)
+          Analyzer->addLock(FSet,
+                            llvm::make_unique<LockableFactEntry>(
+                                AssertLock,
+                                A->isShared() ? LK_Shared : LK_Exclusive, Loc,
+                                false, true),
+                            ClassifyDiagnostic(A));
+        break;
+      }
+
       // When we encounter an unlock function, we need to remove unlocked
       // mutexes from the lockset, and flag a warning if they are not there.
       case attr::ReleaseCapability: {
         auto *A = cast<ReleaseCapabilityAttr>(At);
         if (A->isGeneric())
           Analyzer->getMutexIDs(GenericLocksToRemove, A, Exp, D, VD);
         else if (A->isShared())
           Analyzer->getMutexIDs(SharedLocksToRemove, A, Exp, D, VD);

cfe/trunk/lib/Sema/SemaDeclAttr.cpp

if (!N.equals_lower("mutex") && !N.equals_lower("role"))
     S.Diag(LiteralLoc, diag::warn_invalid_capability_name) << N;
 
   D->addAttr(::new (S.Context) CapabilityAttr(Attr.getRange(), S.Context, N,
                                         Attr.getAttributeSpellingListIndex()));
 }
 
 static void handleAssertCapabilityAttr(Sema &S, Decl *D,
                                        const AttributeList &Attr) {
+  SmallVector<Expr*, 1> Args;
+  if (!checkLockFunAttrCommon(S, D, Attr, Args))
+    return;
+
   D->addAttr(::new (S.Context) AssertCapabilityAttr(Attr.getRange(), S.Context,
-                                                    Attr.getArgAsExpr(0),
+                                                    Args.data(), Args.size(),
                                         Attr.getAttributeSpellingListIndex()));
 }
 
 static void handleAcquireCapabilityAttr(Sema &S, Decl *D,
                                         const AttributeList &Attr) {
   SmallVector<Expr*, 1> Args;
   if (!checkLockFunAttrCommon(S, D, Attr, Args))
     return;

cfe/trunk/test/Sema/attr-capabilities.c

 void Func4(void) __attribute__((requires_shared_capability)) {}  // expected-error {{'requires_shared_capability' attribute takes at least 1 argument}}
 
 void Func5(void) __attribute__((requires_capability(1))) {}  // expected-warning {{'requires_capability' attribute requires arguments whose type is annotated with 'capability' attribute; type here is 'int'}}
 void Func6(void) __attribute__((requires_shared_capability(BadCapability))) {}  // expected-warning {{'requires_shared_capability' attribute requires arguments whose type is annotated with 'capability' attribute; type here is 'struct NotACapability'}}
 
 void Func7(void) __attribute__((assert_capability(GUI))) {}
 void Func8(void) __attribute__((assert_shared_capability(GUI))) {}
 
-void Func9(void) __attribute__((assert_capability())) {} // expected-error {{'assert_capability' attribute takes one argument}}
-void Func10(void) __attribute__((assert_shared_capability())) {} // expected-error {{'assert_shared_capability' attribute takes one argument}}
-
 void Func11(void) __attribute__((acquire_capability(GUI))) {}
 void Func12(void) __attribute__((acquire_shared_capability(GUI))) {}
 
 void Func15(void) __attribute__((release_capability(GUI))) {}
 void Func16(void) __attribute__((release_shared_capability(GUI))) {}
 void Func17(void) __attribute__((release_generic_capability(GUI))) {}
 
 void Func21(void) __attribute__((try_acquire_capability(1))) {}

cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=0 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=1 %s
 
 // FIXME: should also run  %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 -Wc++98-compat %s
 // FIXME: should also run  %clang_cc1 -fsyntax-only -verify -Wthread-safety %s
 
 #define LOCKABLE             __attribute__((lockable))
 #define SCOPED_LOCKABLE      __attribute__((scoped_lockable))
 #define GUARDED_BY(x)        __attribute__((guarded_by(x)))
 #define GUARDED_VAR          __attribute__((guarded_var))
 #define PT_GUARDED_BY(x)     __attribute__((pt_guarded_by(x)))
 #define PT_GUARDED_VAR       __attribute__((pt_guarded_var))
 #define ACQUIRED_AFTER(...)  __attribute__((acquired_after(__VA_ARGS__)))
 #define ACQUIRED_BEFORE(...) __attribute__((acquired_before(__VA_ARGS__)))
 #define EXCLUSIVE_LOCK_FUNCTION(...)    __attribute__((exclusive_lock_function(__VA_ARGS__)))
 #define SHARED_LOCK_FUNCTION(...)       __attribute__((shared_lock_function(__VA_ARGS__)))
+
+#if USE_ASSERT_CAPABILITY
+#define ASSERT_EXCLUSIVE_LOCK(...)      __attribute__((assert_capability(__VA_ARGS__)))
+#define ASSERT_SHARED_LOCK(...)         __attribute__((assert_shared_capability(__VA_ARGS__)))
+#else
 #define ASSERT_EXCLUSIVE_LOCK(...)      __attribute__((assert_exclusive_lock(__VA_ARGS__)))
 #define ASSERT_SHARED_LOCK(...)         __attribute__((assert_shared_lock(__VA_ARGS__)))
+#endif
+
 #define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__((exclusive_trylock_function(__VA_ARGS__)))
 #define SHARED_TRYLOCK_FUNCTION(...)    __attribute__((shared_trylock_function(__VA_ARGS__)))
 #define UNLOCK_FUNCTION(...)            __attribute__((unlock_function(__VA_ARGS__)))
 #define EXCLUSIVE_UNLOCK_FUNCTION(...)  __attribute__((release_capability(__VA_ARGS__)))
 #define SHARED_UNLOCK_FUNCTION(...)     __attribute__((release_shared_capability(__VA_ARGS__)))
 #define LOCK_RETURNED(x)                __attribute__((lock_returned(x)))
 #define LOCKS_EXCLUDED(...)             __attribute__((locks_excluded(__VA_ARGS__)))
 #define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__((exclusive_locks_required(__VA_ARGS__)))
class Foo {
   void unlock()        UNLOCK_FUNCTION(mu1_)
                        UNLOCK_FUNCTION(mu2_);
   bool trylock()       EXCLUSIVE_TRYLOCK_FUNCTION(true, mu1_)
                        EXCLUSIVE_TRYLOCK_FUNCTION(true, mu2_);
   bool readertrylock() SHARED_TRYLOCK_FUNCTION(true, mu1_)
                        SHARED_TRYLOCK_FUNCTION(true, mu2_);
   void assertBoth() ASSERT_EXCLUSIVE_LOCK(mu1_)
                     ASSERT_EXCLUSIVE_LOCK(mu2_);
+
+  void alsoAssertBoth() ASSERT_EXCLUSIVE_LOCK(mu1_, mu2_);
+
   void assertShared() ASSERT_SHARED_LOCK(mu1_)
                       ASSERT_SHARED_LOCK(mu2_);
 
+  void alsoAssertShared() ASSERT_SHARED_LOCK(mu1_, mu2_);
+
   void test();
   void testAssert();
   void testAssertShared();
 };
 
 
 void Foo::foo1() {
   a = 1;
void Foo::test() {
   if (readertrylock()) {
     int zz = a + b;
     unlock();
   }
 }
 
 // Force duplication of attributes
 void Foo::assertBoth() { }
+void Foo::alsoAssertBoth() { }
 void Foo::assertShared() { }
+void Foo::alsoAssertShared() { }
 
 void Foo::testAssert() {
+  {
     assertBoth();
     a = 0;
     b = 0;
   }
+  {
+    alsoAssertBoth();
+    a = 0;
+    b = 0;
+  }
+}
 
 void Foo::testAssertShared() {
+  {
     assertShared();
     int zz = a + b;
   }
 
+  {
+    alsoAssertShared();
+    int zz = a + b;
+  }
+}
+
 
 }  // end namespace MultipleAttributeTest
 
 
 namespace GuardedNonPrimitiveTypeTest {
 
 
 class Data {