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>
This commit is contained in:
Wan-Teh Chang 2016-12-07 16:16:02 -08:00 committed by Vittorio Giovara
parent d5759701a8
commit 2170017a1c
6 changed files with 96 additions and 22 deletions

View File

@ -145,3 +145,5 @@ TESTPROGS = adler32 \
sha \
tree \
xtea \
TESTPROGS-$(HAVE_THREADS) += cpu_init

View File

@ -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);
static int get_cpu_flags(void)
{
if (ARCH_AARCH64)
return ff_get_cpu_flags_aarch64();
if (ARCH_ARM)
return ff_get_cpu_flags_arm();
if (ARCH_PPC)
return ff_get_cpu_flags_ppc();
if (ARCH_X86)
return ff_get_cpu_flags_x86();
return 0;
}
int av_get_cpu_flags(void)
{
static int flags;
if (checked)
return flags;
if (ARCH_AARCH64)
flags = ff_get_cpu_flags_aarch64();
if (ARCH_ARM)
flags = ff_get_cpu_flags_arm();
if (ARCH_PPC)
flags = ff_get_cpu_flags_ppc();
if (ARCH_X86)
flags = ff_get_cpu_flags_x86();
flags &= cpuflags_mask;
checked = 1;
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)

View File

@ -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);

View File

@ -5,6 +5,7 @@
/base64
/blowfish
/cpu
/cpu_init
/crc
/des
/eval

View File

@ -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;
}

View File

@ -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)