Postgres Pro
Downloads
Home   >   mailing lists

Thread: Configure with thread sanitizer fails the thread test

Configure with thread sanitizer fails the thread test

From
Ewan Higgs
Date:
Hi all,
I throught I would try sqlsmith with more sanitizers. So, I tried the ThreadSanitizer. Building off HEAD (522400a5198f63823406202e18fdaf3621619d98), it fails to complete the thread check since the thread test because there are some volatile ints being accessed between threads[1]. Volatile prevents caching of the variable in each thread context but it doesn't provide all the guarantees needed for multithreaded programming. This should probably use _Atomic (C11) on platforms with C11 compilers and whatever else is needed for platforms that refuse to update their compilers past C89.

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'.

I haven't investigated this in depth yet since I figured if I flagged it on the list someone might be in a better position to determine if it's worth fixing and what the fix might be.

My configure flags:
./configure CFLAGS='-O1 -g -fsanitize=thread -fno-omit-frame-pointer -fno-optimize-sibling-calls' LDFLAGS='-ltsan -ldl' --enable-cassert --prefix=$HOME/pkg

Yours,
Ewan


[1] Failed test in configure yields:
==================                                                             
WARNING: ThreadSanitizer: data race (pid=31420)                                
  Write of size 4 at 0x000000602100 by thread T2:                              
    #0 func_call_2 src/test/thread/thread_test.c:416 (conftest+0x000000400db9) 
    #1 <null> <null> (libtsan.so.0+0x0000000235b9)                             
                                                                               
  Previous read of size 4 at 0x000000602100 by thread T1:                      
    #0 func_call_1 src/test/thread/thread_test.c:326 (conftest+0x000000400f3a) 
    #1 <null> <null> (libtsan.so.0+0x0000000235b9)                             
                                                                               
  Location is global 'errno2_set' of size 4 at 0x000000602100 (conftest+0x000000602100)
                                                                               
  Thread T2 (tid=31423, running) created by main thread at:                    
    #0 pthread_create <null> (libtsan.so.0+0x000000027a67)                     
    #1 main src/test/thread/thread_test.c:165 (conftest+0x0000004010e1)        
                                                                               
  Thread T1 (tid=31422, running) created by main thread at:                    
    #0 pthread_create <null> (libtsan.so.0+0x000000027a67)                     
    #1 main src/test/thread/thread_test.c:158 (conftest+0x00000040107e)        
                                                                               
SUMMARY: ThreadSanitizer: data race src/test/thread/thread_test.c:416 func_call_2
==================                                                             
WARNING: ThreadSanitizer: data race (pid=31420)                                
  Read of size 4 at 0x000000602104 by thread T2:                               
    #0 func_call_2 src/test/thread/thread_test.c:417 (conftest+0x000000400dcd) 
    #1 <null> <null> (libtsan.so.0+0x0000000235b9)                             
                                                                               
  Previous write of size 4 at 0x000000602104 by thread T1:                     
    #0 func_call_1 src/test/thread/thread_test.c:325 (conftest+0x000000400f26) 
    #1 <null> <null> (libtsan.so.0+0x0000000235b9)                             
                                                                               
  Location is global 'errno1_set' of size 4 at 0x000000602104 (conftest+0x000000602104)
                                                                               
  Thread T2 (tid=31423, running) created by main thread at:                    
    #0 pthread_create <null> (libtsan.so.0+0x000000027a67)                     
    #1 main src/test/thread/thread_test.c:165 (conftest+0x0000004010e1)        
                                                                               
  Thread T1 (tid=31422, running) created by main thread at:                    
    #0 pthread_create <null> (libtsan.so.0+0x000000027a67)                     
    #1 main src/test/thread/thread_test.c:158 (conftest+0x00000040107e)        
                                                                               
SUMMARY: ThreadSanitizer: data race src/test/thread/thread_test.c:417 func_call_2
==================                                                             
errno not thread-safe **                                                       
exiting                                                                        
==================                                                             
WARNING: ThreadSanitizer: data race (pid=31420)                                
  Write of size 4 at 0x00000060210c by thread T1:                              
    #0 func_call_1 src/test/thread/thread_test.c:381 (conftest+0x000000400fc8) 
    #1 <null> <null> (libtsan.so.0+0x0000000235b9)                             
                                                                               
  Previous read of size 4 at 0x00000060210c by main thread (mutexes: write M6):
    #0 main src/test/thread/thread_test.c:176 (conftest+0x000000401123)        
                                                                               
  Location is global 'thread1_done' of size 4 at 0x00000060210c (conftest+0x00000060210c)
                                                                               
  Mutex M6 (0x000000602120) created at:                                        
    #0 pthread_mutex_lock <null> (libtsan.so.0+0x000000037776)                 
    #1 main src/test/thread/thread_test.c:156 (conftest+0x000000401066)        
                                                                               
  Thread T1 (tid=31422, running) created by main thread at:                    
    #0 pthread_create <null> (libtsan.so.0+0x000000027a67)                     
    #1 main src/test/thread/thread_test.c:158 (conftest+0x00000040107e)        
                                                                               
