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

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

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

alexw
This commit adds test to the -Wthread-safety check for C code.

Co-authored-by: Ethan Jackson <[hidden email]>
Signed-off-by: Alex Wang <[hidden email]>

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
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] Thread-safety: Enable -Wthread-safety check for c.

alexw
Before this commit, the -Wthread-safety check is only for C++ code.
This commit makes it available for C code as well.

Co-authored-by: Ethan Jackson <[hidden email]>
Signed-off-by: Alex Wang <[hidden email]>

Index: SemaDeclAttr.cpp
===================================================================
--- SemaDeclAttr.cpp (revision 186655)
+++ SemaDeclAttr.cpp (working copy)
@@ -586,7 +586,7 @@
     return false;
 
   // FIXME: Lockable structs for C code.
-  if (!isa<CXXRecordDecl>(D)) {
+  if (!isa<RecordDecl>(D)) {
     S.Diag(Attr.getLoc(), diag::warn_thread_attribute_wrong_decl_type)
       << Attr.getName() << ThreadExpectedClassOrStruct;
     return false;
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Thread-safety: Enable -Wthread-safety check for c.

David Blaikie
Please include the tests and product code change in the same patch (&
ideally attach the patch at least (including it inline in the email is
optional) or consider using Phabricator

For details:
http://llvm.org/docs/DeveloperPolicy.html
http://llvm.org/docs/Phabricator.html

On Wed, Jul 24, 2013 at 6:16 PM, Alex Wang <[hidden email]> wrote:

> Before this commit, the -Wthread-safety check is only for C++ code.
> This commit makes it available for C code as well.
>
> Co-authored-by: Ethan Jackson <[hidden email]>
> Signed-off-by: Alex Wang <[hidden email]>
>
> Index: SemaDeclAttr.cpp
> ===================================================================
> --- SemaDeclAttr.cpp    (revision 186655)
> +++ SemaDeclAttr.cpp    (working copy)
> @@ -586,7 +586,7 @@
>      return false;
>
>    // FIXME: Lockable structs for C code.
> -  if (!isa<CXXRecordDecl>(D)) {
> +  if (!isa<RecordDecl>(D)) {
>      S.Diag(Attr.getLoc(), diag::warn_thread_attribute_wrong_decl_type)
>        << Attr.getName() << ThreadExpectedClassOrStruct;
>      return false;
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Thread-safety: Enable -Wthread-safety check for c.

alexw
Hey David,

Thanks for your quick response.

Please check the attachment for the combined patch.

Kind Regards,
Alex Wang,

==================================================

Before this commit, the -Wthread-safety check is only for C++ code.

This commit makes it available for C code and adds tests.

Co-authored-by: Ethan Jackson <[hidden email]>
Signed-off-by: Alex Wang <[hidden email]>


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

wthread_safety_for_c.diff (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

David Chisnall-4
In reply to this post by alexw
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 <[hidden email]> wrote:

> This commit adds test to the -Wthread-safety check for C code.
>
> Co-authored-by: Ethan Jackson <[hidden email]>
> Signed-off-by: Alex Wang <[hidden email]>
>
> 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
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

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

alexw
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 <[hidden email]> 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 <[hidden email]> wrote:

> This commit adds test to the -Wthread-safety check for C code.
>
> Co-authored-by: Ethan Jackson <[hidden email]>
> Signed-off-by: Alex Wang <[hidden email]>
>
> 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
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

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

David Chisnall-4
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> wrote:
>
> > This commit adds test to the -Wthread-safety check for C code.
> >
> > Co-authored-by: Ethan Jackson <[hidden email]>
> > Signed-off-by: Alex Wang <[hidden email]>
> >
> > 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
> > [hidden email]
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

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

alexw
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> wrote:
>
> > This commit adds test to the -Wthread-safety check for C code.
> >
> > Co-authored-by: Ethan Jackson <[hidden email]>
> > Signed-off-by: Alex Wang <[hidden email]>
> >
> > 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
> > [hidden email]
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

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

David Chisnall-4
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> wrote:
> >
> > > This commit adds test to the -Wthread-safety check for C code.
> > >
> > > Co-authored-by: Ethan Jackson <[hidden email]>
> > > Signed-off-by: Alex Wang <[hidden email]>
> > >
> > > 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
> > > [hidden email]
> > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> >
> >
>
>


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

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

alexw
Hey David,

Thanks for the comments,

On Fri, Jul 26, 2013 at 2:41 AM, David Chisnall <[hidden email]> wrote:
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.

Great! ;D
I attach the entire patch again with this reply.

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...

I can hack on this over my spare time. Right now, we eagerly look forward to having "c_thread_safety_attributes". Is there anything else you think required for applying the patch?

Kind Regards,
Alex Wang,


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

c_thread_safety_attributes.diff (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Thread-safety: Enable -Wthread-safety check for c.

David Blaikie
In reply to this post by alexw
This was committed in r187365, but the test case was found to be flawed in 199347 - would you mind fixing it?


On Wed, Jul 24, 2013 at 8:42 PM, Alex Wang <[hidden email]> wrote:
Hey David,

Thanks for your quick response.

Please check the attachment for the combined patch.

Kind Regards,
Alex Wang,

==================================================

Before this commit, the -Wthread-safety check is only for C++ code.

This commit makes it available for C code and adds tests.


Co-authored-by: Ethan Jackson <[hidden email]>
Signed-off-by: Alex Wang <[hidden email]>



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Thread-safety: Enable -Wthread-safety check for c.

alexw
Hey David,

Yes, i'll take a look and fix it.  Thx for the update.

Alex Wang,

On Wed, Jan 15, 2014 at 6:52 PM, David Blaikie <[hidden email]> wrote:
This was committed in r187365, but the test case was found to be flawed in 199347 - would you mind fixing it?


On Wed, Jul 24, 2013 at 8:42 PM, Alex Wang <[hidden email]> wrote:
Hey David,

Thanks for your quick response.

Please check the attachment for the combined patch.

Kind Regards,
Alex Wang,

==================================================

Before this commit, the -Wthread-safety check is only for C++ code.

This commit makes it available for C code and adds tests.


Co-authored-by: Ethan Jackson <[hidden email]>
Signed-off-by: Alex Wang <[hidden email]>




_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Thread-safety: Enable -Wthread-safety check for c.

alexw
Hey David,

Please see the attachment for fix patch.

Sorry again, I must have run the wrong test the first time.
The test is flawed at the beginning.

I tested the fix, output:
root@server329:~/llvm/llvm/tools/clang/test/Sema# ~/llvm/llvm/utils/lit/lit.py -v   warn-
thread-safety-analysis.c
lit.py: lit.cfg:196: note: using clang: '/root/llvm/llvm/Debug+Asserts/bin/clang'
-- Testing: 1 tests, 1 threads --
PASS: Clang :: Sema/warn-thread-safety-analysis.c (1 of 1)
Testing Time: 0.03s
  Expected Passes    : 1

Thanks,
Alex Wang,
test_fix_wthread_safety_for_c.diff
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Thread-safety: Enable -Wthread-safety check for c.

David Blaikie
Thanks for that.

Committed as r199762.


On Thu, Jan 16, 2014 at 1:51 PM, alexw <[hidden email]> wrote:
Hey David,

Please see the attachment for fix patch.

Sorry again, I must have run the wrong test the first time.
The test is flawed at the beginning.

I tested the fix, output:
root@server329:~/llvm/llvm/tools/clang/test/Sema#
~/llvm/llvm/utils/lit/lit.py -v   warn-
thread-safety-analysis.c
lit.py: lit.cfg:196: note: using clang:
'/root/llvm/llvm/Debug+Asserts/bin/clang'
-- Testing: 1 tests, 1 threads --
PASS: Clang :: Sema/warn-thread-safety-analysis.c (1 of 1)
Testing Time: 0.03s
  Expected Passes    : 1

Thanks,
Alex Wang,
test_fix_wthread_safety_for_c.diff
<http://clang-developers.42468.n3.nabble.com/file/n4037219/test_fix_wthread_safety_for_c.diff>



--
View this message in context: http://clang-developers.42468.n3.nabble.com/PATCH-2-2-Thread-safety-Add-test-for-Wthread-safety-check-for-C-tp4033444p4037219.html
Sent from the Clang Developers mailing list archive at Nabble.com.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Thread-safety: Enable -Wthread-safety check for c.

Alp Toker
Thanks guys

On 21/01/2014 19:12, David Blaikie wrote:

> Thanks for that.
>
> Committed as r199762.
>
>
> On Thu, Jan 16, 2014 at 1:51 PM, alexw <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Hey David,
>
>     Please see the attachment for fix patch.
>
>     Sorry again, I must have run the wrong test the first time.
>     The test is flawed at the beginning.
>
>     I tested the fix, output:
>     root@server329:~/llvm/llvm/tools/clang/test/Sema#
>     ~/llvm/llvm/utils/lit/lit.py -v   warn-
>     thread-safety-analysis.c
>     lit.py: lit.cfg:196: note: using clang:
>     '/root/llvm/llvm/Debug+Asserts/bin/clang'
>     -- Testing: 1 tests, 1 threads --
>     PASS: Clang :: Sema/warn-thread-safety-analysis.c (1 of 1)
>     Testing Time: 0.03s
>       Expected Passes    : 1
>
>     Thanks,
>     Alex Wang,
>     test_fix_wthread_safety_for_c.diff
>     <http://clang-developers.42468.n3.nabble.com/file/n4037219/test_fix_wthread_safety_for_c.diff>
>
>
>
>     --
>     View this message in context:
>     http://clang-developers.42468.n3.nabble.com/PATCH-2-2-Thread-safety-Add-test-for-Wthread-safety-check-for-C-tp4033444p4037219.html
>     Sent from the Clang Developers mailing list archive at Nabble.com.
>     _______________________________________________
>     cfe-dev mailing list
>     [hidden email] <mailto:[hidden email]>
>     http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
>
>
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

--
http://www.nuanti.com
the browser experts

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev