[Bug sanitizer/55561] New: TSAN crashes for Fortran

[Bug sanitizer/55561] New: TSAN crashes for Fortran

[Joost.VandeVondele at mat dot ethz.ch]

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

             Bug #: 55561
           Summary: TSAN crashes for Fortran
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: Joost.VandeVondele@mat.ethz.ch
                CC: dodji@gcc.gnu.org, dvyukov@gcc.gnu.org,
                    jakub@gcc.gnu.org, kcc@gcc.gnu.org


Current trunk crashes if -fsanitize=thread is used for Fortran programs. This
is contrary to -fsanitize=address, which seems to work (PR55341). I think there
would be quite some interest in having -fsanitize=thread to work for OMP'ed
Fortran programs.

> cat test.f90
J=0
!$OMP PARALLEL DO
DO I=1,10
   J=J+1
ENDDO
WRITE(6,*) J
END

> gfortran -fopenmp -fsanitize=thread test.f90 
test.f90:7:0: internal compiler error: Segmentation fault
 END
 ^
0x98cc1f crash_signal
        ../../gcc/gcc/toplev.c:334
0x6718e0 contains_struct_check
        ../../gcc/gcc/tree.h:3784
0x6718e0 build_call_expr_loc_array(unsigned int, tree_node*, int, tree_node**)
        ../../gcc/gcc/builtins.c:11206
0x672f82 build_call_expr(tree_node*, int, ...)
        ../../gcc/gcc/builtins.c:11256
0x9a0d76 tsan_finish_file()
        ../../gcc/gcc/tsan.c:316
Please submit a full bug report,

[Bug sanitizer/55561] TSAN crashes for Fortran

[dvyukov at google dot com]

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

Dmitry Vyukov <dvyukov at google dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dvyukov at google dot com

--- Comment #1 from Dmitry Vyukov <dvyukov at google dot com> 2012-12-02 09:21:19 UTC ---
Note that ThreadSanitizer does not understand Fortran/OpenMP synchronization,
so most likely it won't be useful anyway.

[Bug sanitizer/55561] TSAN crashes for Fortran

[Joost.VandeVondele at mat dot ethz.ch]

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

Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |Joost.VandeVondele at mat
                   |                            |dot ethz.ch

