Support instrumenting the STL with ASan#4313
Merged
StephanTLavavej merged 10 commits intomicrosoft:mainfrom Jan 17, 2024
Merged
Support instrumenting the STL with ASan#4313StephanTLavavej merged 10 commits intomicrosoft:mainfrom
StephanTLavavej merged 10 commits intomicrosoft:mainfrom
Conversation
VSO-1913897 is the result the STL's early initialization of locale and stream data - which must happen before init of user globals so their initializers can use streams - being instrumented with ASan and sometimes running before the ASan initialization. This change adds a new force-included header `vso1913897.hpp` that dynamically initializes a `thread_local`; programs that use TLS engage a more reliable early initialization path in the ASan runtime.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
Contributor
Author
|
STL-ASan-CI validation: https://dev.azure.com/vclibs/STL/_build/results?buildId=15919&view=results |
CaseyCarter
commented
Jan 16, 2024
StephanTLavavej
approved these changes
Jan 16, 2024
Member
|
LGTM. I pushed a conflict-free merge with |
Member
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
Member
|
I had to push a commit because
|
StephanTLavavej
approved these changes
Jan 17, 2024
Member
|
Thanks for this major ASan achievement! 🧗 ⛰️ 😻 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add support for instrumenting the STL with ASan by setting the
STL_ASAN_BUILDcmake option and enable this support in the STL-ASan-CI pipeline. This lets us run "full ASan" tests with coverage of the header-only code and binary libraries. For now that requires client programs to be ASan instrumented since they statically link with either the entire STL (/MTor/MTd) or the contents of the import library (/MDor/MDd).This includes a fairly icky workaround for an initialization ordering bug (VSO-1913897) which injects a dynamically-initialized
thread_localinto each test TU. It's a matter of opinion whether it's more horrible to have ASan and non-ASan test coverage diverge by using this workaround only for ASan testing, or more horrible to have the ASan workaround affect non-ASan testing. I've chosen to use the workaround universally. This should truly be temporary since there's ongoing work to fix the issue with a coordinated change in the compiler and ASan runtime.Drive-by: increase paranoia around google/sanitizers#328. This change isn't strictly needed but would have saved me some time and confusion.
.. to add
tests/std/includeto the include path for the libc++ test suite to support the VSO-1913897 workaround.