This is an archive of the discontinued LLVM Phabricator instance.

Only test sanitizers that are built when COMPILER_RT_SANITIZERS_TO_BUILD is used
ClosedPublic

Authored by fjricci on Jun 26 2017, 1:58 PM.

Event Timeline

fjricci created this revision.Jun 26 2017, 1:58 PM
fjricci updated this revision to Diff 104020.Jun 26 2017, 2:00 PM

Actually fix typo

compnerd added inline comments.Jun 26 2017, 5:37 PM
test/CMakeLists.txt
67–79

Why not just walk COMPILER_RT_SANITIZERS_TO_BUILD rather than doing an intersection for each one?

compnerd accepted this revision.Jun 26 2017, 5:37 PM
This revision is now accepted and ready to land.Jun 26 2017, 5:37 PM
fjricci added inline comments.Jun 27 2017, 8:21 AM
test/CMakeLists.txt
67–79

Will address in a follow-up commit for the build case as well as the test case.

This revision was automatically updated to reflect the committed changes.
fjricci reopened this revision.Jun 27 2017, 10:28 AM

Broke cfi in cases when the cfi runtime isn't built.

This revision is now accepted and ready to land.Jun 27 2017, 10:28 AM
fjricci updated this revision to Diff 104207.Jun 27 2017, 10:28 AM

Test cfi even when cfi runtime isn't built

pcc edited edge metadata.Jun 27 2017, 10:57 AM

I don't understand why we have this COMPILER_RT_SANITIZERS_TO_BUILD feature, but given that we have it, this seems reasonable to me.

test/CMakeLists.txt
70

I guess if you want this to be controlled by COMPILER_RT_SANITIZERS_TO_BUILD it would need to be calling compiler_rt_test_sanitizer.

72

Probably want to preserve this comment.

fjricci marked an inline comment as done.Jun 27 2017, 11:06 AM

As far as I'm aware, the feature exists both to save time building and testing sanitizers you don't need, and to get around differing build requirements across the sanitizers (it's possible, especially when cross-compiling, that some sanitizers build in a given configuration and others do not).

test/CMakeLists.txt
70

The components built as part of the common runtime aren't controlled by COMPILER_RT_SANITIZERS_TO_BUILD (lsan, ubsan, sanitizer_common, interception, etc).

fjricci updated this revision to Diff 104217.Jun 27 2017, 11:07 AM

Add back cfi comment

pcc accepted this revision.Jun 27 2017, 11:23 AM

As far as I'm aware, the feature exists both to save time building and testing sanitizers you don't need, and to get around differing build requirements across the sanitizers (it's possible, especially when cross-compiling, that some sanitizers build in a given configuration and others do not).

You can achieve that though by choosing not to build the sanitizer runtimes that you don't want when you run "make" or "ninja" or whatever.

But anyway, this seems fine to me. LGTM.

This revision was automatically updated to reflect the committed changes.