avutil: fix data race in av_get_cpu_flags()

Make the one-time initialization in av_get_cpu_flags() thread-safe. The
static variables |flags|, |cpuflags_mask|, and |checked| in
libavutil/cpu.c are read and written using normal load and store
operations. These are considered as data races. The fix is to use atomic
load and store operations.

Remove the |checked| variable because the invalid value of -1 for
|flags| can be used to indicate the same condition. Rename |flags| to
|cpu_flags| and move it to file scope.

The fix can be verified by running the libavutil/tests/cpu_init.c test
program under ThreadSanitizer:
    ./configure --toolchain=clang-tsan
    make libavutil/tests/cpu_init
    libavutil/tests/cpu_init

There should be no warnings from ThreadSanitizer.

Co-author: Dmitry Vyukov of Google, who suggested the data race fix.

Signed-off-by: Wan-Teh Chang <wtc@google.com>

5 files changed, 86 insertions, 18 deletions

diff --git a/libavutil/Makefile b/libavutil/Makefile
index 28372c9bef..f34c79950e 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -145,3 +145,5 @@ TESTPROGS = adler32                                                     \
             sha                                                         \
             tree                                                        \
             xtea                                                        \
+
+TESTPROGS-$(HAVE_THREADS)               += cpu_init
diff --git a/libavutil/cpu.c b/libavutil/cpu.c
index 7d7390a756..0109c9e8d1 100644
--- a/libavutil/cpu.c
+++ b/libavutil/cpu.c
@@ -17,6 +17,7 @@
  */
 
 #include <stdint.h>
+#include <stdatomic.h>
 
 #include "cpu.h"
 #include "cpu_internal.h"
@@ -42,34 +43,35 @@
 #include <unistd.h>
 #endif
 
-static int cpuflags_mask = -1, checked;
+static atomic_int cpu_flags = ATOMIC_VAR_INIT(-1);
 
-int av_get_cpu_flags(void)
+static int get_cpu_flags(void)
 {
-    static int flags;
-
-    if (checked)
-        return flags;
-
     if (ARCH_AARCH64)
-        flags = ff_get_cpu_flags_aarch64();
+        return ff_get_cpu_flags_aarch64();
     if (ARCH_ARM)
-        flags = ff_get_cpu_flags_arm();
+        return ff_get_cpu_flags_arm();
     if (ARCH_PPC)
-        flags = ff_get_cpu_flags_ppc();
+        return ff_get_cpu_flags_ppc();
     if (ARCH_X86)
-        flags = ff_get_cpu_flags_x86();
-
-    flags  &= cpuflags_mask;
-    checked = 1;
+        return ff_get_cpu_flags_x86();
+    return 0;
+}
 
+int av_get_cpu_flags(void)
+{
+    int flags = atomic_load_explicit(&cpu_flags, memory_order_relaxed);
+    if (flags == -1) {
+        flags = get_cpu_flags();
+        atomic_store_explicit(&cpu_flags, flags, memory_order_relaxed);
+    }
     return flags;
 }
 
 void av_set_cpu_flags_mask(int mask)
 {
-    cpuflags_mask = mask;
-    checked       = 0;
+    atomic_store_explicit(&cpu_flags, get_cpu_flags() & mask,
+                          memory_order_relaxed);
 }
 
 int av_parse_cpu_flags(const char *s)
diff --git a/libavutil/cpu.h b/libavutil/cpu.h
index deff4ccf55..c205ee16f6 100644
--- a/libavutil/cpu.h
+++ b/libavutil/cpu.h
@@ -73,8 +73,6 @@ int av_get_cpu_flags(void);
 /**
  * Set a mask on flags returned by av_get_cpu_flags().
  * This function is mainly useful for testing.
- *
- * @warning this function is not thread safe.
  */
 void av_set_cpu_flags_mask(int mask);
 
diff --git a/libavutil/tests/.gitignore b/libavutil/tests/.gitignore
index c4d1d065fb..c8f37a7f90 100644
--- a/libavutil/tests/.gitignore
+++ b/libavutil/tests/.gitignore
@@ -5,6 +5,7 @@
 /base64
 /blowfish
 /cpu
+/cpu_init
 /crc
 /des
 /eval
diff --git a/libavutil/tests/cpu_init.c b/libavutil/tests/cpu_init.c
new file mode 100644
index 0000000000..beb95c911f
--- /dev/null
+++ b/libavutil/tests/cpu_init.c
@@ -0,0 +1,65 @@
+/*
+ * This file is part of Libav.
+ *
+ * Libav is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * Libav is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Libav; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/*
+ * This test program tests whether the one-time initialization in
+ * av_get_cpu_flags() has data races.
+ */
+
+#include <stdio.h>
+#include <string.h>
+
+#include "libavutil/cpu.h"
+#include "libavutil/thread.h"
+
+static void *thread_main(void *arg)
+{
+    int *flags = arg;
+
+    *flags = av_get_cpu_flags();
+    return NULL;
+}
+
+int main(void)
+{
+    int cpu_flags1;
+    int cpu_flags2;
+    int ret;
+    pthread_t thread1;
+    pthread_t thread2;
+
+    if ((ret = pthread_create(&thread1, NULL, thread_main, &cpu_flags1))) {
+        fprintf(stderr, "pthread_create failed: %s.\n", strerror(ret));
+        return 1;
+    }
+    if ((ret = pthread_create(&thread2, NULL, thread_main, &cpu_flags2))) {
+        fprintf(stderr, "pthread_create failed: %s.\n", strerror(ret));
+        return 1;
+    }
+    pthread_join(thread1, NULL);
+    pthread_join(thread2, NULL);
+
+    if (cpu_flags1 < 0)
+        return 2;
+    if (cpu_flags2 < 0)
+        return 2;
+    if (cpu_flags1 != cpu_flags2)
+        return 3;
+
+    return 0;
+}