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

Anna Zaks via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 20 10:42:02 PST 2015


Author: zaks
Date: Fri Nov 20 12:42:01 2015
New Revision: 253689

URL: http://llvm.org/viewvc/llvm-project?rev=253689&view=rev
Log:
[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

Modified:
    compiler-rt/trunk/lib/asan/asan_report.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_libcdep.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc

Modified: compiler-rt/trunk/lib/asan/asan_report.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_report.cc?rev=253689&r1=253688&r2=253689&view=diff
==============================================================================
--- compiler-rt/trunk/lib/asan/asan_report.cc (original)
+++ compiler-rt/trunk/lib/asan/asan_report.cc Fri Nov 20 12:42:01 2015
@@ -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);
-  uptr remaining = error_message_buffer_size - error_message_buffer_pos;
+  RAW_CHECK(kErrorMessageBufferSize >= 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);
+    }
+
+    // Remove color sequences since logs cannot print them.
+    RemoveANSIEscapeSequencesFromString(buffer_copy.data());
+
+    LogFullErrorReport(buffer_copy.data());
 
-      if (error_report_callback) {
-        error_report_callback(error_message_buffer);
-      }
+    if (error_report_callback) {
+      error_report_callback(buffer_copy.data());
     }
     CommonSanitizerReportMutex.Unlock();
     reporting_thread_tid_ = kInvalidTid;

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc?rev=253689&r1=253688&r2=253689&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc Fri Nov 20 12:42:01 2015
@@ -164,11 +164,12 @@ void NORETURN CheckFailed(const char *fi
 }
 
 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();
   }

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h?rev=253689&r1=253688&r2=253689&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h Fri Nov 20 12:42:01 2015
@@ -49,6 +49,8 @@ static const uptr kMaxNumberOfModules =
 
 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, upt
                           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 siz
 }
 
 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

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_libcdep.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_libcdep.cc?rev=253689&r1=253688&r2=253689&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_libcdep.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_libcdep.cc Fri Nov 20 12:42:01 2015
@@ -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) {
-  char *copy = internal_strdup(buffer);
-  char *p = copy;
+void WriteToSyslog(const char *msg) {
+  InternalScopedString msg_copy(kErrorMessageBufferSize);
+  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() {

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc?rev=253689&r1=253688&r2=253689&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc Fri Nov 20 12:42:01 2015
@@ -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"

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc?rev=253689&r1=253688&r2=253689&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.cc Fri Nov 20 12:42:01 2015
@@ -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;
 }

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc?rev=253689&r1=253688&r2=253689&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc Fri Nov 20 12:42:01 2015
@@ -83,10 +83,11 @@ void GetThreadStackTopAndBottom(bool at_
 }
 #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;
 }
 




More information about the llvm-commits mailing list