SUMMARY: ThreadSanitizer: data race src/test/thread/thread_test.c:381 func_call_1
==================                                                             
ThreadSanitizer: reported 3 warnings 
Attachment

Re: Configure with thread sanitizer fails the thread test

From
Alvaro Herrera
Date:
On 2021-Jul-23, Mikhail Matrosov wrote:

> I am not sure what is the proper fix for the issue, but I at least suggest
> to disable this test when building with thread sanitizer:
>
https://github.com/conan-io/conan-center-index/pull/6472/files#diff-b8639f81e30f36c8ba29a0878f1ef4d9f1552293bc9098ebb9b429ddb1f0935f

Here's the proposed patch.  Patches posted to the mailing list by their
authors proposed for inclusion are considered to be under the PostgreSQL
license, but this patch hasn't been posted by the author so let's assume
they're not authorizing them to use it.  (Otherwise, why wouldn't they
just post it here instead of doing the absurdly convoluted dance of a
github PR?)

diff --git a/src/test/thread/thread_test.c b/src/test/thread/thread_test.c
index e1bec01..e4ffd78 100644
--- a/src/test/thread/thread_test.c
+++ b/src/test/thread/thread_test.c
@@ -61,6 +61,14 @@ main(int argc, char *argv[])
     fprintf(stderr, "Perhaps rerun 'configure' using '--enable-thread-safety'.\n");
     return 1;
 }
+
+#elif __has_feature(thread_sanitizer)
+int
+main(int argc, char *argv[])
+{
+    printf("Thread safety check is skipped since it does not work with thread sanitizer.\n");
+    return 0;
+}
 #else
 
 /* This must be down here because this is the code that uses threads. */

-- 
Álvaro Herrera

Re: Configure with thread sanitizer fails the thread test

From
Alvaro Herrera
Date:
On 2021-Jul-23, Alvaro Herrera wrote:

> On 2021-Jul-23, Mikhail Matrosov wrote:
> 
> > I am not sure what is the proper fix for the issue, but I at least suggest
> > to disable this test when building with thread sanitizer:
> >
https://github.com/conan-io/conan-center-index/pull/6472/files#diff-b8639f81e30f36c8ba29a0878f1ef4d9f1552293bc9098ebb9b429ddb1f0935f
> 
> Here's the proposed patch.  Patches posted to the mailing list by their
> authors proposed for inclusion are considered to be under the PostgreSQL
> license, but this patch hasn't been posted by the author so let's assume
> they're not authorizing them to use it.  (Otherwise, why wouldn't they
> just post it here instead of doing the absurdly convoluted dance of a
> github PR?)

... that said, I wonder why would we do this in the thread_test program
rather than in configure itself.  Wouldn't it make more sense for the
configure test to be skipped altogether (marking the result as
thread-safe) when running under thread sanitizer, if there's a way to
detect that?

-- 
Álvaro Herrera              Valdivia, Chile  —  https://www.EnterpriseDB.com/
"No me acuerdo, pero no es cierto.  No es cierto, y si fuera cierto,
 no me acuerdo."                 (Augusto Pinochet a una corte de justicia)

Re: Configure with thread sanitizer fails the thread test

From
Tom Lane
Date:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> ... that said, I wonder why would we do this in the thread_test program
> rather than in configure itself.  Wouldn't it make more sense for the
> configure test to be skipped altogether (marking the result as
> thread-safe) when running under thread sanitizer, if there's a way to
> detect that?

TBH, I wonder why we don't just nuke thread_test.c altogether.
Is it still useful in 2021?  Machines that still need
--disable-thread-safety can doubtless be counted without running
out of fingers, and I think their owners can be expected to know
that they need that.

            regards, tom lane

Re: Configure with thread sanitizer fails the thread test

From
Tom Lane
Date:
Andres Freund <andres@anarazel.de> writes:
> On 2021-07-23 13:42:41 -0400, Tom Lane wrote:
>> TBH, I wonder why we don't just nuke thread_test.c altogether.

> +1. And before long it might be time to remove support for systems
> without threads...

I'm not prepared to go that far just yet; but certainly we can stop
expending configure cycles on the case.

Should we back-patch this, or just do it in HEAD?  Maybe HEAD+v14?

            regards, tom lane

Re: Configure with thread sanitizer fails the thread test

From
Tom Lane
Date:
Andres Freund <andres@anarazel.de> writes:
> On 2021-07-23 17:18:37 -0400, Tom Lane wrote:
>> Should we back-patch this, or just do it in HEAD?  Maybe HEAD+v14?

> I'm ok with all, with a preference for HEAD+v14, followed by HEAD, and
> all branches.

After a bit more thought, HEAD+v14 is also my preference.  I'm not
eager to change this in stable branches, but it doesn't seem too
late for v14.

            regards, tom lane

Re: Configure with thread sanitizer fails the thread test

From
Tom Lane
Date:
I wrote:
> After a bit more thought, HEAD+v14 is also my preference.  I'm not
> eager to change this in stable branches, but it doesn't seem too
> late for v14.

Perusing the commit log, I realized that there's another reason why
v14 is a good cutoff: thread_test.c was in a different place with
an allegedly different raison d'etre before 8a2121185.  So done
that way.

            regards, tom lane