[asan] Fix deadlock issue on FreeBSD, caused by use of .preinit_array in rL325240

[MaskRay]

Summary

Without this patch,
clang -fsanitize=address -xc =(printf 'int main(){}') -o a; ./a => deadlock in __asan_init>AsanInitInternal>AsanTSDInit>...>__getcontextx_size>_rtld_bind>rlock_acquire(rtld_bind_lock, &lockstate)

libexec/rtld-elf/rtld.c

wlock_acquire(rtld_bind_lock, &lockstate);
if (obj_main->crt_no_init)
  preinit_main(); // unresolved PLT functions cannot be called here

lib/libthr/thread/thr_rtld.c

  uc_len = __getcontextx_size(); // unresolved PLT function in libthr.so.3

/* So sad that I cannot attend LLVM Social - Shanghai: July 1st, 2018 */
/* This revision is dedicated to hotpxl who will start a new adventure in New York City. */

[devnexen]

Is Xray instrumentation working now ?

[MaskRay]

In D48806#1148927, @devnexen wrote:

Is Xray instrumentation working now ?

% fclang++ -fxray-instrument -fxray-instruction-threshold=1 a.cc -o a
% XRAY_OPTIONS=patch_premain=true:verbosity=1:xray_mode=xray-basic ./a                                         
==29379==Missing rdtscp support.                                                                               
==29379==Missing rdtscp support.                                                                               
==29379==WARNING: Required CPU features missing for XRay instrumentation, using                                
emulation instead.                                                                                             
==29379==XRay: Log file in 'xray-log.a.q1WUln'                                                                 
meow                                                                                                           
meow                                                                                                           
==29379==Cleaned up log for TID: 100557  
% ls -l xray-log.a.q1WUln                                                                                      
-rw-------  1 ray  wheel  224 Jun 30 15:41 xray-log.a.q1WUln
% fllvm-xray account xray-log.a.q1WUln
Functions with latencies: 2
   funcid      count [      min,       med,       90p,       99p,       max]
   sum  function
        1          2 [ 0.000015,  0.000143,  0.000143,  0.000143,  0.000143]  0.000158  (unknown): #1
        2          1 [ 0.000191,  0.000191,  0.000191,  0.000191,  0.000191]  0.000191  (unknown): #2

[devnexen]

Hopefully unit tests still passing.

[MaskRay]

In D48806#1148944, @devnexen wrote:

Hopefully unit tests still passing.

Some check-xray tests will fail on FreeBSD, e.g. test/xray/TestCases/Posix/fdr-mode.cc. It seems these xray tests rely on eager initialization of __xray_init, but I don't think this revision should be blocked by these failures. The current check-xray status isn't clean either:

Failing Tests (2):
  XRay-x86_64-freebsd :: TestCases/Posix/profiling-multi-threaded.cc
  XRay-x86_64-freebsd :: TestCases/Posix/profiling-single-threaded.cc

Note, rL325240 makes ASAN seriously broken on FreeBSD: all -fsanitize=address tests will deadlock (they will dead lock in __asan_init) (so ninja check-asan will hang forever)

clang -fsanitize=address -xc =(printf 'int main(){}') -o a; ./a => deadlock in __asan_init>AsanInitInternal>AsanTSDInit>...>__getcontextx_size>_rtld_bind>rlock_acquire(rtld_bind_lock, &lockstate)

[MaskRay]

80 columns rule

[devnexen]

This issue is well known in fact, we were trying to solve it. @dim for advice here.

[devnexen]

The failing tests might depend on FreeBSD version, on 10.4

[100%] Running the XRay tests
Testing Time: 2.81s
  Expected Passes    : 57
  Unsupported Tests  : 1
[100%] Built target check-xray

Also, as an alternative, what if we disable the early init in asan only instead of generalising it ? Works too.

[MaskRay]

Special case xray to use unsafe .preinit_array

[MaskRay]

In D48806#1148977, @devnexen wrote:

The failing tests might depend on FreeBSD version, on 10.4

[100%] Running the XRay tests
Testing Time: 2.81s
  Expected Passes    : 57
  Unsupported Tests  : 1
[100%] Built target check-xray

Also, as an alternative, what if we disable the early init in asan only instead of generalising it ? Works too.

Made a special case in xray_init.cc

[dim]

Sorry for not getting to this one earlier, indeed it fixes the deadlock for me too!

Now the only "big" thing left is the hundreds of:

==41641==AddressSanitizer CHECK failed: /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/asan_posix.cc:48 "((0)) == ((pthread_key_create(&tsd_key, destructor)))" (0x0, 0x4e)
`

failures. :-)

[devnexen]

In D48806#1149021, @dim wrote:

Sorry for not getting to this one earlier, indeed it fixes the deadlock for me too!

Now the only "big" thing left is the hundreds of:

==41641==AddressSanitizer CHECK failed: /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/asan_posix.cc:48 "((0)) == ((pthread_key_create(&tsd_key, destructor)))" (0x0, 0x4e)
`

failures. :-)

I guess it s FreeBSD 11.2 ? Seems his fix working on 10.4, will check on 11.x when I can ... otherwise there is always the thread local solution I threw a while ago. WE might need to come up with another solution anyway @krytarowski reports it is still an issue on NetBSD.

