Thread safety: Run tests with both lock and capability attributes

[aaronpuchert]

Summary

Running all tests with both sets of attributes should make sure there is
no regression in either variant.

[aaron.ballman]

LGTM! Thank you for this!

[aaronpuchert]

Thanks for the review. Could you commit this for me (Aaron Puchert <[e-mail]>)?

[aaron.ballman]

In D49275#1162204, @aaronpuchert wrote:

Thanks for the review. Could you commit this for me (Aaron Puchert <[e-mail]>)?

Happy to do so. I've commit in r337125. Thank you for the patch!

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 -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
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=0 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=1 -DUSE_TRY_ACQUIRE_CAPABILITY %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=0 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=1 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_CAPABILITY=0 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_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
+#if USE_CAPABILITY
+#define LOCKABLE                        __attribute__((capability("mutex")))
 #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
-
-#ifdef USE_TRY_ACQUIRE_CAPABILITY
+#define EXCLUSIVE_LOCK_FUNCTION(...)    __attribute__((acquire_capability(__VA_ARGS__)))
+#define SHARED_LOCK_FUNCTION(...)       __attribute__((acquire_shared_capability(__VA_ARGS__)))
 #define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__((try_acquire_capability(__VA_ARGS__)))
 #define SHARED_TRYLOCK_FUNCTION(...)    __attribute__((try_acquire_shared_capability(__VA_ARGS__)))
+#define EXCLUSIVE_UNLOCK_FUNCTION(...)  __attribute__((release_capability(__VA_ARGS__)))
+#define SHARED_UNLOCK_FUNCTION(...)     __attribute__((release_shared_capability(__VA_ARGS__)))
+#define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__((requires_capability(__VA_ARGS__)))
+#define SHARED_LOCKS_REQUIRED(...)      __attribute__((requires_shared_capability(__VA_ARGS__)))
 #else
+#define LOCKABLE                        __attribute__((lockable))
+#define ASSERT_EXCLUSIVE_LOCK(...)      __attribute__((assert_exclusive_lock(__VA_ARGS__)))
+#define ASSERT_SHARED_LOCK(...)         __attribute__((assert_shared_lock(__VA_ARGS__)))
+#define EXCLUSIVE_LOCK_FUNCTION(...)    __attribute__((exclusive_lock_function(__VA_ARGS__)))
+#define SHARED_LOCK_FUNCTION(...)       __attribute__((shared_lock_function(__VA_ARGS__)))
 #define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__((exclusive_trylock_function(__VA_ARGS__)))
 #define SHARED_TRYLOCK_FUNCTION(...)    __attribute__((shared_trylock_function(__VA_ARGS__)))
+#define EXCLUSIVE_UNLOCK_FUNCTION(...)  __attribute__((unlock_function(__VA_ARGS__)))
+#define SHARED_UNLOCK_FUNCTION(...)     __attribute__((unlock_function(__VA_ARGS__)))
+#define EXCLUSIVE_LOCKS_REQUIRED(...)   __attribute__((exclusive_locks_required(__VA_ARGS__)))
+#define SHARED_LOCKS_REQUIRED(...)      __attribute__((shared_locks_required(__VA_ARGS__)))
 #endif
 #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__)))
