From 68144a81da6594fccb150d9856df68de7369dcd6 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Mon, 8 Jun 2026 16:52:06 -0500 Subject: [PATCH] fixes from AI review: wolfcrypt/src/wc_port.c: fix several missed refactors to WC_ATOMIC_[U]INT_ARG. wolfssl/wolfcrypt/wc_port.h: * harmonize the return type of WOLFSSL_ATOMIC_STORE() (always void). * fix MSVC WOLFSSL_ATOMIC_LOAD() and _STORE() with correct atomic semantics, and add gating on USE_WINDOWS_API. --- wolfcrypt/src/wc_port.c | 26 ++++++++++++++++---------- wolfssl/wolfcrypt/wc_port.h | 16 ++++++++++++---- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/wolfcrypt/src/wc_port.c b/wolfcrypt/src/wc_port.c index 02706d49c7..63c1bb57f5 100644 --- a/wolfcrypt/src/wc_port.c +++ b/wolfcrypt/src/wc_port.c @@ -795,7 +795,7 @@ int wolfCrypt_Cleanup(void) ret = wc_local_InitDown(&wolfcrypt_init_state); if (ret < 0) { if (ret == WC_NO_ERR_TRACE(ALREADY_E)) - WOLFSSL_MSG("wolfCrypt_Cleanup() called with initRefCount <= 0."); + WOLFSSL_MSG("wolfCrypt_Cleanup() called during or after prior final cleanup."); else if (ret == WC_NO_ERR_TRACE(BAD_STATE_E)) WOLFSSL_MSG("wolfCrypt_Cleanup() failed: bad internal state."); #ifdef WC_INIT_ERROR_WHEN_CONTENDED @@ -1766,14 +1766,16 @@ WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_FetchSub(wolfSSL_Atomic_Int* c, WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_AddFetch(wolfSSL_Atomic_Int* c, WC_ATOMIC_INT_ARG i) { - int ret = atomic_fetch_add_explicit(c, i, memory_order_relaxed); + WC_ATOMIC_INT_ARG ret = + atomic_fetch_add_explicit(c, i, memory_order_relaxed); return ret + i; } WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_SubFetch(wolfSSL_Atomic_Int* c, WC_ATOMIC_INT_ARG i) { - int ret = atomic_fetch_sub_explicit(c, i, memory_order_relaxed); + WC_ATOMIC_INT_ARG ret = + atomic_fetch_sub_explicit(c, i, memory_order_relaxed); return ret - i; } @@ -1812,14 +1814,16 @@ WC_ATOMIC_UINT_ARG wolfSSL_Atomic_Uint_FetchSub(wolfSSL_Atomic_Uint* c, WC_ATOMIC_UINT_ARG wolfSSL_Atomic_Uint_AddFetch(wolfSSL_Atomic_Uint* c, WC_ATOMIC_UINT_ARG i) { - unsigned int ret = atomic_fetch_add_explicit(c, i, memory_order_relaxed); + WC_ATOMIC_UINT_ARG ret = + atomic_fetch_add_explicit(c, i, memory_order_relaxed); return ret + i; } WC_ATOMIC_UINT_ARG wolfSSL_Atomic_Uint_SubFetch(wolfSSL_Atomic_Uint* c, WC_ATOMIC_UINT_ARG i) { - unsigned int ret = atomic_fetch_sub_explicit(c, i, memory_order_relaxed); + WC_ATOMIC_UINT_ARG ret = + atomic_fetch_sub_explicit(c, i, memory_order_relaxed); return ret - i; } @@ -1987,14 +1991,16 @@ WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_FetchSub(wolfSSL_Atomic_Int* c, WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_AddFetch(wolfSSL_Atomic_Int* c, WC_ATOMIC_INT_ARG i) { - int ret = (int)_InterlockedExchangeAdd(c, (long)i); + WC_ATOMIC_INT_ARG ret = + (WC_ATOMIC_INT_ARG)_InterlockedExchangeAdd(c, (long)i); return ret + i; } WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_SubFetch(wolfSSL_Atomic_Int* c, WC_ATOMIC_INT_ARG i) { - int ret = (int)_InterlockedExchangeAdd(c, (long)-i); + WC_ATOMIC_INT_ARG ret = + (WC_ATOMIC_INT_ARG)_InterlockedExchangeAdd(c, (long)-i); return ret - i; } @@ -2002,7 +2008,7 @@ WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_Exchange(wolfSSL_Atomic_Int* c, WC_ATOMIC_INT_ARG new_i) { long actual_i = InterlockedExchange(c, (long)new_i); - return (int)actual_i; + return (WC_ATOMIC_INT_ARG)actual_i; } int wolfSSL_Atomic_Int_CompareExchange(wolfSSL_Atomic_Int* c, @@ -2015,7 +2021,7 @@ int wolfSSL_Atomic_Int_CompareExchange(wolfSSL_Atomic_Int* c, return 1; } else { - *expected_i = (int)actual_i; + *expected_i = (WC_ATOMIC_INT_ARG)actual_i; return 0; } } @@ -2060,7 +2066,7 @@ int wolfSSL_Atomic_Uint_CompareExchange( return 1; } else { - *expected_i = (unsigned int)actual_i; + *expected_i = (WC_ATOMIC_UINT_ARG)actual_i; return 0; } } diff --git a/wolfssl/wolfcrypt/wc_port.h b/wolfssl/wolfcrypt/wc_port.h index 37e6fe8954..0bd5f793a2 100644 --- a/wolfssl/wolfcrypt/wc_port.h +++ b/wolfssl/wolfcrypt/wc_port.h @@ -539,7 +539,7 @@ typedef WC_ATOMIC_UINT_ARG wolfSSL_Atomic_Uint; #define WOLFSSL_ATOMIC_INITIALIZER(x) (x) #define WOLFSSL_ATOMIC_LOAD(x) (x) - #define WOLFSSL_ATOMIC_STORE(x, val) (x) = (val) + #define WOLFSSL_ATOMIC_STORE(x, val) (void)((x) = (val)) #define WOLFSSL_ATOMIC_OPS #elif defined(WOLFSSL_BSDKM) /* Note: can be safely included in both linux kernel and @@ -567,7 +567,8 @@ #define WOLFSSL_ATOMIC_STORE(x, val) __atomic_store_n(&(x), \ val, __ATOMIC_RELEASE) #define WOLFSSL_ATOMIC_OPS - #elif defined(_MSC_VER) && !defined(WOLFSSL_NOT_WINDOWS_API) + #elif defined(_MSC_VER) && defined(USE_WINDOWS_API) && \ + !defined(WOLFSSL_NOT_WINDOWS_API) /* Use MSVC compiler intrinsics for atomic ops */ #ifdef _WIN32_WCE #include @@ -577,8 +578,15 @@ typedef volatile long wolfSSL_Atomic_Int; typedef volatile unsigned long wolfSSL_Atomic_Uint; #define WOLFSSL_ATOMIC_INITIALIZER(x) (x) - #define WOLFSSL_ATOMIC_LOAD(x) (x) - #define WOLFSSL_ATOMIC_STORE(x, val) (x) = (val) + /* Acquire-ordered load via idempotent RMW: OR-with-0 leaves the value + * unchanged but provides atomicity + acquire ordering. On cl.exe this + * is a locked RMW; LLVM (clang-cl) may lower it to a plain acquire + * load. + */ + #define WOLFSSL_ATOMIC_LOAD(x) \ + InterlockedOrAcquire((volatile long *)&(x), 0) + #define WOLFSSL_ATOMIC_STORE(x, val) \ + (void)InterlockedExchange((volatile long *)&(x), (long)(val)) #define WOLFSSL_ATOMIC_OPS #endif