`<optional>`: Add Address Sanitizer annotations by ozguronsoy · Pull Request #6010 · microsoft/STL

ozguronsoy · 2026-01-14T00:23:52Z

When optional is empty the internal storage is poisoned, it's unpoisoned when a value is assigned. We need to unpoison in the destructor, so the annotations must be restricted to non-trivially destructible value types.

Added _ANNOTATE_OPTIONAL, _DISABLE_OPTIONAL_ANNOTATION, etc. to __msvc_sanitizer_annotate_container.hpp.
Added poisoning storage in _Optional_destruct_base on empty construction and reset.
Added unpoisoning of storage in _Optional_construct_base::_Construct and ~_Optional_destruct_base().
Added poisoning and unpoisoning tests.

Resolves #5974

ozguronsoy · 2026-01-14T00:29:04Z

@microsoft-github-policy-service agree

davidmrdavid

Left some very minor feedback. The implementation seems sound, but from comparing the test matrix to the one in the asan vector and asan string annotations, I suspect we'll need a few more scenarios. I'll defer to @StephanTLavavej as usual.

StephanTLavavej · 2026-03-17T13:08:16Z

I reviewed the product code and pushed changes to merge main and address nitpicks. However, before getting to the test code, I realized that the product code is doomed in its current form. Consider the following conforming code, which is rejected by these changes:

D:\GitHub\STL\out\x64>type meow.cpp

#include <optional>
#include <print>
#include <string>
#include <variant>
using namespace std;

int main() {
    variant<optional<string>, int> var{"cats"};
    println("{}", get<0>(var).value());
    var = 1729;
    println("{}", get<1>(var));
}

D:\GitHub\STL\out\x64>cl /EHsc /nologo /W4 /std:c++latest /MT /Od meow.cpp && meow
meow.cpp
cats
1729

D:\GitHub\STL\out\x64>cl /EHsc /nologo /W4 /std:c++latest /MT /Od /fsanitize=address /Zi meow.cpp && meow
meow.cpp
cats
=================================================================
==21032==ERROR: AddressSanitizer: use-after-poison on address 0x006358cffcb0 at pc 0x7ff700521fe3 bp 0x006358cffa10 sp 0x006358cffa18
WRITE of size 4 at 0x006358cffcb0 thread T0
    #0 0x7ff700521fe2 in std::_Variant_storage_<1, int>::_Variant_storage_<1, int><int>(struct std::integral_constant<unsigned __int64, 0>, int &&) D:\GitHub\STL\out\x64\out\inc\variant:357
[...]

Click to expand full ASan diagnostic:

==21032==ERROR: AddressSanitizer: use-after-poison on address 0x006358cffcb0 at pc 0x7ff700521fe3 bp 0x006358cffa10 sp 0x006358cffa18
WRITE of size 4 at 0x006358cffcb0 thread T0
    #0 0x7ff700521fe2 in std::_Variant_storage_<1, int>::_Variant_storage_<1, int><int>(struct std::integral_constant<unsigned __int64, 0>, int &&) D:\GitHub\STL\out\x64\out\inc\variant:357
    #1 0x7ff7005214ea in std::_Variant_storage_<0, class std::optional<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>, int>::_Variant_storage_<0, class std::optional<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>, int><1, int, 0>(struct std::integral_constant<unsigned __int64, 1>, int &&) D:\GitHub\STL\out\x64\out\inc\variant:406
    #2 0x7ff700527fe0 in std::_Construct_in_place<class std::_Variant_storage_<0, class std::optional<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>, int>, struct std::integral_constant<unsigned __int64, 1>, int>(class std::_Variant_storage_<0, class std::optional<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>, int> &, struct std::integral_constant<unsigned __int64, 1> &&, int &&) D:\GitHub\STL\out\x64\out\inc\xutility:621
    #3 0x7ff700528b3e in std::variant<class std::optional<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>, int>::_Emplace_valueless<1, int>(int &&) D:\GitHub\STL\out\x64\out\inc\variant:1136
    #4 0x7ff70052269c in std::variant<class std::optional<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>, int>::operator=<int, 0, 0>(int &&) D:\GitHub\STL\out\x64\out\inc\variant:1013
    #5 0x7ff700521306 in main D:\GitHub\STL\out\x64\meow.cpp:10
    #6 0x7ff7005faefb in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
    #7 0x7ff7005faefb in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #8 0x7ff8d89ae8d6  (C:\Windows\System32\KERNEL32.DLL+0x18002e8d6)
    #9 0x7ff8da24c53b  (C:\Windows\SYSTEM32\ntdll.dll+0x18008c53b)

