-
Notifications
You must be signed in to change notification settings - Fork 82
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 AddressSanitizer to CI tests #812
Conversation
PR missing one of the required labels: {'documentation', 'dependencies', 'new feature', 'internal', 'breaking-change', 'enhancement', 'bug'} |
89b9505
to
39ea2b0
Compare
39ea2b0
to
7c96d1b
Compare
a91ce1b
to
5f3499d
Compare
5f3499d
to
b64706a
Compare
@@ -453,6 +454,11 @@ if(BUILD_EXAMPLES) | |||
add_subdirectory(examples) | |||
endif() | |||
|
|||
if(ASAN) | |||
add_compile_options(-fsanitize=address) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not correct, since -fsanitize is a clang/gcc feature only and thus will fail with other compilers (for example for MSVC it is /fsanitize=address)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't tested it with MSVC, but as far as I see there
https://learn.microsoft.com/en-us/cpp/build/reference/compiler-options?view=msvc-170
and there
https://learn.microsoft.com/en-us/cpp/sanitizers/asan?view=msvc-170
this type of option is also correct.
In addition, this is an optional option that does not have to be enabled for all compilers.
Add AddressSanitizer to CI tests and fix memory issues:
Closes: #801