[cfe-dev] [PATCH 2/2] Thread-safety: Add test for -Wthread-safety check for C.

[David Chisnall]

Hi Alex,

I think you misunderstand.  If I want to use these in, for example, pthread.h, then I want to make sure that the attributes are only used when a compiler that knows about them is being used.  Clang provides the __has_feature() mechanism in the preprocessor for these checks at a coarse granularity, so that library headers can conditionally expose compiler and compiler-version specific features.

How do I check, from within a header, that I am using a version of clang that supports thread-safety annotations in C?

David

On 25 Jul 2013, at 18:04, Alex Wang <alexw at nicira.com> wrote:

> Hey David,
> 
> Thanks for your reply, please correct me if my understanding is wrong,
> 
> Since I only use the thread safety attributes defined in clang, there should be no requirement on gcc extension.
> 
> Actually, I think it does not make very much sense to check for each of the attributes individually, since we only use these attributes for semantic analysis and your cpp test (warn-thread-safety-analysis.cpp) is every detailed.
> 
> Kind Regards,
> Alex Wang,
> 
> 
> On Thu, Jul 25, 2013 at 4:31 AM, David Chisnall <David.Chisnall at cl.cam.ac.uk> wrote:
> Hi,
> 
> What is the __has_extension() variable to check for support for these?  Or do we need to check for each of the attributes individually?
> 
> David
> 
> On 25 Jul 2013, at 02:16, Alex Wang <alexw at nicira.com> wrote:
> 
> > This commit adds test to the -Wthread-safety check for C code.
> >
> > Co-authored-by: Ethan Jackson <ethan at nicira.com>
> > Signed-off-by: Alex Wang <alexw at nicira.com>
> >
> > Index: warn-thread-safety-analysis.c
> > ===================================================================
> > --- warn-thread-safety-analysis.c     (revision 0)
> > +++ warn-thread-safety-analysis.c     (revision 0)
> > @@ -0,0 +1,132 @@
> > +// RUN: %clang -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta -fcxx-exceptions %s
> > +
> > +#include <assert.h>
> > +
> > +#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__)))
> > +#define ASSERT_EXCLUSIVE_LOCK(...)      __attribute__ ((assert_exclusive_lock(__VA_ARGS__)))
> > +#define ASSERT_SHARED_LOCK(...)         __attribute__ ((assert_shared_lock(__VA_ARGS__)))
> > +#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 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))
> > +
> > +// Define the mutex struct.
> > +// Simplified only for test purpose.
> > +struct LOCKABLE Mutex {};
> > +
> > +struct Foo {
> > +  struct Mutex *mu_;
> > +};
> > +
> > +// Define mutex lock/unlock functions.
> > +void mutex_exclusive_lock(struct Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu) {
> > +}
> > +
> > +void mutex_shared_lock(struct Mutex *mu) SHARED_LOCK_FUNCTION(mu) {
> > +}
> > +
> > +void mutex_unlock(struct Mutex *mu) UNLOCK_FUNCTION(mu) {
> > +}
> > +
> > +// Define global variables.
> > +struct Mutex mu1;
> > +struct Mutex mu2 ACQUIRED_AFTER(mu1);
> > +struct Foo foo_ = {&mu1};
> > +int a_ GUARDED_BY(foo_.mu_);
> > +int *b_ PT_GUARDED_BY(foo_.mu_) = &a_;
> > +int c_ GUARDED_VAR;
> > +int *d_ PT_GUARDED_VAR = &c_;
> > +
> > +// Define test functions.
> > +int Foo_fun1(int i) SHARED_LOCKS_REQUIRED(mu2) EXCLUSIVE_LOCKS_REQUIRED(mu1) {
> > +  return i;
> > +}
> > +
> > +int Foo_fun2(int i) EXCLUSIVE_LOCKS_REQUIRED(mu2) SHARED_LOCKS_REQUIRED(mu1) {
> > +  return i;
> > +}
> > +
> > +int Foo_func3(int i) LOCKS_EXCLUDED(mu1, mu2) {
> > +  return i;
> > +}
> > +
> > +static int Bar_fun1(int i) EXCLUSIVE_LOCKS_REQUIRED(mu1) {
> > +  return i;
> > +}
> > +
> > +void set_value(int *a, int value) EXCLUSIVE_LOCKS_REQUIRED(foo_.mu_) {
> > +  *a = value;
> > +}
> > +
> > +int get_value(int *p) SHARED_LOCKS_REQUIRED(foo_.mu_){
> > +  return *p;
> > +}
> > +
> > +int main() {
> > +
> > +  Foo_fun1(1); // \
> > +      // expected-warning{{calling function 'Foo_fun1' requires shared lock on 'mu2'}}
> > +      // expected-warning{{calling function 'Foo_fun1' requires exclusive lock on 'mu1'}}
> > +
> > +  mutex_exclusive_lock(&mu1);
> > +  mutex_shared_lock(&mu2);
> > +  Foo_fun1(1);
> > +
> > +  mutex_shared_lock(&mu1);  // \
> > +      // expected-warning{{locking 'mu1' that is already locked}}
> > +  mutex_unlock(&mu1);
> > +  mutex_unlock(&mu2);
> > +  mutex_shared_lock(&mu1);
> > +  mutex_exclusive_lock(&mu2);
> > +  Foo_fun2(2);
> > +
> > +  mutex_unlock(&mu2);
> > +  mutex_unlock(&mu1);
> > +  mutex_exclusive_lock(&mu1);
> > +  Bar_fun1(3);
> > +  mutex_unlock(&mu1);
> > +
> > +  mutex_exclusive_lock(&mu1);
> > +  Foo_func3(4);  // \
> > +      // expected-warning{{cannot call function 'Foo_func3' while mutex 'mu1' is locked}}
> > +  mutex_unlock(&mu1);
> > +
> > +  Foo_func3(5);
> > +
> > +  set_value(&a_, 0);  // \
> > +      // expected-warning{{calling function 'setA' requires exclusive lock on 'foo_.mu_'}}
> > +  get_value(b_);  // \
> > +      // expected-warning{{calling function 'getB' requires shared lock on 'foo_.mu_'}}
> > +  mutex_exclusive_lock(foo_.mu_);
> > +  set_value(&a_, 1);
> > +  mutex_unlock(foo_.mu_);
> > +  mutex_shared_lock(foo_.mu_);
> > +  assert(get_value(b_) == 1);
> > +  mutex_unlock(foo_.mu_);
> > +
> > +  c_ = 0;  // \
> > +      // expected-warning{{writing variable 'c_' requires locking any mutex exclusively}}
> > +  assert(*d_ == 0);  // \
> > +      // expected-warning{{reading the value pointed to by 'd_' requires locking any mutex}}
> > +  mutex_exclusive_lock(foo_.mu_);
> > +  c_ = 1;
> > +  assert(*d_ == 1);
> > +  mutex_unlock(foo_.mu_);
> > +
> > +  return 0;
> > +}
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> 
>