[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
This revision was automatically updated to reflect the committed changes. Closed by commit rL293197: Disable thread safety analysis for some functions in __thread_support (authored by dim). Changed prior to commit: https://reviews.llvm.org/D28520?vs=85794=85939#toc Repository: rL LLVM https://reviews.llvm.org/D28520 Files: libcxx/trunk/include/__threading_support Index: libcxx/trunk/include/__threading_support === --- libcxx/trunk/include/__threading_support +++ libcxx/trunk/include/__threading_support @@ -40,6 +40,12 @@ #define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY #endif +#if defined(__FreeBSD__) && defined(__clang__) && __has_attribute(no_thread_safety_analysis) +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) +#else +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS +#endif + _LIBCPP_BEGIN_NAMESPACE_STD #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) @@ -102,25 +108,25 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS bool __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_lock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS bool __libcpp_mutex_trylock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_unlock(__libcpp_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY @@ -133,10 +139,10 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m, timespec *__ts); Index: libcxx/trunk/include/__threading_support === --- libcxx/trunk/include/__threading_support +++ libcxx/trunk/include/__threading_support @@ -40,6 +40,12 @@ #define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY #endif +#if defined(__FreeBSD__) && defined(__clang__) && __has_attribute(no_thread_safety_analysis) +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) +#else +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS +#endif + _LIBCPP_BEGIN_NAMESPACE_STD #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) @@ -102,25 +108,25 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS bool __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_lock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS bool __libcpp_mutex_trylock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_unlock(__libcpp_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY @@ -133,10 +139,10 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Thank you for being patient while we figured this out. :-) https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
delesley added a comment. This looks good to me. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim updated this revision to Diff 85794. dim added a comment. Back to the previous version, using `no_thread_safety_analysis` for FreeBSD only. Optionally, we could delete the `defined(__FreeBSD__)` part, and disable the analysis for all platforms. https://reviews.llvm.org/D28520 Files: include/__threading_support Index: include/__threading_support === --- include/__threading_support +++ include/__threading_support @@ -40,6 +40,12 @@ #define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY #endif +#if defined(__FreeBSD__) && defined(__clang__) && __has_attribute(no_thread_safety_analysis) +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) +#else +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS +#endif + _LIBCPP_BEGIN_NAMESPACE_STD #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) @@ -102,25 +108,25 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS bool __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_lock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS bool __libcpp_mutex_trylock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_unlock(__libcpp_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY @@ -133,10 +139,10 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m, timespec *__ts); Index: include/__threading_support === --- include/__threading_support +++ include/__threading_support @@ -40,6 +40,12 @@ #define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY #endif +#if defined(__FreeBSD__) && defined(__clang__) && __has_attribute(no_thread_safety_analysis) +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) +#else +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS +#endif + _LIBCPP_BEGIN_NAMESPACE_STD #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) @@ -102,25 +108,25 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS bool __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_lock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS bool __libcpp_mutex_trylock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_unlock(__libcpp_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY @@ -133,10 +139,10 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m, timespec *__ts);
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim added a comment. In https://reviews.llvm.org/D28520#656202, @delesley wrote: > The big question for me is whether these functions are exposed as part of the > public libcxx API, or whether they are only used internally. As far as I can see, they are only used internally, in `src/algorithm.cpp` and `src/mutex.cpp`. Of course they aren't "hidden" in any way in the `include/__thread_support` header, but obviously functions starting with double underscores are not meant as a public API. > If they are only used internally, then no_thread_safety_analysis is probably > the correct option. (Unless, of course, the libcxx developers want to start > using the analysis themselves, but I suspect they don't.) Yes, and since @EricWF also prefers this, I going to switch this review back to the previous form again. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
delesley added a comment. The big question for me is whether these functions are exposed as part of the public libcxx API, or whether they are only used internally. (Again, I'm not a libcxx expert.) If they are part of the public API, then users may want to switch them on and off in their own code. In that case, I'm happy with the current variant. If they are only used internally, then no_thread_safety_analysis is probably the correct option. (Unless, of course, the libcxx developers want to start using the analysis themselves, but I suspect they don't.) https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
EricWF added a comment. @dim I would really rather just suppress these warnings if we want them merged into 4.0. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim updated this revision to Diff 85440.
dim added a comment.
In https://reviews.llvm.org/D28520#653360, @aaron.ballman wrote:
> In https://reviews.llvm.org/D28520#652607, @dim wrote:
>
> >
>
[...]
>> I'm really open to any variant, as long as something that works can get in
>> before the 4.0.0 release. :)
>
> I feel like there's still some confusion here (likely on my part). DeLesley
> was asking if there was a way to turn this off for everyone *except* FreeBSD
> (that is user-controllable), but your code turns it off specifically *for*
> FreeBSD with no option to enable. The part that has me confused is that
> FreeBSD is annotating their functionality specifically to enable thread
> safety checking; it seems like turning that checking off rather than
> supporting it is the wrong way to go. I think that may be why DeLesley was
> saying to disable the functionality for non-FreeBSD systems while still
> honoring it on FreeBSD, but if I'm confused, hopefully he'll clarify.
Ok, so let's go back to the previous variant, but with conditionals to be able
to turn checking on, if so desired, using
`_LIBCPP_ENABLE_THREAD_SAFETY_ATTRIBUTE`. Checking is on by default for
FreeBSD only, but can still be disabled explicitly by defining
`_LIBCPP_DISABLE_THREAD_SAFETY_ATTRIBUTE`.
https://reviews.llvm.org/D28520
Files:
include/__threading_support
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -40,14 +40,30 @@
#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
#endif
+#if defined(__FreeBSD__) && !defined(_LIBCPP_DISABLE_THREAD_SAFETY_ATTRIBUTE) && \
+!defined(_LIBCPP_ENABLE_THREAD_SAFETY_ATTRIBUTE)
+# define _LIBCPP_ENABLE_THREAD_SAFETY_ATTRIBUTE
+#endif
+
+#ifndef _LIBCPP_THREAD_SAFETY_ATTRIBUTE
+# if defined(__clang__) && __has_attribute(acquire_capability) && \
+ defined(_LIBCPP_ENABLE_THREAD_SAFETY_ATTRIBUTE)
+# define _LIBCPP_THREAD_SAFETY_ATTRIBUTE(x) __attribute__((x))
+# else
+# define _LIBCPP_THREAD_SAFETY_ATTRIBUTE(x)
+# endif
+#endif
+
_LIBCPP_BEGIN_NAMESPACE_STD
#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
// Mutex
-typedef pthread_mutex_t __libcpp_mutex_t;
+typedef pthread_mutex_t __libcpp_mutex_t
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(capability("mutex"));
#define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
-typedef pthread_mutex_t __libcpp_recursive_mutex_t;
+typedef pthread_mutex_t __libcpp_recursive_mutex_t
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(capability("mutex"));
// Condition Variable
typedef pthread_cond_t __libcpp_condvar_t;
@@ -71,10 +87,12 @@
#define _LIBCPP_TLS_DESTRUCTOR_CC
#else
// Mutex
-typedef SRWLOCK __libcpp_mutex_t;
+typedef SRWLOCK __libcpp_mutex_t
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(capability("mutex"));
#define _LIBCPP_MUTEX_INITIALIZER SRWLOCK_INIT
-typedef CRITICAL_SECTION __libcpp_recursive_mutex_t;
+typedef CRITICAL_SECTION __libcpp_recursive_mutex_t
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(capability("mutex"));
// Condition Variable
typedef CONDITION_VARIABLE __libcpp_condvar_t;
@@ -103,25 +121,31 @@
int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m);
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m);
+int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(acquire_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
-bool __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m);
+bool __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(try_acquire_capability(true, *__m));
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m);
+int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(release_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m);
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_mutex_lock(__libcpp_mutex_t *__m);
+int __libcpp_mutex_lock(__libcpp_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(acquire_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
-bool __libcpp_mutex_trylock(__libcpp_mutex_t *__m);
+bool __libcpp_mutex_trylock(__libcpp_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(try_acquire_capability(true, *__m));
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_mutex_unlock(__libcpp_mutex_t *__m);
+int __libcpp_mutex_unlock(__libcpp_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(release_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_mutex_destroy(__libcpp_mutex_t *__m);
@@ -134,11 +158,13 @@
int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv);
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m);
+int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m)
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
aaron.ballman added a comment. In https://reviews.llvm.org/D28520#652607, @dim wrote: > In https://reviews.llvm.org/D28520#648880, @delesley wrote: > > > Sorry about the slow response. My main concern here is that the thread > > safety analysis was designed for use with a library that wraps the system > > mutex in a separate Mutex class. We did that specifically to avoid > > breaking anything; code has to opt-in to the static checking by defining > > and using a Mutex class, and the API of that class is restricted to calls > > that can be easily checked via annotations. Including attributes directly > > in the standard library has the potential to cause lots of breakage and > > false positives. > > > Yes, I agree with that. However, on FreeBSD the pthread functions themselves > are already annotated, so the libc++ wrapper functions cause -Werror warnings > during the tests. Therefore one of my suggestions was to explicitly turn off > warnings using `no_thread_safety_analysis` attributes. Is there any > disadvantage in doing that? > > > Is there some way to control the #ifdefs so that the annotations are turned > > off by default for everyone except possibly freebsd, but there's still a > > way to turn them on for users who want to see the warnings? I'm not a > > libcxx expert. > > Yes, that was one of my other suggestions, using `#if defined(__FreeBSD__)` > to limit these attributes to FreeBSD only. We can do that either with the > `no_thread_safety_analysis` attributes, or with the 'real' annotations, > though the latter are not really useful in this case. > > I'm really open to any variant, as long as something that works can get in > before the 4.0.0 release. :) I feel like there's still some confusion here (likely on my part). DeLesley was asking if there was a way to turn this off for everyone *except* FreeBSD (that is user-controllable), but your code turns it off specifically *for* FreeBSD with no option to enable. The part that has me confused is that FreeBSD is annotating their functionality specifically to enable thread safety checking; it seems like turning that checking off rather than supporting it is the wrong way to go. I think that may be why DeLesley was saying to disable the functionality for non-FreeBSD systems while still honoring it on FreeBSD, but if I'm confused, hopefully he'll clarify. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim updated this revision to Diff 85283. dim added a comment. Let's try it with this much simpler version instead, which disables the thread safety analysis _only_ for FreeBSD, and nowhere else. And no messing with capabilities, either. https://reviews.llvm.org/D28520 Files: include/__threading_support Index: include/__threading_support === --- include/__threading_support +++ include/__threading_support @@ -40,6 +40,12 @@ #define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY #endif +#if defined(__FreeBSD__) && defined(__clang__) && __has_attribute(no_thread_safety_analysis) +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) +#else +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS +#endif + _LIBCPP_BEGIN_NAMESPACE_STD #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) @@ -102,25 +108,25 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS bool __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_lock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS bool __libcpp_mutex_trylock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_unlock(__libcpp_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY @@ -133,10 +139,10 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m, timespec *__ts); Index: include/__threading_support === --- include/__threading_support +++ include/__threading_support @@ -40,6 +40,12 @@ #define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY #endif +#if defined(__FreeBSD__) && defined(__clang__) && __has_attribute(no_thread_safety_analysis) +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) +#else +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS +#endif + _LIBCPP_BEGIN_NAMESPACE_STD #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) @@ -102,25 +108,25 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS bool __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_lock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS bool __libcpp_mutex_trylock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_unlock(__libcpp_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY @@ -133,10 +139,10 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m, timespec *__ts); ___
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim added a comment. In https://reviews.llvm.org/D28520#648880, @delesley wrote: > Sorry about the slow response. My main concern here is that the thread > safety analysis was designed for use with a library that wraps the system > mutex in a separate Mutex class. We did that specifically to avoid breaking > anything; code has to opt-in to the static checking by defining and using a > Mutex class, and the API of that class is restricted to calls that can be > easily checked via annotations. Including attributes directly in the > standard library has the potential to cause lots of breakage and false > positives. Yes, I agree with that. However, on FreeBSD the pthread functions themselves are already annotated, so the libc++ wrapper functions cause -Werror warnings during the tests. Therefore one of my suggestions was to explicitly turn off warnings using `no_thread_safety_analysis` attributes. Is there any disadvantage in doing that? > Is there some way to control the #ifdefs so that the annotations are turned > off by default for everyone except possibly freebsd, but there's still a way > to turn them on for users who want to see the warnings? I'm not a libcxx > expert. Yes, that was one of my other suggestions, using `#if defined(__FreeBSD__)` to limit these attributes to FreeBSD only. We can do that either with the `no_thread_safety_analysis` attributes, or with the 'real' annotations, though the latter are not really useful in this case. I'm really open to any variant, as long as something that works can get in before the 4.0.0 release. :) https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
delesley added a comment. Sorry about the slow response. My main concern here is that the thread safety analysis was designed for use with a library that wraps the system mutex in a separate Mutex class. We did that specifically to avoid breaking anything; code has to opt-in to the static checking by defining and using a Mutex class, and the API of that class is restricted to calls that can be easily checked via annotations. Including attributes directly in the standard library has the potential to cause lots of breakage and false positives. Is there some way to control the #ifdefs so that the annotations are turned off by default for everyone except possibly freebsd, but there's still a way to turn them on for users who want to see the warnings? I'm not a libcxx expert. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim added inline comments. Comment at: include/__threading_support:43 +#if defined(__clang__) && __has_attribute(acquire_capability) +#define _LIBCPP_THREAD_SAFETY_ATTRIBUTE(x) __attribute__((x)) I think the least intrusive way would be to add a `defined(__FreeBSD__)` here, as that is the only OS with thread annotations for pthread functions, as far as I know. The alternative is to turn off the capability annotations for `__libcpp_mutex_t` again, and use the `no_thread_safety_analysis` annotation for the functions. Any particular preference? https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim added a comment. Note that my earlier approach of just disabling -Wthread-safety for those few functions might be easier. This should not cause any trouble for any platforms which don't use annotated pthread functions. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim added a comment. In https://reviews.llvm.org/D28520#646564, @EricWF wrote: > This breaks on all platforms were pthread_mutex_t isn't annotated. Hm, sorry about that, I didn't realize. I'm building it on Ubuntu now to see what breaks, and how to fix it. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
EricWF added a comment. This breaks on all platforms were pthread_mutex_t isn't annotated. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim updated this revision to Diff 84465.
dim added a comment.
Rebase after recent changes.
https://reviews.llvm.org/D28520
Files:
include/__threading_support
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -40,14 +40,22 @@
#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
#endif
+#if defined(__clang__) && __has_attribute(acquire_capability)
+#define _LIBCPP_THREAD_SAFETY_ATTRIBUTE(x) __attribute__((x))
+#else
+#define _LIBCPP_THREAD_SAFETY_ATTRIBUTE(x) __attribute__((x))
+#endif
+
_LIBCPP_BEGIN_NAMESPACE_STD
#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
// Mutex
-typedef pthread_mutex_t __libcpp_mutex_t;
+typedef pthread_mutex_t __libcpp_mutex_t
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(capability("mutex"));
#define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
-typedef pthread_mutex_t __libcpp_recursive_mutex_t;
+typedef pthread_mutex_t __libcpp_recursive_mutex_t
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(capability("mutex"));
// Condition Variable
typedef pthread_cond_t __libcpp_condvar_t;
@@ -69,10 +77,12 @@
#define _LIBCPP_TLS_DESTRUCTOR_CC
#else
// Mutex
-typedef SRWLOCK __libcpp_mutex_t;
+typedef SRWLOCK __libcpp_mutex_t
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(capability("mutex"));
#define _LIBCPP_MUTEX_INITIALIZER SRWLOCK_INIT
-typedef CRITICAL_SECTION __libcpp_recursive_mutex_t;
+typedef CRITICAL_SECTION __libcpp_recursive_mutex_t
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(capability("mutex"));
// Condition Variable
typedef CONDITION_VARIABLE __libcpp_condvar_t;
@@ -99,25 +109,31 @@
int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m);
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m);
+int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(acquire_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
-bool __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m);
+bool __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(try_acquire_capability(true, *__m));
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m);
+int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(release_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m);
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_mutex_lock(__libcpp_mutex_t *__m);
+int __libcpp_mutex_lock(__libcpp_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(acquire_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
-bool __libcpp_mutex_trylock(__libcpp_mutex_t *__m);
+bool __libcpp_mutex_trylock(__libcpp_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(try_acquire_capability(true, *__m));
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_mutex_unlock(__libcpp_mutex_t *__m);
+int __libcpp_mutex_unlock(__libcpp_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(release_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_mutex_destroy(__libcpp_mutex_t *__m);
@@ -130,11 +146,13 @@
int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv);
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m);
+int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(requires_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m,
- timespec *__ts);
+ timespec *__ts)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(requires_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_condvar_destroy(__libcpp_condvar_t* __cv);
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -40,14 +40,22 @@
#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
#endif
+#if defined(__clang__) && __has_attribute(acquire_capability)
+#define _LIBCPP_THREAD_SAFETY_ATTRIBUTE(x) __attribute__((x))
+#else
+#define _LIBCPP_THREAD_SAFETY_ATTRIBUTE(x) __attribute__((x))
+#endif
+
_LIBCPP_BEGIN_NAMESPACE_STD
#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
// Mutex
-typedef pthread_mutex_t __libcpp_mutex_t;
+typedef pthread_mutex_t __libcpp_mutex_t
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(capability("mutex"));
#define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
-typedef pthread_mutex_t __libcpp_recursive_mutex_t;
+typedef pthread_mutex_t __libcpp_recursive_mutex_t
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(capability("mutex"));
// Condition Variable
typedef pthread_cond_t __libcpp_condvar_t;
@@ -69,10 +77,12 @@
#define _LIBCPP_TLS_DESTRUCTOR_CC
#else
// Mutex
-typedef SRWLOCK
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim added a comment. Actually, according to https://svnweb.freebsd.org/base?view=revision=270943 (where the annotations were added), this helped to uncover existing bugs. I don't see why it would interfere with anything; if you ask for -Wthread-safety warnings, you should get them, right? What use are the warnings otherwise? https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim updated this revision to Diff 84451.
dim added a comment.
Something like this might work, maybe. (I haven't yet run the full test suite,
as that takes quite a while on my machines.)
I did not re-use the `_LIBCPP_THREAD_SAFETY_ANNOTATION` macro from
`__mutex_base`, since that is included *after* this file, and it is only
enabled when the user defines `_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS`. The
`_LIBCPP_THREAD_SAFETY_ATTRIBUTE` macro is defined iff clang supports any of
the thread safety stuff.
Note that I had to add a capability attribute to the `__libcpp_mutex` types,
otherwise this would not work.
Also, I had to place the thread safety attributes *after* the function
declarations, since otherwise the compiler complains that it does not know the
parameter name `__m`.
Furthermore, I am still not sure whether the `true` value for the
`try_acquire_capability` attribute it correct. If I understand the
documentation correctly, `true` means that the function only succeeds when
trying the lock actually acquires it, and it fails when the lock was already
acquired.
https://reviews.llvm.org/D28520
Files:
include/__threading_support
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -40,14 +40,22 @@
#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
#endif
+#if defined(__clang__) && __has_attribute(acquire_capability))
+#define _LIBCPP_THREAD_SAFETY_ATTRIBUTE(x) __attribute__((x))
+#else
+#define _LIBCPP_THREAD_SAFETY_ATTRIBUTE(x) __attribute__((x))
+#endif
+
_LIBCPP_BEGIN_NAMESPACE_STD
#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
// Mutex
-typedef pthread_mutex_t __libcpp_mutex_t;
+typedef pthread_mutex_t __libcpp_mutex_t
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(capability("mutex"));
#define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
-typedef pthread_mutex_t __libcpp_recursive_mutex_t;
+typedef pthread_mutex_t __libcpp_recursive_mutex_t
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(capability("mutex"));
// Condition Variable
typedef pthread_cond_t __libcpp_condvar_t;
@@ -99,25 +107,31 @@
int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m);
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m);
+int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(acquire_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m);
+int __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(try_acquire_capability(true, *__m));
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m);
+int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(release_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m);
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_mutex_lock(__libcpp_mutex_t *__m);
+int __libcpp_mutex_lock(__libcpp_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(acquire_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_mutex_trylock(__libcpp_mutex_t *__m);
+int __libcpp_mutex_trylock(__libcpp_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(try_acquire_capability(true, *__m));
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_mutex_unlock(__libcpp_mutex_t *__m);
+int __libcpp_mutex_unlock(__libcpp_mutex_t *__m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(release_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_mutex_destroy(__libcpp_mutex_t *__m);
@@ -130,11 +144,13 @@
int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv);
_LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m);
+int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(requires_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m,
- timespec *__ts);
+ timespec *__ts)
+_LIBCPP_THREAD_SAFETY_ATTRIBUTE(requires_capability(*__m));
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_condvar_destroy(__libcpp_condvar_t* __cv);
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -40,14 +40,22 @@
#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
#endif
+#if defined(__clang__) && __has_attribute(acquire_capability))
+#define _LIBCPP_THREAD_SAFETY_ATTRIBUTE(x) __attribute__((x))
+#else
+#define _LIBCPP_THREAD_SAFETY_ATTRIBUTE(x) __attribute__((x))
+#endif
+
_LIBCPP_BEGIN_NAMESPACE_STD
#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
//
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim added a subscriber: ed. dim added a comment. In https://reviews.llvm.org/D28520#646160, @aaron.ballman wrote: > I feel like I must be missing something; why is this disabling rather than > specifying the thread safety behavior? e.g., `__libcpp_mutex_lock()` > specifying that it acquires the capability and `__libcpp_mutex_unlock()` > specifying that it releases it? I wasn't able to figure out how that should work. The Thread Safety Analysis documentation specifies a number of macros that can be used, but does not directly document the attributes themselves. It looks like some of the attributes have a slightly different signature than the actual pthread functions, and the `__libcpp` wrapper for them, e.g. the example for `TRY_ACQUIRE` seems to assume a `bool` return value: // Try to acquire the mutex. Returns true on success, and false on failure. bool TryLock() TRY_ACQUIRE(true); where `TRY_ACQUIRE(true)` gets replaced by `__attribute__ ((try_acquire_capability(true)))`. However, the signature for the `__libcpp` variant, and the "real" pthread function, is: int __libcpp_mutex_trylock(__libcpp_mutex_t *__m) so the return value is an `int`, where `0` means success, and any other value an error. I am unsure how this can be mapped to the attribute, and the documentation does not specify it. In https://reviews.llvm.org/D28520#646169, @EricWF wrote: > Also how is `pthread_mutex_t` getting annotated as a mutex type? Is it now > done automatically? Since a few years, FreeBSD's has locking annotations, for example https://svnweb.freebsd.org/base/head/include/pthread.h?view=markup#l228 which has: int pthread_mutex_init(pthread_mutex_t *__mutex, const pthread_mutexattr_t *) __requires_unlocked(*__mutex); int pthread_mutex_lock(pthread_mutex_t *__mutex) __locks_exclusive(*__mutex); int pthread_mutex_trylock(pthread_mutex_t *__mutex) __trylocks_exclusive(0, *__mutex); int pthread_mutex_timedlock(pthread_mutex_t *__mutex, const struct timespec *) __trylocks_exclusive(0, *__mutex); These annotations expand to `__attribute__((locks_excluded))`, `__attribute__((exclusive_lock_function))`, and so on, if `__has_extension(c_thread_safety_attributes)` is true. Apparently @ed added these. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
EricWF added a comment. In https://reviews.llvm.org/D28520#646160, @aaron.ballman wrote: > I feel like I must be missing something; why is this disabling rather than > specifying the thread safety behavior? e.g., `__libcpp_mutex_lock()` > specifying that it acquires the capability and `__libcpp_mutex_unlock()` > specifying that it releases it? I agree. Also how is `pthread_mutex_t` getting annotated as a mutex type? Is it now done automatically? https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
aaron.ballman added a comment. I feel like I must be missing something; why is this disabling rather than specifying the thread safety behavior? e.g., `__libcpp_mutex_lock()` specifying that it acquires the capability and `__libcpp_mutex_unlock()` specifying that it releases it? https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim updated this revision to Diff 83851. dim added a comment. Let's try this simpler version instead. https://reviews.llvm.org/D28520 Files: include/__threading_support Index: include/__threading_support === --- include/__threading_support +++ include/__threading_support @@ -40,6 +40,12 @@ #define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY #endif +#if defined(__clang__) && __has_attribute(no_thread_safety_analysis) +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) +#else +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS +#endif + _LIBCPP_BEGIN_NAMESPACE_STD #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) @@ -98,25 +104,25 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_lock(__libcpp_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_mutex_trylock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_unlock(__libcpp_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY @@ -129,10 +135,10 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m, timespec *__ts); Index: include/__threading_support === --- include/__threading_support +++ include/__threading_support @@ -40,6 +40,12 @@ #define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY #endif +#if defined(__clang__) && __has_attribute(no_thread_safety_analysis) +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) +#else +#define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS +#endif + _LIBCPP_BEGIN_NAMESPACE_STD #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) @@ -98,25 +104,25 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_lock(__libcpp_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_mutex_trylock(__libcpp_mutex_t *__m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_mutex_unlock(__libcpp_mutex_t *__m); _LIBCPP_THREAD_ABI_VISIBILITY @@ -129,10 +135,10 @@ _LIBCPP_THREAD_ABI_VISIBILITY int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m); -_LIBCPP_THREAD_ABI_VISIBILITY +_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m, timespec *__ts); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim added a comment. Hmm, actually this does not work. The definition of `_LIBCPP_THREAD_SAFETY_ANNOTATION` I moved from `__mutex_base` to `__config` is only enabled if `_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS` is manually defined. There must have been some reason to do it like this in `__mutex_base`, but for `__thread_support` we unconditionally need such a macro. I will try defining them slightly differently in `__thread_support` only instead. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim updated this revision to Diff 83843.
dim added a comment.
- Move `_LIBCPP_THREAD_SAFETY_ANNOTATION` macro definition to `__config`.
- Add `_LIBCPP_THREAD_SAFETY_ANNOTATION(no_thread_safety_analysis)` macros to
`__threading_support` function declarations which require them.
Note that I was not able to figure out how to make the other thread safety
annotation attributes, such as `try_acquire_capability()`, match to the
function signatures, as these use `int` instead of `bool`, for example.
https://reviews.llvm.org/D28520
Files:
include/__config
include/__mutex_base
include/__threading_support
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -98,25 +98,25 @@
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m);
-_LIBCPP_THREAD_ABI_VISIBILITY
+_LIBCPP_THREAD_ABI_VISIBILITY
_LIBCPP_THREAD_SAFETY_ANNOTATION(no_thread_safety_analysis)
int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m);
-_LIBCPP_THREAD_ABI_VISIBILITY
+_LIBCPP_THREAD_ABI_VISIBILITY
_LIBCPP_THREAD_SAFETY_ANNOTATION(no_thread_safety_analysis)
int __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m);
-_LIBCPP_THREAD_ABI_VISIBILITY
+_LIBCPP_THREAD_ABI_VISIBILITY
_LIBCPP_THREAD_SAFETY_ANNOTATION(no_thread_safety_analysis)
int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m);
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m);
-_LIBCPP_THREAD_ABI_VISIBILITY
+_LIBCPP_THREAD_ABI_VISIBILITY
_LIBCPP_THREAD_SAFETY_ANNOTATION(no_thread_safety_analysis)
int __libcpp_mutex_lock(__libcpp_mutex_t *__m);
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_mutex_trylock(__libcpp_mutex_t *__m);
-_LIBCPP_THREAD_ABI_VISIBILITY
+_LIBCPP_THREAD_ABI_VISIBILITY
_LIBCPP_THREAD_SAFETY_ANNOTATION(no_thread_safety_analysis)
int __libcpp_mutex_unlock(__libcpp_mutex_t *__m);
_LIBCPP_THREAD_ABI_VISIBILITY
@@ -129,10 +129,10 @@
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv);
-_LIBCPP_THREAD_ABI_VISIBILITY
+_LIBCPP_THREAD_ABI_VISIBILITY
_LIBCPP_THREAD_SAFETY_ANNOTATION(no_thread_safety_analysis)
int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m);
-_LIBCPP_THREAD_ABI_VISIBILITY
+_LIBCPP_THREAD_ABI_VISIBILITY
_LIBCPP_THREAD_SAFETY_ANNOTATION(no_thread_safety_analysis)
int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m,
timespec *__ts);
Index: include/__mutex_base
===
--- include/__mutex_base
+++ include/__mutex_base
@@ -24,14 +24,6 @@
#ifndef _LIBCPP_HAS_NO_THREADS
-#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
{
#ifndef _LIBCPP_HAS_NO_CONSTEXPR
Index: include/__config
===
--- include/__config
+++ include/__config
@@ -981,6 +981,16 @@
#define _LIBCPP_HAS_THREAD_SAFETY_ANNOTATIONS
#endif
+#if !defined(_LIBCPP_HAS_NO_THREADS)
+# if !defined(_LIBCPP_THREAD_SAFETY_ANNOTATION)
+# if defined(_LIBCPP_HAS_THREAD_SAFETY_ANNOTATIONS)
+# define _LIBCPP_THREAD_SAFETY_ANNOTATION(x) __attribute__((x))
+# else
+# define _LIBCPP_THREAD_SAFETY_ANNOTATION(x)
+# endif
+# endif
+#endif
+
#if __has_attribute(require_constant_initialization)
#define _LIBCPP_SAFE_STATIC
__attribute__((__require_constant_initialization__))
#else
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -98,25 +98,25 @@
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m);
-_LIBCPP_THREAD_ABI_VISIBILITY
+_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_THREAD_SAFETY_ANNOTATION(no_thread_safety_analysis)
int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m);
-_LIBCPP_THREAD_ABI_VISIBILITY
+_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_THREAD_SAFETY_ANNOTATION(no_thread_safety_analysis)
int __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m);
-_LIBCPP_THREAD_ABI_VISIBILITY
+_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_THREAD_SAFETY_ANNOTATION(no_thread_safety_analysis)
int __libcpp_recursive_mutex_unlock(__libcpp_recursive_mutex_t *__m);
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m);
-_LIBCPP_THREAD_ABI_VISIBILITY
+_LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_THREAD_SAFETY_ANNOTATION(no_thread_safety_analysis)
int
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
EricWF added a comment. In https://reviews.llvm.org/D28520#641569, @dim wrote: > In https://reviews.llvm.org/D28520#641563, @EricWF wrote: > > > Also look in `__mutex` where libc++ defines its own macros for the > > annotations. > > > I assume you mean `__mutex_base`. Do we want to reuse the macros from that > file? If so we'd have to include it in `__thread_support`. Maybe it is > better to move the macros to `__config` instead, then. Yes sorry `__mutex_base`. Moving them to `__config` sounds reasonable. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim added a comment. In https://reviews.llvm.org/D28520#641563, @EricWF wrote: > Also look in `__mutex` where libc++ defines its own macros for the > annotations. I assume you mean `__mutex_base`. Do we want to reuse the macros from that file? If so we'd have to include it in `__thread_support`. Maybe it is better to move the macros to `__config` instead, then. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
EricWF added a comment. Also look in `__mutex` where libc++ defines its own macros for the annotations. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
aaron.ballman added a comment. In https://reviews.llvm.org/D28520#641536, @dim wrote: > In https://reviews.llvm.org/D28520#641534, @aaron.ballman wrote: > > > Alternatively, these functions could be given the proper thread safety > > annotations, couldn't they? > > > Aha, I was not aware of the existence of these attributes. I'll take a look. This may be of some help: http://clang.llvm.org/docs/ThreadSafetyAnalysis.html https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
aaron.ballman added a reviewer: delesley. aaron.ballman added a comment. Alternatively, these functions could be given the proper thread safety annotations, couldn't they? https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support
dim created this revision.
dim added reviewers: EricWF, mclow.lists.
dim added subscribers: cfe-commits, emaste, joerg.
Many thread-related libc++ test cases fail on FreeBSD, due to the following
-Werror warnings:
In file included from
/share/dim/src/llvm/trunk/projects/libcxx/test/std/thread/thread.threads/thread.thread.this/sleep_until.pass.cpp:17:
In file included from
/share/dim/src/llvm/trunk/projects/libcxx/include/thread:97:
In file included from
/share/dim/src/llvm/trunk/projects/libcxx/include/__mutex_base:17:
/share/dim/src/llvm/trunk/projects/libcxx/include/__threading_support:222:1:
error: mutex '__m' is still held at the end of function
[-Werror,-Wthread-safety-analysis]
}
^
/share/dim/src/llvm/trunk/projects/libcxx/include/__threading_support:221:10:
note: mutex acquired here
return pthread_mutex_lock(__m);
^
/share/dim/src/llvm/trunk/projects/libcxx/include/__threading_support:231:10:
error: releasing mutex '__m' that was not held
[-Werror,-Wthread-safety-analysis]
return pthread_mutex_unlock(__m);
^
/share/dim/src/llvm/trunk/projects/libcxx/include/__threading_support:242:1:
error: mutex '__m' is still held at the end of function
[-Werror,-Wthread-safety-analysis]
}
^
/share/dim/src/llvm/trunk/projects/libcxx/include/__threading_support:241:10:
note: mutex acquired here
return pthread_mutex_lock(__m);
^
/share/dim/src/llvm/trunk/projects/libcxx/include/__threading_support:251:10:
error: releasing mutex '__m' that was not held
[-Werror,-Wthread-safety-analysis]
return pthread_mutex_unlock(__m);
^
/share/dim/src/llvm/trunk/projects/libcxx/include/__threading_support:272:10:
error: calling function 'pthread_cond_wait' requires holding mutex '__m'
exclusively [-Werror,-Wthread-safety-analysis]
return pthread_cond_wait(__cv, __m);
^
/share/dim/src/llvm/trunk/projects/libcxx/include/__threading_support:278:10:
error: calling function 'pthread_cond_timedwait' requires holding mutex '__m'
exclusively [-Werror,-Wthread-safety-analysis]
return pthread_cond_timedwait(__cv, __m, __ts);
^
6 errors generated.
Obviously, these warnings are false, since the functions are exactly doing what
they are supposed to do.
Therefore, add pragmas to ignored -Wthread-safety-analysis warnings. These
could also be set for the whole block of thread-related functions, but in this
version I have added them per function.
I also considered doing this with macros, as Joerg suggested, but macros that
expand to multi-line pragma statements are rather unwieldy, and don't make it
much nicer, in my opinion. Suggestions welcome, of course.
https://reviews.llvm.org/D28520
Files:
include/__threading_support
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -216,40 +216,68 @@
return 0;
}
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wthread-safety-analysis"
+#endif
int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m)
{
return pthread_mutex_lock(__m);
}
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
int __libcpp_recursive_mutex_trylock(__libcpp_recursive_mutex_t *__m)
{
return pthread_mutex_trylock(__m);
}
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wthread-safety-analysis"
+#endif
int __libcpp_recursive_mutex_unlock(__libcpp_mutex_t *__m)
{
return pthread_mutex_unlock(__m);
}
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
int __libcpp_recursive_mutex_destroy(__libcpp_recursive_mutex_t *__m)
{
return pthread_mutex_destroy(__m);
}
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wthread-safety-analysis"
+#endif
int __libcpp_mutex_lock(__libcpp_mutex_t *__m)
{
return pthread_mutex_lock(__m);
}
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
int __libcpp_mutex_trylock(__libcpp_mutex_t *__m)
{
return pthread_mutex_trylock(__m);
}
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wthread-safety-analysis"
+#endif
int __libcpp_mutex_unlock(__libcpp_mutex_t *__m)
{
return pthread_mutex_unlock(__m);
}
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
int __libcpp_mutex_destroy(__libcpp_mutex_t *__m)
{
@@ -267,16 +295,30 @@
return pthread_cond_broadcast(__cv);
}
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wthread-safety-analysis"
+#endif
int __libcpp_condvar_wait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m)
{
return pthread_cond_wait(__cv, __m);
}
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wthread-safety-analysis"
+#endif
int
