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

[Evgeniy Stepanov via llvm-commits]

eugenis added inline comments.

================
Comment at: lib/sanitizer_common/sanitizer_common.h:659
@@ +658,3 @@
+  SANITIZER_TYPE_UNKNOWN
+};
+
----------------
zaks.anna wrote:
> eugenis wrote:
> > We already have knowledge of the current sanitizer: SanitizerToolName. Could you either dispatch on that, or use it directly?
> One of the APIs we use only allows string literals (for privacy reasons).
> 
> Dispatching on the string literal instead of the enum feels bad. 
> 
> Also, how does this global SanitizerToolName works when we have more than one sanitizer on at the same time? I think UBSan and ASan are not mutually exclusive. It might not matter which tool failed in some cases; however, when a tool reports an error, differentiating between the two might be useful. One way of doing that would be to have the error reporting APIs take the tool name as a parameter.
> 
> In most cases, SanitizerToolName is used in calls to Report() when reporting an internal error. It would be useful to log the internal errors to syslog on OS X as well (at least that should be the long term goal).
> 
> Any further opinions on this?
> 
I don't like 2 things about this: duplication between SanitizerType and SanitizerToolName, and the fact the SanitizerType list is incomplete and looks arbitrary.

Maybe extend the list to include, at least, all possible values of SanitizerToolName, and define a global instance of that to replace SanitizerToolName? With a static list of pretty tool names indexed by SanitizerType.

>> Also, how does this global SanitizerToolName works when we have more than one sanitizer on at the same time?

There is always a "main sanitizer" which provides the name to be used in reports. In the case of asan+ubsan the main sanitizer is asan. We never really saw this as a problem. After all, the report contains a type string like "use-after-free" which clearly tells the user what it is about.



http://reviews.llvm.org/D13452