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

Run tests with address, undefined, and thread sanitizer in CI #4

Merged
merged 3 commits into from
Sep 6, 2024

Conversation

msimberg
Copy link
Collaborator

@msimberg msimberg commented Jul 26, 2024

tsan is maybe a bit unnecessary, but doesn't hurt much, and then it's there in case there will be threading in the future.

msan is not supported by GCC so leaving that out (I believe that also requires recompiling the standard library to be effective).

asan reports false positives during release mode compilation so I've kept asan in debug builds only, and added lsan for release and debug.

@msimberg
Copy link
Collaborator Author

Sigh, there's either a bug in libstdc++ or GCC just reports false positives (https://github.com/bcumming/uenv2/actions/runs/10110902791/job/27962015058?pr=4). This seems to be an issue all the way up to GCC 14 (https://godbolt.org/z/o6zq4TT8M) with a combination of -O3 and -fsanitize=address. Staying a draft until I decide how best to suppress this.

@msimberg msimberg force-pushed the sanitizers-ci branch 2 times, most recently from 0a8ede8 to cf83da7 Compare September 4, 2024 08:09
@msimberg
Copy link
Collaborator Author

msimberg commented Sep 4, 2024

I downgraded asan to lsan to avoid the false positive and at least have some sanitizers used.

@bcumming is this skipped test expected to be skipped: https://github.com/eth-cscs/uenv2/actions/runs/10697698093/job/29655442215?pr=4#step:9:20?

@msimberg msimberg marked this pull request as ready for review September 4, 2024 08:19
@bcumming
Copy link
Member

bcumming commented Sep 4, 2024

@bcumming is this skipped test expected to be skipped: https://github.com/eth-cscs/uenv2/actions/runs/10697698093/job/29655442215?pr=4#step:9:20?

That test needs to be fixed - a "setup" step that creates the input data sets needs to be performed first.

@msimberg
Copy link
Collaborator Author

msimberg commented Sep 6, 2024

Thanks @bcumming for fixing the test setup. This should now be ready to go.

@bcumming bcumming merged commit a6143a9 into eth-cscs:main Sep 6, 2024
10 checks passed
@msimberg msimberg deleted the sanitizers-ci branch September 6, 2024 09:52
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.

2 participants