From ec0718b8e06dc172eb87ede6a493865ccf7610ec Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 30 Jun 2026 14:58:17 +0000 Subject: [PATCH] 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) --- lib/arm/cpu_features.c | 5 +++-- lib/arm/cpu_features.h | 16 +++++++++++++--- lib/x86/cpu_features.c | 5 +++-- lib/x86/cpu_features.h | 16 +++++++++++++--- 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/lib/arm/cpu_features.c b/lib/arm/cpu_features.c index 42f81f5e..18b8018d 100644 --- a/lib/arm/cpu_features.c +++ b/lib/arm/cpu_features.c @@ -202,7 +202,7 @@ static const struct cpu_feature arm_cpu_feature_table[] = { {ARM_CPU_FEATURE_DOTPROD, "dotprod"}, }; -volatile u32 libdeflate_arm_cpu_features = 0; +u32 libdeflate_arm_cpu_features = 0; void libdeflate_init_arm_cpu_features(void) { @@ -224,7 +224,8 @@ void libdeflate_init_arm_cpu_features(void) disable_cpu_features_for_testing(&features, arm_cpu_feature_table, ARRAY_LEN(arm_cpu_feature_table)); - libdeflate_arm_cpu_features = features | ARM_CPU_FEATURES_KNOWN; + __atomic_store_n(&libdeflate_arm_cpu_features, + features | ARM_CPU_FEATURES_KNOWN, __ATOMIC_RELAXED); } #endif /* ARM_CPU_FEATURES_KNOWN */ diff --git a/lib/arm/cpu_features.h b/lib/arm/cpu_features.h index dc9ab8ad..2fd291e1 100644 --- a/lib/arm/cpu_features.h +++ b/lib/arm/cpu_features.h @@ -51,15 +51,25 @@ (defined(_WIN32) && defined(ARCH_ARM64))) /* Runtime ARM CPU feature detection is supported. */ # define ARM_CPU_FEATURES_KNOWN (1U << 31) -extern volatile u32 libdeflate_arm_cpu_features; +extern u32 libdeflate_arm_cpu_features; void libdeflate_init_arm_cpu_features(void); +/* + * Resolved on the first call. Accessed with relaxed atomics: the first-call initialization is a + * benign race (every thread computes the same features bitmask, a pure function of the CPU), but + * a plain load racing with the store in libdeflate_init_arm_cpu_features() is undefined behavior + * and is flagged by ThreadSanitizer. Relaxed ordering suffices because no other memory is + * published through it. + */ static inline u32 get_arm_cpu_features(void) { - if (libdeflate_arm_cpu_features == 0) + u32 features = __atomic_load_n(&libdeflate_arm_cpu_features, __ATOMIC_RELAXED); + if (features == 0) { libdeflate_init_arm_cpu_features(); - return libdeflate_arm_cpu_features; + features = __atomic_load_n(&libdeflate_arm_cpu_features, __ATOMIC_RELAXED); + } + return features; } #else static inline u32 get_arm_cpu_features(void) { return 0; } diff --git a/lib/x86/cpu_features.c b/lib/x86/cpu_features.c index a4e09ada..31e7f805 100644 --- a/lib/x86/cpu_features.c +++ b/lib/x86/cpu_features.c @@ -86,7 +86,7 @@ static const struct cpu_feature x86_cpu_feature_table[] = { {X86_CPU_FEATURE_AVXVNNI, "avx_vnni"}, }; -volatile u32 libdeflate_x86_cpu_features = 0; +u32 libdeflate_x86_cpu_features = 0; static inline bool os_supports_avx512(u64 xcr0) @@ -207,7 +207,8 @@ void libdeflate_init_x86_cpu_features(void) disable_cpu_features_for_testing(&features, x86_cpu_feature_table, ARRAY_LEN(x86_cpu_feature_table)); - libdeflate_x86_cpu_features = features | X86_CPU_FEATURES_KNOWN; + __atomic_store_n(&libdeflate_x86_cpu_features, + features | X86_CPU_FEATURES_KNOWN, __ATOMIC_RELAXED); } #endif /* X86_CPU_FEATURES_KNOWN */ diff --git a/lib/x86/cpu_features.h b/lib/x86/cpu_features.h index cb225b98..99b80180 100644 --- a/lib/x86/cpu_features.h +++ b/lib/x86/cpu_features.h @@ -53,15 +53,25 @@ #if defined(__GNUC__) || defined(__clang__) || defined(_MSC_VER) /* Runtime x86 CPU feature detection is supported. */ # define X86_CPU_FEATURES_KNOWN (1U << 31) -extern volatile u32 libdeflate_x86_cpu_features; +extern u32 libdeflate_x86_cpu_features; void libdeflate_init_x86_cpu_features(void); +/* + * Resolved on the first call. Accessed with relaxed atomics: the first-call initialization is a + * benign race (every thread computes the same features bitmask, a pure function of the CPU), but + * a plain load racing with the store in libdeflate_init_x86_cpu_features() is undefined behavior + * and is flagged by ThreadSanitizer. Relaxed ordering suffices because no other memory is + * published through it. + */ static inline u32 get_x86_cpu_features(void) { - if (libdeflate_x86_cpu_features == 0) + u32 features = __atomic_load_n(&libdeflate_x86_cpu_features, __ATOMIC_RELAXED); + if (features == 0) { libdeflate_init_x86_cpu_features(); - return libdeflate_x86_cpu_features; + features = __atomic_load_n(&libdeflate_x86_cpu_features, __ATOMIC_RELAXED); + } + return features; } /* * x86 intrinsics are also supported. Include the headers needed to use them.