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

David Chisnall David.Chisnall at cl.cam.ac.uk
Fri Jul 26 02:41:53 PDT 2013


Yes, that's the sort of thing.  c_thread_safety_attributes would probably be a bit more in keeping with the naming convention for __has_feature entries.

On a mostly unrelated issue, we can do clang -E -dM to find out the list of predefined macros, but the only way of finding the valid __has_feature() and __has_extension() values that I know of is to read the source code.  It would be good if there was some command line option to output all of these...

David

On 26 Jul 2013, at 02:03, Alex Wang <alexw at nicira.com> wrote:

> Hey David,
> 
> Sorry for the delayed reply and thanks for your explanation,
> 
> I did modification below to add this new "c_wthread_safety" so that it can be checked by "__has_feature()". Is this what you mean?
> 
> Hope to hear more comments,
> 
> Kind Regards,
> Alex Wang,
> 
> Index: lib/Lex/PPMacroExpansion.cpp
> ===================================================================
> --- lib/Lex/PPMacroExpansion.cpp	(revision 186655)
> +++ lib/Lex/PPMacroExpansion.cpp	(working copy)
> @@ -727,6 +727,7 @@
>             .Case("attribute_unavailable_with_message", true)
>             .Case("attribute_unused_on_fields", true)
>             .Case("blocks", LangOpts.Blocks)
> +           .Case("c_wthread_safety", true)
>             .Case("cxx_exceptions", LangOpts.Exceptions)
>             .Case("cxx_rtti", LangOpts.RTTI)
>             .Case("enumerator_attributes", true)
> 
> 
> 
> On Thu, Jul 25, 2013 at 2:57 PM, David Chisnall <David.Chisnall at cl.cam.ac.uk> wrote:
> 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
> >
> >
> 
> 





More information about the cfe-dev mailing list