[PATCH] D11435: [sanitizer] Implement logging to syslog

Evgeniy Stepanov eugenis at google.com
Wed Jul 22 16:30:49 PDT 2015


eugenis added inline comments.

================
Comment at: lib/sanitizer_common/sanitizer_common.h:633
@@ -632,2 +632,3 @@
 // Initialize Android logging. Any writes before this are silently lost.
 void AndroidLogInit();
+void WriteToSyslog(const char *buffer);
----------------
samsonov wrote:
> Do you actually need `AndroidLogInit` on non-Android platforms?
That's how we do platform-specific things. You can find more examples below in the same file. The alternative is to add ugly ifdefs at all call sites.

================
Comment at: lib/sanitizer_common/sanitizer_posix_libcdep.cc:306
@@ +305,3 @@
+void WriteToSyslog(const char *buffer) {
+  char *copy = internal_strdup(buffer);
+  char *p = copy;
----------------
samsonov wrote:
> Hm, calling an internal allocator on every printf... I think that local or mmaped buffer would work better.
We have a check that stack frame is small.
Message length in unpredictable.
By mmaped, do you  mean MmapOrDie in every printf? A thread-local mmaped buffer?

Why is this a problem in the first place?


================
Comment at: lib/sanitizer_common/tests/CMakeLists.txt:176
@@ -176,1 +175,3 @@
+                               $<TARGET_OBJECTS:RTSanitizerCommon.x86_64>
+                               $<TARGET_OBJECTS:RTSanitizerCommonNoLibc.x86_64>)
     endif()
----------------
samsonov wrote:
> You probably would need to add this library to dfsan.
Strangely enough, the no-libc variant of dfsan is not linked to the "dfsan" target, nor does it have any tests.
Added the new library.


http://reviews.llvm.org/D11435







More information about the llvm-commits mailing list