Skip to content

Commit ec0718b

Browse files
Fix data race in CPU-feature global, not just the dispatch pointers
The previous fix (PR #4) made the per-codec dispatch function pointers (crc32_impl, adler32_impl, ...) atomic, but each dispatcher still calls get_x86_cpu_features() / get_arm_cpu_features(), which lazily initialize the global libdeflate_x86_cpu_features / libdeflate_arm_cpu_features bitmask on the first call. That global was only 'volatile', so a plain load racing with the store in libdeflate_init_*_cpu_features() is undefined behavior and was still flagged by ThreadSanitizer: WARNING: ThreadSanitizer: data race Write ... libdeflate_init_x86_cpu_features cpu_features.c Read ... get_x86_cpu_features cpu_features.h Location is global 'libdeflate_x86_cpu_features' This reproduced as a "Server died" failure when two HTTP connections first compressed a gzip response concurrently under TSan. Access the global with relaxed __atomic_load_n / __atomic_store_n, matching the dispatch-pointer fix. The first-call initialization is a benign race (every thread computes the same bitmask, a pure function of the CPU), and relaxed ordering suffices because no other memory is published through it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3a9791b commit ec0718b

4 files changed

Lines changed: 32 additions & 10 deletions

File tree

lib/arm/cpu_features.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ static const struct cpu_feature arm_cpu_feature_table[] = {
202202
{ARM_CPU_FEATURE_DOTPROD, "dotprod"},
203203
};
204204

205-
volatile u32 libdeflate_arm_cpu_features = 0;
205+
u32 libdeflate_arm_cpu_features = 0;
206206

207207
void libdeflate_init_arm_cpu_features(void)
208208
{
@@ -224,7 +224,8 @@ void libdeflate_init_arm_cpu_features(void)
224224
disable_cpu_features_for_testing(&features, arm_cpu_feature_table,
225225
ARRAY_LEN(arm_cpu_feature_table));
226226

227-
libdeflate_arm_cpu_features = features | ARM_CPU_FEATURES_KNOWN;
227+
__atomic_store_n(&libdeflate_arm_cpu_features,
228+
features | ARM_CPU_FEATURES_KNOWN, __ATOMIC_RELAXED);
228229
}
229230

230231
#endif /* ARM_CPU_FEATURES_KNOWN */

lib/arm/cpu_features.h

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,25 @@
5151
(defined(_WIN32) && defined(ARCH_ARM64)))
5252
/* Runtime ARM CPU feature detection is supported. */
5353
# define ARM_CPU_FEATURES_KNOWN (1U << 31)
54-
extern volatile u32 libdeflate_arm_cpu_features;
54+
extern u32 libdeflate_arm_cpu_features;
5555

5656
void libdeflate_init_arm_cpu_features(void);
5757

58+
/*
59+
* Resolved on the first call. Accessed with relaxed atomics: the first-call initialization is a
60+
* benign race (every thread computes the same features bitmask, a pure function of the CPU), but
61+
* a plain load racing with the store in libdeflate_init_arm_cpu_features() is undefined behavior
62+
* and is flagged by ThreadSanitizer. Relaxed ordering suffices because no other memory is
63+
* published through it.
64+
*/
5865
static inline u32 get_arm_cpu_features(void)
5966
{
60-
if (libdeflate_arm_cpu_features == 0)
67+
u32 features = __atomic_load_n(&libdeflate_arm_cpu_features, __ATOMIC_RELAXED);
68+
if (features == 0) {
6169
libdeflate_init_arm_cpu_features();
62-
return libdeflate_arm_cpu_features;
70+
features = __atomic_load_n(&libdeflate_arm_cpu_features, __ATOMIC_RELAXED);
71+
}
72+
return features;
6373
}
6474
#else
6575
static inline u32 get_arm_cpu_features(void) { return 0; }

lib/x86/cpu_features.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ static const struct cpu_feature x86_cpu_feature_table[] = {
8686
{X86_CPU_FEATURE_AVXVNNI, "avx_vnni"},
8787
};
8888

89-
volatile u32 libdeflate_x86_cpu_features = 0;
89+
u32 libdeflate_x86_cpu_features = 0;
9090

9191
static inline bool
9292
os_supports_avx512(u64 xcr0)
@@ -207,7 +207,8 @@ void libdeflate_init_x86_cpu_features(void)
207207
disable_cpu_features_for_testing(&features, x86_cpu_feature_table,
208208
ARRAY_LEN(x86_cpu_feature_table));
209209

210-
libdeflate_x86_cpu_features = features | X86_CPU_FEATURES_KNOWN;
210+
__atomic_store_n(&libdeflate_x86_cpu_features,
211+
features | X86_CPU_FEATURES_KNOWN, __ATOMIC_RELAXED);
211212
}
212213

213214
#endif /* X86_CPU_FEATURES_KNOWN */

lib/x86/cpu_features.h

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,25 @@
5353
#if defined(__GNUC__) || defined(__clang__) || defined(_MSC_VER)
5454
/* Runtime x86 CPU feature detection is supported. */
5555
# define X86_CPU_FEATURES_KNOWN (1U << 31)
56-
extern volatile u32 libdeflate_x86_cpu_features;
56+
extern u32 libdeflate_x86_cpu_features;
5757

5858
void libdeflate_init_x86_cpu_features(void);
5959

60+
/*
61+
* Resolved on the first call. Accessed with relaxed atomics: the first-call initialization is a
62+
* benign race (every thread computes the same features bitmask, a pure function of the CPU), but
63+
* a plain load racing with the store in libdeflate_init_x86_cpu_features() is undefined behavior
64+
* and is flagged by ThreadSanitizer. Relaxed ordering suffices because no other memory is
65+
* published through it.
66+
*/
6067
static inline u32 get_x86_cpu_features(void)
6168
{
62-
if (libdeflate_x86_cpu_features == 0)
69+
u32 features = __atomic_load_n(&libdeflate_x86_cpu_features, __ATOMIC_RELAXED);
70+
if (features == 0) {
6371
libdeflate_init_x86_cpu_features();
64-
return libdeflate_x86_cpu_features;
72+
features = __atomic_load_n(&libdeflate_x86_cpu_features, __ATOMIC_RELAXED);
73+
}
74+
return features;
6575
}
6676
/*
6777
* x86 intrinsics are also supported. Include the headers needed to use them.

0 commit comments

Comments
 (0)