This is an archive of the discontinued LLVM Phabricator instance.

[Sanitizers] Use SANITIZER_* macros in lib/interception
ClosedPublic

Authored by ro on Nov 8 2017, 6:06 AM.

Details

Summary

Unlike the rest of the sanitizer code, lib/interception uses native macros like linux
to check for specific targets instead of the common ones like SANITIZER_LINUX.

When working on the Solaris port of the sanitizers, the current style was found to not
only be inconsistent, but clumsy to use because the canonical way to check for Solaris
is to check for sun && svr4 which is a mouthful.

Therefore, this patch switches to use SANITIZER_* macros instead.

Tested on x86_64-pc-linux-gnu.

Diff Detail

Event Timeline

ro created this revision.Nov 8 2017, 6:06 AM
krytarowski added inline comments.Nov 8 2017, 7:42 AM
lib/interception/interception.h
267

SANITIZER_NETBSD

lib/interception/interception_mac.cc
17

Does it make sense?

ro added inline comments.Nov 8 2017, 7:47 AM
lib/interception/interception_mac.cc
17

How do you mean? One could do away with the effectively empty
file completely.

ro updated this revision to Diff 123552.Nov 20 2017, 1:29 AM

Fixed capitalization.

lib/interception/interception_mac.cc
17

Sorry but I still don't know what you mean here.

krytarowski added inline comments.Nov 20 2017, 1:41 AM
lib/interception/interception_mac.cc
17

I would just drop this:

#if SANITIZER_MAC
#endif
ro updated this revision to Diff 123553.Nov 20 2017, 1:50 AM

Remove unused guard.

ro marked 2 inline comments as done.Nov 20 2017, 1:52 AM
ro added inline comments.
lib/interception/interception_mac.cc
17

Fine with me. However, there's the risk of breaking non-OS X
builds if there's code added to the file and the guard forgotten...

But let's cross that bridge when/if we get there.

ro added a project: Restricted Project.Nov 20 2017, 5:12 AM
ro marked an inline comment as done.
ro set the repository for this revision to rCRT Compiler Runtime.Nov 30 2017, 12:12 PM
vitalybuka accepted this revision.Dec 5 2017, 4:13 PM
vitalybuka added inline comments.
lib/interception/interception_mac.cc
17

I'd probably keep this for consistency with other platform specific files.

This revision is now accepted and ready to land.Dec 5 2017, 4:13 PM
ro updated this revision to Diff 125679.Dec 6 2017, 1:11 AM
ro marked an inline comment as done.

Effectively restored to previous revision.

Herald added a subscriber: Restricted Project. · View Herald TranscriptDec 6 2017, 1:11 AM
ro added a comment.Dec 6 2017, 1:12 AM

Thanks for the approval. Could you (or someone else) please commit this for me?

krytarowski closed this revision.Dec 6 2017, 9:02 AM