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

[ASan] Disable annotations on unsupported platforms #4058

Merged

Conversation

strega-nil-ms
Copy link
Contributor

@strega-nil-ms strega-nil-ms commented Sep 28, 2023

Add a new define, _DISABLE_ASAN_ANNOTATIONS, which disables all ASan annotations in the standard library as opposed to requiring a person to define both _DISABLE_STRING_ANNOTATION and
_DISABLE_VECTOR_ANNOTATION.

Disable ASan annotations on unsupported platforms like ARM64. If someone is working on adding address sanitizer support on those platforms (* wink wink nudge nudge *) they can define the
_ENABLE_ASAN_ANNOTATIONS_ON_UNSUPPORTED_PLATFORMS escape hatch to get STL annotations anyways, though they will need to provide their own stl_asan.lib.

Additionally, add two new defines:

  • _DISABLE_STL_ANNOTATION
  • _ANNOTATE_STL

which do the job of _DISABLE_STRING_ANNOTATION and _DISABLE_VECTOR_ANNOTATION, and _ANNOTATE_STRING and _ANNOTATE_VECTOR respectively. This allows users to pass these defines without worrying we'll add new ones for other containers.

Also, fix a bug where annotate_string (and annotate_vector) were not getting set to 0 if you passed both _DISABLE_STRING_ANNOTATION and _DISABLE_VECTOR_ANNOTATION.

Fixes #4035

Add a new define, `_DISABLE_ASAN_ANNOTATIONS`, which disables all ASan
annotations in the standard library as opposed to requiring a person to
define both `_DISABLE_STRING_ANNOTATION` and
`_DISABLE_VECTOR_ANNOTATION`.

Disable ASan annotations on unsupported platforms like ARM64. If someone
is working on adding address sanitizer support on those platforms
(* wink wink nudge nudge *) they can define the
`_ENABLE_ASAN_ANNOTATIONS_ON_UNSUPPORTED_PLATFORMS` escape hatch to get
STL annotations anyways, though they will need to provide their own
stl_asan.lib.

Also, fix a bug where `annotate_string` (and `annotate_vector`) were not
getting set to `0` if you passed both `_DISABLE_STRING_ANNOTATION` and
`_DISABLE_VECTOR_ANNOTATION`.
@strega-nil-ms strega-nil-ms requested a review from a team as a code owner September 28, 2023 18:41
@github-actions github-actions bot added this to Initial Review in Code Reviews Sep 28, 2023
@strega-nil-ms strega-nil-ms moved this from Initial Review to Final Review in Code Reviews Sep 28, 2023
@CaseyCarter CaseyCarter added the bug Something isn't working label Sep 28, 2023
stl/inc/__msvc_sanitizer_annotate_container.hpp Outdated Show resolved Hide resolved
stl/inc/__msvc_sanitizer_annotate_container.hpp Outdated Show resolved Hide resolved
stl/inc/__msvc_sanitizer_annotate_container.hpp Outdated Show resolved Hide resolved
stl/inc/__msvc_sanitizer_annotate_container.hpp Outdated Show resolved Hide resolved
stl/inc/__msvc_sanitizer_annotate_container.hpp Outdated Show resolved Hide resolved
Code Reviews automation moved this from Final Review to Work In Progress Sep 28, 2023
@strega-nil-ms strega-nil-ms moved this from Work In Progress to Final Review in Code Reviews Sep 29, 2023
benchmarks/google-benchmark Outdated Show resolved Hide resolved
stl/inc/__msvc_sanitizer_annotate_container.hpp Outdated Show resolved Hide resolved
stl/inc/__msvc_sanitizer_annotate_container.hpp Outdated Show resolved Hide resolved
stl/inc/__msvc_sanitizer_annotate_container.hpp Outdated Show resolved Hide resolved
Code Reviews automation moved this from Final Review to Work In Progress Sep 29, 2023
I've also cleaned up everything in a way that's hopefully more obvious.
@CaseyCarter CaseyCarter moved this from Work In Progress to Ready To Merge in Code Reviews Oct 2, 2023
stl/inc/__msvc_sanitizer_annotate_container.hpp Outdated Show resolved Hide resolved
stl/CMakeLists.txt Show resolved Hide resolved
stl/inc/__msvc_sanitizer_annotate_container.hpp Outdated Show resolved Hide resolved
stl/inc/__msvc_sanitizer_annotate_container.hpp Outdated Show resolved Hide resolved
stl/inc/__msvc_sanitizer_annotate_container.hpp Outdated Show resolved Hide resolved
Code Reviews automation moved this from Ready To Merge to Work In Progress Oct 2, 2023
@StephanTLavavej StephanTLavavej added the ASan Address Sanitizer label Oct 2, 2023
@StephanTLavavej StephanTLavavej moved this from Work In Progress to Ready To Merge in Code Reviews Oct 2, 2023
@CaseyCarter

This comment was marked as resolved.

@azure-pipelines

This comment was marked as resolved.

@StephanTLavavej StephanTLavavej self-assigned this Oct 3, 2023
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 172d0c0 into microsoft:main Oct 4, 2023
69 checks passed
Code Reviews automation moved this from Ready To Merge to Done Oct 4, 2023
@StephanTLavavej
Copy link
Member

Thanks for improving the ASan experience! 🎉 📈 ⚙️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASan Address Sanitizer bug Something isn't working
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

MSVC 14.37.32822 does not ship with stl_asan.lib for arm64
4 participants