[PATCH] D13452: [asan] On OS X, log reports to syslog and os_trace (version 2)

[Filipe Cabecinhas via llvm-commits]

filcab added a comment.

Looks good, but wait on another LGTM.
Minor comments above.


================
Comment at: lib/asan/asan_report.cc:47
@@ -46,2 +46,3 @@
 static ReportData report_data = {};
+static BlockingMutex append_to_error_buf;
 
----------------
I'd prefer this to have "mutex" or "lock" (or a variation of one) in the name.

================
Comment at: lib/sanitizer_common/sanitizer_mac.cc:408
@@ +407,3 @@
+  // for GCD to dispatch a new thread, the process will deadlock, because the
+  // pthread_create wrapper needs to acquire the lock as well.
+  if (common_flags()->log_to_syslog)
----------------
Can we verify any/some of these things (e.g: someone was holding the mutex and called us) with some CHECK() calls?

================
Comment at: lib/sanitizer_common/sanitizer_printf.cc:282
@@ -282,1 +281,3 @@
+  // Log to syslog on Android.
+  if (common_flags()->log_to_syslog && ShouldLogAfterPrintf())
     WriteToSyslog(buffer);
----------------
It's not really testing Android, it's something more general.
Both rephrasing or removing the comment are ok for me.

================
Comment at: test/ubsan/lit.common.cfg:45
@@ -44,2 +44,3 @@
   default_ubsan_opts += ['abort_on_error=0']
+  default_ubsan_opts += [':log_to_syslog=0']
 default_ubsan_opts_str = ':'.join(default_ubsan_opts)
----------------
Extra ':'.


http://reviews.llvm.org/D13452