Skip to content

Allow ASan container annotations to activate piecemeal #6211

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.

Sign up for GitHub

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

Open
amyw-msft wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Allow ASan container annotations to activate piecemeal #6211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62e7af2
Remove stl_asan.lib as a dependency so that annotations can be activa…
amyw-msft Apr 3, 2026
d1baeac
Add comments to stl_asan.lib definitions.
amyw-msft Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions stl/inc/__msvc_sanitizer_annotate_container.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,15 +129,15 @@ _STL_DISABLE_CLANG_WARNINGS
#endif // ^^^ !defined(_INSERT_OPTIONAL_ANNOTATION) ^^^

#ifdef _ACTIVATE_STRING_ANNOTATION
#pragma comment(lib, "stl_asan")
extern "C" __declspec(selectany) const bool _Asan_string_should_annotate = true;
Copy link
Copy Markdown
Member

@StephanTLavavej StephanTLavavej Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of individually marking these as extern "C", I believe it would be better to move up the extern "C" { scope beginning at right line 148, so that these definitions look more similar to the declarations below. The only stuff in between are pragmas and undefs.

#pragma detect_mismatch("annotate_string", "1")
#endif // ^^^ defined(_ACTIVATE_STRING_ANNOTATION) ^^^
#ifdef _ACTIVATE_VECTOR_ANNOTATION
#pragma comment(lib, "stl_asan")
extern "C" __declspec(selectany) const bool _Asan_vector_should_annotate = true;
#pragma detect_mismatch("annotate_vector", "1")
#endif // ^^^ defined(_ACTIVATE_VECTOR_ANNOTATION) ^^^
#ifdef _ACTIVATE_OPTIONAL_ANNOTATION
#pragma comment(lib, "stl_asan")
extern "C" __declspec(selectany) const bool _Asan_optional_should_annotate = true;
#pragma detect_mismatch("annotate_optional", "1")
#endif // ^^^ defined(_ACTIVATE_OPTIONAL_ANNOTATION) ^^^

Expand Down
11 changes: 8 additions & 3 deletions stl/src/asan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@

namespace std {
extern "C" {
extern const bool _Asan_string_should_annotate = true;
extern const bool _Asan_vector_should_annotate = true;
extern const bool _Asan_optional_should_annotate = true;
// Retained for compatibility with old headers that add stl_asan.lib to the link.
Copy link
Copy Markdown
Member

@StephanTLavavej StephanTLavavej Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use the magic words "TRANSITION, ABI".

// Use __declspec(selectany) to be compatible with new version that define these
Copy link
Copy Markdown
Member

@StephanTLavavej StephanTLavavej Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"with new version that define these" isn't grammatical. Should probably be "with the new version that defines these".

// variables as __declspec(selectany) in __msvc_sanitizer_annotate_container.hpp.
// The new method is preferred because previously enabling just string would also
// enable vector and optional.
Copy link
Copy Markdown
Member

@StephanTLavavej StephanTLavavej Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding ", see GH-6186." would be helpful (we try to avoid the need to git blame for programmer-archaeology).

extern __declspec(selectany) const bool _Asan_string_should_annotate = true;
extern __declspec(selectany) const bool _Asan_vector_should_annotate = true;
extern __declspec(selectany) const bool _Asan_optional_should_annotate = true;
Comment on lines +11 to +13
Copy link
Copy Markdown
Member

@StephanTLavavej StephanTLavavej Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned that bundling these 3 definitions into a single object can still lead to scenarios where annotations are partially activated, the linker drags in asan.obj here, and (through the magic of selectany) we end up activating annotations that the user didn't request.

Conversely, I am even more concerned that asan_noop.cpp isn't being changed, and is still a monolithic source file producing a single object file.

Should we break up asan.cpp and asan_noop.cpp into 3 separate files each, prominently commented that they need to remain separate and never fused? stl_asan.lib would still be a single LIB for back-compat containing the 3 OBJs. The linker's behavior operates at an OBJ level of granularity.

} // extern "C"
} // namespace std
1 change: 1 addition & 0 deletions tests/std/test.lst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ tests\GH_005800_stable_sort_large_alignment
tests\GH_005816_numeric_limits_traps
tests\GH_005968_headers_provide_begin_end
tests\GH_005974_asan_annotate_optional
tests\GH_006186_asan_annotate_partial
tests\LWG2381_num_get_floating_point
tests\LWG2510_tag_classes
tests\LWG2597_complex_branch_cut
Expand Down
41 changes: 41 additions & 0 deletions tests/std/tests/GH_006186_asan_annotate_partial/env.lst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

# This test matrix is the usual test matrix, with all currently unsupported options removed, crossed with the ASan flags.

# TRANSITION, google/sanitizers#328: clang-cl does not support /MDd or /MTd with ASan
RUNALL_INCLUDE ..\prefix.lst
RUNALL_CROSSLIST
PM_CL="/fsanitize=address /DTEST_ENSURE_VECTOR_ENABLED /DTEST_ENSURE_STRING_ENABLED /DTEST_ENSURE_OPTIONAL_ENABLED"
PM_CL="/fsanitize=address /D_DISABLE_VECTOR_ANNOTATION /DTEST_ENSURE_STRING_ENABLED /DTEST_ENSURE_OPTIONAL_ENABLED"
PM_CL="/fsanitize=address /DTEST_ENSURE_VECTOR_ENABLED /D_DISABLE_STRING_ANNOTATION /DTEST_ENSURE_OPTIONAL_ENABLED"
PM_CL="/fsanitize=address /DTEST_ENSURE_VECTOR_ENABLED /DTEST_ENSURE_STRING_ENABLED /D_DISABLE_OPTIONAL_ANNOTATION"
PM_CL="/D_ANNOTATE_STL"
RUNALL_CROSSLIST
PM_CL="/Zi /wd4611 /w14640 /Zc:threadSafeInit-" PM_LINK="/debug"
RUNALL_CROSSLIST
PM_CL="/BE /c /EHsc /MD /std:c++14"
PM_CL="/BE /c /EHsc /MDd /std:c++17 /permissive-"
PM_CL="/BE /c /EHsc /MT /std:c++20 /permissive-"
PM_CL="/BE /c /EHsc /MTd /std:c++latest /permissive-"
PM_CL="/EHsc /MD /std:c++14"
PM_CL="/EHsc /MD /std:c++17"
PM_CL="/EHsc /MD /std:c++20"
PM_CL="/EHsc /MD /std:c++latest /permissive- /Zc:char8_t- /Zc:preprocessor"
PM_CL="/EHsc /MD /std:c++latest /permissive- /Zc:noexceptTypes-"
PM_CL="/EHsc /MDd /std:c++14 /fp:except /Zc:preprocessor"
PM_CL="/EHsc /MDd /std:c++17 /permissive-"
PM_CL="/EHsc /MDd /std:c++20 /permissive-"
PM_CL="/EHsc /MDd /std:c++latest /permissive- /Zc:wchar_t-"
PM_CL="/EHsc /MDd /std:c++latest /permissive-"
PM_CL="/EHsc /MT /std:c++latest /permissive- /analyze:only /analyze:autolog-"
PM_CL="/EHsc /MT /std:c++latest /permissive-"
PM_CL="/EHsc /MTd /std:c++latest /permissive"
PM_CL="/EHsc /MTd /std:c++latest /permissive- /analyze:only /analyze:autolog-"
PM_CL="/EHsc /MTd /std:c++latest /permissive- /fp:strict"
PM_CL="/EHsc /MTd /std:c++latest /permissive-"
# TRANSITION, we don't use /ALTERNATENAME for Clang (see GH-5224) so we cannot test /D_ANNOTATE_VECTOR without -fsanitize=address
Copy link
Copy Markdown
Member

@StephanTLavavej StephanTLavavej Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment was copy-pasted from GH_002030_asan_annotate_vector/env.lst, but as you're crossing with PM_CL="/D_ANNOTATE_STL", I think this comment is inapplicable and should be removed here.

PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /MD /std:c++14"
PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /MD /std:c++17"
PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /MT /std:c++20 /permissive-"
PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /MT /std:c++latest /permissive- /fp:strict"
37 changes: 37 additions & 0 deletions tests/std/tests/GH_006186_asan_annotate_partial/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright (c) Microsoft Corporation.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
// REQUIRES: x64 || x86 || arm64
Comment on lines +2 to +3
Copy link
Copy Markdown
Member

@StephanTLavavej StephanTLavavej Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: We conventionally put an empty line after the license banner.


#if defined(__clang__) && defined(_M_ARM64) // TRANSITION, LLVM-184902, fixed in Clang 23
#pragma comment(linker, "/INFERASANLIBS")
int main() {}
#else // ^^^ workaround / no workaround vvv

#include <cassert>
#include <vector> // include __msvc_sanitizer_annotate_container.hpp

extern "C" const bool _Asan_vector_should_annotate;
extern "C" const bool _Asan_string_should_annotate;
extern "C" const bool _Asan_optional_should_annotate;

int main() {
#ifdef TEST_ENSURE_VECTOR_ENABLED
assert(_Asan_vector_should_annotate == true);
#else
assert(_Asan_vector_should_annotate == false);
#endif

#ifdef TEST_ENSURE_STRING_ENABLED
assert(_Asan_string_should_annotate == true);
#else
assert(_Asan_string_should_annotate == false);
#endif

#ifdef TEST_ENSURE_OPTIONAL_ENABLED
assert(_Asan_optional_should_annotate == true);
#else
assert(_Asan_optional_should_annotate == false);
#endif
}

#endif // ^^^ no workaround ^^^