This is an archive of the discontinued LLVM Phabricator instance.

[sanitizer] Do not introduce __sanitizer namespace globally
ClosedPublic

Authored by zaks.anna on Jul 1 2016, 5:24 PM.

Details

Summary

The definitions in sanitizer_common may conflict with definitions from system headers because:

  1. The runtime includes the system headers after the project headers (as per LLVM coding guidelines).
  2. lib/sanitizer_common/sanitizer_internal_defs.h pollutes the namespace of everything defined after it, which is all/most of the sanitizer .h and .cc files and the included system headers with: using namespace __sanitizer; // NOLINT

This patch solves the problem by introducing the namespace only within the sanitizer namespaces as proposed by Dmitry.

(TODO: This needs to be tested on Linux since we do not build a few sanitizers on macOS. I'll probably have to do more cleanup once I test on Linux.)

Diff Detail

Event Timeline

zaks.anna updated this revision to Diff 62578.Jul 1 2016, 5:24 PM
zaks.anna retitled this revision from to [sanitizer] Do not introduce __sanitizer namespace globally.
zaks.anna updated this object.
zaks.anna added reviewers: dvyukov, kubamracek.
zaks.anna added subscribers: llvm-commits, kcc.
dvyukov accepted this revision.Jul 3 2016, 5:33 AM
dvyukov edited edge metadata.

looks good to me, but please wait several days in case somebody else wants to chime in

This revision is now accepted and ready to land.Jul 3 2016, 5:33 AM
zaks.anna updated this revision to Diff 70256.Sep 2 2016, 4:54 PM
zaks.anna edited edge metadata.

I tested my earlier patch on Linux and needed to add a few more updates as many sanitizers are not available on a mac.

Will wait a couple of days and commit if there are no more comments.

This revision was automatically updated to reflect the committed changes.