[PATCH] D47225: Add nonnull; use it for atomics
jfb added a comment. GCC in libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03 seems to mis-handle ATOMIC_VAR_INIT: File /home/llvm-builder/llvm-buildslave-root/libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03/llvm/projects/libcxx/test/libcxx/atomics/diagnose_nonnull.fail.cpp Line 20: non-aggregate type 'std::atomic' cannot be initialized with an initializer list File /home/llvm-builder/llvm-buildslave-root/libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03/llvm/projects/libcxx/test/libcxx/atomics/diagnose_nonnull.fail.cpp Line 21: non-aggregate type 'volatile std::atomic' cannot be initialized with an initializer list I'll drop the initialization for now, it's not required anyways. Repository: rL LLVM https://reviews.llvm.org/D47225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47225: Add nonnull; use it for atomics
This revision was automatically updated to reflect the committed changes.
Closed by commit rL25: Add nonnull; use it for atomics (authored by jfb,
committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D47225
Files:
libcxx/trunk/include/__config
libcxx/trunk/include/atomic
libcxx/trunk/test/libcxx/atomics/diagnose_nonnull.fail.cpp
Index: libcxx/trunk/include/atomic
===
--- libcxx/trunk/include/atomic
+++ libcxx/trunk/include/atomic
@@ -646,19 +646,23 @@
} // namespace __gcc_atomic
template
+_LIBCPP_NONNULL(1)
static inline
typename enable_if<
__gcc_atomic::__can_assign::value>::type
-__c11_atomic_init(volatile _Atomic(_Tp)* __a, _Tp __val) {
+__c11_atomic_init(volatile _Atomic(_Tp)* __a, _Tp __val)
+{
__a->__a_value = __val;
}
template
+_LIBCPP_NONNULL(1)
static inline
typename enable_if<
!__gcc_atomic::__can_assign::value &&
__gcc_atomic::__can_assign< _Atomic(_Tp)*, _Tp>::value>::type
-__c11_atomic_init(volatile _Atomic(_Tp)* __a, _Tp __val) {
+__c11_atomic_init(volatile _Atomic(_Tp)* __a, _Tp __val)
+{
// [atomics.types.generic]p1 guarantees _Tp is trivially copyable. Because
// the default operator= in an object is not volatile, a byte-by-byte copy
// is required.
@@ -671,7 +675,9 @@
}
template
-static inline void __c11_atomic_init(_Atomic(_Tp)* __a, _Tp __val) {
+_LIBCPP_NONNULL(1)
+static inline void __c11_atomic_init(_Atomic(_Tp)* __a, _Tp __val)
+{
__a->__a_value = __val;
}
@@ -684,88 +690,108 @@
}
template
+_LIBCPP_NONNULL(1)
static inline void __c11_atomic_store(volatile _Atomic(_Tp)* __a, _Tp __val,
- memory_order __order) {
+ memory_order __order)
+{
return __atomic_store(&__a->__a_value, &__val,
__gcc_atomic::__to_gcc_order(__order));
}
template
+_LIBCPP_NONNULL(1)
static inline void __c11_atomic_store(_Atomic(_Tp)* __a, _Tp __val,
- memory_order __order) {
+ memory_order __order)
+{
__atomic_store(&__a->__a_value, &__val,
__gcc_atomic::__to_gcc_order(__order));
}
template
+_LIBCPP_NONNULL(1)
static inline _Tp __c11_atomic_load(volatile _Atomic(_Tp)* __a,
-memory_order __order) {
+memory_order __order)
+{
_Tp __ret;
__atomic_load(&__a->__a_value, &__ret,
__gcc_atomic::__to_gcc_order(__order));
return __ret;
}
template
-static inline _Tp __c11_atomic_load(_Atomic(_Tp)* __a, memory_order __order) {
+_LIBCPP_NONNULL(1)
+static inline _Tp __c11_atomic_load(_Atomic(_Tp)* __a, memory_order __order)
+{
_Tp __ret;
__atomic_load(&__a->__a_value, &__ret,
__gcc_atomic::__to_gcc_order(__order));
return __ret;
}
template
+_LIBCPP_NONNULL(1)
static inline _Tp __c11_atomic_exchange(volatile _Atomic(_Tp)* __a,
-_Tp __value, memory_order __order) {
+_Tp __value, memory_order __order)
+{
_Tp __ret;
__atomic_exchange(&__a->__a_value, &__value, &__ret,
__gcc_atomic::__to_gcc_order(__order));
return __ret;
}
template
+_LIBCPP_NONNULL(1)
static inline _Tp __c11_atomic_exchange(_Atomic(_Tp)* __a, _Tp __value,
-memory_order __order) {
+memory_order __order)
+{
_Tp __ret;
__atomic_exchange(&__a->__a_value, &__value, &__ret,
__gcc_atomic::__to_gcc_order(__order));
return __ret;
}
template
+_LIBCPP_NONNULL(1, 2)
static inline bool __c11_atomic_compare_exchange_strong(
volatile _Atomic(_Tp)* __a, _Tp* __expected, _Tp __value,
-memory_order __success, memory_order __failure) {
+memory_order __success, memory_order __failure)
+{
return __atomic_compare_exchange(&__a->__a_value, __expected, &__value,
false,
__gcc_atomic::__to_gcc_order(__success),
__gcc_atomic::__to_gcc_failure_order(__failure));
}
template
+_LIBCPP_NONNULL(1, 2)
static inline bool __c11_atomic_compare_exchange_strong(
_Atomic(_Tp)* __a, _Tp* __expected, _Tp __value, memory_order __success,
-memory_order __failure) {
+memory_order __failure)
+{
return __atomic_compare_exchange(&__a->__a_value, __expected, &__value,
false,
__gcc_atomic::__to_gcc_order(__success),
__gcc_atomic::__to_gcc_failure_order(__failure));
}
template
+_LIBCPP_NONNULL(1, 2)
static inline bool __c11_atomic_compare_exchange_weak(
volatile _Atomic(_Tp)* __a, _Tp* __
[PATCH] D47225: Add nonnull; use it for atomics
arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM (FYI, Please use a weekly frequency for Pings). Repository: rCXX libc++ https://reviews.llvm.org/D47225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47225: Add nonnull; use it for atomics
jfb added a comment. Ping! clang side landed in https://reviews.llvm.org/rL333246 Repository: rCXX libc++ https://reviews.llvm.org/D47225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47225: Add nonnull; use it for atomics
jfb created this revision.
jfb added reviewers: arphaman, EricWF.
Herald added subscribers: cfe-commits, christof, aheejin.
The atomic non-member functions accept pointers to std::atomic /
std::atomic_flag as well as to the non-atomic value. These are all dereferenced
unconditionally when lowered, and therefore will fault if null. It's a tiny
gotcha for new users, especially when they pass in NULL as expected value
(instead of passing a pointer to a NULL value). We can therefore use the
nonnull attribute to denote that:
- A warning should be generated if the argument is null
- It is undefined behavior if the argument is null (because a dereference will
segfault)
This patch adds support for this attribute for clang and GCC, and sticks to the
subset of the syntax both supports. In particular, work around this GCC oddity:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60625
The attributes are documented:
- https://gcc.gnu.org/onlinedocs/gcc-4.0.0/gcc/Function-Attributes.html
- https://clang.llvm.org/docs/AttributeReference.html#nullability-attributes
I'm authoring a companion clang patch for the __c11_* and __atomic_* builtins,
which currently only warn on a subset of the pointer parameters.
In all cases the check needs to be explicit and not use the empty nonnull list,
because some of the overloads are for atomic and the values themselves are
allowed to be null.
rdar://problem/18473124
Repository:
rCXX libc++
https://reviews.llvm.org/D47225
Files:
include/__config
include/atomic
test/libcxx/atomics/diagnose_nonnull.fail.cpp
Index: test/libcxx/atomics/diagnose_nonnull.fail.cpp
===
--- /dev/null
+++ test/libcxx/atomics/diagnose_nonnull.fail.cpp
@@ -0,0 +1,92 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: verify-support
+// UNSUPPORTED: libcpp-has-no-threads
+
+//
+
+// Test that null pointer parameters are diagnosed.
+
+#include
+
+int main() {
+ std::atomic ai = ATOMIC_VAR_INIT(0);
+ volatile std::atomic vai = ATOMIC_VAR_INIT(0);
+ int i = 42;
+
+ atomic_is_lock_free((const volatile std::atomic*)0); // expected-error {{null passed to a callee that requires a non-null argument}}
+ atomic_is_lock_free((const std::atomic*)0); // expected-error {{null passed to a callee that requires a non-null argument}}
+ atomic_init((volatile std::atomic*)0, 42); // expected-error {{null passed to a callee that requires a non-null argument}}
+ atomic_init((std::atomic*)0, 42); // expected-error {{null passed to a callee that requires a non-null argument}}
+ atomic_store((volatile std::atomic*)0, 42); // expected-error {{null passed to a callee that requires a non-null argument}}
+ atomic_store((std::atomic*)0, 42); // expected-error {{null passed to a callee that requires a non-null argument}}
+ atomic_store_explicit((volatile std::atomic*)0, 42, std::memory_order_relaxed); // expected-error {{null passed to a callee that requires a non-null argument}}
+ atomic_store_explicit((std::atomic*)0, 42, std::memory_order_relaxed); // expected-error {{null passed to a callee that requires a non-null argument}}
+ (void)atomic_load((const volatile std::atomic*)0); // expected-error {{null passed to a callee that requires a non-null argument}}
+ (void)atomic_load((const std::atomic*)0); // expected-error {{null passed to a callee that requires a non-null argument}}
+ (void)atomic_load_explicit((const volatile std::atomic*)0, std::memory_order_relaxed); // expected-error {{null passed to a callee that requires a non-null argument}}
+ (void)atomic_load_explicit((const std::atomic*)0, std::memory_order_relaxed); // expected-error {{null passed to a callee that requires a non-null argument}}
+ (void)atomic_exchange((volatile std::atomic*)0, 42); // expected-error {{null passed to a callee that requires a non-null argument}}
+ (void)atomic_exchange((std::atomic*)0, 42); // expected-error {{null passed to a callee that requires a non-null argument}}
+ (void)atomic_exchange_explicit((volatile std::atomic*)0, 42, std::memory_order_relaxed); // expected-error {{null passed to a callee that requires a non-null argument}}
+ (void)atomic_exchange_explicit((std::atomic*)0, 42, std::memory_order_relaxed); // expected-error {{null passed to a callee that requires a non-null argument}}
+ (void)atomic_compare_exchange_weak((volatile std::atomic*)0, &i, 42); // expected-error {{null passed to a callee that requires a non-null argument}}
+ (void)atomic_compare_exchange_weak((std::atomic*)0, &i, 42); // expected-error {{null passed to a callee that requires a non-null argument}}
+ (void)atomic_compare_exchange