Address 0x006358cffcb0 is located in stack of thread T0 at offset 32 in frame
    #0 0x7ff70052110f in main D:\GitHub\STL\out\x64\meow.cpp:7

  This frame has 2 object(s):
    [32, 80) 'var' <== Memory access at offset 32 is inside this variable
    [48, 52) 'compiler temporary'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp, SEH and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: use-after-poison D:\GitHub\STL\out\x64\out\inc\variant:357 in std::_Variant_storage_<1, int>::_Variant_storage_<1, int><int>(struct std::integral_constant<unsigned __int64, 0>, int &&)
Shadow bytes around the buggy address:
  0x006358cffa00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x006358cffa80: 00 00 f1 f1 f1 f1 01 f3 f3 f3 f3 00 00 00 00 00
  0x006358cffb00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x006358cffb80: f1 f1 f1 f1 01 f3 f3 f3 f3 00 00 00 00 00 00 00
  0x006358cffc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x006358cffc80: 00 00 f1 f1 f1 f1[f7]f7 f7 f7 00 00 f2 f2 f2 f2
  0x006358cffd00: 04 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00
  0x006358cffd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x006358cffe00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x006358cffe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x006358cfff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb

NOTE: the stack trace above identifies the code that *accessed* the poisoned memory.
To identify the code that *poisoned* the memory, try the experimental setting ASAN_OPTIONS=poison_history_size=<size>.
==21032==ABORTING

I arrived at this realization by noticing the asymmetry between the trivial dtor and non-trivial dtor cases, which got me thinking about the poisoned/unpoisoned state of the memory after the optional object's lifetime has ended.

The problem is that if an optional object (whether non-trivial like the optional<string> in the repro above, or trivial like an optional<double> would be) leaves its memory in a poisoned state (e.g. as the non-trivial dtor does; a default-constructed or reset() trivial optional will also be poisoned), then any attempted reuse of that memory will lead to an ASan false positive failure. I used variant in my repro, but other scenarios are possible.

