[PATCH] D15396: [sanitizers] Log all output to CrashReport on OS X

[Anna Zaks via llvm-commits]

zaks.anna marked an inline comment as done.

================
Comment at: lib/sanitizer_common/sanitizer_mac.h:49
@@ -50,1 +48,3 @@
+INLINE void CRAppendCrashLogMessage(const char *msg) {
+  internal_strlcat(__crashreporter_info_buff__, msg,
                    sizeof(__crashreporter_info_buff__)); }
----------------
kubabrecka wrote:
> Don't we need a lock here?  Can we optimize the strcat here (by remembering the position where to append)?
> 
> Or maybe we could unify this completely with AppendToErrorMessageBuffer (in asan_report.cc), which basically does the same thing.  We could only have a single buffer to log into.
> Don't we need a lock here? 
Yes, we need it. Will address.
> Can we optimize the strcat here (by remembering the position where to append)?
I am not convinced that increasing code complexity is worth it in this case. This is logging code...

> Or maybe we could unify this completely with 
> AppendToErrorMessageBuffer (in asan_report.cc), which basically does > the same thing. We could only have a single buffer to log into.

Currently, AppendToErrorMessageBuffer is ASan specific. We should log output from any sanitizer.

================
Comment at: lib/sanitizer_common/sanitizer_printf.cc:283
@@ -283,1 +282,3 @@
+  // Remove color sequences from the message.
+  RemoveANSIEscapeSequencesFromString(buffer);
   CallPrintfAndReportCallback(buffer);
----------------
kubabrecka wrote:
> This means that CallPrintfAndReportCallback is now called on a color-less string.  I'm actually in favor of this change, but we should make sure everyone's okay with it.
Yes, calling CallPrintfAndReportCallback on a color-less string seems to be the right thing to do, at least to me.

I can update the commit message to call this out.


http://reviews.llvm.org/D15396