sanitizer_common: deduplicate CheckFailed
authorDmitry Vyukov <dvyukov@google.com>
Tue, 11 May 2021 08:18:48 +0000 (10:18 +0200)
committerDmitry Vyukov <dvyukov@google.com>
Wed, 12 May 2021 06:50:53 +0000 (08:50 +0200)
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.

Reviewed By: vitalybuka

Differential Revision: https://reviews.llvm.org/D102221

14 files changed:
compiler-rt/lib/asan/asan_rtl.cpp
compiler-rt/lib/asan/asan_stack.h
compiler-rt/lib/hwasan/hwasan.cpp
compiler-rt/lib/hwasan/hwasan.h
compiler-rt/lib/memprof/memprof_rtl.cpp
compiler-rt/lib/memprof/memprof_stack.h
compiler-rt/lib/msan/msan.cpp
compiler-rt/lib/msan/msan.h
compiler-rt/lib/sanitizer_common/sanitizer_common.h
compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp
compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
compiler-rt/lib/tsan/rtl/tsan_rtl.h
compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
compiler-rt/test/msan/check-handler.cpp

index f42de41..e715d77 100644 (file)
@@ -62,19 +62,9 @@ static void AsanDie() {
   }
 }
 
-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
@@ -432,7 +422,7 @@ static void AsanInitInternal() {
 
   // Install tool-specific callbacks in sanitizer_common.
   AddDieCallback(AsanDie);
-  SetCheckFailedCallback(AsanCheckFailed);
+  SetCheckUnwindCallback(CheckUnwind);
   SetPrintfAndReportCallback(AppendToErrorMessageBuffer);
 
   __sanitizer_set_report_path(common_flags()->log_path);
index 47ca85a..b9575d2 100644 (file)
@@ -54,9 +54,6 @@ u32 GetMallocContextSize();
 #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)
 
@@ -71,10 +68,4 @@ u32 GetMallocContextSize();
     stack.Print();              \
   }
 
-#define PRINT_CURRENT_STACK_CHECK() \
-  {                                 \
-    GET_STACK_TRACE_CHECK_HERE;     \
-    stack.Print();                  \
-  }
-
 #endif // ASAN_STACK_H
index 5c0d804..8d6c252 100644 (file)
@@ -128,12 +128,9 @@ static void InitializeFlags() {
   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) {
@@ -271,7 +268,7 @@ void __hwasan_init() {
   InitializeFlags();
 
   // Install tool-specific callbacks in sanitizer_common.
-  SetCheckFailedCallback(HWAsanCheckFailed);
+  SetCheckUnwindCallback(CheckUnwind);
 
   __sanitizer_set_report_path(common_flags()->log_path);
 
index 24d96ce..8515df5 100644 (file)
@@ -127,15 +127,6 @@ void InstallAtExitHandler();
   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();
 
index d6d606f..fee2912 100644 (file)
@@ -48,19 +48,9 @@ static void MemprofDie() {
   }
 }
 
-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
@@ -174,7 +164,7 @@ static void MemprofInitInternal() {
 
   // 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.
index 289a61e..a8fdfc9 100644 (file)
@@ -50,9 +50,6 @@ u32 GetMallocContextSize();
 #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                                                 \
@@ -66,10 +63,4 @@ u32 GetMallocContextSize();
     stack.Print();                                                             \
   }
 
-#define PRINT_CURRENT_STACK_CHECK()                                            \
-  {                                                                            \
-    GET_STACK_TRACE_CHECK_HERE;                                                \
-    stack.Print();                                                             \
-  }
-
 #endif // MEMPROF_STACK_H
index 4be1630..611b04d 100644 (file)
@@ -410,12 +410,9 @@ 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() {
@@ -430,7 +427,7 @@ void __msan_init() {
   InitializeFlags();
 
   // Install tool-specific callbacks in sanitizer_common.
-  SetCheckFailedCallback(MsanCheckFailed);
+  SetCheckUnwindCallback(CheckUnwind);
 
   __sanitizer_set_report_path(common_flags()->log_path);
 
index a46e42c..963b94a 100644 (file)
@@ -365,15 +365,6 @@ const int STACK_TRACE_TAG_POISON = StackTrace::TAG_CUSTOM + 1;
     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(); }
index dcd625d..7b65dd7 100644 (file)
@@ -304,8 +304,8 @@ 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
@@ -317,9 +317,7 @@ 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
index 84be6fc..6a54734 100644 (file)
@@ -59,26 +59,31 @@ void NORETURN Die() {
   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);
+  u32 tid = GetTid();
+  Printf("%s: CHECK failed: %s:%d \"%s\" (0x%zx, 0x%zx) (tid=%u)\n",
+         SanitizerToolName, StripModuleName(file), line, cond, (uptr)v1,
+         (uptr)v2, tid);
+  static atomic_uint32_t first_tid;
+  u32 cmp = 0;
+  if (!atomic_compare_exchange_strong(&first_tid, &cmp, tid,
+                                      memory_order_relaxed)) {
+    if (cmp == tid) {
+      // Recursing into CheckFailed.
+    } else {
+      // Another thread fails already, let it 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);
+  if (CheckUnwindCallback)
+    CheckUnwindCallback();
   Die();
 }
 
index 81517a2..0efa997 100644 (file)
@@ -374,6 +374,18 @@ static void TsanOnDeadlySignal(int signo, void *siginfo, void *context) {
 }
 #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;
@@ -384,7 +396,7 @@ void Initialize(ThreadState *thr) {
   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";
index 689f95f..3ae519d 100644 (file)
@@ -84,9 +84,6 @@ 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):
index cedf1a3..706794f 100644 (file)
@@ -31,23 +31,6 @@ 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);
@@ -752,8 +735,7 @@ void PrintCurrentStack(ThreadState *thr, uptr pc) {
 // 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 =
index 4721f8c..0bdb433 100644 (file)
@@ -10,7 +10,7 @@ int main(void) {
   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;
 }