[dim]

In D48806#1149211, @devnexen wrote:

In D48806#1149021, @dim wrote:

Sorry for not getting to this one earlier, indeed it fixes the deadlock for me too!

Now the only "big" thing left is the hundreds of:

==41641==AddressSanitizer CHECK failed: /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/asan_posix.cc:48 "((0)) == ((pthread_key_create(&tsd_key, destructor)))" (0x0, 0x4e)
`

failures. :-)

I guess it s FreeBSD 11.2 ? Seems his fix working on 10.4, will check on 11.x when I can ... otherwise there is always the thread local solution I threw a while ago. WE might need to come up with another solution anyway @krytarowski reports it is still an issue on NetBSD.

This is on FreeBSD 12-CURRENT, but it could also show the same issue on 11.x.

[dim]

In D48806#1149655, @dim wrote:

In D48806#1149211, @devnexen wrote:

In D48806#1149021, @dim wrote:

Sorry for not getting to this one earlier, indeed it fixes the deadlock for me too!

Now the only "big" thing left is the hundreds of:

==41641==AddressSanitizer CHECK failed: /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/asan_posix.cc:48 "((0)) == ((pthread_key_create(&tsd_key, destructor)))" (0x0, 0x4e)
`

failures. :-)

I guess it s FreeBSD 11.2 ? Seems his fix working on 10.4, will check on 11.x when I can ... otherwise there is always the thread local solution I threw a while ago. WE might need to come up with another solution anyway @krytarowski reports it is still an issue on NetBSD.

This is on FreeBSD 12-CURRENT, but it could also show the same issue on 11.x.

I verified this, just to be sure, and it also happens on 11.x.

[devnexen]

Maybe we need to move from pthread_get/set_specific ? https://reviews.llvm.org/D47448

[krytarowski]

In D48806#1150208, @devnexen wrote:

Maybe we need to move from pthread_get/set_specific ? https://reviews.llvm.org/D47448

Yes, please. I was thinking about the same _lwp_kill(2) trick like in TSan or LSan for NetBSD. I've noted that FreeBSD adopted this solution there too.

compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h

 //   #endif
 //
 // And then use it as: if (compare_hook) compare_hook(a, b);
 //----------------------------------------------------------------------------//
 
 
 // We can use .preinit_array section on Linux to call sanitizer initialization
 // functions very early in the process startup (unless PIC macro is defined).
+//
+// On FreeBSD, .preinit_array functions are called with rtld_bind_lock writer
+// lock held. It will lead to dead lock if unresolved PLT functions (which helds
+// rtld_bind_lock reader lock) are called inside .preinit_array functions.
+//
 // FIXME: do we have anything like this on Mac?
 #ifndef SANITIZER_CAN_USE_PREINIT_ARRAY
-#if ((SANITIZER_LINUX && !SANITIZER_ANDROID) || \
-  SANITIZER_FREEBSD || SANITIZER_OPENBSD) && !defined(PIC)
-# define SANITIZER_CAN_USE_PREINIT_ARRAY 1
+#if ((SANITIZER_LINUX && !SANITIZER_ANDROID) || SANITIZER_OPENBSD) && \
+    !defined(PIC)
+#define SANITIZER_CAN_USE_PREINIT_ARRAY 1
 // Before Solaris 11.4, .preinit_array is fully supported only with GNU ld.
 // FIXME: Check for those conditions.
 #elif SANITIZER_SOLARIS && !defined(PIC)
 # define SANITIZER_CAN_USE_PREINIT_ARRAY 1
 #else
 # define SANITIZER_CAN_USE_PREINIT_ARRAY 0
 #endif
 #endif  // SANITIZER_CAN_USE_PREINIT_ARRAY

compiler-rt/trunk/lib/xray/xray_init.cc

void __xray_init() XRAY_NEVER_INSTRUMENT {
   atomic_store(&XRayInitialized, true, memory_order_release);
 
 #ifndef XRAY_NO_PREINIT
   if (flags()->patch_premain)
     __xray_patch();
 #endif
 }
 
-#if !defined(XRAY_NO_PREINIT) && SANITIZER_CAN_USE_PREINIT_ARRAY
+// FIXME: Make check-xray tests work on FreeBSD without
+// SANITIZER_CAN_USE_PREINIT_ARRAY.
+// See sanitizer_internal_defs.h where the macro is defined.
+// Calling unresolved PLT functions in .preinit_array can lead to deadlock on
+// FreeBSD but here it seems benign.
+#if !defined(XRAY_NO_PREINIT) &&                                               \
+    (SANITIZER_CAN_USE_PREINIT_ARRAY || SANITIZER_FREEBSD)
 // Only add the preinit array initialization if the sanitizers can.
 __attribute__((section(".preinit_array"),
                used)) void (*__local_xray_preinit)(void) = __xray_init;
 #else
 // If we cannot use the .preinit_array section, we should instead use dynamic
 // initialisation.
 static bool UNUSED __local_xray_dyninit = [] {
   __xray_init();
   return true;
 }();
 #endif