diff --git a/BUILD.generated.bzl b/BUILD.generated.bzl index 64bf19981..58df08c8b 100644 --- a/BUILD.generated.bzl +++ b/BUILD.generated.bzl @@ -415,7 +415,8 @@ crypto_sources = [ "src/crypto/rand_extra/windows.c", "src/crypto/rc4/rc4.c", "src/crypto/refcount_c11.c", - "src/crypto/refcount_lock.c", + "src/crypto/refcount_no_threads.c", + "src/crypto/refcount_win.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 89e60bdb6..e83dc9983 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -408,7 +408,8 @@ add_library( src/crypto/rand_extra/windows.c src/crypto/rc4/rc4.c src/crypto/refcount_c11.c - src/crypto/refcount_lock.c + src/crypto/refcount_no_threads.c + src/crypto/refcount_win.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 cbb3654af..b0e3caf99 100644 --- a/sources.json +++ b/sources.json @@ -144,7 +144,8 @@ "src/crypto/rand_extra/windows.c", "src/crypto/rc4/rc4.c", "src/crypto/refcount_c11.c", - "src/crypto/refcount_lock.c", + "src/crypto/refcount_no_threads.c", + "src/crypto/refcount_win.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 bc307023c..c427b20c5 100644 --- a/src/crypto/CMakeLists.txt +++ b/src/crypto/CMakeLists.txt @@ -203,7 +203,8 @@ add_library( rand_extra/windows.c rc4/rc4.c refcount_c11.c - refcount_lock.c + refcount_no_threads.c + refcount_win.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 a4cd92912..5c0473596 100644 --- a/src/crypto/internal.h +++ b/src/crypto/internal.h @@ -548,6 +548,24 @@ OPENSSL_EXPORT void CRYPTO_once(CRYPTO_once_t *once, void (*init)(void)); #define OPENSSL_C11_ATOMIC #endif +// 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 + +// 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 + // CRYPTO_REFCOUNT_MAX is the value at which the reference count saturates. #define CRYPTO_REFCOUNT_MAX 0xffffffff diff --git a/src/crypto/refcount_lock.c b/src/crypto/refcount_no_threads.c similarity index 75% rename from src/crypto/refcount_lock.c rename to src/crypto/refcount_no_threads.c index 173267e38..096b4fa93 100644 --- a/src/crypto/refcount_lock.c +++ b/src/crypto/refcount_no_threads.c @@ -18,35 +18,25 @@ #include -#if !defined(OPENSSL_C11_ATOMIC) +#if !defined(OPENSSL_THREADS) static_assert((CRYPTO_refcount_t)-1 == CRYPTO_REFCOUNT_MAX, "CRYPTO_REFCOUNT_MAX is incorrect"); -static struct CRYPTO_STATIC_MUTEX g_refcount_lock = CRYPTO_STATIC_MUTEX_INIT; - void CRYPTO_refcount_inc(CRYPTO_refcount_t *count) { - CRYPTO_STATIC_MUTEX_lock_write(&g_refcount_lock); if (*count < CRYPTO_REFCOUNT_MAX) { (*count)++; } - CRYPTO_STATIC_MUTEX_unlock_write(&g_refcount_lock); } int CRYPTO_refcount_dec_and_test_zero(CRYPTO_refcount_t *count) { - int ret; - - CRYPTO_STATIC_MUTEX_lock_write(&g_refcount_lock); if (*count == 0) { abort(); } if (*count < CRYPTO_REFCOUNT_MAX) { (*count)--; } - ret = (*count == 0); - CRYPTO_STATIC_MUTEX_unlock_write(&g_refcount_lock); - - return ret; + return *count == 0; } -#endif // OPENSSL_C11_ATOMIC +#endif // !OPENSSL_THREADS diff --git a/src/crypto/refcount_win.c b/src/crypto/refcount_win.c new file mode 100644 index 000000000..7a2740bc2 --- /dev/null +++ b/src/crypto/refcount_win.c @@ -0,0 +1,89 @@ +/* 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