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

Support instrumenting the STL with ASan #4313

Merged
merged 10 commits into from
Jan 17, 2024

Conversation

CaseyCarter
Copy link
Member

@CaseyCarter CaseyCarter commented Jan 12, 2024

Add support for instrumenting the STL with ASan by setting the STL_ASAN_BUILD cmake 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 (/MT or /MTd) or the contents of the import library (/MD or /MDd).

This includes a fairly icky workaround for an initialization ordering bug (VSO-1913897) which injects a dynamically-initialized thread_local into 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.

⚠️ Requires coordinated changes in the internal test infrastructure ⚠️

.. to add tests/std/include to the include path for the libc++ test suite to support the VSO-1913897 workaround.

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.
@CaseyCarter CaseyCarter added infrastructure Related to repository automation test Related to test code labels Jan 12, 2024
@CaseyCarter CaseyCarter changed the title Asan build Support instrumenting the STL with ASan Jan 12, 2024
@CaseyCarter

This comment was marked as resolved.

This comment was marked as resolved.

@CaseyCarter

This comment was marked as resolved.

This comment was marked as resolved.

@CaseyCarter

This comment was marked as duplicate.

This comment was marked as duplicate.

@CaseyCarter

This comment was marked as duplicate.

This comment was marked as duplicate.

@CaseyCarter
Copy link
Member Author

@CaseyCarter CaseyCarter marked this pull request as ready for review January 16, 2024 20:53
@CaseyCarter CaseyCarter requested a review from a team as a code owner January 16, 2024 20:53
@StephanTLavavej
Copy link
Member

LGTM. I pushed a conflict-free merge with main, plus the comment cleanup you noted, plus a TRANSITION comment that I wanted.

@StephanTLavavej StephanTLavavej self-assigned this Jan 17, 2024
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej added the ASan Address Sanitizer label Jan 17, 2024
@StephanTLavavej
Copy link
Member

I had to push a commit because /clr and /clr:pure hate thread_local:

/clr error:

D:\a01\_work\5\s\src\vctools\crt\github\tests\std\include\vso1913897.hpp(11): error C2482: '__stl_asan_init_variable': dynamic initialization of thread local data not allowed in managed code

/clr:pure errors:

D:\a01\_work\5\s\src\vctools\crt\github\tests\std\include\vso1913897.hpp(11): error C3403: thread_local cannot be used with /clr:pure or /clr:safe
D:\a01\_work\5\s\src\vctools\crt\github\tests\std\include\vso1913897.hpp(11): error C2482: '__stl_asan_init_variable': dynamic initialization of thread local data not allowed in managed code

@StephanTLavavej StephanTLavavej merged commit 0cb9eb4 into microsoft:main Jan 17, 2024
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks for this major ASan achievement! 🧗 ⛰️ 😻

@CaseyCarter CaseyCarter deleted the asan-build branch January 17, 2024 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASan Address Sanitizer infrastructure Related to repository automation test Related to test code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants