sanitizer_common: deduplicate CheckFailed

[dvyukov]

Summary

We have some significant amount of duplication around
CheckFailed functionality. Each sanitizer copy-pasted
a chunk of code. Some got random improvements like
dealing with recursive failures better. These improvements
could benefit all sanitizers, but they don't.

Deduplicate CheckFailed logic across sanitizers and let each
sanitizer only print the current stack trace.
I've tried to dedup stack printing as well,
but this got me into cmake hell. So let's keep this part
duplicated in each sanitizer for now.

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp

67–90

maybe?

[dvyukov]

Simplify CheckFailed logic.

[dvyukov]

compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp

67–90

Good point.
I see you removed num_calls, but we will still print the second CHECK failure as we recurse, right? I think that's enough.
PTAL

For future: for the diff you posted is there a way to look at it as side-by-side diff, or apply locally, or download raw diff with +/- marks?

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp

67–90

I can't find anything useful we can do with such comments. I suspect that maybe pasting code as-is in comment is more convenient:

void f();

compiler-rt/lib/asan/asan_rtl.cpp

if (kMidMemBeg) {
       UnmapOrDie((void*)kMidMemEnd, kHighShadowEnd - kMidMemEnd);
     } else {
       if (kHighShadowEnd)
         UnmapOrDie((void*)kLowShadowBeg, kHighShadowEnd - kLowShadowBeg);
     }
   }
 }
 
