Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add additional handling for AddressSanitizer #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aceckel
Copy link
Contributor

@aceckel aceckel commented Aug 20, 2020

This commit is a continuation of commit 2230e3e ("ignore sanitizer"),
which added the "no_sanitize_address" attribute to ctest_main to
prevent ASan from flagging reads while discovering the registered test
structs. Clang's ASan implementation adds a redzone around globals, so
some additional handling is required.

Starting with clang 4.0, the "no_sanitize_address" can also be applied
to globals to inhibit creating the globals redzone. However, for
earlier versions of clang (3.1 was the first to include ASan), we need
to work around the redzone. Thanks to ASAN_UNPOISON_MEMORY_REGION(),
this is possible. By unpoisoning the memory region and skipping over
any alignment padding, we are able to derive the test list, but the
"magic" value is now required to bookend the ctest struct.

This commit also attempts to make the compiler platform and version
checking logic more readable.

@aceckel aceckel force-pushed the ctest-vs-asan branch 7 times, most recently from b47ecbe to 330f9df Compare August 20, 2020 21:41
(test + direction) + offset);

#if CTEST_IMPL_POISIONED_GLOBALS
while (ASAN_UNPOISON_MEMORY_REGION(magic, sizeof(*magic)), *magic == 0) {
Copy link
Contributor Author

@aceckel aceckel Aug 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative to trying to work around sanitizers would be to change the test discovery method to not involve the use of a linker set. Linker sets are a handy tool, but are difficult to do portably (as this PR emphasizes). An approach that shouldn't anger sanitizers would be to construct a global linked list of tests using static constructors (mentioned as a solution to a different issue in #35 (comment)). I think we should consider this approach since it may kill two birds with one stone (fixes both the sanitizer issue and the Windows incremental linking issue while adding comparatively little platform-specific code).

This commit is a continuation of commit 2230e3e ("ignore sanitizer"),
which added the "no_sanitize_address" attribute to ctest_main to
prevent ASan from flagging reads while discovering the registered test
structs.  Clang's ASan implementation adds a redzone around globals, so
some additional handling is required.

Starting with clang 4.0, the "no_sanitize_address" can also be applied
to globals to inhibit creating the globals redzone.  However, for
earlier versions of clang (3.1 was the first to include ASan), we need
to work around the redzone.  Thanks to ASAN_UNPOISON_MEMORY_REGION(),
this is possible.  By unpoisoning the memory region and skipping over
any alignment padding, we are able to derive the test list, but the
"magic" value is now required to bookend the ctest struct.

This commit also attempts to make the compiler platform and version
checking logic more readable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant