diff --git a/BUILD.generated.bzl b/BUILD.generated.bzl index 58df08c8b..259d4f53f 100644 --- a/BUILD.generated.bzl +++ b/BUILD.generated.bzl @@ -414,9 +414,7 @@ crypto_sources = [ "src/crypto/rand_extra/rand_extra.c", "src/crypto/rand_extra/windows.c", "src/crypto/rc4/rc4.c", - "src/crypto/refcount_c11.c", - "src/crypto/refcount_no_threads.c", - "src/crypto/refcount_win.c", + "src/crypto/refcount.c", "src/crypto/rsa_extra/rsa_asn1.c", "src/crypto/rsa_extra/rsa_crypt.c", "src/crypto/rsa_extra/rsa_print.c", diff --git a/CMakeLists.txt b/CMakeLists.txt index e83dc9983..402b9b7d9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -407,9 +407,7 @@ add_library( src/crypto/rand_extra/rand_extra.c src/crypto/rand_extra/windows.c src/crypto/rc4/rc4.c - src/crypto/refcount_c11.c - src/crypto/refcount_no_threads.c - src/crypto/refcount_win.c + src/crypto/refcount.c src/crypto/rsa_extra/rsa_asn1.c src/crypto/rsa_extra/rsa_crypt.c src/crypto/rsa_extra/rsa_print.c diff --git a/sources.json b/sources.json index b0e3caf99..510e3070c 100644 --- a/sources.json +++ b/sources.json @@ -143,9 +143,7 @@ "src/crypto/rand_extra/rand_extra.c", "src/crypto/rand_extra/windows.c", "src/crypto/rc4/rc4.c", - "src/crypto/refcount_c11.c", - "src/crypto/refcount_no_threads.c", - "src/crypto/refcount_win.c", + "src/crypto/refcount.c", "src/crypto/rsa_extra/rsa_asn1.c", "src/crypto/rsa_extra/rsa_crypt.c", "src/crypto/rsa_extra/rsa_print.c", diff --git a/src/crypto/CMakeLists.txt b/src/crypto/CMakeLists.txt index c427b20c5..ef47623cc 100644 --- a/src/crypto/CMakeLists.txt +++ b/src/crypto/CMakeLists.txt @@ -202,9 +202,7 @@ add_library( rand_extra/rand_extra.c rand_extra/windows.c rc4/rc4.c - refcount_c11.c - refcount_no_threads.c - refcount_win.c + refcount.c rsa_extra/rsa_asn1.c rsa_extra/rsa_crypt.c rsa_extra/rsa_print.c diff --git a/src/crypto/internal.h b/src/crypto/internal.h index 5c0473596..00f0582fe 100644 --- a/src/crypto/internal.h +++ b/src/crypto/internal.h @@ -155,6 +155,32 @@ #if defined(OPENSSL_THREADS) && !defined(OPENSSL_PTHREADS) && \ defined(OPENSSL_WINDOWS) #define OPENSSL_WINDOWS_THREADS +#endif + +// Determine the atomics implementation to use with C. +#if !defined(__cplusplus) +#if !defined(OPENSSL_C11_ATOMIC) && defined(OPENSSL_THREADS) && \ + !defined(__STDC_NO_ATOMICS__) && defined(__STDC_VERSION__) && \ + __STDC_VERSION__ >= 201112L +#define OPENSSL_C11_ATOMIC +#endif + +// Older MSVC does not support C11 atomics, so we fallback to the Windows APIs. +// When both are available (e.g. clang-cl), we prefer the C11 ones. The Windows +// APIs don't allow some operations to be implemented as efficiently. This can +// be removed once we can rely on +// https://devblogs.microsoft.com/cppblog/c11-atomics-in-visual-studio-2022-version-17-5-preview-2/ +#if !defined(OPENSSL_C11_ATOMIC) && defined(OPENSSL_THREADS) && \ + defined(OPENSSL_WINDOWS) +#define OPENSSL_WINDOWS_ATOMIC +#endif +#endif // !__cplusplus + +#if defined(OPENSSL_C11_ATOMIC) +#include +#endif + +#if defined(OPENSSL_WINDOWS_THREADS) || defined(OPENSSL_WINDOWS_ATOMIC) OPENSSL_MSVC_PRAGMA(warning(push, 3)) #include OPENSSL_MSVC_PRAGMA(warning(pop)) @@ -539,33 +565,102 @@ typedef pthread_once_t CRYPTO_once_t; OPENSSL_EXPORT void CRYPTO_once(CRYPTO_once_t *once, void (*init)(void)); -// Reference counting. +// Atomics. +// +// The following functions provide an API analogous to from C11 +// and abstract between a few variations on atomics we need to support. -// Automatically enable C11 atomics if implemented. -#if !defined(OPENSSL_C11_ATOMIC) && defined(OPENSSL_THREADS) && \ - !defined(__STDC_NO_ATOMICS__) && defined(__STDC_VERSION__) && \ - __STDC_VERSION__ >= 201112L -#define OPENSSL_C11_ATOMIC -#endif +#if defined(__cplusplus) -// Older MSVC does not support C11 atomics, so we fallback to the Windows APIs. -// This can be removed once we can rely on -// https://devblogs.microsoft.com/cppblog/c11-atomics-in-visual-studio-2022-version-17-5-preview-2/ -#if !defined(OPENSSL_C11_ATOMIC) && defined(OPENSSL_THREADS) && \ - defined(OPENSSL_WINDOWS) -#define OPENSSL_WINDOWS_ATOMIC -#endif +// In C++, we can't easily detect whether C will use |OPENSSL_C11_ATOMIC| or +// |OPENSSL_WINDOWS_ATOMIC|. Instead, we define a layout-compatible type without +// the corresponding functions. When we can rely on C11 atomics in MSVC, that +// will no longer be a concern. +typedef uint32_t CRYPTO_atomic_u32; + +#elif defined(OPENSSL_C11_ATOMIC) + +typedef _Atomic uint32_t CRYPTO_atomic_u32; + +// This should be const, but the |OPENSSL_WINDOWS_ATOMIC| implementation is not +// const due to Windows limitations. When we can rely on C11 atomics, make this +// const-correct. +OPENSSL_INLINE uint32_t CRYPTO_atomic_load_u32(CRYPTO_atomic_u32 *val) { + return atomic_load(val); +} + +OPENSSL_INLINE int CRYPTO_atomic_compare_exchange_weak_u32( + CRYPTO_atomic_u32 *val, uint32_t *expected, uint32_t desired) { + return atomic_compare_exchange_weak(val, expected, desired); +} + +#elif defined(OPENSSL_WINDOWS_ATOMIC) + +typedef LONG CRYPTO_atomic_u32; + +OPENSSL_INLINE uint32_t CRYPTO_atomic_load_u32(volatile CRYPTO_atomic_u32 *val) { + // This is not ideal because it still writes to a cacheline. MSVC is not able + // to optimize this to a true atomic read, and Windows does not provide an + // InterlockedLoad function. + // + // The Windows documentation [1] does say "Simple reads and writes to + // properly-aligned 32-bit variables are atomic operations", but this is not + // phrased in terms of the C11 and C++11 memory models, and indeed a read or + // write seems to produce slightly different code on MSVC than a sequentially + // consistent std::atomic::load in C++. Moreover, it is unclear if non-MSVC + // compilers on Windows provide the same guarantees. Thus we avoid relying on + // this and instead still use an interlocked function. This is still + // preferable a global mutex, and eventually this code will be replaced by + // [2]. Additionally, on clang-cl, we'll use the |OPENSSL_C11_ATOMIC| path. + // + // [1] https://learn.microsoft.com/en-us/windows/win32/sync/interlocked-variable-access + // [2] https://devblogs.microsoft.com/cppblog/c11-atomics-in-visual-studio-2022-version-17-5-preview-2/ + return (uint32_t)InterlockedCompareExchange(val, 0, 0); +} + +OPENSSL_INLINE int CRYPTO_atomic_compare_exchange_weak_u32( + volatile CRYPTO_atomic_u32 *val, uint32_t *expected32, uint32_t desired) { + LONG expected = (LONG)*expected32; + LONG actual = InterlockedCompareExchange(val, (LONG)desired, expected); + *expected32 = (uint32_t)actual; + return actual == expected; +} + +#elif !defined(OPENSSL_THREADS) + +typedef uint32_t CRYPTO_atomic_u32; + +OPENSSL_INLINE uint32_t CRYPTO_atomic_load_u32(CRYPTO_atomic_u32 *val) { + return *val; +} + +OPENSSL_INLINE int CRYPTO_atomic_compare_exchange_weak_u32( + CRYPTO_atomic_u32 *val, uint32_t *expected, uint32_t desired) { + if (*val != *expected) { + *expected = *val; + return 0; + } + *val = desired; + return 1; +} + +#else // Require some atomics implementation. Contact BoringSSL maintainers if you // have a platform with fails this check. -// -// Note this check can only be done in C. From C++, we don't know whether the -// corresponding C mode would support C11 atomics. -#if !defined(__cplusplus) && defined(OPENSSL_THREADS) && \ - !defined(OPENSSL_C11_ATOMIC) && !defined(OPENSSL_WINDOWS_ATOMIC) #error "Thread-compatible configurations require atomics" + #endif +// See the comment in the |__cplusplus| section above. +static_assert(sizeof(CRYPTO_atomic_u32) == sizeof(uint32_t), + "CRYPTO_atomic_u32 does not match uint32_t size"); +static_assert(alignof(CRYPTO_atomic_u32) == alignof(uint32_t), + "CRYPTO_atomic_u32 does not match uint32_t alignment"); + + +// Reference counting. + // CRYPTO_REFCOUNT_MAX is the value at which the reference count saturates. #define CRYPTO_REFCOUNT_MAX 0xffffffff diff --git a/src/crypto/refcount_c11.c b/src/crypto/refcount.c similarity index 66% rename from src/crypto/refcount_c11.c rename to src/crypto/refcount.c index a1781c66a..74ebdd7c7 100644 --- a/src/crypto/refcount_c11.c +++ b/src/crypto/refcount.c @@ -14,39 +14,35 @@ #include "internal.h" - -#if defined(OPENSSL_C11_ATOMIC) - #include #include -#include #include // See comment above the typedef of CRYPTO_refcount_t about these tests. -static_assert(alignof(CRYPTO_refcount_t) == alignof(_Atomic CRYPTO_refcount_t), - "_Atomic alters the needed alignment of a reference count"); -static_assert(sizeof(CRYPTO_refcount_t) == sizeof(_Atomic CRYPTO_refcount_t), - "_Atomic alters the size of a reference count"); +static_assert(alignof(CRYPTO_refcount_t) == alignof(CRYPTO_atomic_u32), + "CRYPTO_refcount_t does not match CRYPTO_atomic_u32 alignment"); +static_assert(sizeof(CRYPTO_refcount_t) == sizeof(CRYPTO_atomic_u32), + "CRYPTO_refcount_t does not match CRYPTO_atomic_u32 size"); static_assert((CRYPTO_refcount_t)-1 == CRYPTO_REFCOUNT_MAX, "CRYPTO_REFCOUNT_MAX is incorrect"); void CRYPTO_refcount_inc(CRYPTO_refcount_t *in_count) { - _Atomic CRYPTO_refcount_t *count = (_Atomic CRYPTO_refcount_t *) in_count; - uint32_t expected = atomic_load(count); + CRYPTO_atomic_u32 *count = (CRYPTO_atomic_u32 *)in_count; + uint32_t expected = CRYPTO_atomic_load_u32(count); while (expected != CRYPTO_REFCOUNT_MAX) { uint32_t new_value = expected + 1; - if (atomic_compare_exchange_weak(count, &expected, new_value)) { + if (CRYPTO_atomic_compare_exchange_weak_u32(count, &expected, new_value)) { break; } } } int CRYPTO_refcount_dec_and_test_zero(CRYPTO_refcount_t *in_count) { - _Atomic CRYPTO_refcount_t *count = (_Atomic CRYPTO_refcount_t *)in_count; - uint32_t expected = atomic_load(count); + CRYPTO_atomic_u32 *count = (CRYPTO_atomic_u32 *)in_count; + uint32_t expected = CRYPTO_atomic_load_u32(count); for (;;) { if (expected == 0) { @@ -55,11 +51,10 @@ int CRYPTO_refcount_dec_and_test_zero(CRYPTO_refcount_t *in_count) { return 0; } else { const uint32_t new_value = expected - 1; - if (atomic_compare_exchange_weak(count, &expected, new_value)) { + if (CRYPTO_atomic_compare_exchange_weak_u32(count, &expected, + new_value)) { return new_value == 0; } } } } - -#endif // OPENSSL_C11_ATOMIC diff --git a/src/crypto/refcount_no_threads.c b/src/crypto/refcount_no_threads.c deleted file mode 100644 index 096b4fa93..000000000 --- a/src/crypto/refcount_no_threads.c +++ /dev/null @@ -1,42 +0,0 @@ -/* Copyright (c) 2015, Google Inc. - * - * Permission to use, copy, modify, and/or distribute this software for any - * purpose with or without fee is hereby granted, provided that the above - * copyright notice and this permission notice appear in all copies. - * - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY - * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION - * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN - * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ - -#include "internal.h" - -#include -#include - - -#if !defined(OPENSSL_THREADS) - -static_assert((CRYPTO_refcount_t)-1 == CRYPTO_REFCOUNT_MAX, - "CRYPTO_REFCOUNT_MAX is incorrect"); - -void CRYPTO_refcount_inc(CRYPTO_refcount_t *count) { - if (*count < CRYPTO_REFCOUNT_MAX) { - (*count)++; - } -} - -int CRYPTO_refcount_dec_and_test_zero(CRYPTO_refcount_t *count) { - if (*count == 0) { - abort(); - } - if (*count < CRYPTO_REFCOUNT_MAX) { - (*count)--; - } - return *count == 0; -} - -#endif // !OPENSSL_THREADS diff --git a/src/crypto/refcount_win.c b/src/crypto/refcount_win.c deleted file mode 100644 index 7a2740bc2..000000000 --- a/src/crypto/refcount_win.c +++ /dev/null @@ -1,89 +0,0 @@ -/* Copyright (c) 2023, Google Inc. - * - * Permission to use, copy, modify, and/or distribute this software for any - * purpose with or without fee is hereby granted, provided that the above - * copyright notice and this permission notice appear in all copies. - * - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY - * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION - * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN - * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ - -#include "internal.h" - -#if defined(OPENSSL_WINDOWS_ATOMIC) - -#include - - -// See comment above the typedef of CRYPTO_refcount_t about these tests. -static_assert(alignof(CRYPTO_refcount_t) == alignof(LONG), - "CRYPTO_refcount_t does not match LONG alignment"); -static_assert(sizeof(CRYPTO_refcount_t) == sizeof(LONG), - "CRYPTO_refcount_t does not match LONG size"); - -static_assert((CRYPTO_refcount_t)-1 == CRYPTO_REFCOUNT_MAX, - "CRYPTO_REFCOUNT_MAX is incorrect"); - -static uint32_t atomic_load_u32(volatile LONG *ptr) { - // This is not ideal because it still writes to a cacheline. MSVC is not able - // to optimize this to a true atomic read, and Windows does not provide an - // InterlockedLoad function. - // - // The Windows documentation [1] does say "Simple reads and writes to - // properly-aligned 32-bit variables are atomic operations", but this is not - // phrased in terms of the C11 and C++11 memory models, and indeed a read or - // write seems to produce slightly different code on MSVC than a sequentially - // consistent std::atomic::load in C++. Moreover, it is unclear if non-MSVC - // compilers on Windows provide the same guarantees. Thus we avoid relying on - // this and instead still use an interlocked function. This is still - // preferable a global mutex, and eventually this code will be replaced by - // [2]. Additionally, on clang-cl, we'll use the |OPENSSL_C11_ATOMIC| path. - // - // [1] https://learn.microsoft.com/en-us/windows/win32/sync/interlocked-variable-access - // [2] https://devblogs.microsoft.com/cppblog/c11-atomics-in-visual-studio-2022-version-17-5-preview-2/ - return (uint32_t)InterlockedCompareExchange(ptr, 0, 0); -} - -static int atomic_compare_exchange_u32(volatile LONG *ptr, uint32_t *expected32, - uint32_t desired) { - LONG expected = (LONG)*expected32; - LONG actual = InterlockedCompareExchange(ptr, (LONG)desired, expected); - *expected32 = (uint32_t)actual; - return actual == expected; -} - -void CRYPTO_refcount_inc(CRYPTO_refcount_t *in_count) { - volatile LONG *count = (volatile LONG *)in_count; - uint32_t expected = atomic_load_u32(count); - - while (expected != CRYPTO_REFCOUNT_MAX) { - const uint32_t new_value = expected + 1; - if (atomic_compare_exchange_u32(count, &expected, new_value)) { - break; - } - } -} - -int CRYPTO_refcount_dec_and_test_zero(CRYPTO_refcount_t *in_count) { - volatile LONG *count = (volatile LONG *)in_count; - uint32_t expected = atomic_load_u32(count); - - for (;;) { - if (expected == 0) { - abort(); - } else if (expected == CRYPTO_REFCOUNT_MAX) { - return 0; - } else { - const uint32_t new_value = expected - 1; - if (atomic_compare_exchange_u32(count, &expected, new_value)) { - return new_value == 0; - } - } - } -} - -#endif // OPENSSL_WINDOWS_ATOMIC diff --git a/src/include/openssl/thread.h b/src/include/openssl/thread.h index afa9f08b1..695182b8b 100644 --- a/src/include/openssl/thread.h +++ b/src/include/openssl/thread.h @@ -100,7 +100,7 @@ typedef union crypto_mutex_st { // _Atomic qualifier. However, this header is included by C++ programs as well // as C code that might not set -std=c11. So, in practice, it's not possible to // do that. Instead we statically assert that the size and native alignment of -// a plain uint32_t and an _Atomic uint32_t are equal in refcount_c11.c. +// a plain uint32_t and an _Atomic uint32_t are equal in refcount.c. typedef uint32_t CRYPTO_refcount_t;