[compiler-rt] 2721e27 - sanitizer_common: deduplicate CheckFailed
Dmitry Vyukov via llvm-commits
llvm-commits at lists.llvm.org
Tue May 11 23:50:58 PDT 2021
Author: Dmitry Vyukov
Date: 2021-05-12T08:50:53+02:00
New Revision: 2721e27c3aa34841db3fc0b4da25890288652d50
URL: https://github.com/llvm/llvm-project/commit/2721e27c3aa34841db3fc0b4da25890288652d50
DIFF: https://github.com/llvm/llvm-project/commit/2721e27c3aa34841db3fc0b4da25890288652d50.diff
LOG: sanitizer_common: deduplicate CheckFailed
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
Added:
Modified:
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
Removed:
################################################################################
diff --git a/compiler-rt/lib/asan/asan_rtl.cpp b/compiler-rt/lib/asan/asan_rtl.cpp
index f42de4189135c..e715d7742289c 100644
--- a/compiler-rt/lib/asan/asan_rtl.cpp
+++ b/compiler-rt/lib/asan/asan_rtl.cpp
@@ -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);
diff --git a/compiler-rt/lib/asan/asan_stack.h b/compiler-rt/lib/asan/asan_stack.h
index 47ca85a164435..b9575d2f427ee 100644
--- a/compiler-rt/lib/asan/asan_stack.h
+++ b/compiler-rt/lib/asan/asan_stack.h
@@ -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
diff --git a/compiler-rt/lib/hwasan/hwasan.cpp b/compiler-rt/lib/hwasan/hwasan.cpp
index 5c0d804561d21..8d6c25261b804 100644
--- a/compiler-rt/lib/hwasan/hwasan.cpp
+++ b/compiler-rt/lib/hwasan/hwasan.cpp
@@ -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);
diff --git a/compiler-rt/lib/hwasan/hwasan.h b/compiler-rt/lib/hwasan/hwasan.h
index 24d96cedc0446..8515df559f30a 100644
--- a/compiler-rt/lib/hwasan/hwasan.h
+++ b/compiler-rt/lib/hwasan/hwasan.h
@@ -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();
diff --git a/compiler-rt/lib/memprof/memprof_rtl.cpp b/compiler-rt/lib/memprof/memprof_rtl.cpp
index d6d606f666ee5..fee2912d64d49 100644
--- a/compiler-rt/lib/memprof/memprof_rtl.cpp
+++ b/compiler-rt/lib/memprof/memprof_rtl.cpp
@@ -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.
diff --git a/compiler-rt/lib/memprof/memprof_stack.h b/compiler-rt/lib/memprof/memprof_stack.h
index 289a61e385a25..a8fdfc9def9d0 100644
--- a/compiler-rt/lib/memprof/memprof_stack.h
+++ b/compiler-rt/lib/memprof/memprof_stack.h
@@ -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
diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index 4be1630cd3027..611b04d7d7e18 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -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);
diff --git a/compiler-rt/lib/msan/msan.h b/compiler-rt/lib/msan/msan.h
index a46e42c0e8e9d..963b94a540874 100644
--- a/compiler-rt/lib/msan/msan.h
+++ b/compiler-rt/lib/msan/msan.h
@@ -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(); }
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
index dcd625d30f779..7b65dd7dfb8ff 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
@@ -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
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp
index 84be6fc323420..6a54734353c5a 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp
@@ -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();
}
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
index 81517a25e290e..0efa99788abe6 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
@@ -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";
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.h b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
index 689f95ff01df9..3ae519d34da4f 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
@@ -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):
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
index cedf1a3ca7c9c..706794fdad10d 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
@@ -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 =
diff --git a/compiler-rt/test/msan/check-handler.cpp b/compiler-rt/test/msan/check-handler.cpp
index 4721f8c3068e3..0bdb433ab78b9 100644
--- a/compiler-rt/test/msan/check-handler.cpp
+++ b/compiler-rt/test/msan/check-handler.cpp
@@ -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;
}
More information about the llvm-commits
mailing list