[asan] Fix the deadlocks introduced by "On OS X, log reports to syslog and os_trace" commit

[asan] On OS X, log reports to syslog and os_trace, has been reverted in r252076 due to deadlocks on earlier versions of OS X. Alexey has also noticed deadlocks in some corner cases on Linux. This patch, if applied on top of the logging patch (http://reviews.llvm.org/D13452), addresses the known deadlock issues.

(This also proactively removes the color escape sequences from the error report buffer since we have to copy the buffer anyway.)

Differential Revision: http://reviews.llvm.org/D14470

llvm-svn: 253689

compiler-rt/lib/asan/asan_report.cc

@@ -30,7 +30,6 @@ namespace __asan {
 static void (*error_report_callback)(const char*);
 static char *error_message_buffer = nullptr;
 static uptr error_message_buffer_pos = 0;
-static uptr error_message_buffer_size = 0;
 static BlockingMutex error_message_buf_mutex(LINKER_INITIALIZED);
 
 struct ReportData {
@@ -49,17 +48,16 @@ static ReportData report_data = {};
 void AppendToErrorMessageBuffer(const char *buffer) {
   BlockingMutexLock l(&error_message_buf_mutex);
   if (!error_message_buffer) {
-    error_message_buffer_size = 1 << 16;
     error_message_buffer =
-      (char*)MmapOrDie(error_message_buffer_size, __func__);
+      (char*)MmapOrDieQuietly(kErrorMessageBufferSize, __func__);
     error_message_buffer_pos = 0;
   }
   uptr length = internal_strlen(buffer);
-  CHECK_GE(error_message_buffer_size, error_message_buffer_pos);
+  RAW_CHECK(kErrorMessageBufferSize >= error_message_buffer_pos);
-  uptr remaining = error_message_buffer_size - error_message_buffer_pos;
+  uptr remaining = kErrorMessageBufferSize - error_message_buffer_pos;
   internal_strncpy(error_message_buffer + error_message_buffer_pos,
                    buffer, remaining);
-  error_message_buffer[error_message_buffer_size - 1] = '\0';
+  error_message_buffer[kErrorMessageBufferSize - 1] = '\0';
   // FIXME: reallocate the buffer instead of truncating the message.
   error_message_buffer_pos += Min(remaining, length);
 }
@@ -686,13 +684,23 @@ class ScopedInErrorReport {
     // Print memory stats.
     if (flags()->print_stats)
       __asan_print_accumulated_stats();
+
+    // Copy the message buffer so that we could start logging without holding a
+    // lock that gets aquired during printing.
+    InternalScopedBuffer<char> buffer_copy(kErrorMessageBufferSize);
     {
       BlockingMutexLock l(&error_message_buf_mutex);
-      LogFullErrorReport(error_message_buffer);
+      internal_memcpy(buffer_copy.data(),
+                      error_message_buffer, kErrorMessageBufferSize);
+    }
 
-      if (error_report_callback) {
+    // Remove color sequences since logs cannot print them.
-        error_report_callback(error_message_buffer);
+    RemoveANSIEscapeSequencesFromString(buffer_copy.data());
-      }
+
+    LogFullErrorReport(buffer_copy.data());
+
+    if (error_report_callback) {
+      error_report_callback(buffer_copy.data());
     }
     CommonSanitizerReportMutex.Unlock();
     reporting_thread_tid_ = kInvalidTid;

compiler-rt/lib/sanitizer_common/sanitizer_common.cc

@@ -164,11 +164,12 @@ void NORETURN CheckFailed(const char *file, int line, const char *cond,
 }
 
 void NORETURN ReportMmapFailureAndDie(uptr size, const char *mem_type,
-                                      const char *mmap_type, error_t err) {
+                                      const char *mmap_type, error_t err,
+                                      bool raw_report) {
   static int recursion_count;
-  if (recursion_count) {
+  if (raw_report || recursion_count) {
+    // If raw report is requested or we went into recursion, just die.
     // The Report() and CHECK calls below may call mmap recursively and fail.
-    // If we went into recursion, just die.
     RawWrite("ERROR: Failed to mmap\n");
     Die();
   }

compiler-rt/lib/sanitizer_common/sanitizer_common.h

@@ -49,6 +49,8 @@ static const uptr kMaxNumberOfModules = 1 << 14;
 
 const uptr kMaxThreadStackSize = 1 << 30;  // 1Gb
 
+static const uptr kErrorMessageBufferSize = 1 << 16;
+
 // Denotes fake PC values that come from JIT/JAVA/etc.
 // For such PC values __tsan_symbolize_external() will be called.
 const u64 kExternalPCBit = 1ULL << 60;
@@ -76,7 +78,10 @@ void GetThreadStackAndTls(bool main, uptr *stk_addr, uptr *stk_size,
                           uptr *tls_addr, uptr *tls_size);
 
 // Memory management
-void *MmapOrDie(uptr size, const char *mem_type);
+void *MmapOrDie(uptr size, const char *mem_type, bool raw_report = false);
+INLINE void *MmapOrDieQuietly(uptr size, const char *mem_type) {
+  return MmapOrDie(size, mem_type, /*raw_report*/ true);
+}
 void UnmapOrDie(void *addr, uptr size);
 void *MmapFixedNoReserve(uptr fixed_addr, uptr size,
                          const char *name = nullptr);
@@ -310,7 +315,8 @@ 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);
+                                      const char *mmap_type, error_t err,
+                                      bool raw_report = false);
 
 // Set the name of the current thread to 'name', return true on succees.
 // The name may be truncated to a system-dependent limit.
@@ -423,7 +429,7 @@ INLINE uptr RoundUpToPowerOfTwo(uptr size) {
 }
 
 INLINE uptr RoundUpTo(uptr size, uptr boundary) {
-  CHECK(IsPowerOfTwo(boundary));
+  RAW_CHECK(IsPowerOfTwo(boundary));
   return (size + boundary - 1) & ~(boundary - 1);
 }
 
@@ -652,9 +658,9 @@ enum AndroidApiLevel {
 void WriteToSyslog(const char *buffer);
 
 #if SANITIZER_MAC
-void LogFullErrorReport(const char *error_message_buffer);
+void LogFullErrorReport(const char *buffer);
 #else
-INLINE void LogFullErrorReport(const char *error_message_buffer) {}
+INLINE void LogFullErrorReport(const char *buffer) {}
 #endif
 
 #if SANITIZER_LINUX || SANITIZER_MAC

compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cc

@@ -12,7 +12,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_common.h"
-#include "sanitizer_allocator_internal.h"
+
 #include "sanitizer_flags.h"
 #include "sanitizer_stackdepot.h"
 #include "sanitizer_stacktrace.h"
@@ -119,13 +119,14 @@ void BackgroundThread(void *arg) {
   }
 }
 
-void WriteToSyslog(const char *buffer) {
+void WriteToSyslog(const char *msg) {
-  char *copy = internal_strdup(buffer);
+  InternalScopedString msg_copy(kErrorMessageBufferSize);
-  char *p = copy;
+  msg_copy.append("%s", msg);
+  char *p = msg_copy.data();
   char *q;
 
   // Remove color sequences since syslogs cannot print them.
-  RemoveANSIEscapeSequencesFromString(copy);
+  RemoveANSIEscapeSequencesFromString(p);
 
   // Print one line at a time.
   // syslog, at least on Android, has an implicit message length limit.
@@ -137,7 +138,6 @@ void WriteToSyslog(const char *buffer) {
     if (q)
       p = q + 1;
   } while (q);
-  InternalFree(copy);
 }
 
 void MaybeStartBackgroudThread() {

compiler-rt/lib/sanitizer_common/sanitizer_mac.cc

@@ -13,6 +13,7 @@
 
 #include "sanitizer_platform.h"
 #if SANITIZER_MAC
+#include "sanitizer_mac.h"
 
 // Use 64-bit inodes in file operations. ASan does not support OS X 10.5, so
 // the clients will most certainly use 64-bit ones as well.
@@ -25,7 +26,6 @@
 #include "sanitizer_flags.h"
 #include "sanitizer_internal_defs.h"
 #include "sanitizer_libc.h"
-#include "sanitizer_mac.h"
 #include "sanitizer_placement_new.h"
 #include "sanitizer_platform_limits_posix.h"
 #include "sanitizer_procmaps.h"

compiler-rt/lib/sanitizer_common/sanitizer_posix.cc

@@ -112,14 +112,14 @@ uptr GetMaxVirtualAddress() {
 #endif  // SANITIZER_WORDSIZE
 }
 
-void *MmapOrDie(uptr size, const char *mem_type) {
+void *MmapOrDie(uptr size, const char *mem_type, bool raw_report) {
   size = RoundUpTo(size, GetPageSizeCached());
   uptr res = internal_mmap(nullptr, size,
                            PROT_READ | PROT_WRITE,
                            MAP_PRIVATE | MAP_ANON, -1, 0);
   int reserrno;
   if (internal_iserror(res, &reserrno))
-    ReportMmapFailureAndDie(size, mem_type, "allocate", reserrno);
+    ReportMmapFailureAndDie(size, mem_type, "allocate", reserrno, raw_report);
   IncreaseTotalMmap(size);
   return (void *)res;
 }

compiler-rt/lib/sanitizer_common/sanitizer_win.cc

@@ -83,10 +83,11 @@ void GetThreadStackTopAndBottom(bool at_initialization, uptr *stack_top,
 }
 #endif  // #if !SANITIZER_GO
 
-void *MmapOrDie(uptr size, const char *mem_type) {
+void *MmapOrDie(uptr size, const char *mem_type, bool raw_report) {
   void *rv = VirtualAlloc(0, size, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE);
   if (rv == 0)
-    ReportMmapFailureAndDie(size, mem_type, "allocate", GetLastError());
+    ReportMmapFailureAndDie(size, mem_type, "allocate",
+                            GetLastError(), raw_report);
   return rv;
 }