-static void AsanCheckFailed(const char *file, int line, const char *cond,
-                            u64 v1, u64 v2) {
-  Report("AddressSanitizer CHECK failed: %s:%d \"%s\" (0x%zx, 0x%zx)\n", file,
-         line, cond, (uptr)v1, (uptr)v2);
-
-  // Print a stack trace the first time we come here. Otherwise, we probably
-  // failed a CHECK during symbolization.
-  static atomic_uint32_t num_calls;
-  if (atomic_fetch_add(&num_calls, 1, memory_order_relaxed) == 0) {
-    PRINT_CURRENT_STACK_CHECK();
-  }
-
-  Die();
+static void CheckUnwind() {
+  GET_STACK_TRACE(kStackTraceMax, common_flags()->fast_unwind_on_check);
+  stack.Print();
 }
 
 // -------------------------- Globals --------------------- {{{1
 int asan_inited;
 bool asan_init_is_running;
 
 #if !ASAN_FIXED_MAPPING
 uptr kHighMemEnd, kMidMemBeg, kMidMemEnd;
static void AsanInitInternal() {
 
   InitializeHighMemEnd();
 
   // Make sure we are not statically linked.
   AsanDoesNotSupportStaticLinkage();
 
   // Install tool-specific callbacks in sanitizer_common.
   AddDieCallback(AsanDie);
-  SetCheckFailedCallback(AsanCheckFailed);
+  SetCheckUnwindCallback(CheckUnwind);
   SetPrintfAndReportCallback(AppendToErrorMessageBuffer);
 
   __sanitizer_set_report_path(common_flags()->log_path);
 
   __asan_option_detect_stack_use_after_return =
       flags()->detect_stack_use_after_return;
 
   __sanitizer::InitializePlatformEarly();

compiler-rt/lib/asan/asan_stack.h

 #define GET_STACK_TRACE_FATAL(pc, bp)              \
   BufferedStackTrace stack;                        \
   stack.Unwind(pc, bp, nullptr,                    \
                common_flags()->fast_unwind_on_fatal)
 
 #define GET_STACK_TRACE_FATAL_HERE                                \
   GET_STACK_TRACE(kStackTraceMax, common_flags()->fast_unwind_on_fatal)
 
-#define GET_STACK_TRACE_CHECK_HERE                                \
-  GET_STACK_TRACE(kStackTraceMax, common_flags()->fast_unwind_on_check)
-
 #define GET_STACK_TRACE_THREAD                                    \
   GET_STACK_TRACE(kStackTraceMax, true)
 
 #define GET_STACK_TRACE_MALLOC                                                 \
   GET_STACK_TRACE(GetMallocContextSize(), common_flags()->fast_unwind_on_malloc)
 
 #define GET_STACK_TRACE_FREE GET_STACK_TRACE_MALLOC
 
 #define PRINT_CURRENT_STACK()   \
   {                             \
     GET_STACK_TRACE_FATAL_HERE; \
     stack.Print();              \
   }
 
-#define PRINT_CURRENT_STACK_CHECK() \
-  {                                 \
-    GET_STACK_TRACE_CHECK_HERE;     \
-    stack.Print();                  \
-  }
-
 #endif // ASAN_STACK_H

compiler-rt/lib/hwasan/hwasan.h

stack.Unwind(StackTrace::GetCurrentPc(), GET_CURRENT_FRAME(),         \
                  nullptr, common_flags()->fast_unwind_on_malloc,          \
                  common_flags()->malloc_context_size)
 
 #define GET_FATAL_STACK_TRACE_PC_BP(pc, bp)              \
   BufferedStackTrace stack;                              \
   if (hwasan_inited)                                     \
     stack.Unwind(pc, bp, nullptr, common_flags()->fast_unwind_on_fatal)
 
-#define GET_FATAL_STACK_TRACE_HERE \
-  GET_FATAL_STACK_TRACE_PC_BP(StackTrace::GetCurrentPc(), GET_CURRENT_FRAME())
-
-#define PRINT_CURRENT_STACK_CHECK() \
-  {                                 \
-    GET_FATAL_STACK_TRACE_HERE;     \
-    stack.Print();                  \
-  }
-
 void HwasanTSDInit();
 void HwasanTSDThreadInit();
 
 void HwasanOnDeadlySignal(int signo, void *info, void *context);
 
 void UpdateMemoryUsage();
 
 void AppendToErrorMessageBuffer(const char *buffer);

compiler-rt/lib/hwasan/hwasan.cpp

#endif
 
   InitializeCommonFlags();
 
   if (Verbosity()) ReportUnrecognizedFlags();
 
   if (common_flags()->help) parser.PrintFlagDescriptions();
 }
 
-static void HWAsanCheckFailed(const char *file, int line, const char *cond,
-                              u64 v1, u64 v2) {
-  Report("HWAddressSanitizer CHECK failed: %s:%d \"%s\" (0x%zx, 0x%zx)\n", file,
-         line, cond, (uptr)v1, (uptr)v2);
-  PRINT_CURRENT_STACK_CHECK();
-  Die();
+static void CheckUnwind() {
+  GET_FATAL_STACK_TRACE_PC_BP(StackTrace::GetCurrentPc(), GET_CURRENT_FRAME());
+  stack.Print();
 }
 
 static void HwasanFormatMemoryUsage(InternalScopedString &s) {
   HwasanThreadList &thread_list = hwasanThreadList();
   auto thread_stats = thread_list.GetThreadStats();
   auto *sds = StackDepotGetStats();
   AllocatorStatCounters asc;
   GetAllocatorStats(asc);
void __hwasan_init() {
   SanitizerToolName = "HWAddressSanitizer";
 
   InitTlsSize();
 
   CacheBinaryName();
   InitializeFlags();
 
   // Install tool-specific callbacks in sanitizer_common.
-  SetCheckFailedCallback(HWAsanCheckFailed);
+  SetCheckUnwindCallback(CheckUnwind);
 
   __sanitizer_set_report_path(common_flags()->log_path);
 
   AndroidTestTlsSlot();
 
   DisableCoreDumperIfNecessary();
 
   InitInstrumentation();

compiler-rt/lib/memprof/memprof_rtl.cpp

static void MemprofDie() {
   if (common_flags()->print_module_map >= 1)
     DumpProcessMap();
   if (flags()->unmap_shadow_on_exit) {
     if (kHighShadowEnd)
       UnmapOrDie((void *)kLowShadowBeg, kHighShadowEnd - kLowShadowBeg);
   }
 }
 
-static void MemprofCheckFailed(const char *file, int line, const char *cond,
-                               u64 v1, u64 v2) {
-  Report("MemProfiler CHECK failed: %s:%d \"%s\" (0x%zx, 0x%zx)\n", file, line,
-         cond, (uptr)v1, (uptr)v2);
-
-  // Print a stack trace the first time we come here. Otherwise, we probably
-  // failed a CHECK during symbolization.
-  static atomic_uint32_t num_calls;
-  if (atomic_fetch_add(&num_calls, 1, memory_order_relaxed) == 0) {
-    PRINT_CURRENT_STACK_CHECK();
-  }
-
-  Die();
+static void CheckUnwind() {
+  GET_STACK_TRACE(kStackTraceMax, common_flags()->fast_unwind_on_check);
+  stack.Print();
 }
 
 // -------------------------- Globals --------------------- {{{1
 int memprof_inited;
 int memprof_init_done;
 bool memprof_init_is_running;
 int memprof_timestamp_inited;
 long memprof_init_timestamp_s;
static void MemprofInitInternal() {
 
   InitializeHighMemEnd();
 
   // Make sure we are not statically linked.
   MemprofDoesNotSupportStaticLinkage();
 
   // Install tool-specific callbacks in sanitizer_common.
   AddDieCallback(MemprofDie);
-  SetCheckFailedCallback(MemprofCheckFailed);
+  SetCheckUnwindCallback(CheckUnwind);
 
   // Use profile name specified via the binary itself if it exists, and hasn't
   // been overrriden by a flag at runtime.
   if (__memprof_profile_filename[0] != 0 && !common_flags()->log_path)
     __sanitizer_set_report_path(__memprof_profile_filename);
   else
     __sanitizer_set_report_path(common_flags()->log_path);
 

compiler-rt/lib/memprof/memprof_stack.h

#define GET_STACK_TRACE(max_size, fast)                                        \
   } else {                                                                     \
     stack.Unwind(StackTrace::GetCurrentPc(), GET_CURRENT_FRAME(), nullptr,     \
                  fast, max_size);                                              \
   }
 
 #define GET_STACK_TRACE_FATAL_HERE                                             \
   GET_STACK_TRACE(kStackTraceMax, common_flags()->fast_unwind_on_fatal)
 
-#define GET_STACK_TRACE_CHECK_HERE                                             \
-  GET_STACK_TRACE(kStackTraceMax, common_flags()->fast_unwind_on_check)
-
 #define GET_STACK_TRACE_THREAD GET_STACK_TRACE(kStackTraceMax, true)
 
 #define GET_STACK_TRACE_MALLOC                                                 \
   GET_STACK_TRACE(GetMallocContextSize(), common_flags()->fast_unwind_on_malloc)
 
 #define GET_STACK_TRACE_FREE GET_STACK_TRACE_MALLOC
 
 #define PRINT_CURRENT_STACK()                                                  \
   {                                                                            \
     GET_STACK_TRACE_FATAL_HERE;                                                \
     stack.Print();                                                             \
   }
 
-#define PRINT_CURRENT_STACK_CHECK()                                            \
-  {                                                                            \
-    GET_STACK_TRACE_CHECK_HERE;                                                \
-    stack.Print();                                                             \
-  }
-
 #endif // MEMPROF_STACK_H

compiler-rt/lib/msan/msan.h

#define GET_STORE_STACK_TRACE \
   GET_STORE_STACK_TRACE_PC_BP(StackTrace::GetCurrentPc(), GET_CURRENT_FRAME())
 
 #define GET_FATAL_STACK_TRACE_PC_BP(pc, bp)                              \
   BufferedStackTrace stack;                                              \
   if (msan_inited) {                                                     \
     stack.Unwind(pc, bp, nullptr, common_flags()->fast_unwind_on_fatal); \
   }
 
-#define GET_FATAL_STACK_TRACE_HERE \
-  GET_FATAL_STACK_TRACE_PC_BP(StackTrace::GetCurrentPc(), GET_CURRENT_FRAME())
-
-#define PRINT_CURRENT_STACK_CHECK() \
-  {                                 \
-    GET_FATAL_STACK_TRACE_HERE;     \
-    stack.Print();                  \
-  }
-
 class ScopedThreadLocalStateBackup {
  public:
   ScopedThreadLocalStateBackup() { Backup(); }
   ~ScopedThreadLocalStateBackup() { Restore(); }
   void Backup();
   void Restore();
  private:
   u64 va_arg_overflow_size_tls;

compiler-rt/lib/msan/msan.cpp

static void OnStackUnwind(const SignalContext &sig, const void *,
   stack->Unwind(StackTrace::GetNextInstructionPc(sig.pc), sig.bp, sig.context,
                 common_flags()->fast_unwind_on_fatal);
 }
 
 static void MsanOnDeadlySignal(int signo, void *siginfo, void *context) {
   HandleDeadlySignal(siginfo, context, GetTid(), &OnStackUnwind, nullptr);
 }
 
-static void MsanCheckFailed(const char *file, int line, const char *cond,
-                            u64 v1, u64 v2) {
-  Report("MemorySanitizer CHECK failed: %s:%d \"%s\" (0x%zx, 0x%zx)\n", file,
-         line, cond, (uptr)v1, (uptr)v2);
-  PRINT_CURRENT_STACK_CHECK();
-  Die();
+static void CheckUnwind() {
+  GET_FATAL_STACK_TRACE_PC_BP(StackTrace::GetCurrentPc(), GET_CURRENT_FRAME());
+  stack.Print();
 }
 
 void __msan_init() {
   CHECK(!msan_init_is_running);
   if (msan_inited) return;
   msan_init_is_running = 1;
   SanitizerToolName = "MemorySanitizer";
 
   AvoidCVE_2016_2143();
 
   CacheBinaryName();
   InitializeFlags();
 
   // Install tool-specific callbacks in sanitizer_common.
-  SetCheckFailedCallback(MsanCheckFailed);
+  SetCheckUnwindCallback(CheckUnwind);
 
   __sanitizer_set_report_path(common_flags()->log_path);
 
   InitializeInterceptors();
   CheckASLR();
   InitTlsSize();
   InstallDeadlySignalHandlers(MsanOnDeadlySignal);
   InstallAtExitHandler(); // Needs __cxa_atexit interceptor.

compiler-rt/lib/sanitizer_common/sanitizer_common.h

 void NORETURN Abort();
 void NORETURN Die();
 void NORETURN
 CheckFailed(const char *file, int line, const char *cond, u64 v1, u64 v2);
 void NORETURN ReportMmapFailureAndDie(uptr size, const char *mem_type,
                                       const char *mmap_type, error_t err,
                                       bool raw_report = false);
 
-// Specific tools may override behavior of "Die" and "CheckFailed" functions
-// to do tool-specific job.
+// Specific tools may override behavior of "Die" function to do tool-specific
+// job.
 typedef void (*DieCallbackType)(void);
 
 // It's possible to add several callbacks that would be run when "Die" is
 // called. The callbacks will be run in the opposite order. The tools are
 // strongly recommended to setup all callbacks during initialization, when there
 // is only a single thread.
 bool AddDieCallback(DieCallbackType callback);
 bool RemoveDieCallback(DieCallbackType callback);
 
 void SetUserDieCallback(DieCallbackType callback);
 
-typedef void (*CheckFailedCallbackType)(const char *, int, const char *,
-                                       u64, u64);
-void SetCheckFailedCallback(CheckFailedCallbackType callback);
+void SetCheckUnwindCallback(void (*callback)());
 
 // Callback will be called if soft_rss_limit_mb is given and the limit is
 // exceeded (exceeded==true) or if rss went down below the limit
 // (exceeded==false).
 // The callback should be registered once at the tool init time.
 void SetSoftRssLimitExceededCallback(void (*Callback)(bool exceeded));
 
 // Functions related to signal handling.

compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp

for (int i = kMaxNumOfInternalDieCallbacks - 1; i >= 0; i--) {
     if (InternalDieCallbacks[i])
       InternalDieCallbacks[i]();
   }
   if (common_flags()->abort_on_error)
     Abort();
   internal__exit(common_flags()->exitcode);
 }
 
-static CheckFailedCallbackType CheckFailedCallback;
-void SetCheckFailedCallback(CheckFailedCallbackType callback) {
-  CheckFailedCallback = callback;
+static void (*CheckUnwindCallback)();
+void SetCheckUnwindCallback(void (*callback)()) {
+  CheckUnwindCallback = callback;
 }
 
-const int kSecondsToSleepWhenRecursiveCheckFailed = 2;
-
 void NORETURN CheckFailed(const char *file, int line, const char *cond,
                           u64 v1, u64 v2) {
-  static atomic_uint32_t num_calls;
-  if (atomic_fetch_add(&num_calls, 1, memory_order_relaxed) > 10) {
-    SleepForSeconds(kSecondsToSleepWhenRecursiveCheckFailed);
+  static atomic_uint32_t first_tid;
+  u32 tid = GetTid();
+  u32 cmp = 0;
+  bool first = atomic_compare_exchange_weak(&first_tid, &cmp, tid,
+                                            memory_order_relaxed) ||
+               cmp == tid;
+  Printf("%s: CHECK failed: %s:%d \"%s\" (0x%zx, 0x%zx) (tid=%u)\n",
+         SanitizerToolName, StripModuleName(file), line, cond, (uptr)v1,
+         (uptr)v2, tid);
+  if (!first) {
+    // Let the first thread print the stack and terminate.
+    SleepForSeconds(2);
     Trap();
   }
-
-  if (CheckFailedCallback) {
-    CheckFailedCallback(file, line, cond, v1, v2);
-  }
-  Report("Sanitizer CHECK failed: %s:%d %s (%lld, %lld)\n", file, line, cond,
-                                                            v1, v2);
+  static atomic_uint32_t num_calls;
+  u32 num = atomic_fetch_add(&num_calls, 1, memory_order_relaxed);
+  if (num > 10)
+    Trap();  // We are probably in an infinite recursion.
+  if (CheckUnwindCallback && num == 0)
+    CheckUnwindCallback();
   Die();
 }
 
 } // namespace __sanitizer
 
 using namespace __sanitizer;
 
 extern "C" {
 SANITIZER_INTERFACE_ATTRIBUTE
 void __sanitizer_set_death_callback(void (*callback)(void)) {
   SetUserDieCallback(callback);
 }
 }  // extern "C"

compiler-rt/lib/tsan/rtl/tsan_rtl.h

 };
 typedef SizeClassAllocator64<AP64> PrimaryAllocator;
 #endif
 typedef CombinedAllocator<PrimaryAllocator> Allocator;
 typedef Allocator::AllocatorCache AllocatorCache;
 Allocator *allocator();
 #endif
 
-void TsanCheckFailed(const char *file, int line, const char *cond,
-                     u64 v1, u64 v2);
-
 const u64 kShadowRodata = (u64)-1;  // .rodata shadow marker
 
 // FastState (from most significant bit):
 //   ignore          : 1
 //   tid             : kTidBits
 //   unused          : -
 //   history_size    : 3
 //   epoch           : kClkBits

compiler-rt/lib/tsan/rtl/tsan_rtl.cpp

stack->Unwind(StackTrace::GetNextInstructionPc(sig.pc), sig.bp, sig.context,
                 common_flags()->fast_unwind_on_fatal);
 }
 
 static void TsanOnDeadlySignal(int signo, void *siginfo, void *context) {
   HandleDeadlySignal(siginfo, context, GetTid(), &OnStackUnwind, nullptr);
 }
 #endif
 
+void CheckUnwind() {
+  // There is high probability that interceptors will check-fail as well,
+  // on the other hand there is no sense in processing interceptors
+  // since we are going to die soon.
+  ScopedIgnoreInterceptors ignore;
+#if !SANITIZER_GO
+  cur_thread()->ignore_sync++;
+  cur_thread()->ignore_reads_and_writes++;
+#endif
+  PrintCurrentStackSlow(StackTrace::GetCurrentPc());
+}
+
 void Initialize(ThreadState *thr) {
   // Thread safe because done before all threads exist.
   static bool is_initialized = false;
   if (is_initialized)
     return;
   is_initialized = true;
   // We are not ready to handle interceptors yet.
   ScopedIgnoreInterceptors ignore;
   SanitizerToolName = "ThreadSanitizer";
   // Install tool-specific callbacks in sanitizer_common.
-  SetCheckFailedCallback(TsanCheckFailed);
+  SetCheckUnwindCallback(CheckUnwind);
 
   ctx = new(ctx_placeholder) Context;
   const char *env_name = SANITIZER_GO ? "GORACE" : "TSAN_OPTIONS";
   const char *options = GetEnv(env_name);
   CacheBinaryName();
   CheckASLR();
   InitializeFlags(&ctx->flags, options, env_name);
   AvoidCVE_2016_2143();

compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp

 #include "tsan_fd.h"
 
 namespace __tsan {
 
 using namespace __sanitizer;
 
 static ReportStack *SymbolizeStack(StackTrace trace);
 
-void TsanCheckFailed(const char *file, int line, const char *cond,
-                     u64 v1, u64 v2) {
-  // There is high probability that interceptors will check-fail as well,
-  // on the other hand there is no sense in processing interceptors
-  // since we are going to die soon.
-  ScopedIgnoreInterceptors ignore;
-#if !SANITIZER_GO
-  cur_thread()->ignore_sync++;
-  cur_thread()->ignore_reads_and_writes++;
-#endif
-  Printf("FATAL: ThreadSanitizer CHECK failed: "
-         "%s:%d \"%s\" (0x%zx, 0x%zx)\n",
-         file, line, cond, (uptr)v1, (uptr)v2);
-  PrintCurrentStackSlow(StackTrace::GetCurrentPc());
-  Die();
-}
-
 // Can be overriden by an application/test to intercept reports.
 #ifdef TSAN_EXTERNAL_HOOKS
 bool OnReport(const ReportDesc *rep, bool suppressed);
 #else
 SANITIZER_WEAK_CXX_DEFAULT_IMPL
 bool OnReport(const ReportDesc *rep, bool suppressed) {
   (void)rep;
   return suppressed;

 
 // Always inlining PrintCurrentStackSlow, because LocatePcInTrace assumes
 // __sanitizer_print_stack_trace exists in the actual unwinded stack, but
 // tail-call to PrintCurrentStackSlow breaks this assumption because
 // __sanitizer_print_stack_trace disappears after tail-call.
 // However, this solution is not reliable enough, please see dvyukov's comment
 // http://reviews.llvm.org/D19148#406208
 // Also see PR27280 comment 2 and 3 for breaking examples and analysis.
-ALWAYS_INLINE
-void PrintCurrentStackSlow(uptr pc) {
+ALWAYS_INLINE USED void PrintCurrentStackSlow(uptr pc) {
 #if !SANITIZER_GO
   uptr bp = GET_CURRENT_FRAME();
   BufferedStackTrace *ptrace =
       new(internal_alloc(MBlockStackTrace, sizeof(BufferedStackTrace)))
           BufferedStackTrace();
   ptrace->Unwind(pc, bp, nullptr, false);
 
   for (uptr i = 0; i < ptrace->size / 2; i++) {

compiler-rt/test/msan/check-handler.cpp

 // RUN: %clangxx_msan -O0 -g %s -o %t && not %run %t 2>&1 | FileCheck %s
 
 // Verify that CHECK handler prints a stack on CHECK fail.
 
 #include <stdlib.h>
 
 int main(void) {
   // Allocate chunk from the secondary allocator to trigger CHECK(IsALigned())
   // in its free() path.
   void *p = malloc(8 << 20);
   free(reinterpret_cast<char*>(p) + 1);
   // CHECK: MemorySanitizer: bad pointer
-  // CHECK: MemorySanitizer CHECK failed
+  // CHECK: MemorySanitizer: CHECK failed
   // CHECK: #0
   return 0;
 }