[FFmpeg-cvslog] avutil: fix data race in av_get_cpu_flags()
Wan-Teh Chang
git at videolan.org
Fri Apr 14 00:26:40 EEST 2017
ffmpeg | branch: master | Wan-Teh Chang <wtc at google.com> | Wed Dec 7 16:16:02 2016 -0800| [2170017a1cd033b6f28e16476921022712a522d8] | committer: Vittorio Giovara
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 at google.com>
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=2170017a1cd033b6f28e16476921022712a522d8
---
libavutil/Makefile | 2 ++
libavutil/cpu.c | 34 ++++++++++++------------
libavutil/cpu.h | 2 --
libavutil/tests/.gitignore | 1 +
libavutil/tests/cpu_init.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
tests/fate/libavutil.mak | 6 +++++
6 files changed, 92 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;
+}
diff --git a/tests/fate/libavutil.mak b/tests/fate/libavutil.mak
index 2824e084eb..8b17411370 100644
--- a/tests/fate/libavutil.mak
+++ b/tests/fate/libavutil.mak
@@ -25,6 +25,11 @@ fate-cpu: libavutil/tests/cpu$(EXESUF)
fate-cpu: CMD = run libavutil/tests/cpu $(CPUFLAGS:%=-c%) $(THREADS:%=-t%)
fate-cpu: REF = /dev/null
+FATE_LIBAVUTIL-$(HAVE_THREADS) += fate-cpu_init
+fate-cpu_init: libavutil/tests/cpu_init$(EXESUF)
+fate-cpu_init: CMD = run libavutil/tests/cpu_init
+fate-cpu_init: REF = /dev/null
+
FATE_LIBAVUTIL += fate-crc
fate-crc: libavutil/tests/crc$(EXESUF)
fate-crc: CMD = run libavutil/tests/crc
@@ -73,5 +78,6 @@ FATE_LIBAVUTIL += fate-xtea
fate-xtea: libavutil/tests/xtea$(EXESUF)
fate-xtea: CMD = run libavutil/tests/xtea
+FATE_LIBAVUTIL += $(FATE_LIBAVUTIL-yes)
FATE-$(CONFIG_AVUTIL) += $(FATE_LIBAVUTIL)
fate-libavutil: $(FATE_LIBAVUTIL)
More information about the ffmpeg-cvslog
mailing list