<optional>: Add Address Sanitizer annotations

[ozguronsoy]

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]

@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]

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.