jamesr updated this revision to Diff 50481.
jamesr added a comment.
This patch guards the new annotations with
_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS and adds a number of tests to check
that the annotations produce errors when the annotations are enabled that code
violates the thread safety rules, that correct code does not produce errors,
and that code that does not enable the annotations does not produce errors even
in code that the thread-safety feature in Clang does not understand.
The feature guard is state in the positive as opposed to the negative as the
annotations may be incompatible with correct code in some situations.
http://reviews.llvm.org/D14731
Files:
include/__config
include/__mutex_base
test/libcxx/test/format.py
test/libcxx/thread/thread.mutex/thread_safety_access_guarded_without_lock.fail.cpp
test/libcxx/thread/thread.mutex/thread_safety_annotations_not_enabled.cpp
test/libcxx/thread/thread.mutex/thread_safety_call_requires_capability_without_having.fail.cpp
test/libcxx/thread/thread.mutex/thread_safety_lock_guard.cpp
test/libcxx/thread/thread.mutex/thread_safety_lock_unlock.cpp
test/libcxx/thread/thread.mutex/thread_safety_missing_unlock.fail.cpp
test/libcxx/thread/thread.mutex/thread_safety_requires_capability.cpp
Index: test/libcxx/thread/thread.mutex/thread_safety_requires_capability.cpp
===================================================================
--- /dev/null
+++ test/libcxx/thread/thread.mutex/thread_safety_requires_capability.cpp
@@ -0,0 +1,22 @@
+#if !defined(__clang__) || !__has_attribute(acquire_capability)
+// This test is only meaningful on versions of clang that understand thread
+// safety annotations.
+
+#define _LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS
+
+#include <mutex>
+
+std::mutex m;
+int foo __attribute__((guarded_by(m)));
+
+void increment() __attribute__((requires_capability(m))) {
+ foo++;
+}
+
+int main() {
+ m.lock();
+ increment();
+ m.unlock();
+}
+
+#endif
Index: test/libcxx/thread/thread.mutex/thread_safety_missing_unlock.fail.cpp
===================================================================
--- /dev/null
+++ test/libcxx/thread/thread.mutex/thread_safety_missing_unlock.fail.cpp
@@ -0,0 +1,15 @@
+#if !defined(__clang__) || !__has_attribute(acquire_capability)
+// This test is only meaningful on versions of clang that understand thread
+// safety annotations.
+#error Test only supported on clang versions that support the acquire_capability annotation
+#endif
+
+#define _LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS
+
+#include <mutex>
+
+std::mutex m;
+
+int main() {
+ m.lock();
+} // expected-error {{mutex 'm' is still held at the end of function}}
Index: test/libcxx/thread/thread.mutex/thread_safety_lock_unlock.cpp
===================================================================
--- /dev/null
+++ test/libcxx/thread/thread.mutex/thread_safety_lock_unlock.cpp
@@ -0,0 +1,18 @@
+#if !defined(__clang__) || !__has_attribute(acquire_capability)
+// This test is only meaningful on versions of clang that understand thread
+// safety annotations.
+
+#define _LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS
+
+#include <mutex>
+
+std::mutex m;
+int foo __attribute__((guarded_by(m)));
+
+int main() {
+ m.lock();
+ foo++;
+ m.unlock();
+}
+
+#endif
Index: test/libcxx/thread/thread.mutex/thread_safety_lock_guard.cpp
===================================================================
--- /dev/null
+++ test/libcxx/thread/thread.mutex/thread_safety_lock_guard.cpp
@@ -0,0 +1,17 @@
+#if !defined(__clang__) || !__has_attribute(acquire_capability)
+// This test is only meaningful on versions of clang that understand thread
+// safety annotations.
+
+#define _LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS
+
+#include <mutex>
+
+std::mutex m;
+int foo __attribute__((guarded_by(m)));
+
+int main() {
+ std::lock_guard<std::mutex> lock(m);
+ foo++;
+}
+
+#endif
Index: test/libcxx/thread/thread.mutex/thread_safety_call_requires_capability_without_having.fail.cpp
===================================================================
--- /dev/null
+++ test/libcxx/thread/thread.mutex/thread_safety_call_requires_capability_without_having.fail.cpp
@@ -0,0 +1,20 @@
+#if !defined(__clang__) || !__has_attribute(acquire_capability)
+// This test is only meaningful on versions of clang that understand thread
+// safety annotations.
+#error Test only supported on clang versions that support the acquire_capability annotation
+#endif
+
+#define _LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS
+
+#include <mutex>
+
+std::mutex m;
+int foo __attribute__((guarded_by(m)));
+
+void increment() __attribute__((requires_capability(m))) {
+ foo++;
+}
+
+int main() {
+ increment(); // expected-error{{calling function 'increment' requires holding mutex 'm' exclusively}}
+}
Index: test/libcxx/thread/thread.mutex/thread_safety_annotations_not_enabled.cpp
===================================================================
--- /dev/null
+++ test/libcxx/thread/thread.mutex/thread_safety_annotations_not_enabled.cpp
@@ -0,0 +1,13 @@
+// This test does not define _LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS so it
+// should compile without any warnings or errors even though this pattern is not
+// understood by the thread safety annotations.
+
+#include <mutex>
+
+int main() {
+ std::mutex m;
+ m.lock();
+ {
+ std::unique_lock<std::mutex> g(m, std::adopt_lock);
+ }
+}
Index: test/libcxx/thread/thread.mutex/thread_safety_access_guarded_without_lock.fail.cpp
===================================================================
--- /dev/null
+++ test/libcxx/thread/thread.mutex/thread_safety_access_guarded_without_lock.fail.cpp
@@ -0,0 +1,16 @@
+#if !defined(__clang__) || !__has_attribute(acquire_capability)
+// This test is only meaningful on versions of clang that understand thread
+// safety annotations.
+#error Test only supported on clang versions that support the acquire_capability annotation
+#endif
+
+#define _LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS
+
+#include <mutex>
+
+std::mutex m;
+int foo __attribute__((guarded_by(m)));
+
+int main() {
+ foo++; // expected-error{{writing variable 'foo' requires holding mutex 'm' exclusively}}
+}
Index: test/libcxx/test/format.py
===================================================================
--- test/libcxx/test/format.py
+++ test/libcxx/test/format.py
@@ -167,7 +167,7 @@
# when using Clang.
extra_flags = []
if self.cxx.type != 'gcc':
- extra_flags += ['-fsyntax-only']
+ extra_flags += ['-fsyntax-only', '-Werror=thread-safety']
if use_verify:
extra_flags += ['-Xclang', '-verify',
'-Xclang', '-verify-ignore-unexpected=note']
Index: include/__mutex_base
===================================================================
--- include/__mutex_base
+++ include/__mutex_base
@@ -26,7 +26,15 @@
#ifndef _LIBCPP_HAS_NO_THREADS
-class _LIBCPP_TYPE_VIS mutex
+#ifndef _LIBCPP_THREAD_SAFETY_ANNOTATION
+# ifdef _LIBCPP_HAS_THREAD_SAFETY_ANNOTATIONS
+# define _LIBCPP_THREAD_SAFETY_ANNOTATION(x) __attribute__((x))
+# else
+# define _LIBCPP_THREAD_SAFETY_ANNOTATION(x)
+# endif
+#endif // _LIBCPP_THREAD_SAFETY_ANNOTATION
+
+class _LIBCPP_TYPE_VIS _LIBCPP_THREAD_SAFETY_ANNOTATION(capability("mutex")) mutex
{
pthread_mutex_t __m_;
@@ -44,9 +52,9 @@
mutex& operator=(const mutex&);// = delete;
public:
- void lock();
- bool try_lock() _NOEXCEPT;
- void unlock() _NOEXCEPT;
+ void lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability());
+ bool try_lock() _NOEXCEPT _LIBCPP_THREAD_SAFETY_ANNOTATION(try_acquire_capability(true));
+ void unlock() _NOEXCEPT _LIBCPP_THREAD_SAFETY_ANNOTATION(release_capability());
typedef pthread_mutex_t* native_handle_type;
_LIBCPP_INLINE_VISIBILITY native_handle_type native_handle() {return &__m_;}
@@ -71,7 +79,7 @@
#endif
template <class _Mutex>
-class _LIBCPP_TYPE_VIS_ONLY lock_guard
+class _LIBCPP_TYPE_VIS_ONLY _LIBCPP_THREAD_SAFETY_ANNOTATION(scoped_lockable) lock_guard
{
public:
typedef _Mutex mutex_type;
@@ -81,13 +89,13 @@
public:
_LIBCPP_INLINE_VISIBILITY
- explicit lock_guard(mutex_type& __m)
+ explicit lock_guard(mutex_type& __m) _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability(__m))
: __m_(__m) {__m_.lock();}
_LIBCPP_INLINE_VISIBILITY
- lock_guard(mutex_type& __m, adopt_lock_t)
+ lock_guard(mutex_type& __m, adopt_lock_t) _LIBCPP_THREAD_SAFETY_ANNOTATION(requires_capability(__m))
: __m_(__m) {}
_LIBCPP_INLINE_VISIBILITY
- ~lock_guard() {__m_.unlock();}
+ ~lock_guard() _LIBCPP_THREAD_SAFETY_ANNOTATION(release_capability()) {__m_.unlock();}
private:
lock_guard(lock_guard const&);// = delete;
Index: include/__config
===================================================================
--- include/__config
+++ include/__config
@@ -859,6 +859,11 @@
#endif
#endif
+#if (defined(_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS) && defined(__clang__) \
+ && __has_attribute(acquire_capability))
+#define _LIBCPP_HAS_THREAD_SAFETY_ANNOTATIONS
+#endif
+
#endif // __cplusplus
#endif // _LIBCPP_CONFIG
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