--- Comment #2 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-02 09:30:17 UTC ---
(In reply to comment #1)
> Note that ThreadSanitizer does not understand Fortran/OpenMP synchronization,
> so most likely it won't be useful anyway.

I was assuming that it would do just fine, as OMP in gcc is just built on
pthreads ? I believe that Fortran and C are relatively similar from a
middle-end perspective.

[Bug sanitizer/55561] TSAN crashes for Fortran

[dvyukov at google dot com]

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

--- Comment #3 from Dmitry Vyukov <dvyukov at google dot com> 2012-12-02 09:35:41 UTC ---
Well, then maybe it will work. But we did not do anything special for Fortran
and OpenMP, and did not do any testing.

[Bug sanitizer/55561] TSAN crashes for Fortran

[kcc at gcc dot gnu.org]

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

--- Comment #4 from Kostya Serebryany <kcc at gcc dot gnu.org> 2012-12-02 10:28:10 UTC ---
Note that tsan currently requires to use -pie linker flag (and either -fPIC or
-fPIE compiler flag). 
Obviously, your failure happens before the run-time library has a chance to
crash.

[Bug sanitizer/55561] TSAN crashes for Fortran

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-12-02 21:11:07 UTC ---
Are you testing with all the pending unreviewed TSAN fixes?
http://gcc.gnu.org/ml/gcc-patches/2012-11/msg01875.html
http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02578.html
http://gcc.gnu.org/ml/gcc-patches/2012-12/msg00018.html

[Bug sanitizer/55561] TSAN crashes for Fortran

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #6 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-03 07:41:29 UTC ---
(In reply to comment #5)
> Are you testing with all the pending unreviewed TSAN fixes?

Ah.. no, I will retest once they are in trunk. Thanks!

[Bug sanitizer/55561] TSAN crashes for Fortran

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #7 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-10 12:43:42 UTC ---
Now, compilation seems to go fine, but I'm not figuring out how to do it
properly so it works at run time. I have:

> gfortran -g  -fsanitize=thread -fPIE PR55561.f90 
> ./a.out
FATAL: ThreadSanitizer can not mmap the shadow memory (something is mapped at
0x400000 < 0x7cf000000000)
FATAL: Make sure to compile with -fPIE and to link with -pie.

next try yields

> gfortran -g  -fsanitize=thread -fPIC -Wl,-pie PR55561.f90 
/data/vjoost/gnu/binutils-2.22/install/bin/ld: error: /usr/lib/../lib64/crt1.o:
requires unsupported dynamic reloc 11; recompile with -fPIC
/data/vjoost/gnu/binutils-2.22/install/bin/ld: error:
/data/vjoost/gnu/gcc_trunk/install/lib/gcc/x86_64-unknown-linux-gnu/4.8.0/crtbegin.o:
requires dynamic R_X86_64_32 reloc against '__TMC_END__' which may overflow at
runtime; recompile with -fPIC

Is there something wrong in the way I configured gcc or do I need a different
command line ?

../gcc/configure --prefix=/data/vjoost/gnu/gcc_trunk/install
--enable-languages=c,c++,fortran --disable-multilib --enable-plugins
--enable-lto --disable-bootstrap

[Bug sanitizer/55561] TSAN crashes for Fortran

[pinskia at gcc dot gnu.org]

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

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-12-10 12:46:56 UTC ---
(In reply to comment #7)
> Now, compilation seems to go fine, but I'm not figuring out how to do it
> properly so it works at run time. I have:
> 
> > gfortran -g  -fsanitize=thread -fPIE PR55561.f90 
> > ./a.out
> FATAL: ThreadSanitizer can not mmap the shadow memory (something is mapped at
> 0x400000 < 0x7cf000000000)
> FATAL: Make sure to compile with -fPIE and to link with -pie.
> 
> next try yields
> 
> > gfortran -g  -fsanitize=thread -fPIC -Wl,-pie PR55561.f90 
Try:
gfortran -g  -fsanitize=thread -fPIC -pie PR55561.f90 

The driver changes the startup files when -pie is passed.

[Bug sanitizer/55561] TSAN crashes for Fortran

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #9 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-10 12:53:22 UTC ---
(In reply to comment #8)
> gfortran -g  -fsanitize=thread -fPIC -pie PR55561.f90 

Thanks! yields the proper warning as expected.. 

> gfortran -g  -fopenmp -fsanitize=thread -fPIC -pie PR55561.f90 ; ./a.out
==================
WARNING: ThreadSanitizer: data race (pid=26592)
  Read of size 4 at 0x7fff74e67d6c by main thread:
    #0 MAIN__._omp_fn.0 PR55561.f90:0 (exe+0x000000000fff)
    #1 MAIN__ PR55561.f90:0 (exe+0x000000000e79)
    #2 main ??:0 (exe+0x000000000f75)

  Previous write of size 4 at 0x7fff74e67d6c by thread 1:
    #0 MAIN__._omp_fn.0 PR55561.f90:0 (exe+0x000000001020)
    #1 gomp_thread_start
/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:116
(libgomp.so.1+0x000000008449)

  Thread 1 (tid=26593, running) created at:
    #0 pthread_create ??:0 (libtsan.so.0+0x00000001be6c)
    #1 gomp_team_start
/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:440
(libgomp.so.1+0x0000000089f5)
    #2 main ??:0 (exe+0x000000000f75)

==================

[Bug sanitizer/55561] TSAN crashes for Fortran

[dvyukov at google dot com]

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

--- Comment #10 from Dmitry Vyukov <dvyukov at google dot com> 2012-12-10 12:57:01 UTC ---
(In reply to comment #9)
> (In reply to comment #8)
> > gfortran -g  -fsanitize=thread -fPIC -pie PR55561.f90 
> 
> Thanks! yields the proper warning as expected.. 
> 
> > gfortran -g  -fopenmp -fsanitize=thread -fPIC -pie PR55561.f90 ; ./a.out
> ==================
> WARNING: ThreadSanitizer: data race (pid=26592)
>   Read of size 4 at 0x7fff74e67d6c by main thread:
>     #0 MAIN__._omp_fn.0 PR55561.f90:0 (exe+0x000000000fff)
>     #1 MAIN__ PR55561.f90:0 (exe+0x000000000e79)
>     #2 main ??:0 (exe+0x000000000f75)
> 
>   Previous write of size 4 at 0x7fff74e67d6c by thread 1:
>     #0 MAIN__._omp_fn.0 PR55561.f90:0 (exe+0x000000001020)
>     #1 gomp_thread_start
> /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:116
> (libgomp.so.1+0x000000008449)
> 
>   Thread 1 (tid=26593, running) created at:
>     #0 pthread_create ??:0 (libtsan.so.0+0x00000001be6c)
>     #1 gomp_team_start
> /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:440
> (libgomp.so.1+0x0000000089f5)
>     #2 main ??:0 (exe+0x000000000f75)
> 
> ==================

Is is a correct report? Or false positive?

[Bug sanitizer/55561] TSAN crashes for Fortran

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #11 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-10 12:59:09 UTC ---
(In reply to comment #10)
> Is is a correct report? Or false positive?

This is a correct report for the testcase in comment #0 (as J is shared between
threads).

[Bug sanitizer/55561] TSAN crashes for Fortran

[dvyukov at google dot com]

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

--- Comment #12 from Dmitry Vyukov <dvyukov at google dot com> 2012-12-10 15:07:11 UTC ---
(In reply to comment #11)
> (In reply to comment #10)
> > Is is a correct report? Or false positive?
> 
> This is a correct report for the testcase in comment #0 (as J is shared between
> threads).

That's great that gcc tsan works for Fortran/OpenMP out of the box!

However, most likely Fortran/OpenMP will require some special handling to
produce better reports and catch more bugs (ThreadSanitizer reasons about
synchronization on pthread level, and not on OpenMP level). We won't have spare
cycles for that in near future.
I am working on ThreadSanitizer tasking API, I would appreciate if you review
it and comment on whether it will fit Fortran/OpenMP model (the work is
currently in very early "what if" stage).
The is the proposed public API:


// The tasking API is intended for programs that use finer-grained tasks
// on top of threads.  For such programs ThreadSanitizer can produce
// bad reports (namely, thread creation stack is useless), and catch less
// bugs because it does not understand task boundaries.
// The tasking API allows programs to annotate tasks to solve both problems.
// The API is intended to be flexible enough to support standard C++ tasks,
// OpenMP tasks as well as custom tasks.  It's also intended to be
// non-intrusive as possible to be used merely as annotations in existing code.

// Creation of a task.
// TASK is an arbitrary pointer that uniquely identifies the task.
// ThreadSanitizer stores any required internal info offline.
// FLAGS see below.
void __tsan_task_create(void *task, unsigned flags);

// Destruction of a task, frees all associated resources.
// After that the TASK pointer can be reused for another task.
void __tsan_task_destroy(void *task);

// Denotes beginning of execution of the TASK.
// When the function returns ThreadSanitizer effectively switches
// to another thread context associated with the task.
// Returns TASK.  This is needed because re-reading it even from stack variable
// can result in a data race with the worker thread (since we are already in
the
// task context).
// Tasks can be recursive, but must be strictly nested.
void* __tsan_task_begin(void *task);

// Denotes end of execution of the TASK.
// When the function returns ThreadSanitizer effectively switches
// back to worker thread context.
void __tsan_task_end(void *task);

// If set, ThreadSanitizer uses separate thread context for task execution.
// If not set, ThreadSanitizer can not detect any new data races but still
// produces better reports (namely, task creation stack).  A user may not want
// to set in two cases:
// 1. If task executor executes tasks in single thread (and that is the part
//    of public contract).
// 2. If ThreadSanitizer produces false postives, but a user still wants
//    to take advantage of better reports.
const unsigned __TSAN_TASK_FLAG_SYNC = 1 << 0;

// Denotes that the task is periodic, i.e. can be executed several times
// (intended for periodic timer tasks and IO rediness notifications).
// If set, then __tsan_task_begin()/__tsan_task_end() can be called several
// times.  After the last execution the task must be explicitly destroyed with
// __tsan_task_destroy().
const unsigned __TSAN_TASK_FLAG_PERIODIC = 1 << 1;

// Usage examples:
/*
1. Non-prdiodic task:

void ThreadPool::Add(Task *t) {
  __tsan_task_create(t, __TSAN_TASK_FLAG_SYNC);
  mutex_.lock();
  queue_.push_back(t);
  mutex_.unlock();  
}

void ThreadPool::WorkerThread() {
  for (;;) {
    mutex_.lock();
    while (queue_.empty())
      convvar_.wait(&mutex_);
    Task *t = queue_.pop_front();    
    mutex_.unlock();
    t = __tsan_task_begin(t);
    t->Execute();
    __tsan_task_end(t);
    __tsan_task_destroy(t);
    delete t;
  }
}

This is semantically equivalent to:

void ThreadPool::Add(Task *t) {
  pthread_create(&t->thread_, 0, &Task::Execute, t);
}

2. Priodic task:

void ThreadPool::AddPeriodic(Task *t) {
  __tsan_task_create(t, __TSAN_TASK_FLAG_SYNC | __TSAN_TASK_FLAG_PERIODIC);
  AddPeriodicImpl(t, t->Period());
}

void ThreadPool::WorkerThread() {
  for (;;) {
    Task* t = CheckPeriodicTasks();
    if (t) {
      t = __tsan_task_begin(t);
      t->Execute();
      __tsan_task_end(t);
      AddPeriodicImpl(t, t->Period());
    }
    ...
  }
}

void ThreadPool::CancelPeriodic(Task *t) {
  CancelPeriodicImpl(t);
  __tsan_task_destroy(t);
  delete t;
}

This is semantically equivalent to:

void ThreadPool::Add(Task *t) {
  pthread_create(&t->thread_, 0, &ExecutePeriodic, t);
}

void *ExecutePeriodic(Task *t) {
  while (!t->Cancelled()) {
    sleep(t->Period());
    t->Execute();
  }
}

void ThreadPool::CancelPeriodic(Task *t) {
  t->SetCancelled();
  pthread_join(t->thread_, 0);
  delete t;
}
*/

[Bug sanitizer/55561] TSAN crashes for Fortran

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #13 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-10 15:55:50 UTC ---
(In reply to comment #12)
> That's great that gcc tsan works for Fortran/OpenMP out of the box!

I'm afraid it yields false positives. Something like this is supposed to be OK
OMP-wise (and gives the right result in 10000 repetitions) but produces
warnings.

> cat test.f90 
INTEGER, PARAMETER :: N=10
INTEGER :: data(N)

!$OMP PARALLEL DO
DO j=1,N
  data(j)=j
ENDDO

!$OMP PARALLEL DO
DO j=N,1,-1
  data(j)=data(j)-j
ENDDO

IF (ANY(data.NE.0)) CALL abort()

END

> gfortran -fsanitize=thread -O1 -g -fopenmp -fPIC -pie test.f90 ; ./a.out
==================
WARNING: ThreadSanitizer: data race (pid=18871)


> However, most likely Fortran/OpenMP will require some special handling to
> produce better reports and catch more bugs (ThreadSanitizer reasons about
> synchronization on pthread level, and not on OpenMP level). We won't have spare
> cycles for that in near future.
> I am working on ThreadSanitizer tasking API, I would appreciate if you review
> it and comment on whether it will fit Fortran/OpenMP model (the work is
> currently in very early "what if" stage).

I'm sure there are knowledgeable people reading the gcc list... posting this
text there seems like a good idea.

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[Joost.VandeVondele at mat dot ethz.ch]

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

Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|TSAN crashes for Fortran    |TSAN: Fortran/OMP yields
                   |                            |false positives

--- Comment #14 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-11 09:47:43 UTC ---
Adjusting the subject, as the crashes have gone (checked also building CP2K
with -fsanitize=thread) but the false positives remain.

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #15 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-25 19:30:15 UTC ---
(In reply to comment #13)
> (In reply to comment #12)
> > That's great that gcc tsan works for Fortran/OpenMP out of the box!
> 
> I'm afraid it yields false positives.

The obvious solution to this seems to be that also the OMP runtime (libgomp)
must be compiled with -fsanitize=thread. If I do that, it appears to work.
That's cool, I will try to do some more testing.

A reasonable approach could be to build two versions of libgomp. One standard
one, and one sanitized one (libgomp_tsan ?). -fsanitize=thread -fopenmp could
link the second version automatically. 

Just for those trying this out...

I used the following to build&install a sanitized libgomp (based on Jakub's
comments in PR55374 and a hack of mine (-L...)) after an initial normal build

cd /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/
make clean
make CFLAGS="-std=gnu99 -g -O2 -fsanitize=thread" FCFLAGS="-g -O2
-fsanitize=thread"
LDFLAGS="-L/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libsanitizer/tsan/.libs/
-B/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libsanitizer/tsan/.libs/
-Wl,-rpath,/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libsanitizer/tsan/.libs/
-fsanitize=thread"
cd /data/vjoost/gnu/gcc_trunk/obj/
make install

compilation as

gfortran -fopenmp -fsanitize=thread -pie -fPIC test.f90

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #16 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-25 20:23:07 UTC ---
many things appear to work fine, but seemingly parallel do loops with a dynamic
schedule generate warnings in libgomp. I also seem to observe that they are not
strictly deterministic, sometimes these warnings happen, sometimes not.

Testcase:

!$OMP PARALLEL PRIVATE(j)

j=OMP_GET_THREAD_NUM()

! no warnings without the dynamic schedule
!$OMP DO SCHEDULE(DYNAMIC,2) 
DO i=1,10
ENDDO

!$OMP END PARALLEL
END

Result:

vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
==================
WARNING: ThreadSanitizer: data race (pid=35190)
  Read of size 8 at 0x7d3000027290 by main thread:
    #0 gomp_iter_dynamic_next
/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/iter.c:190
(libgomp.so.1+0x000000006678)
    #1 GOMP_loop_dynamic_start
/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/loop.c:128
(libgomp.so.1+0x000000007a03)
    #2 MAIN__._omp_fn.0 test.f90:0 (exe+0x000000000d7d)
    #3 MAIN__ test.f90:0 (exe+0x000000000ccb)
    #4 main ??:0 (exe+0x000000000d1a)

  Previous write of size 8 at 0x7d3000027290 by thread 1:
    #0 gomp_loop_init
/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/loop.c:41
(libgomp.so.1+0x000000007a96)
    #1 MAIN__._omp_fn.0 test.f90:0 (exe+0x000000000d7d)
    #2 gomp_thread_start
/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:116
(libgomp.so.1+0x00000000d012)

  Location is heap block of size 1568 at 0x7d3000027100 allocated by main
thread:
    #0 malloc ??:0 (libtsan.so.0+0x00000001896e)
    #1 gomp_malloc
/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/alloc.c:36
(libgomp.so.1+0x00000000417a)
    #2 gomp_new_team
/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:145
(libgomp.so.1+0x00000000d27a)
    #3 GOMP_parallel_start
/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/parallel.c:108
(libgomp.so.1+0x00000000afc7)
    #4 MAIN__ test.f90:0 (exe+0x000000000cc1)
    #5 main ??:0 (exe+0x000000000d1a)

  Thread 1 (tid=35191, running) created at:
    #0 pthread_create ??:0 (libtsan.so.0+0x00000001a868)
    #1 gomp_team_start
/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:440
(libgomp.so.1+0x00000000d908)
    #2 GOMP_parallel_start
/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/parallel.c:108
(libgomp.so.1+0x00000000afd7)
    #3 MAIN__ test.f90:0 (exe+0x000000000cc1)
    #4 main ??:0 (exe+0x000000000d1a)

==================

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #17 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-26 19:34:29 UTC ---
Another testcase that yields warnings with a sanitized libgomp:

!$omp parallel default(none) private(i,j,k) 
!$omp do collapse(3)
    DO k = 1,10
    DO j = 1,10
    DO i = 1,10
    ENDDO
    ENDDO
    ENDDO
!$omp end parallel

END 

WARNING: ThreadSanitizer: data race (pid=16133)
  Read of size 8 at 0x7d30000272c8 by main thread:
    #0 gomp_iter_static_next
/data/vjoost/gnu/gcc_exp/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/iter.c:71
(libgomp.so.1+0x00000000623d)
    #1 GOMP_loop_static_start
/data/vjoost/gnu/gcc_exp/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/loop.c:110
(libgomp.so.1+0x0000000078ac)
    #2 MAIN__._omp_fn.0 /data/vjoost/clean/cp2k/test.f90:2 (exe+0x000000000c64)

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[dvyukov at google dot com]

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

--- Comment #18 from Dmitry Vyukov <dvyukov at google dot com> 2012-12-29 09:32:53 UTC ---
On Tue, Dec 25, 2012 at 11:30 PM, Joost.VandeVondele at mat dot
ethz.ch <gcc-bugzilla@gcc.gnu.org> wrote:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55561
>
> --- Comment #15 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-25 19:30:15 UTC ---
> (In reply to comment #13)
>> (In reply to comment #12)
>> > That's great that gcc tsan works for Fortran/OpenMP out of the box!
>>
>> I'm afraid it yields false positives.
>
> The obvious solution to this seems to be that also the OMP runtime (libgomp)
> must be compiled with -fsanitize=thread. If I do that, it appears to work.
> That's cool, I will try to do some more testing.

Good idea!

It should do for now. It should eliminate all false positives since
tsan must understand synchronization in libgomp (pthread, __sync).
It can have false negatives, though. E.g. all gomp_iter_dynamic_next()
synchronize with each other.




>
> A reasonable approach could be to build two versions of libgomp. One standard
> one, and one sanitized one (libgomp_tsan ?). -fsanitize=thread -fopenmp could
> link the second version automatically.
>
> Just for those trying this out...
>
> I used the following to build&install a sanitized libgomp (based on Jakub's
> comments in PR55374 and a hack of mine (-L...)) after an initial normal build
>
> cd /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/
> make clean
> make CFLAGS="-std=gnu99 -g -O2 -fsanitize=thread" FCFLAGS="-g -O2
> -fsanitize=thread"
> LDFLAGS="-L/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libsanitizer/tsan/.libs/
> -B/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libsanitizer/tsan/.libs/
> -Wl,-rpath,/data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libsanitizer/tsan/.libs/
> -fsanitize=thread"
> cd /data/vjoost/gnu/gcc_trunk/obj/
> make install
>
> compilation as
>
> gfortran -fopenmp -fsanitize=thread -pie -fPIC test.f90
>
> --
> Configure bugmail: http://gcc.gnu.org/bugzilla/userprefs.cgi?tab=email
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug.

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[dvyukov at google dot com]

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

--- Comment #19 from Dmitry Vyukov <dvyukov at google dot com> 2012-12-29 09:38:13 UTC ---
On Wed, Dec 26, 2012 at 12:23 AM, Joost.VandeVondele at mat dot
ethz.ch <gcc-bugzilla@gcc.gnu.org> wrote:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55561
>
> --- Comment #16 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-25 20:23:07 UTC ---
> many things appear to work fine, but seemingly parallel do loops with a dynamic
> schedule generate warnings in libgomp. I also seem to observe that they are not
> strictly deterministic, sometimes these warnings happen, sometimes not.
>
> Testcase:
>
> !$OMP PARALLEL PRIVATE(j)
>
> j=OMP_GET_THREAD_NUM()
>
> ! no warnings without the dynamic schedule
> !$OMP DO SCHEDULE(DYNAMIC,2)
> DO i=1,10
> ENDDO
>
> !$OMP END PARALLEL
> END
>
> Result:
>
> vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
> vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
> vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
> vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
> ==================
> WARNING: ThreadSanitizer: data race (pid=35190)
>   Read of size 8 at 0x7d3000027290 by main thread:
>     #0 gomp_iter_dynamic_next
> /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/iter.c:190
> (libgomp.so.1+0x000000006678)
>     #1 GOMP_loop_dynamic_start
> /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/loop.c:128
> (libgomp.so.1+0x000000007a03)
>     #2 MAIN__._omp_fn.0 test.f90:0 (exe+0x000000000d7d)
>     #3 MAIN__ test.f90:0 (exe+0x000000000ccb)
>     #4 main ??:0 (exe+0x000000000d1a)
>
>   Previous write of size 8 at 0x7d3000027290 by thread 1:
>     #0 gomp_loop_init
> /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/loop.c:41
> (libgomp.so.1+0x000000007a96)
>     #1 MAIN__._omp_fn.0 test.f90:0 (exe+0x000000000d7d)
>     #2 gomp_thread_start
> /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:116
> (libgomp.so.1+0x00000000d012)
>
>   Location is heap block of size 1568 at 0x7d3000027100 allocated by main
> thread:
>     #0 malloc ??:0 (libtsan.so.0+0x00000001896e)
>     #1 gomp_malloc
> /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/alloc.c:36
> (libgomp.so.1+0x00000000417a)
>     #2 gomp_new_team
> /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:145
> (libgomp.so.1+0x00000000d27a)
>     #3 GOMP_parallel_start
> /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/parallel.c:108
> (libgomp.so.1+0x00000000afc7)
>     #4 MAIN__ test.f90:0 (exe+0x000000000cc1)
>     #5 main ??:0 (exe+0x000000000d1a)
>
>   Thread 1 (tid=35191, running) created at:
>     #0 pthread_create ??:0 (libtsan.so.0+0x00000001a868)
>     #1 gomp_team_start
> /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:440
> (libgomp.so.1+0x00000000d908)
>     #2 GOMP_parallel_start
> /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/parallel.c:108
> (libgomp.so.1+0x00000000afd7)
>     #3 MAIN__ test.f90:0 (exe+0x000000000cc1)
>     #4 main ??:0 (exe+0x000000000d1a)


Looks like unsafe publication of gomp_work_share data.

Can you show disassembly of
>     #2 MAIN__._omp_fn.0 test.f90:0 (exe+0x000000000d7d)
?
How does it choose between calling gomp_loop_init() and
GOMP_loop_dynamic_start()?

Humm... do omp generated functions (like MAIN__._omp_fn.0) pass
through tsan pass? Perhaps it contains some atomic op that tsan does
not see.

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[dvyukov at google dot com]

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

--- Comment #20 from Dmitry Vyukov <dvyukov at google dot com> 2012-12-29 10:13:00 UTC ---
(In reply to comment #19)
> On Wed, Dec 26, 2012 at 12:23 AM, Joost.VandeVondele at mat dot
> ethz.ch <gcc-bugzilla@gcc.gnu.org> wrote:
> >
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55561
> >
> > --- Comment #16 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-25 20:23:07 UTC ---
> > many things appear to work fine, but seemingly parallel do loops with a dynamic
> > schedule generate warnings in libgomp. I also seem to observe that they are not
> > strictly deterministic, sometimes these warnings happen, sometimes not.
> >
> > Testcase:
> >
> > !$OMP PARALLEL PRIVATE(j)
> >
> > j=OMP_GET_THREAD_NUM()
> >
> > ! no warnings without the dynamic schedule
> > !$OMP DO SCHEDULE(DYNAMIC,2)
> > DO i=1,10
> > ENDDO
> >
> > !$OMP END PARALLEL
> > END
> >
> > Result:
> >
> > vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
> > vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
> > vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
> > vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
> > ==================
> > WARNING: ThreadSanitizer: data race (pid=35190)
> >   Read of size 8 at 0x7d3000027290 by main thread:
> >     #0 gomp_iter_dynamic_next
> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/iter.c:190
> > (libgomp.so.1+0x000000006678)
> >     #1 GOMP_loop_dynamic_start
> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/loop.c:128
> > (libgomp.so.1+0x000000007a03)
> >     #2 MAIN__._omp_fn.0 test.f90:0 (exe+0x000000000d7d)
> >     #3 MAIN__ test.f90:0 (exe+0x000000000ccb)
> >     #4 main ??:0 (exe+0x000000000d1a)
> >
> >   Previous write of size 8 at 0x7d3000027290 by thread 1:
> >     #0 gomp_loop_init
> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/loop.c:41
> > (libgomp.so.1+0x000000007a96)
> >     #1 MAIN__._omp_fn.0 test.f90:0 (exe+0x000000000d7d)
> >     #2 gomp_thread_start
> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:116
> > (libgomp.so.1+0x00000000d012)
> >
> >   Location is heap block of size 1568 at 0x7d3000027100 allocated by main
> > thread:
> >     #0 malloc ??:0 (libtsan.so.0+0x00000001896e)
> >     #1 gomp_malloc
> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/alloc.c:36
> > (libgomp.so.1+0x00000000417a)
> >     #2 gomp_new_team
> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:145
> > (libgomp.so.1+0x00000000d27a)
> >     #3 GOMP_parallel_start
> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/parallel.c:108
> > (libgomp.so.1+0x00000000afc7)
> >     #4 MAIN__ test.f90:0 (exe+0x000000000cc1)
> >     #5 main ??:0 (exe+0x000000000d1a)
> >
> >   Thread 1 (tid=35191, running) created at:
> >     #0 pthread_create ??:0 (libtsan.so.0+0x00000001a868)
> >     #1 gomp_team_start
> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:440
> > (libgomp.so.1+0x00000000d908)
> >     #2 GOMP_parallel_start
> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/parallel.c:108
> > (libgomp.so.1+0x00000000afd7)
> >     #3 MAIN__ test.f90:0 (exe+0x000000000cc1)
> >     #4 main ??:0 (exe+0x000000000d1a)
> 
> 
> Looks like unsafe publication of gomp_work_share data.
> 
> Can you show disassembly of
> >     #2 MAIN__._omp_fn.0 test.f90:0 (exe+0x000000000d7d)
> ?
> How does it choose between calling gomp_loop_init() and
> GOMP_loop_dynamic_start()?
> 
> Humm... do omp generated functions (like MAIN__._omp_fn.0) pass
> through tsan pass? Perhaps it contains some atomic op that tsan does
> not see.


Congratulations! We've found racy unsafe publication in libgomp with
ThreadSanitizer:

gomp_loop_dynamic_start() uses the following functions to synchronize with each
other.  As you can see gomp_ptrlock_get() contains fast-and-dead unsafe
fast-path.

libgomp/config/posix/ptrlock.h

static inline void *gomp_ptrlock_get (gomp_ptrlock_t *ptrlock)
{
  if (ptrlock->ptr != NULL)
    return ptrlock->ptr;

  gomp_mutex_lock (&ptrlock->lock);
  if (ptrlock->ptr != NULL)
    {
      gomp_mutex_unlock (&ptrlock->lock);
      return ptrlock->ptr;
    }

  return NULL;
}

static inline void gomp_ptrlock_set (gomp_ptrlock_t *ptrlock, void *ptr)
{
  ptrlock->ptr = ptr;
  gomp_mutex_unlock (&ptrlock->lock);
}

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[dvyukov at google dot com]

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

--- Comment #21 from Dmitry Vyukov <dvyukov at google dot com> 2012-12-29 10:21:06 UTC ---
It is a known issue http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40362#c7
and Jakub said it's safe. I am not that sure.

On Sat, Dec 29, 2012 at 2:13 PM, dvyukov at google dot com
<gcc-bugzilla@gcc.gnu.org> wrote:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55561
>
> --- Comment #20 from Dmitry Vyukov <dvyukov at google dot com> 2012-12-29 10:13:00 UTC ---
> (In reply to comment #19)
>> On Wed, Dec 26, 2012 at 12:23 AM, Joost.VandeVondele at mat dot
>> ethz.ch <gcc-bugzilla@gcc.gnu.org> wrote:
>> >
>> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55561
>> >
>> > --- Comment #16 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-25 20:23:07 UTC ---
>> > many things appear to work fine, but seemingly parallel do loops with a dynamic
>> > schedule generate warnings in libgomp. I also seem to observe that they are not
>> > strictly deterministic, sometimes these warnings happen, sometimes not.
>> >
>> > Testcase:
>> >
>> > !$OMP PARALLEL PRIVATE(j)
>> >
>> > j=OMP_GET_THREAD_NUM()
>> >
>> > ! no warnings without the dynamic schedule
>> > !$OMP DO SCHEDULE(DYNAMIC,2)
>> > DO i=1,10
>> > ENDDO
>> >
>> > !$OMP END PARALLEL
>> > END
>> >
>> > Result:
>> >
>> > vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
>> > vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
>> > vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
>> > vjoost@nanosim-s01.ethz.ch:/data/vjoost/clean/cp2k/cp2k/src> ./a.out
>> > ==================
>> > WARNING: ThreadSanitizer: data race (pid=35190)
>> >   Read of size 8 at 0x7d3000027290 by main thread:
>> >     #0 gomp_iter_dynamic_next
>> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/iter.c:190
>> > (libgomp.so.1+0x000000006678)
>> >     #1 GOMP_loop_dynamic_start
>> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/loop.c:128
>> > (libgomp.so.1+0x000000007a03)
>> >     #2 MAIN__._omp_fn.0 test.f90:0 (exe+0x000000000d7d)
>> >     #3 MAIN__ test.f90:0 (exe+0x000000000ccb)
>> >     #4 main ??:0 (exe+0x000000000d1a)
>> >
>> >   Previous write of size 8 at 0x7d3000027290 by thread 1:
>> >     #0 gomp_loop_init
>> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/loop.c:41
>> > (libgomp.so.1+0x000000007a96)
>> >     #1 MAIN__._omp_fn.0 test.f90:0 (exe+0x000000000d7d)
>> >     #2 gomp_thread_start
>> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:116
>> > (libgomp.so.1+0x00000000d012)
>> >
>> >   Location is heap block of size 1568 at 0x7d3000027100 allocated by main
>> > thread:
>> >     #0 malloc ??:0 (libtsan.so.0+0x00000001896e)
>> >     #1 gomp_malloc
>> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/alloc.c:36
>> > (libgomp.so.1+0x00000000417a)
>> >     #2 gomp_new_team
>> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:145
>> > (libgomp.so.1+0x00000000d27a)
>> >     #3 GOMP_parallel_start
>> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/parallel.c:108
>> > (libgomp.so.1+0x00000000afc7)
>> >     #4 MAIN__ test.f90:0 (exe+0x000000000cc1)
>> >     #5 main ??:0 (exe+0x000000000d1a)
>> >
>> >   Thread 1 (tid=35191, running) created at:
>> >     #0 pthread_create ??:0 (libtsan.so.0+0x00000001a868)
>> >     #1 gomp_team_start
>> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:440
>> > (libgomp.so.1+0x00000000d908)
>> >     #2 GOMP_parallel_start
>> > /data/vjoost/gnu/gcc_trunk/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/parallel.c:108
>> > (libgomp.so.1+0x00000000afd7)
>> >     #3 MAIN__ test.f90:0 (exe+0x000000000cc1)
>> >     #4 main ??:0 (exe+0x000000000d1a)
>>
>>
>> Looks like unsafe publication of gomp_work_share data.
>>
>> Can you show disassembly of
>> >     #2 MAIN__._omp_fn.0 test.f90:0 (exe+0x000000000d7d)
>> ?
>> How does it choose between calling gomp_loop_init() and
>> GOMP_loop_dynamic_start()?
>>
>> Humm... do omp generated functions (like MAIN__._omp_fn.0) pass
>> through tsan pass? Perhaps it contains some atomic op that tsan does
>> not see.
>
>
> Congratulations! We've found racy unsafe publication in libgomp with
> ThreadSanitizer:
>
> gomp_loop_dynamic_start() uses the following functions to synchronize with each
> other.  As you can see gomp_ptrlock_get() contains fast-and-dead unsafe
> fast-path.
>
> libgomp/config/posix/ptrlock.h
>
> static inline void *gomp_ptrlock_get (gomp_ptrlock_t *ptrlock)
> {
>   if (ptrlock->ptr != NULL)
>     return ptrlock->ptr;
>
>   gomp_mutex_lock (&ptrlock->lock);
>   if (ptrlock->ptr != NULL)
>     {
>       gomp_mutex_unlock (&ptrlock->lock);
>       return ptrlock->ptr;
>     }
>
>   return NULL;
> }
>
> static inline void gomp_ptrlock_set (gomp_ptrlock_t *ptrlock, void *ptr)
> {
>   ptrlock->ptr = ptr;
>   gomp_mutex_unlock (&ptrlock->lock);
> }
>
> --
> Configure bugmail: http://gcc.gnu.org/bugzilla/userprefs.cgi?tab=email
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug.

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #22 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-30 09:03:15 UTC ---
(In reply to comment #18)
> > The obvious solution to this seems to be that also the OMP runtime (libgomp)
> > must be compiled with '-fsanitize=thread. If I do that, it appears to work.
> > That's cool, I will try to do some more testing.
> 
> Good idea!
> 

I did do some more testing, and the combination '-fsanitize=thread -fopenmp'
was really very useful. Apart from the libgomp-related warning discussed here
and PR40362, everything worked well, and I was able to find a couple of
correctly identified OMP bugs in our code that would have been (in part) very
difficult to find without the tool. I'll also allow us to start an automatic
regression tester for our OMP code, which is something we didn't manage so far.

I would be great to get this combination to work out-of-the-box.

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[dvyukov at google dot com]

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

--- Comment #23 from Dmitry Vyukov <dvyukov at google dot com> 2012-12-30 09:57:44 UTC ---
(In reply to comment #22)
> I did do some more testing, and the combination '-fsanitize=thread -fopenmp'
> was really very useful. Apart from the libgomp-related warning discussed here
> and PR40362, everything worked well, and I was able to find a couple of
> correctly identified OMP bugs in our code that would have been (in part) very
> difficult to find without the tool. I'll also allow us to start an automatic
> regression tester for our OMP code, which is something we didn't manage so far.
> 
> I would be great to get this combination to work out-of-the-box.


Wow! Great news!
I am not really a gcc hacker. Perhaps Jakub or Dodji can help.

It would be interesting to run some OpenMP test suite with tsan to test libgomp
better (various schedules and clause types -- section, task, single, etc).

Later we may want to instrument libgomp in one way or another to provide better
detection and reporting.

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[dvyukov at google dot com]

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

--- Comment #24 from Dmitry Vyukov <dvyukov at google dot com> 2012-12-30 10:11:27 UTC ---
For testing you can comment out first 2 lines of gomp_ptrlock_get(). That
should fix the race in libgomp. It's not a good fix form performance pov, but
should be OK for testing.

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #25 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-30 14:52:51 UTC ---
(In reply to comment #24)
> For testing you can comment out first 2 lines of gomp_ptrlock_get(). That
> should fix the race in libgomp. It's not a good fix form performance pov, but
> should be OK for testing.

Thanks.. it turns out that gomp_ptrlock_get is actually used from

gcc/libgomp/config/linux/ptrlock.h:74

on linux...  is it still sufficient to comment the first if condition only and
keep the atomic_compare_exchange, or should one directly have the 'return
gomp_ptrlock_get_slow' ?

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[dvyukov at google dot com]

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

--- Comment #26 from Dmitry Vyukov <dvyukov at google dot com> 2012-12-30 17:07:01 UTC ---
(In reply to comment #25)
> (In reply to comment #24)
> > For testing you can comment out first 2 lines of gomp_ptrlock_get(). That
> > should fix the race in libgomp. It's not a good fix form performance pov, but
> > should be OK for testing.
> 
> Thanks.. it turns out that gomp_ptrlock_get is actually used from
> 
> gcc/libgomp/config/linux/ptrlock.h:74
> 
> on linux...  is it still sufficient to comment the first if condition only and
> keep the atomic_compare_exchange, or should one directly have the 'return
> gomp_ptrlock_get_slow' ?

Ah, I was looking at config/posix/ptrlock.

For config/linux/ptrlock the changes are:

  if ((uintptr_t) *ptrlock > 2)
    return *ptrlock;
\/\/\/\/\/\/\/\/\/\/\/\/
  uintptr_t v = (uintptr_t)__atomic_load(ptrlock, MEMMODEL_ACQUIRE);
  if (v > 2)
    return v;


  return *ptrlock;
\/\/\/\/\/\/\/\/\/\/\/\/
  return __atomic_load(ptrlock, MEMMODEL_ACQUIRE);


Also loads of intptr needs to be done with __atomic_load(MEMMODEL_RELAXED).
and do_spin() needs to load *addr with __atomic_load(MEMMODEL_RELAXED) as well.

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #27 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2012-12-30 19:57:24 UTC ---
(In reply to comment #24)
> For testing you can comment out first 2 lines of gomp_ptrlock_get(). That
> should fix the race in libgomp. It's not a good fix form performance pov, but
> should be OK for testing.

I found that the routines in config/posix can be used instead of the
config/linux by configuring gcc with '--disable-linux-futex' . With your
suggestion above, I still get warnings (absent in the linux case) from the
following testcase:

[vjoost@nanosim-s03 bugs]$ cat test_23.f90 
INTEGER :: i,j,OMP_GET_THREAD_NUM
!$OMP PARALLEL  PRIVATE(i,j)
j=OMP_GET_THREAD_NUM()
!$OMP DO
DO i=1,10
!$OMP CRITICAL(xxx)
!$OMP END CRITICAL(xxx)
! !$OMP CRITICAL(yyy)
! !$OMP END CRITICAL(yyy)
END DO
!$OMP END PARALLEL
END
[vjoost@nanosim-s03 bugs]$ gfortran -fsanitize=thread -fopenmp -gdwarf-3 -O1 
-fPIE -pie test_23.f90 
[vjoost@nanosim-s03 bugs]$ export OMP_NUM_THREADS=4 ; ./a.out
==================
WARNING: ThreadSanitizer: data race (pid=29039)
  Read of size 1 at 0x7d020003d050 by thread 1:
    #0 pthread_mutex_lock ??:0 (libtsan.so.0+0x00000001a863)
    #1 GOMP_critical_name_start
/data/vjoost/gnu/gcc_exp/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/config/posix/mutex.h:44
(libgomp.so.1+0x000000005e5d)
    #2 MAIN__._omp_fn.0 /data/vjoost/gnu/bugs/test_23.f90:6
(exe+0x000000000dcc)
    #3 gomp_thread_start
/data/vjoost/gnu/gcc_exp/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:116
(libgomp.so.1+0x00000000c6b2)

  Previous write of size 8 at 0x7d020003d050 by main thread:
    #0 gomp_barrier_wait_end
/data/vjoost/gnu/gcc_exp/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/config/posix/bar.c:93
(libgomp.so.1+0x00000000e691)
    #1 gomp_malloc
/data/vjoost/gnu/gcc_exp/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/alloc.c:36
(libgomp.so.1+0x000000005cba)
    #2 GOMP_critical_name_start
/data/vjoost/gnu/gcc_exp/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/critical.c:71
(libgomp.so.1+0x000000005e79)
    #3 MAIN__._omp_fn.0 /data/vjoost/gnu/bugs/test_23.f90:6
(exe+0x000000000dcc)
    #4 MAIN__ /data/vjoost/gnu/bugs/test_23.f90:2 (exe+0x000000000e43)
    #5 __libc_start_main ??:0 (libc.so.6+0x0037a121ecdc)

  Location is heap block of size 40 at 0x7d020003d050 allocated by main thread:
    #0 malloc ??:0 (libtsan.so.0+0x00000001839e)
    #1 gomp_malloc
/data/vjoost/gnu/gcc_exp/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/alloc.c:36
(libgomp.so.1+0x000000005cba)
    #2 GOMP_critical_name_start
/data/vjoost/gnu/gcc_exp/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/critical.c:71
(libgomp.so.1+0x000000005e79)
    #3 MAIN__._omp_fn.0 /data/vjoost/gnu/bugs/test_23.f90:6
(exe+0x000000000dcc)
    #4 MAIN__ /data/vjoost/gnu/bugs/test_23.f90:2 (exe+0x000000000e43)
    #5 __libc_start_main ??:0 (libc.so.6+0x0037a121ecdc)

  Thread 1 (tid=29040, running) created at:
    #0 pthread_create ??:0 (libtsan.so.0+0x00000001a298)
    #1 gomp_team_start
/data/vjoost/gnu/gcc_exp/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/team.c:440
(libgomp.so.1+0x00000000d000)
    #2 GOMP_parallel_start
/data/vjoost/gnu/gcc_exp/obj/x86_64-unknown-linux-gnu/libgomp/../../../gcc/libgomp/parallel.c:108
(libgomp.so.1+0x00000000abb7)
    #3 MAIN__ /data/vjoost/gnu/bugs/test_23.f90:2 (exe+0x000000000e39)
    #4 __libc_start_main ??:0 (libc.so.6+0x0037a121ecdc)

==================
ThreadSanitizer: reported 1 warnings

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #28 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2013-01-01 17:13:39 UTC ---
(In reply to comment #26)
> For config/linux/ptrlock the changes are:
> [...]

Following your suggestions, I applied the following patch (mistakes are mine),
which allows me to run without warnings from libgomp:

Index: config/linux/wait.h
===================================================================
--- config/linux/wait.h (revision 194730)
+++ config/linux/wait.h (working copy)
@@ -51,7 +51,7 @@
   if (__builtin_expect (gomp_managed_threads > gomp_available_cpus, 0))
     count = gomp_throttled_spin_count_var;
   for (i = 0; i < count; i++)
-    if (__builtin_expect (*addr != val, 0))
+    if (__builtin_expect (__atomic_load_n(addr,MEMMODEL_RELAXED) != val, 0))
       return 0;
     else
       cpu_relax ();
Index: config/linux/ptrlock.c
===================================================================
--- config/linux/ptrlock.c      (revision 194730)
+++ config/linux/ptrlock.c      (working copy)
@@ -50,9 +50,9 @@
 #endif
   do
     do_wait (intptr, 2);
-  while (*intptr == 2);
+  while (__atomic_load_n(intptr, MEMMODEL_RELAXED) == 2);
   __asm volatile ("" : : : "memory");
-  return *ptrlock;
+  return (void*)__atomic_load_n(ptrlock, MEMMODEL_ACQUIRE);
 }

 void
Index: config/linux/ptrlock.h
===================================================================
--- config/linux/ptrlock.h      (revision 194730)
+++ config/linux/ptrlock.h      (working copy)
@@ -48,8 +48,9 @@
 {
   uintptr_t oldval;

-  if ((uintptr_t) *ptrlock > 2)
-    return *ptrlock;
+  uintptr_t v = (uintptr_t)__atomic_load_n(ptrlock, MEMMODEL_ACQUIRE);
+  if (v > 2)
+    return (void*)v;

   oldval = 0;
   if (__atomic_compare_exchange_n (ptrlock, &oldval, 1, false,

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[dvyukov at google dot com]

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

--- Comment #29 from Dmitry Vyukov <dvyukov at google dot com> 2013-01-02 09:09:11 UTC ---
(In reply to comment #28)
> (In reply to comment #26)
> > For config/linux/ptrlock the changes are:
> > [...]
> 
> Following your suggestions, I applied the following patch (mistakes are mine),
> which allows me to run without warnings from libgomp:
> 
> Index: config/linux/wait.h
> ===================================================================
> --- config/linux/wait.h (revision 194730)
> +++ config/linux/wait.h (working copy)
> @@ -51,7 +51,7 @@
>    if (__builtin_expect (gomp_managed_threads > gomp_available_cpus, 0))
>      count = gomp_throttled_spin_count_var;
>    for (i = 0; i < count; i++)
> -    if (__builtin_expect (*addr != val, 0))
> +    if (__builtin_expect (__atomic_load_n(addr,MEMMODEL_RELAXED) != val, 0))
>        return 0;
>      else
>        cpu_relax ();
> Index: config/linux/ptrlock.c
> ===================================================================
> --- config/linux/ptrlock.c      (revision 194730)
> +++ config/linux/ptrlock.c      (working copy)
> @@ -50,9 +50,9 @@
>  #endif
>    do
>      do_wait (intptr, 2);
> -  while (*intptr == 2);
> +  while (__atomic_load_n(intptr, MEMMODEL_RELAXED) == 2);
>    __asm volatile ("" : : : "memory");
> -  return *ptrlock;
> +  return (void*)__atomic_load_n(ptrlock, MEMMODEL_ACQUIRE);
>  }
> 
>  void
> Index: config/linux/ptrlock.h
> ===================================================================
> --- config/linux/ptrlock.h      (revision 194730)
> +++ config/linux/ptrlock.h      (working copy)
> @@ -48,8 +48,9 @@
>  {
>    uintptr_t oldval;
> 
> -  if ((uintptr_t) *ptrlock > 2)
> -    return *ptrlock;
> +  uintptr_t v = (uintptr_t)__atomic_load_n(ptrlock, MEMMODEL_ACQUIRE);
> +  if (v > 2)
> +    return (void*)v;
> 
>    oldval = 0;
>    if (__atomic_compare_exchange_n (ptrlock, &oldval, 1, false,


Great!
Please post the patch to http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40362
I believe it is the correct fix for the bug.

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[jakub at gcc dot gnu.org]

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

--- Comment #30 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-02 09:43:29 UTC ---
The formatting in the patch is wrong (multiple issues).

I don't see a point in the __atomic_load_n (addr, MEMMODEL_RELAXED), for
aligned ints or pointers the loads are atomic on all architectures libgomp is
supported on, after all kernel is also using just a normal load in the futex
syscall, not __atomic_load_n (which expands to the normal load only anyway).

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[dvyukov at google dot com]

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

--- Comment #31 from Dmitry Vyukov <dvyukov at google dot com> 2013-01-02 10:28:00 UTC ---
(In reply to comment #30)
> The formatting in the patch is wrong (multiple issues).
> 
> I don't see a point in the __atomic_load_n (addr, MEMMODEL_RELAXED), for
> aligned ints or pointers the loads are atomic on all architectures libgomp is
> supported on, after all kernel is also using just a normal load in the futex
> syscall, not __atomic_load_n (which expands to the normal load only anyway).

Do you agree about MEMMODEL_ACQUIRE?

Regarding MEMMODEL_RELAXED, there are 2 reasons to use it correctness aside.
First, it greatly contributes to code readability and self-documentation, and
allows readers to easily distinguish between plain loads and inter-thread
synchronization shared loads which are hideously different things. Seconds, it
allows tools like tsan to work properly on such code and point to more serious
issues (like that MEMMODEL_ACQUIRE above).

As for correctness, below is an example that I usually provide (and there is
also "How to miscompile programs with benign data races" paper by Boehm with
other good examples):

-----

Consider that you have an "atomic" (which is not actually marked as atomic for
compiler) store and some code w/o sync operations around it (potentially from
inlined functions):

...
*p = x;
...

C++ compiler assumes absence of data races. So if it sees a store to p, then it
is allowed to use it as a scratch storage for any garbage in the same
synchronization region. I.e. it can do:

...
*p = foo;
...
*p = x;
...

Note that it can't affect any correct race-free program.
This way, other threads will read random garbage from p.

Now imagine that foo is a function pointer:

...
*p = foo; // spill from register
...
foo = *p; // restore to register
if (bar) foo(); // and execute
...
*p = x;
...

Now imagine that this thread spills &ReadFile to p, and another thread spills
&LaunchNuclearMissle to p in between (but was not intended to execute it due to
bar==0).
Ooops, this "benign" race just caused accidental launch of nuclear missiles.

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #32 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2013-01-07 21:35:25 UTC ---
(In reply to comment #30)
> The formatting in the patch is wrong (multiple issues).

I've tried to fix them in the version below.

It would be really cool to have -fopenmp -fsanitize=thread to work
out-of-the-box with gcc. 

2012-12-30  Dmitry Vyukov  <dvyukov@gcc.gnu.org>

        PR libgomp/55561
        PR libgomp/40362
        * config/linux/wait.h (do_spin): Use atomic load for addr.
        * config/linux/ptrlock.c (gomp_ptrlock_get_slow): Use atomic
        for intptr and ptrlock.
        * onfig/linux/ptrlock.h (gomp_ptrlock_get): Use atomic load
        for ptrlock.

Index: libgomp/config/linux/wait.h
===================================================================
--- libgomp/config/linux/wait.h (revision 194730)
+++ libgomp/config/linux/wait.h (working copy)
@@ -51,7 +51,7 @@ static inline int do_spin (int *addr, in
   if (__builtin_expect (gomp_managed_threads > gomp_available_cpus, 0))
     count = gomp_throttled_spin_count_var;
   for (i = 0; i < count; i++)
-    if (__builtin_expect (*addr != val, 0))
+    if (__builtin_expect (__atomic_load_n (addr, MEMMODEL_RELAXED) != val, 0))
       return 0;
     else
       cpu_relax ();
Index: libgomp/config/linux/ptrlock.c
===================================================================
--- libgomp/config/linux/ptrlock.c      (revision 194730)
+++ libgomp/config/linux/ptrlock.c      (working copy)
@@ -50,9 +50,9 @@ gomp_ptrlock_get_slow (gomp_ptrlock_t *p
 #endif
   do
     do_wait (intptr, 2);
-  while (*intptr == 2);
+  while (__atomic_load_n (intptr, MEMMODEL_RELAXED) == 2);
   __asm volatile ("" : : : "memory");
-  return *ptrlock;
+  return (void *) __atomic_load_n (ptrlock, MEMMODEL_ACQUIRE);
 }

 void
Index: libgomp/config/linux/ptrlock.h
===================================================================
--- libgomp/config/linux/ptrlock.h      (revision 194730)
+++ libgomp/config/linux/ptrlock.h      (working copy)
@@ -48,8 +48,9 @@ static inline void *gomp_ptrlock_get (go
 {
   uintptr_t oldval;

-  if ((uintptr_t) *ptrlock > 2)
-    return *ptrlock;
+  uintptr_t v = (uintptr_t) __atomic_load_n (ptrlock, MEMMODEL_ACQUIRE);
+  if (v > 2)
+    return (void *) v;

   oldval = 0;
   if (__atomic_compare_exchange_n (ptrlock, &oldval, 1, false,

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[dvyukov at google dot com]

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

--- Comment #33 from Dmitry Vyukov <dvyukov at google dot com> 2013-01-08 09:17:31 UTC ---
(In reply to comment #32)
> (In reply to comment #30)
> > The formatting in the patch is wrong (multiple issues).
> 
> I've tried to fix them in the version below.
> 
> It would be really cool to have -fopenmp -fsanitize=thread to work
> out-of-the-box with gcc. 

Agree.

Can you sent it to review? You can also mention that it fixes issue 40362.

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #34 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2013-01-10 11:26:23 UTC ---
(In reply to comment #33)

> Can you sent it to review? You can also mention that it fixes issue 40362.

I had a closer look at PR40362. Actually, I don't think this patch fixes
PR40362, as it only addresses the config/linux/ variant and not the
config/posix/ (which is what the --disable-linux-futex choice in PR40362
follows). I would be happy if you could sent the patch, I wouldn't be able to
explain the rationale for the changes, I just followed your instructions in
comment #26. You'll notice that the changelog I prepared also puts your name, I
think this is appropriate.

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[jakub at gcc dot gnu.org]

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

--- Comment #35 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-10 11:37:08 UTC ---
For config/posix it is not that easy, because you can't assume that atomics are
available.  You'd need to guard it with #ifdef HAVE_SYNC_BUILTINS and do
something else (nothing?) otherwise.  Those targets don't support tsan
anyway...

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[amodra at gmail dot com]

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

Alan Modra <amodra at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amodra at gmail dot com

--- Comment #36 from Alan Modra <amodra at gmail dot com> 2013-01-31 07:42:52 UTC ---
*** Bug 56159 has been marked as a duplicate of this bug. ***

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[jakub at gcc dot gnu.org]

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

--- Comment #37 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-31 14:29:27 UTC ---
As discussed elsewhere, __atomic_load_n actually should work on all targets, if
the backend doesn't have special support for that, it is emitted as normal load
for loads with sizes <= wordsize, plus perhaps some barrier (if target has
any).
Can you please post the patch to gcc-patches?

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[jakub at gcc dot gnu.org]

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

--- Comment #38 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-31 16:57:15 UTC ---
Author: jakub
Date: Thu Jan 31 16:57:09 2013
New Revision: 195618

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195618
Log:
    PR libgomp/55561
    * config/linux/wait.h (do_spin): Use atomic load for addr.
    * config/linux/ptrlock.c (gomp_ptrlock_get_slow): Use atomic
    for intptr and ptrlock.
    * config/linux/ptrlock.h (gomp_ptrlock_get): Use atomic load
    for ptrlock.

Modified:
    trunk/libgomp/ChangeLog
    trunk/libgomp/config/linux/ptrlock.c
    trunk/libgomp/config/linux/ptrlock.h
    trunk/libgomp/config/linux/wait.h

[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[dvyukov at google dot com]

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

--- Comment #39 from Dmitry Vyukov <dvyukov at google dot com> 2013-02-01 20:00:22 UTC ---
Sorry, I was busy previous weeks. Thanks for fixing this.

[Bug sanitizer/55561] TSAN: provide a TSAN instrumented libgomp

[Joost.VandeVondele at mat dot ethz.ch]

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

Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|TSAN: Fortran/OMP yields    |TSAN: provide a TSAN
                   |false positives             |instrumented libgomp

--- Comment #40 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> 2013-03-29 08:11:17 UTC ---
I'm updating the summary for this bug. It is my impression that tsan & omp now
work well together, but in order for this to be useful, a tsan instrumented
libgomp needs to be linked in. In my opinion, it would be great if gcc would
build to versions of the runtime library (one standard, one tsan instrumented)
and link the tsan instrumented libgomp as needed.

As a side effect, it will provide good checking for the libgomp library. 

I also believe this is a more sustainable approach than writing wrappers for
all omp functionality.

[Bug sanitizer/55561] TSAN: provide a TSAN instrumented libgomp

[dvyukov at google dot com]

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

--- Comment #41 from Dmitry Vyukov <dvyukov at google dot com> ---
Similar problem with libstdc++
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57507

[Bug sanitizer/55561] TSAN: provide a TSAN instrumented libgomp

[emil.styrke at gmail dot com]

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

Emil Styrke <emil.styrke at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |emil.styrke at gmail dot com

--- Comment #42 from Emil Styrke <emil.styrke at gmail dot com> ---
Created attachment 31905
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31905&action=edit
Patch idea

(In reply to Joost VandeVondele from comment #40)
> I'm updating the summary for this bug. It is my impression that tsan & omp
> now work well together, but in order for this to be useful, a tsan
> instrumented libgomp needs to be linked in. In my opinion, it would be great
> if gcc would build to versions of the runtime library (one standard, one
> tsan instrumented) and link the tsan instrumented libgomp as needed.
> 
> As a side effect, it will provide good checking for the libgomp library. 
> 
> I also believe this is a more sustainable approach than writing wrappers for
> all omp functionality.

I have made a try to create a patch for this, just to get the ball rolling. 
However, my autotools knowledge is a bit thin and my experience with the GCC
codebase even thinner, so I didn't get all the way.

The idea is to create a tsan-instrumented libgomp as libgomp_tsan.so and change
the libgomp.spec to select that library instead if -fsanitize=thread is used.

Issues I didn't manage to sort out:
 * Correctly detecting if tsan support is available (as evident by the
commented-out logic in the patch).
 * The vpath magic for finding the files in config/ breaks for the new target
because explicit targets are created for each source file.  I hacked around
this 
 by hard-coding the paths for Linux, but obviously this is ugly.
 * It seems the multilib support broke with this patch (I get the resulting
library and spec file in lib/ instead of lib32/ and lib64/), but maybe I just
handled the build scripts wrong...
 * There are no test cases...

Anyway, after manually fixing up the install it seems to work alright.  If this
looks like a reasonable way forward, I'd be happy to improve the patch after
feedback or leave this embryo in the hands of someone more knowledgeable.

[Bug sanitizer/55561] TSAN: provide a TSAN instrumented libgomp

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #43 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(In reply to Emil Styrke from comment #42)
> Anyway, after manually fixing up the install it seems to work alright.  If
> this looks like a reasonable way forward, I'd be happy to improve the patch
> after feedback or leave this embryo in the hands of someone more
> knowledgeable.

I would welcome such a solution very much. From looking at the patch, it seems
a relatively small amount of work for a significant benefit. I hope you get
some feedback&help from libgomp/sanitizer experts.

[Bug sanitizer/55561] TSAN: provide a TSAN instrumented libgomp

[roland at rschulz dot eu]

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

Roland Schulz <roland at rschulz dot eu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |roland at rschulz dot eu

--- Comment #44 from Roland Schulz <roland at rschulz dot eu> ---
If I run tsan on our code with libgomp compiled with --disable-linux-futex, I
only see false positives for omp-atomic constructs. Everything else seems fine.
If I compile libgomp with tsan and without --disable-linux-futex I get a lot of
false positives. And if I compile libgomp with both tsan and
--disable-linux-futex, I get also the false positives for omp-atomic. I used
gcc 4.8.2.

For those who reported success with compiling libgomp with tsan:
- Do you also use --disable-linux-futex or did you only use -fsanitize=thread?
- Did you test with code using "#pragma omp atomic update"?

Is there a way to compile libgomp to not get false positives for omp-atomic?

[Bug sanitizer/55561] TSAN: provide a TSAN instrumented libgomp

[Joost.VandeVondele at mat dot ethz.ch]

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

--- Comment #45 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(In reply to Roland Schulz from comment #44)

> Is there a way to compile libgomp to not get false positives for omp-atomic?

yes, this is fixed in gcc for 4.9.0 see PR59194 or r206572

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59194#c11

I believe that some of the other aspects you're seeing with 4.8.2 might also be
fixed in 4.9 (not sure).

[Bug sanitizer/55561] TSAN: provide a TSAN instrumented libgomp

[dvyukov at google dot com]

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

--- Comment #46 from Dmitry Vyukov <dvyukov at google dot com> ---
Roland, why do you think that what you see is false positives? I think these
are real, potentially harmful, races. Please test with gcc 4.9, and file bugs
if you still see any races.

[Bug sanitizer/55561] TSAN: provide a TSAN instrumented libgomp

[Joost.VandeVondele at mat dot ethz.ch]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55561

--- Comment #48 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(In reply to Roland Schulz from comment #47)
> What is the advantage of a TSAN instrumented libgomp over one with
> --disable-linux-futex?

I would be happy to know that '--disable-linux-futex' is a suitable approach,
as that would be rather practical. However, my first idea would be that, since
libgomp is not sanitized, not all races in 'user land' would be detected. I'm
just guessing ...

[Bug sanitizer/55561] TSAN: provide a TSAN instrumented libgomp

[dvyukov at google dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55561

--- Comment #49 from Dmitry Vyukov <dvyukov at google dot com> ---
> However, my first idea would be that, since libgomp is not sanitized, not all races in 'user land' would be detected. I'm just guessing ...

If libgomp does not access user data (which I suspect it does not generally
do), then un-instrumented libgomp can only lead to false positives due to
missed synchronization in libgomp. The missed synchronization can come mostly
from atomic operations, because tsan intercepts pthread_mutex and other library
functions.

[Bug sanitizer/55561] TSAN: provide a TSAN instrumented libgomp

[roland at rschulz dot eu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55561

--- Comment #50 from Roland Schulz <roland at rschulz dot eu> ---
I must say I don't know how the internals work. But I assume that reductions
are implemented in libgomp (I know they are in iomp). Thus for any code which
uses OpenMP reduce statements, libgomp would touch user data.

[Bug sanitizer/55561] TSAN: provide a TSAN instrumented libgomp

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55561

--- Comment #51 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Roland Schulz from comment #50)
> I must say I don't know how the internals work. But I assume that reductions
> are implemented in libgomp (I know they are in iomp). Thus for any code
> which uses OpenMP reduce statements, libgomp would touch user data.

Your assumption is wrong, reductions are not handled in libgomp, but in the
code emitted by the compiler.

[Bug sanitizer/55561] TSAN: provide a TSAN instrumented libgomp

[Joost.VandeVondele at mat dot ethz.ch]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55561

--- Comment #52 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(In reply to Jakub Jelinek from comment #51)

> Your assumption is wrong, reductions are not handled in libgomp, but in the
> code emitted by the compiler.

does this imply that the combination of -fsanitize=thread and a libgomp as
obtained with --disable-linux-futex is indeed a 'proper' solution for this PR,
or do you think that building two libraries such as suggested in comment #42
would be best ?

[Bug sanitizer/55561] TSAN: provide a TSAN instrumented libgomp

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55561

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |egallager at gcc dot gnu.org
   Last reconfirmed|                            |2024-02-29
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #54 from Eric Gallager <egallager at gcc dot gnu.org> ---
Confirmed that providing an instrumented libgomp would be worthwhile.