Re: Configure with thread sanitizer fails the thread test

On 2015-08-17 07:37:45 +0000, Ewan Higgs wrote:
> So I changed volatile to _Atomic and continued (patch is in
> thread_test_atomic.patch). I then ran it against sqlsmith. The good
> news: I didn't happen to find any problems in normal use. The bad
> news: I did find a lot of warnings about improper use of functions
> like malloc and free from signal handlers. I attached the log under
> 'errors.log'.

Aren't pretty much all of those false positives? I checked the first few
and they all look like:

> ==================
> WARNING: ThreadSanitizer: signal-unsafe call inside of a signal (pid=26767)
>     #0 free <null> (libtsan.so.0+0x000000025d09)
>     #1 AllocSetDelete /home/ehiggs/src/download/postgresql/src/backend/utils/mmgr/aset.c:643
(postgres+0x0000009ece3d)
>     #2 MemoryContextDelete /home/ehiggs/src/download/postgresql/src/backend/utils/mmgr/mcxt.c:226
(postgres+0x0000009ef7cc)
>     #3 MemoryContextDeleteChildren /home/ehiggs/src/download/postgresql/src/backend/utils/mmgr/mcxt.c:246
(postgres+0x0000009ef83b)
>     #4 MemoryContextDelete /home/ehiggs/src/download/postgresql/src/backend/utils/mmgr/mcxt.c:209
(postgres+0x0000009ef798)
>     #5 StartChildProcess /home/ehiggs/src/download/postgresql/src/backend/postmaster/postmaster.c:5211
(postgres+0x0000007b2e72)
>     #6 reaper /home/ehiggs/src/download/postgresql/src/backend/postmaster/postmaster.c:2717
(postgres+0x0000007b44ac)
>     #7 <null> <null> (libtsan.so.0+0x0000000236e3)
>     #8 ServerLoop /home/ehiggs/src/download/postgresql/src/backend/postmaster/postmaster.c:1631
(postgres+0x0000007b6f78)
>     #9 PostmasterMain /home/ehiggs/src/download/postgresql/src/backend/postmaster/postmaster.c:1274
(postgres+0x0000007b6f78)
>     #10 main /home/ehiggs/src/download/postgresql/src/backend/main/main.c:223 (postgres+0x0000006f2da9)

This is after a fork. And fork is a async-signal-safe function. So it
seems tsan doesn't properly recognize that we actually "escaped" the
signal handler and are in a new process.

>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <stdatomic.h>
>  #include <unistd.h>
>  #include <netdb.h>
>  #include <sys/types.h>
> @@ -84,11 +85,11 @@ static void func_call_2(void);
>  
>  static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
>  
> -static volatile int thread1_done = 0;
> -static volatile int thread2_done = 0;
> +static _Atomic int thread1_done = 0;
> +static _Atomic int thread2_done = 0;
>  
> -static volatile int errno1_set = 0;
> -static volatile int errno2_set = 0;
> +static _Atomic int errno1_set = 0;
> +static _Atomic int errno2_set = 0;
>  
>  #ifndef HAVE_STRERROR_R
>  static char *strerror_p1;

We can't do that because we have to work on older compilers as well. My
suggestion for now would be to disable tsan during configure and only
enable it during the actual build (you can do stuff like make
COPT='-fsanitize...'.

Greetings,

Andres Freund