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

            Bug ID: 110799
           Summary: [tsan] False positive due to -fhoist-adjacent-loads
           Product: gcc
           Version: 13.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vries at gcc dot gnu.org
                CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
                    jakub at gcc dot gnu.org, kcc at gcc dot gnu.org, marxin at 
gcc dot gnu.org
  Target Milestone: ---

I build gdb with -O2 -fsanitizer=thread, and ran into a false positive due to
-fhoist-adjacent-loads.

Minimal example:
...
$ cat race.c
#include <pthread.h>
#include <stdio.h>

int c;

struct s
{
  int a;
  int b;
};

struct s v1;

int v3;

void *
thread1 (void *x)
{
  v1.a = 1;
  return NULL;
}

void *
thread2 (void *x)
{
  v3 = c ? v1.a : v1.b;
  return NULL;
}

int
main (void)
{
  pthread_t t[2];

  pthread_create(&t[0], NULL, thread1, NULL);
  pthread_create(&t[1], NULL, thread2, NULL);

  pthread_join(t[0], NULL);
  pthread_join(t[1], NULL);

  return 0;
}
...

With O0, runs fine:
...
$ gcc race.c -fsanitize=thread -g
$ ./a.out 
$
...

With O2, a race is reported:
...
$ gcc race.c -fsanitize=thread -g -O2
$ ./a.out 
==================
WARNING: ThreadSanitizer: data race (pid=24538)
  Read of size 4 at 0x000000404060 by thread T2:
    #0 thread2 /data/vries/gdb/race.c:26 (a.out+0x401299) (BuildId:
295673549b1e99c73c70a2a8d26944f177f88c15)
    #1 <null> <null> (libtsan.so.2+0x3c329) (BuildId:
8f2a9be581a0fcb3d7109755a6067408093b9dbd)

  Previous write of size 4 at 0x000000404060 by thread T1:
    #0 thread1 /data/vries/gdb/race.c:19 (a.out+0x401257) (BuildId:
295673549b1e99c73c70a2a8d26944f177f88c15)
    #1 <null> <null> (libtsan.so.2+0x3c329) (BuildId:
8f2a9be581a0fcb3d7109755a6067408093b9dbd)
...

With -fno-hoist-adjacent-loads, it's fine again:
...
$ gcc race.c -fsanitize=thread -g -O2 -fno-hoist-adjacent-loads
$ ./a.out 
$
...

The optimization transforms these loads:
...
  v3 = c ? v1.a : v1.b;
...
into:
...
  int tmp_a = v1.a;
  int tmp_b = v1.b;
  v3 = c ? tmp_a : tmp_b
...
which introduces the false positive.

So I wonder if there should be a change like this:
...
 static bool
 gate_hoist_loads (void)
 {
   return (flag_hoist_adjacent_loads == 1
+          && (flag_sanitize & SANITIZE_THREAD) == 0
           && param_l1_cache_line_size
           && HAVE_conditional_move);
 }
...

Reply via email to