-bf-by-func is not thread safe

[cdaley]

There are data races when using the -bf-by-func Byfl instrumentation option. The race conditions happen when accessing objects in bf_incr_func_tally.

==78232==  Lock at 0x56F5AC0 was first observed
==78232==    at 0x4C2C466: mutex_lock_WRK (hg_intercepts.c:901)
==78232==    by 0x4C30201: pthread_mutex_lock (hg_intercepts.c:917)
==78232==    by 0x50E82FF: bf_acquire_mega_lock (threading.cpp:24)
==78232==    by 0x40143C: main (init.c:4)
==78232==  Location 0x56f5ac0 is 0 bytes inside global var "megalock"
==78232==  declared at threading.cpp:12
==78232== 
==78232== Possible data race during read of size 8 at 0x65CFFD0 by thread #2
==78232== Locks held: none
==78232==    at 0x50CA461: find (cachemap.h:58)
==78232==    by 0x50CA461: CachedAnyMap<std::unordered_map<unsigned long, unsigned long, std::hash<unsigned long>, 
std::equal_to<unsigned long>, std::allocator<std::pair<unsigned long const, unsigned long> > >, unsigned long, unsi
gned long, std::equal_to<unsigned long> >::operator[](unsigned long const&) (cachemap.h:113)
==78232==    by 0x50CAB67: bf_incr_func_tally (byfl.cpp:209)
==78232==    by 0x401C93: main._omp_fn.0 (init.c:9)
==78232==    by 0x591BB89: gomp_thread_start (team.c:115)
==78232==    by 0x4C2EF50: mythread_wrapper (hg_intercepts.c:389)
==78232==    by 0x56FD805: start_thread (in /lib64/libpthread-2.11.3.so)
==78232==    by 0x5E099BC: clone (in /lib64/libc-2.11.3.so)
==78232== 
==78232== This conflicts with a previous write of size 8 by thread #1
==78232== Locks held: 1, at address 0x56F5AC0
==78232==    at 0x50CA578: find (cachemap.h:88)
==78232==    by 0x50CA578: CachedAnyMap<std::unordered_map<unsigned long, unsigned long, std::hash<unsigned long>, 
std::equal_to<unsigned long>, std::allocator<std::pair<unsigned long const, unsigned long> > >, unsigned long, unsi
gned long, std::equal_to<unsigned long> >::operator[](unsigned long const&) (cachemap.h:113)
==78232==    by 0x50CAB67: bf_incr_func_tally (byfl.cpp:209)
==78232==    by 0x401EF3: main._omp_fn.0 (init.c:9)
==78232==    by 0x401629: main (init.c:9)
==78232==  Address 0x65cffd0 is 0 bytes inside a block of size 16 alloc'd
==78232==    at 0x4C29C15: operator new(unsigned long) (vg_replace_malloc.c:333)
==78232==    by 0x50CA55E: find (cachemap.h:84)
==78232==    by 0x50CA55E: CachedAnyMap<std::unordered_map<unsigned long, unsigned long, std::hash<unsigned long>, 
std::equal_to<unsigned long>, std::allocator<std::pair<unsigned long const, unsigned long> > >, unsigned long, unsi
gned long, std::equal_to<unsigned long> >::operator[](unsigned long const&) (cachemap.h:113)
==78232==    by 0x50CAB67: bf_incr_func_tally (byfl.cpp:209)
==78232==    by 0x4013EE: main (init.c:4)
==78232==  Block was alloc'd by thread #1

I am using byfl-1.5-llvm-3.5.2 and gcc-4.8.1 on Cori at NERSC. You should be able to reproduce it using:

> cat init.c
#include <stdio.h>
#define UB 10000

int main()
{
  int x[UB];
  int i, sum;

#pragma omp parallel for
  for (i=0; i<UB; i++) {
    x[i] = 1;
  }
  printf("Yay, we initialized an array. First element is %d\n", x[0]);
  return 0;
}
> export BF_OPTS="-bf-by-func -bf-thread-safe -bf-verbose"
> bf-gcc init.c -fopenmp -o init
> export OMP_NUM_THREADS=2
> valgrind --tool=helgrind --read-inline-info=yes --read-var-info=yes --check-stack-refs=yes --log-file=helgrind.out ./init

You mentioned by email that locking is handled in the Byfl function instrument.cpp. There are definitely TallyByFunction code blocks which are not protected by the "mega-lock", but I don't know which ones require the lock calls.