optional and vector are fundamentally different because vector owns remote storage that nobody else can reuse. string is vector-like except for the Small String Optimization, but notice that we specifically avoid using ASan annotations for the Small String Optimization (IIRC this happened in the very-complicated #3164):

STL/stl/inc/xstring

Lines 701 to 704 in 2626cf1

    
           // Don't annotate small strings; only annotate on the heap.
 
           if (_Capacity <= _Small_string_capacity || !_Asan_string_should_annotate) {
 
               return;
 
           }

I'm going to need to run this past our ASan experts to see if some part of these changes can be salvaged. I expect that the trivial dtor case can't be salvaged. However, it might be possible to annotate the non-trivial dtor case; for the lifetime of the optional object, we do control its memory, and I think we can poison it if we're default-constructed or reset(), as long as we (perhaps counterintuitively) unpoison it in the destructor.

Please hold off on making changes to this PR while I go check. I'll also want to add the variant scenario to the test coverage, of course.

StephanTLavavej · 2026-03-21T17:51:26Z

I'm mirroring this to the MSVC-internal repo. Please notify me if any further changes are pushed, otherwise no action is required.

StephanTLavavej · 2026-03-26T08:11:04Z

Thanks for this major safety improvement - it was surprisingly tricky to get merged on the MSVC-internal side but with the ASan team's help we did it! 😻 🎉 🐈

ozguronsoy requested a review from a team as a code owner January 14, 2026 00:23

github-project-automation Bot added this to STL Code Reviews Jan 14, 2026

github-project-automation Bot moved this to Initial Review in STL Code Reviews Jan 14, 2026

StephanTLavavej changed the title ~~Add address sanitizer annotations to optional~~ <optional>: Add Address Sanitizer annotations Jan 14, 2026

StephanTLavavej added enhancement Something can be improved ASan Address Sanitizer labels Jan 14, 2026

This comment was marked as resolved.

Sign in to view

frederick-vs-ja reviewed Jan 15, 2026

View reviewed changes

ozguronsoy added 6 commits January 28, 2026 21:06

add asan annotation to optional

3be5fb2

fix poison tests

37bcf48

use __asan_address_is_poisoned in tests

c9f424c

fix unused var error in tests

b4fd22b

add _is_constant_evaluated and should_annotate checks before annotations

93879fe

add constexpr test

1be4772

ozguronsoy force-pushed the optional-asan branch from 297ab67 to 1be4772 Compare January 28, 2026 18:21

This comment was marked as resolved.

Sign in to view

davidmrdavid reviewed Feb 25, 2026

View reviewed changes

Comment thread tests/std/tests/GH_005974_optional_asan/env.lst

Comment thread tests/std/tests/GH_005974_optional_asan/env.lst

address review suggestions

eaa64c4

AlexGuteniev reviewed Feb 26, 2026

View reviewed changes

Comment thread tests/std/tests/GH_005974_asan_annotate_optional/test.cpp

StephanTLavavej moved this from Initial Review to Work In Progress in STL Code Reviews Feb 26, 2026

disable arm tests

51b6466

StephanTLavavej moved this from Work In Progress to Initial Review in STL Code Reviews Feb 26, 2026

StephanTLavavej added 2 commits March 16, 2026 14:46

Merge branch 'main' into optional-asan

80189e0

Enable ARM64 test coverage.

c0c5a78

This comment was marked as resolved.

Sign in to view

StephanTLavavej self-assigned this Mar 16, 2026

void const volatile* => const volatile void*

ad97b4b

Consistently poison before assigning _Has_value = false;.

39a9624

StephanTLavavej reviewed Mar 17, 2026

View reviewed changes

Comment thread stl/src/asan_noop.cpp Outdated

Comment thread stl/inc/optional Outdated

Comment thread tests/std/tests/GH_005974_asan_annotate_optional/test.cpp Outdated

StephanTLavavej moved this from Initial Review to Work In Progress in STL Code Reviews Mar 17, 2026

StephanTLavavej added 3 commits March 17, 2026 08:06

Fix product code, add lots of comments.

b36dc3d

Re-derive the test matrix from usual_17_matrix.lst.

4b4347c

Overhaul the test.

4a5db0e

StephanTLavavej reviewed Mar 17, 2026

View reviewed changes

Comment thread tests/std/tests/GH_005974_asan_annotate_optional/env.lst Outdated

Comment thread tests/std/tests/GH_005974_asan_annotate_optional/test.cpp Outdated

StephanTLavavej approved these changes Mar 17, 2026

View reviewed changes

StephanTLavavej removed their assignment Mar 17, 2026

StephanTLavavej moved this from Work In Progress to Ready To Merge in STL Code Reviews Mar 17, 2026

AraHaan approved these changes Mar 18, 2026

View reviewed changes

StephanTLavavej moved this from Ready To Merge to Merging in STL Code Reviews Mar 21, 2026

StephanTLavavej added a commit to StephanTLavavej/STL that referenced this pull request Mar 21, 2026

microsoftGH-6010

df21344

StephanTLavavej added a commit to StephanTLavavej/STL that referenced this pull request Mar 21, 2026

microsoftGH-6010

f7bee2e

StephanTLavavej merged commit 5e47670 into microsoft:main Mar 26, 2026
49 checks passed

github-project-automation Bot moved this from Merging to Done in STL Code Reviews Mar 26, 2026

ozguronsoy deleted the optional-asan branch March 26, 2026 20:18

Conversation

ozguronsoy commented Jan 14, 2026 • edited by StephanTLavavej Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ozguronsoy commented Jan 14, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

davidmrdavid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

StephanTLavavej commented Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

StephanTLavavej commented Mar 21, 2026

Uh oh!

Uh oh!

StephanTLavavej commented Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ozguronsoy commented Jan 14, 2026 •

edited by StephanTLavavej

Loading