[Bug middle-end/48076] New: Unsafe double checked locking in __emutls_get_address

["eugeni.stepanov at gmail dot com" <gcc-bugzilla at gcc dot gnu dot org>]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48076

           Summary: Unsafe double checked locking in __emutls_get_address
           Product: gcc
           Version: 4.4.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: eugeni.stepanov@gmail.com


The following piece of code around emutls.c:138 uses double checked locking to
initialize a tls offset to the next available value.

  pointer offset = obj->loc.offset;

  if (__builtin_expect (offset == 0, 0))
    {
      static __gthread_once_t once = __GTHREAD_ONCE_INIT;
      __gthread_once (&once, emutls_init);
      __gthread_mutex_lock (&emutls_mutex);
      offset = obj->loc.offset;
      if (offset == 0)
        {
          offset = ++emutls_size;
          obj->loc.offset = offset;
        }
      __gthread_mutex_unlock (&emutls_mutex);
    }

  struct __emutls_array *arr = __gthread_getspecific (emutls_key);

This code needs more barriers to be correct. For example, it is entirely
possible for emutls_key value used in the last line of the code snippet to be
prefetched before obj->loc.offset is loaded, and, consequently, before
emutls_init is called. In short, there needs to be a happens-before
relationship between obj->loc.offset assignment and the unprotected read at the
first line.

This looks unlikely on x86, but it may be a much bigger deal on ARM.

This was detected with ThreadSanitizer
(http://code.google.com/p/data-race-test/wiki/ThreadSanitizer) on an Android
device.

==822== WARNING: Possible data race during read of size 4 at 0x44B74: {{{
==822==    T14 (L{}):
==822==     #0  0xB7E0: __emutls_get_address gcc/emutls.c:138
==822==     #1  0x1BFD5: NegativeTests_PerThreadTest::RealWorker()
unittest/racecheck_unittest.cc:5665
==822==     #2  0x80107324: ThreadSanitizerStartThread
tsan/ts_valgrind_intercepts.c:679
==822==   Concurrent write(s) happened at (OR AFTER) these points:
==822==    T12 (L{L312}):
==822==     #0  0xB80C: __emutls_get_address gcc/emutls.c:145
==822==     #1  0x1BFD5: NegativeTests_PerThreadTest::RealWorker()
unittest/racecheck_unittest.cc:5665
==822==     #2  0x80107324: ThreadSanitizerStartThread
tsan/ts_valgrind_intercepts.c:679
==822==   Address 0x44B74 is 8 bytes inside data symbol
"__emutls_v._ZN27NegativeTests_PerThreadTestL17per_thread_globalE"
==822==   Locks involved in this report (reporting last lock sites): {L312}
==822==    L312 (0x44C18)
==822==     #0  0x80108084: pthread_mutex_lock
tsan/ts_valgrind_intercepts.c:934
==822==     #1  0xB808: __emutls_get_address gcc/gthr-posix.h:768
==822==     #2  0x1BFD5: NegativeTests_PerThreadTest::RealWorker()
unittest/racecheck_unittest.cc:5665
==822==     #3  0x80107324: ThreadSanitizerStartThread
tsan/ts_valgrind_intercepts.c:679
==822==    Race verifier data: 0xB7E0,0xB80C
==822== }}}