[Sanitizers] Enable stack traces on FreeBSD

[kutuzov.viktor.84]

Summary

On FreeBSD 10.0 _Unwind_Backtrace() behaves similarly to its Linux implementation when called from within a signal handler.

[emaste]

lib/sanitizer_common/sanitizer_stacktrace.h

56

Did you check that 9.3 works? (I'm curious if the test should actually be for FreeBSD version < 10.0 .)

[kutuzov.viktor.84]

lib/sanitizer_common/sanitizer_stacktrace.h

56

I didn't check it on 9.3 and I'm not sure if there is a simple way to find the revision affected the behavior of _Unwind_Backtrace().

[samsonov]

lib/sanitizer_common/sanitizer_stacktrace.h

56

Why did you switch to const bool instead of returns?
Also, I think it would be cleaner to introduce SANITIZER_CAN_SLOW_UNWIND macro next to SANITIZER_CAN_FAST_UNWIND

[kutuzov.viktor.84]

On FreeBSD 10.0 _Unwind_Backtrace() behaves similarly to its Linux implementation when called from within a signal handler.

Wrong. It still return the handler's stack on SIGSEGV. Sorry for misleading. I'm going to prepare another patch that introduce SANITIZER_CAN_SLOW_UNWIND, enable stack traces on FreeBSD and rely on fast unwinding when called from within the Asan's SIGSEGV handler.

[kutuzov.viktor.84]

Updated as described above.

[samsonov]

lib/asan/asan_posix.cc

50 ↗

(On Diff #15754)

I'd prefer to have RAII object that sets/unsets it. Also, won't we have a data race when one thread checks if it wants to use a fast unwind (e.g. to save an alllocation stack), and another thread hits a segfault and invokes a signal handler?

lib/sanitizer_common/sanitizer_internal_defs.h

214 ↗

(On Diff #15754)

Please put this in sanitizer_common.h. Also, there are other options apart from SEGV, in_deadly_signal_handler would be a more proper name.

lib/sanitizer_common/sanitizer_stacktrace.cc

20 ↗

(On Diff #15754)

sanitizer_common.cc

lib/sanitizer_common/sanitizer_stacktrace.h

18

Do you still need this?

[kutuzov.viktor.84]

Also, won't we have a data race when one thread checks if it wants to use a fast unwind (e.g. to save an alllocation stack), and another thread hits a segfault and invokes a signal handler?

Sure we will. Thanks for the catch.

Since THREADLOCAL is not supported on OSX and Android and thread-associated data is sanitizer-specific, what if we keep this in Asan and move the code that determines whether we are in a deadly signal handler on FreeBSD from WillUseFastUnwind() (sanitizer_common) to GetStackTraceWithPcBpAndContext() (asan_stack), like in this new patch?

[samsonov]

glider, isn't THREADLOCAL supported on all the version of Mac OS X we care about? Not sure about Android, though.

That said, I'm ok with the current version of the patch.

[glider]

THREADLOCAL is indeed supported on OSX (we haven't switched to using it yet, though). IIRC TLS support isn't finished on Android yet.

The change looks good to me.

lib/sanitizer_common/sanitizer_stacktrace.h

36

We'll need to revisit this when slow unwind works on Mac. Please add a comment linking to https://code.google.com/p/address-sanitizer/issues/detail?id=137

Closed by commit rL221595 (authored by vkutuzov).

lib/sanitizer_common/sanitizer_stacktrace.h

 // This file is shared between AddressSanitizer and ThreadSanitizer
 // run-time libraries.
 //===----------------------------------------------------------------------===//
 #ifndef SANITIZER_STACKTRACE_H
 #define SANITIZER_STACKTRACE_H
 
 #include "sanitizer_internal_defs.h"
 
+#if SANITIZER_FREEBSD
+#include <osreldate.h>  // for __FreeBSD_version
+#endif
+
 namespace __sanitizer {
 
 static const uptr kStackTraceMax = 256;
 
 #if SANITIZER_LINUX && (defined(__aarch64__) || defined(__powerpc__) || \
                         defined(__powerpc64__) || defined(__sparc__) || \
                         defined(__mips__))
 # define SANITIZER_CAN_FAST_UNWIND 0
 #elif SANITIZER_WINDOWS
 # define SANITIZER_CAN_FAST_UNWIND 0
 #else
 # define SANITIZER_CAN_FAST_UNWIND 1
 #endif
 
 struct StackTrace {
   const uptr *trace;
   uptr size;
 
   StackTrace() : trace(nullptr), size(0) {}
   StackTrace(const uptr *trace, uptr size) : trace(trace), size(size) {}
 
   // Prints a symbolized stacktrace, followed by an empty line.
   void Print() const;
 
   static bool WillUseFastUnwind(bool request_fast_unwind) {
     // Check if fast unwind is available. Fast unwind is the only option on Mac.
-    // It is also the only option on FreeBSD as the slow unwinding that
+    // It is also the only option on FreeBSD 9.2 as the slow unwinding that
     // leverages _Unwind_Backtrace() yields the call stack of the signal's
     // handler and not of the code that raised the signal (as it does on Linux).
-    if (!SANITIZER_CAN_FAST_UNWIND)
-      return false;
-    else if (SANITIZER_MAC != 0 || SANITIZER_FREEBSD != 0)
-      return true;
-    return request_fast_unwind;
+#if !SANITIZER_CAN_FAST_UNWIND
+    const bool UseFastUnwind = false;
+#elif SANITIZER_MAC
+    const bool UseFastUnwind = true;
+#elif SANITIZER_FREEBSD
+# if __FreeBSD_version <= 902001  // v9.2
+    const bool UseFastUnwind = true;
+# else
+    const bool UseFastUnwind = request_fast_unwind;
+# endif
+#else
+    const bool UseFastUnwind = request_fast_unwind;
+#endif
+    return UseFastUnwind;
   }
 
   static uptr GetCurrentPc();
   static uptr GetPreviousInstructionPc(uptr pc);
   typedef bool (*SymbolizeCallback)(const void *pc, char *out_buffer,
                                     int out_size);
 };