Page MenuHomePhabricator

[sanitizer] Implement logging to syslog
ClosedPublic

Authored by eugenis on Jul 22 2015, 3:33 PM.

Details

Reviewers
kcc
samsonov
Summary

Previously, Android target has logic of duplicating all sanitizer output to logcat.
This change extends it to all posix platforms via the use of syslog, controlled by log_to_syslog flag.
Enabled by default on Android, off everywhere else.

A bit of cmake magic is required to allow Printf() to call a libc function. I'm adding a stub implementation to support no-libc builds like dfsan and safestack.

Diff Detail

Event Timeline

eugenis updated this revision to Diff 30411.Jul 22 2015, 3:33 PM
eugenis retitled this revision from to [sanitizer] Implement logging to syslog.
eugenis updated this object.
eugenis added reviewers: samsonov, kcc.
eugenis added a subscriber: llvm-commits.
samsonov added inline comments.Jul 22 2015, 4:02 PM
lib/safestack/CMakeLists.txt
16

There is no such thing on osx

lib/sanitizer_common/CMakeLists.txt
36

Please clarify why/how these sources are used.

lib/sanitizer_common/sanitizer_common.h
633

Do you actually need AndroidLogInit on non-Android platforms?

lib/sanitizer_common/sanitizer_posix_libcdep.cc
306

Hm, calling an internal allocator on every printf... I think that local or mmaped buffer would work better.

lib/sanitizer_common/tests/CMakeLists.txt
176

You probably would need to add this library to dfsan.

eugenis updated this revision to Diff 30425.Jul 22 2015, 4:30 PM
eugenis marked 2 inline comments as done.
eugenis added inline comments.
lib/sanitizer_common/sanitizer_common.h
633

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.

lib/sanitizer_common/sanitizer_posix_libcdep.cc
306

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?

lib/sanitizer_common/tests/CMakeLists.txt
176

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.

samsonov accepted this revision.Jul 22 2015, 4:49 PM
samsonov edited edge metadata.

LGTM with a couple of nits.

lib/sanitizer_common/sanitizer_linux.cc
17–18

You can now remove this include.

lib/sanitizer_common/sanitizer_posix_libcdep.cc
295

Note that if android_log_initialized will be initialized while you are looping inside WriteToSyslog, the latter will start logging since the middle of the buffer, which is probably undesirable. Yeah, I know that it's highly unlikely, but you may still add static bool IsSyslogAvailable(), and call it once at the beginning of WriteToSyslog.

This revision is now accepted and ready to land.Jul 22 2015, 4:49 PM
eugenis updated this revision to Diff 30430.Jul 22 2015, 4:55 PM
eugenis edited edge metadata.
eugenis marked an inline comment as done.
eugenis added inline comments.
lib/sanitizer_common/sanitizer_posix_libcdep.cc
295

Good idea.

eugenis closed this revision.Jul 22 2015, 4:58 PM

Thanks.
Landed as r242975.