-#define SHARED_LOCKS_REQUIRED(...)      __attribute__((shared_locks_required(__VA_ARGS__)))
 #define NO_THREAD_SAFETY_ANALYSIS       __attribute__((no_thread_safety_analysis))
 
 
 class LOCKABLE Mutex {
  public:
-  void Lock() __attribute__((exclusive_lock_function));
-  void ReaderLock() __attribute__((shared_lock_function));
-  void Unlock() __attribute__((unlock_function));
+  void Lock() EXCLUSIVE_LOCK_FUNCTION();
+  void ReaderLock() SHARED_LOCK_FUNCTION();
+  void Unlock() UNLOCK_FUNCTION();
   bool TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(true);
   bool ReaderTryLock() SHARED_TRYLOCK_FUNCTION(true);
-  void LockWhen(const int &cond) __attribute__((exclusive_lock_function));
+  void LockWhen(const int &cond) EXCLUSIVE_LOCK_FUNCTION();
 
   // for negative capabilities
   const Mutex& operator!() const { return *this; }
 
   void AssertHeld()       ASSERT_EXCLUSIVE_LOCK();
   void AssertReaderHeld() ASSERT_SHARED_LOCK();
 };
 

 class SCOPED_LOCKABLE ReleasableMutexLock {
  public:
   ReleasableMutexLock(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
   ~ReleasableMutexLock() UNLOCK_FUNCTION();
 
   void Release() UNLOCK_FUNCTION();
 };
 
-class __attribute__((scoped_lockable)) DoubleMutexLock {
+class SCOPED_LOCKABLE DoubleMutexLock {
 public:
-  DoubleMutexLock(Mutex *mu1, Mutex *mu2)
-      __attribute__((exclusive_lock_function(mu1, mu2)));
-  ~DoubleMutexLock() __attribute__((unlock_function));
+  DoubleMutexLock(Mutex *mu1, Mutex *mu2) EXCLUSIVE_LOCK_FUNCTION(mu1, mu2);
+  ~DoubleMutexLock() UNLOCK_FUNCTION();
 };
 
 // The universal lock, written "*", allows checking to be selectively turned
 // off for a particular piece of code.
 void beginNoWarnOnReads()  SHARED_LOCK_FUNCTION("*");
 void endNoWarnOnReads()    UNLOCK_FUNCTION("*");
 void beginNoWarnOnWrites() EXCLUSIVE_LOCK_FUNCTION("*");
 void endNoWarnOnWrites()   UNLOCK_FUNCTION("*");

 int sls_guardby_var __attribute__((guarded_by(sls_mu))) = 0;
 
 bool getBool();
 
 class MutexWrapper {
 public:
    Mutex mu;
    int x __attribute__((guarded_by(mu)));
-   void MyLock() __attribute__((exclusive_lock_function(mu)));
+   void MyLock() EXCLUSIVE_LOCK_FUNCTION(mu);
 };
 
 MutexWrapper sls_mw;
 
 void sls_fun_0() {
   sls_mw.mu.Lock();
   sls_mw.x = 5;
   sls_mw.mu.Unlock();

 //-----------------------------------------//
 // Handling lock expressions in attribute args
 // -------------------------------------------//
 
 Mutex aa_mu;
 
 class GlobalLocker {
 public:
-  void globalLock() __attribute__((exclusive_lock_function(aa_mu)));
-  void globalUnlock() __attribute__((unlock_function(aa_mu)));
+  void globalLock() EXCLUSIVE_LOCK_FUNCTION(aa_mu);
+  void globalUnlock() UNLOCK_FUNCTION(aa_mu);
 };
 
 GlobalLocker glock;
 
 void aa_fun_1() {
   glock.globalLock();
   glock.globalUnlock();
 }

  public:
   int gb_field __attribute__((guarded_by(sls_mu)));
 
   void testFoo() {
     gb_field = 0; // \
       // expected-warning {{writing variable 'gb_field' requires holding mutex 'sls_mu' exclusively}}
   }
 
-  void testNoAnal() __attribute__((no_thread_safety_analysis)) {
+  void testNoAnal() NO_THREAD_SAFETY_ANALYSIS {
     gb_field = 0;
   }
 };
 
 GBFoo GlobalGBFoo __attribute__((guarded_by(sls_mu)));
 
 void gb_fun_0() {
   sls_mu.Lock();

 // Warnings on variables with late parsed attributes
 // ----------------------------------------------//
 
 class LateFoo {
 public:
   int a __attribute__((guarded_by(mu)));
   int b;
 
-  void foo() __attribute__((exclusive_locks_required(mu))) { }
+  void foo() EXCLUSIVE_LOCKS_REQUIRED(mu) { }
 
   void test() {
     a = 0; // \
       // expected-warning{{writing variable 'a' requires holding mutex 'mu' exclusively}}
     b = a; // \
       // expected-warning {{reading variable 'a' requires holding mutex 'mu'}}
     c = 0; // \
       // expected-warning {{writing variable 'c' requires holding mutex 'mu' exclusively}}
sls_mu.Lock(); // \
       // expected-note {{the other acquisition of mutex 'sls_mu' is here}}
   *pgb_var = 1;
   sls_mu.Unlock();
 }
 
 // FIXME: Add support for functions (not only methods)
 class LRBar {
  public:
-  void aa_elr_fun() __attribute__((exclusive_locks_required(aa_mu)));
-  void aa_elr_fun_s() __attribute__((shared_locks_required(aa_mu)));
+  void aa_elr_fun() EXCLUSIVE_LOCKS_REQUIRED(aa_mu);
+  void aa_elr_fun_s() SHARED_LOCKS_REQUIRED(aa_mu);
   void le_fun() __attribute__((locks_excluded(sls_mu)));
 };
 
 class LRFoo {
  public:
-  void test() __attribute__((exclusive_locks_required(sls_mu)));
-  void testShared() __attribute__((shared_locks_required(sls_mu2)));
+  void test() EXCLUSIVE_LOCKS_REQUIRED(sls_mu);
+  void testShared() SHARED_LOCKS_REQUIRED(sls_mu2);
 };
 
-void elr_fun() __attribute__((exclusive_locks_required(sls_mu)));
+void elr_fun() EXCLUSIVE_LOCKS_REQUIRED(sls_mu);
 void elr_fun() {}
 
 LRFoo MyLRFoo;
 LRBar Bar;
 
 void es_fun_0() {
   aa_mu.Lock();
   Bar.aa_elr_fun();

 }
 
 void es_fun_7() {
   sls_mu.Lock();
   elr_fun();
   sls_mu.Unlock();
 }
 
-void es_fun_8() __attribute__((no_thread_safety_analysis));
+void es_fun_8() NO_THREAD_SAFETY_ANALYSIS;
 
 void es_fun_8() {
   Bar.aa_elr_fun_s();
 }
 
-void es_fun_9() __attribute__((shared_locks_required(aa_mu)));
+void es_fun_9() SHARED_LOCKS_REQUIRED(aa_mu);
 void es_fun_9() {
   Bar.aa_elr_fun_s();
 }
 
-void es_fun_10() __attribute__((exclusive_locks_required(aa_mu)));
+void es_fun_10() EXCLUSIVE_LOCKS_REQUIRED(aa_mu);
 void es_fun_10() {
   Bar.aa_elr_fun_s();
 }
 
 void es_bad_0() {
   Bar.aa_elr_fun(); // \
     // expected-warning {{calling function 'aa_elr_fun' requires holding mutex 'aa_mu' exclusively}}
 }

 } // end namespace thread_annot_lock_67_modified
 
 
 namespace substitution_test {
   class MyData  {
   public:
     Mutex mu;
 
-    void lockData()    __attribute__((exclusive_lock_function(mu)));
-    void unlockData()  __attribute__((unlock_function(mu)));
+    void lockData()    EXCLUSIVE_LOCK_FUNCTION(mu);
+    void unlockData()  UNLOCK_FUNCTION(mu);
 
-    void doSomething() __attribute__((exclusive_locks_required(mu)))  { }
+    void doSomething() EXCLUSIVE_LOCKS_REQUIRED(mu)  { }
   };
 
 
   class DataLocker {
   public:
-    void lockData  (MyData *d) __attribute__((exclusive_lock_function(d->mu)));
-    void unlockData(MyData *d) __attribute__((unlock_function(d->mu)));
+    void lockData  (MyData *d) EXCLUSIVE_LOCK_FUNCTION(d->mu);
+    void unlockData(MyData *d) UNLOCK_FUNCTION(d->mu);
   };
 
 
   class Foo {
   public:
-    void foo(MyData* d) __attribute__((exclusive_locks_required(d->mu))) { }
+    void foo(MyData* d) EXCLUSIVE_LOCKS_REQUIRED(d->mu) { }
 
     void bar1(MyData* d) {
       d->lockData();
       foo(d);
       d->unlockData();
     }
 
     void bar2(MyData* d) {

 
 
 namespace constructor_destructor_tests {
   Mutex fooMu;
   int myVar GUARDED_BY(fooMu);
 
   class Foo {
   public:
-    Foo()  __attribute__((exclusive_lock_function(fooMu))) { }
-    ~Foo() __attribute__((unlock_function(fooMu))) { }
+    Foo()  EXCLUSIVE_LOCK_FUNCTION(fooMu) { }
+    ~Foo() UNLOCK_FUNCTION(fooMu) { }
   };
 
   void fooTest() {
     Foo foo;
     myVar = 0;
   }
 }
 
void test() {
   myfoo.foo1(&myfoo);
   myfoo.mu_.Unlock();
 }
 
 };
 
 namespace GoingNative {
 
-  struct __attribute__((lockable)) mutex {
-    void lock() __attribute__((exclusive_lock_function));
-    void unlock() __attribute__((unlock_function));
+  struct LOCKABLE mutex {
+    void lock() EXCLUSIVE_LOCK_FUNCTION();
+    void unlock() UNLOCK_FUNCTION();
     // ...
   };
   bool foo();
   bool bar();
   mutex m;
   void test() {
     m.lock();
     while (foo()) {

 struct Cell {
   T dummy_;
   Mutex* mu_;
 };
 
 class Foo {
 public:
   template <class T>
-  void elr(Cell<T>* c) __attribute__((exclusive_locks_required(c->mu_)));
+  void elr(Cell<T>* c) EXCLUSIVE_LOCKS_REQUIRED(c->mu_);
 
   void test();
 };
 
 template<class T>
 void Foo::elr(Cell<T>* c1) { }
 
 void Foo::test() {
   Cell<int> cell;
   elr(&cell); // \
     // expected-warning {{calling function 'elr<int>' requires holding mutex 'cell.mu_' exclusively}}
 }
 
 
 template<class T>
-void globalELR(Cell<T>* c) __attribute__((exclusive_locks_required(c->mu_)));
+void globalELR(Cell<T>* c) EXCLUSIVE_LOCKS_REQUIRED(c->mu_);
 
 template<class T>
 void globalELR(Cell<T>* c1) { }
 
 void globalTest() {
   Cell<int> cell;
   globalELR(&cell); // \
     // expected-warning {{calling function 'globalELR<int>' requires holding mutex 'cell.mu_' exclusively}}
 }
 
 
 template<class T>
-void globalELR2(Cell<T>* c) __attribute__((exclusive_locks_required(c->mu_)));
+void globalELR2(Cell<T>* c) EXCLUSIVE_LOCKS_REQUIRED(c->mu_);
 
 // second declaration
 template<class T>
 void globalELR2(Cell<T>* c2);
 
 template<class T>
 void globalELR2(Cell<T>* c3) { }
 
 // re-declaration after definition
 template<class T>
 void globalELR2(Cell<T>* c4);
 
 void globalTest2() {
   Cell<int> cell;
   globalELR2(&cell); // \
     // expected-warning {{calling function 'globalELR2<int>' requires holding mutex 'cell.mu_' exclusively}}
 }
 
 
 template<class T>
 class FooT {
 public:
-  void elr(Cell<T>* c) __attribute__((exclusive_locks_required(c->mu_)));
+  void elr(Cell<T>* c) EXCLUSIVE_LOCKS_REQUIRED(c->mu_);
 };
 
 template<class T>
 void FooT<T>::elr(Cell<T>* c1) { }
 
 void testFooT() {
   Cell<int> cell;
   FooT<int> foo;

 
 namespace NonMemberCalleeICETest {
 
 class A {
   void Run() {
   (RunHelper)();  // expected-warning {{calling function 'RunHelper' requires holding mutex 'M' exclusively}}
  }
 
- void RunHelper() __attribute__((exclusive_locks_required(M)));
+ void RunHelper() EXCLUSIVE_LOCKS_REQUIRED(M);
  Mutex M;
 };
 
 }  // end namespace NonMemberCalleeICETest
 
 
 namespace pt_guard_attribute_type {
   int i PT_GUARDED_BY(sls_mu);  // expected-warning {{'pt_guarded_by' only applies to pointer types; type here is 'int'}}

 
 }   // end namespace NegativeRequirements
 
 
 namespace NegativeThreadRoles {
 
 typedef int __attribute__((capability("role"))) ThreadRole;
 
-void acquire(ThreadRole R) __attribute__((exclusive_lock_function(R))) __attribute__((no_thread_safety_analysis)) {}
-void release(ThreadRole R) __attribute__((unlock_function(R))) __attribute__((no_thread_safety_analysis)) {}
+void acquire(ThreadRole R) EXCLUSIVE_LOCK_FUNCTION(R) NO_THREAD_SAFETY_ANALYSIS {}
+void release(ThreadRole R) UNLOCK_FUNCTION(R) NO_THREAD_SAFETY_ANALYSIS {}
 
 ThreadRole FlightControl, Logger;
 
 extern void enque_log_msg(const char *msg);
 void log_msg(const char *msg) {
   enque_log_msg(msg);
 }