From 313692d51efc2ea41058a18b8ec1abd59674c9b7 Mon Sep 17 00:00:00 2001 From: Wan-Teh Chang Date: Fri, 1 Aug 2025 19:07:41 -0700 Subject: [PATCH 1/2] Use file static variables in WEBP_DSP_INIT_FUNC() Function static variables are initialized on the first call to the function. In C the initialization of function static variables is not thread-safe. Use file static variables instead in the WEBP_DSP_INIT_FUNC() macro. Remove the volatile qualifier for the pthread version of the func##_last_cpuinfo_used variable because the variable is only accessed while holding the mutex. Change-Id: I1237904a49d2467d7ce79fc53f9e7f966aa7a5c1 --- src/dsp/cpu.h | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/dsp/cpu.h b/src/dsp/cpu.h index 8dc0c0cb..913e857f 100644 --- a/src/dsp/cpu.h +++ b/src/dsp/cpu.h @@ -193,23 +193,25 @@ #include // NOLINT // clang-format off -#define WEBP_DSP_INIT(func) \ - do { \ - static volatile VP8CPUInfo func##_last_cpuinfo_used = \ - (VP8CPUInfo)&func##_last_cpuinfo_used; \ - static pthread_mutex_t func##_lock = PTHREAD_MUTEX_INITIALIZER; \ - if (pthread_mutex_lock(&func##_lock)) break; \ - if (func##_last_cpuinfo_used != VP8GetCPUInfo) func(); \ - func##_last_cpuinfo_used = VP8GetCPUInfo; \ - (void)pthread_mutex_unlock(&func##_lock); \ +#define WEBP_DSP_INIT_VARS(func) \ + static VP8CPUInfo func##_last_cpuinfo_used = \ + (VP8CPUInfo)&func##_last_cpuinfo_used; \ + static pthread_mutex_t func##_lock = PTHREAD_MUTEX_INITIALIZER +#define WEBP_DSP_INIT(func) \ + do { \ + if (pthread_mutex_lock(&func##_lock)) break; \ + if (func##_last_cpuinfo_used != VP8GetCPUInfo) func(); \ + func##_last_cpuinfo_used = VP8GetCPUInfo; \ + (void)pthread_mutex_unlock(&func##_lock); \ } while (0) // clang-format on #else // !(defined(WEBP_USE_THREAD) && !defined(_WIN32)) // clang-format off +#define WEBP_DSP_INIT_VARS(func) \ + static volatile VP8CPUInfo func##_last_cpuinfo_used = \ + (VP8CPUInfo)&func##_last_cpuinfo_used #define WEBP_DSP_INIT(func) \ do { \ - static volatile VP8CPUInfo func##_last_cpuinfo_used = \ - (VP8CPUInfo)&func##_last_cpuinfo_used; \ if (func##_last_cpuinfo_used == VP8GetCPUInfo) break; \ func(); \ func##_last_cpuinfo_used = VP8GetCPUInfo; \ @@ -225,6 +227,7 @@ } */ #define WEBP_DSP_INIT_FUNC(name) \ + WEBP_DSP_INIT_VARS(name##_body); \ static WEBP_TSAN_IGNORE_FUNCTION void name##_body(void); \ WEBP_TSAN_IGNORE_FUNCTION void name(void) { WEBP_DSP_INIT(name##_body); } \ static WEBP_TSAN_IGNORE_FUNCTION void name##_body(void) From 54f23b049e06b2397722c2b3ee8e0e7bc4e6bf9b Mon Sep 17 00:00:00 2001 From: Wan-Teh Chang Date: Fri, 1 Aug 2025 16:20:52 -0700 Subject: [PATCH 2/2] Implement WEBP_DSP_INIT with SRWLOCK for Windows A slim reader/writer (SRW) lock can be initialized statically with the constant SRWLOCK_INIT. It is the only Windows synchronization object I can find with this property. Note: On old Windows versions that don't have SRWLOCK, use the fallback, thread-unsafe implementation. Change a NOLINT comment to a NOLINTNEXTLINE comment to prevent clang-format from aligning the #else and #endif comments in undesired way. Bug: 435213378 Change-Id: Iecff615a14a1905aedd2c05ad9444889f711cc17 --- src/dsp/cpu.h | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/src/dsp/cpu.h b/src/dsp/cpu.h index 913e857f..7c0cb376 100644 --- a/src/dsp/cpu.h +++ b/src/dsp/cpu.h @@ -189,8 +189,40 @@ #endif #endif -#if defined(WEBP_USE_THREAD) && !defined(_WIN32) -#include // NOLINT +#if defined(WEBP_USE_THREAD) +#if defined(_WIN32) +#include + +#if _WIN32_WINNT >= 0x0600 +// clang-format off +#define WEBP_DSP_INIT_VARS(func) \ + static VP8CPUInfo func##_last_cpuinfo_used = \ + (VP8CPUInfo)&func##_last_cpuinfo_used; \ + static SRWLOCK func##_lock = SRWLOCK_INIT +#define WEBP_DSP_INIT(func) \ + do { \ + AcquireSRWLockExclusive(&func##_lock); \ + if (func##_last_cpuinfo_used != VP8GetCPUInfo) func(); \ + func##_last_cpuinfo_used = VP8GetCPUInfo; \ + ReleaseSRWLockExclusive(&func##_lock); \ + } while (0) +// clang-format on +#else // _WIN32_WINNT < 0x0600 +// clang-format off +#define WEBP_DSP_INIT_VARS(func) \ + static volatile VP8CPUInfo func##_last_cpuinfo_used = \ + (VP8CPUInfo)&func##_last_cpuinfo_used +#define WEBP_DSP_INIT(func) \ + do { \ + if (func##_last_cpuinfo_used == VP8GetCPUInfo) break; \ + func(); \ + func##_last_cpuinfo_used = VP8GetCPUInfo; \ + } while (0) +// clang-format on +#endif // _WIN32_WINNT >= 0x0600 +#else // !defined(_WIN32) +// NOLINTNEXTLINE +#include // clang-format off #define WEBP_DSP_INIT_VARS(func) \ @@ -205,7 +237,8 @@ (void)pthread_mutex_unlock(&func##_lock); \ } while (0) // clang-format on -#else // !(defined(WEBP_USE_THREAD) && !defined(_WIN32)) +#endif // defined(_WIN32) +#else // !defined(WEBP_USE_THREAD) // clang-format off #define WEBP_DSP_INIT_VARS(func) \ static volatile VP8CPUInfo func##_last_cpuinfo_used = \ @@ -217,7 +250,7 @@ func##_last_cpuinfo_used = VP8GetCPUInfo; \ } while (0) // clang-format on -#endif // defined(WEBP_USE_THREAD) && !defined(_WIN32) +#endif // defined(WEBP_USE_THREAD) // Defines an Init + helper function that control multiple initialization of // function pointers / tables.