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

[Anna Zaks via llvm-commits]

zaks.anna added inline comments.

================
Comment at: lib/sanitizer_common/sanitizer_mac.cc:378
@@ +377,3 @@
+
+void LogFullErrorReport(const char *error_message_buffer) {
+  // Log with os_trace. This will make it into the crash log.
----------------
glider wrote:
> There's a name clash between this parameter and the global variable.
The variable is a static from another file, so technically there is no clash, but I can rename the parameter to buffer.

================
Comment at: lib/sanitizer_common/sanitizer_mac.cc:399
@@ +398,3 @@
+  // Log to syslog.
+  // The logging on OS X may call pthread_create so we need the threading
+  // environment to be fully initialized. Also, this should never be called when
----------------
glider wrote:
> Any idea about the conditions under which the logging calls pthread_create?
> Maybe we can warm up the logging subsystem when initializing ASan?
> Will it help to create actual asl clients using asl_open()?
asl_log relies on libdispatch, for example, it calls dispatch_once. We cannot control when GCD will spawn a new thread. The deadlocks are unpredictable, they often happen when the client code uses GCD.

asl_open should be used if we want to log from multiple threads without locking:
"A client handle may only be safely shared amongst multiple threads if the application uses locks or some synchronization strategy to ensure single-threaded access." 

LogFullErrorReport is only called behind a lock right now. However, I could add another lock just to protect logging to syslog. Ex: maybe we could lock before calling WriteToSyslog below and check the lock in WriteOneLineToSyslog. This way we will future proof the code.



http://reviews.llvm.org/D13452