Skip to content

Commit

Permalink
blake3_dispatch: Fix race condition initializing g_cpu_features.
Browse files Browse the repository at this point in the history
If multiple threads try to compute a hash simultaneously before the library has been used for the first time,
the logic in get_cpu_features that detects CPU features will write to g_cpu_features without synchronization,
which is a race condition and flagged by ThreadSanitizer.

This change marks g_cpu_features as an atomic variable to address the race condition.
  • Loading branch information
jblazquez authored and oconnor663 committed Jul 22, 2023
1 parent e302cdf commit 12823b8
Showing 1 changed file with 34 additions and 5 deletions.
39 changes: 34 additions & 5 deletions c/blake3_dispatch.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#if defined(IS_X86)
#if defined(_MSC_VER)
#include <Windows.h>
#include <intrin.h>
#elif defined(__GNUC__)
#include <immintrin.h>
Expand All @@ -14,6 +15,32 @@
#endif
#endif

#if !defined(BLAKE3_ATOMICS)
#if defined(__has_include)
#if __has_include(<stdatomic.h>) && !defined(_MSC_VER)
#define BLAKE3_ATOMICS 1
#else
#define BLAKE3_ATOMICS 0
#endif /* __has_include(<stdatomic.h>) && !defined(_MSC_VER) */
#else
#define BLAKE3_ATOMICS 0
#endif /* defined(__has_include) */
#endif /* BLAKE3_ATOMICS */

#if BLAKE3_ATOMICS
#define ATOMIC_INT _Atomic int
#define ATOMIC_LOAD(x) x
#define ATOMIC_STORE(x, y) x = y
#elif defined(_MSC_VER)
#define ATOMIC_INT LONG
#define ATOMIC_LOAD(x) InterlockedOr(&x, 0)
#define ATOMIC_STORE(x, y) InterlockedExchange(&x, y)
#else
#define ATOMIC_INT int
#define ATOMIC_LOAD(x) x
#define ATOMIC_STORE(x, y) x = y
#endif

#define MAYBE_UNUSED(x) (void)((x))

#if defined(IS_X86)
Expand Down Expand Up @@ -76,22 +103,24 @@ enum cpu_feature {
#if !defined(BLAKE3_TESTING)
static /* Allow the variable to be controlled manually for testing */
#endif
enum cpu_feature g_cpu_features = UNDEFINED;
ATOMIC_INT g_cpu_features = UNDEFINED;

#if !defined(BLAKE3_TESTING)
static
#endif
enum cpu_feature
get_cpu_features(void) {

if (g_cpu_features != UNDEFINED) {
return g_cpu_features;
/* If TSAN detects a data race here, try compiling with -DBLAKE3_ATOMICS=1 */
enum cpu_feature features = ATOMIC_LOAD(g_cpu_features);
if (features != UNDEFINED) {
return features;
} else {
#if defined(IS_X86)
uint32_t regs[4] = {0};
uint32_t *eax = &regs[0], *ebx = &regs[1], *ecx = &regs[2], *edx = &regs[3];
(void)edx;
enum cpu_feature features = 0;
features = 0;
cpuid(regs, 0);
const int max_id = *eax;
cpuid(regs, 1);
Expand Down Expand Up @@ -124,7 +153,7 @@ static
}
}
}
g_cpu_features = features;
ATOMIC_STORE(g_cpu_features, features);
return features;
#else
/* How to detect NEON? */
Expand Down

0 comments on commit 12823b8

Please sign in to comment.