<optional>: Add Address Sanitizer annotations#6010
<optional>: Add Address Sanitizer annotations#6010ozguronsoy wants to merge 6 commits intomicrosoft:mainfrom
<optional>: Add Address Sanitizer annotations#6010Conversation
|
@microsoft-github-policy-service agree |
<optional>: Add Address Sanitizer annotations
|
Thank you! I need to focus on landing |
| #endif | ||
|
|
||
| #ifdef _INSERT_OPTIONAL_ANNOTATION | ||
| extern const bool _Asan_optional_should_annotate; |
There was a problem hiding this comment.
This variable isn't used. Is it really necessary?
There was a problem hiding this comment.
Added a check for this variable inside the if(!_STD _Is_constant_evaluated()) block.
| #endif | ||
| } // extern "C" | ||
|
|
||
| #if defined(_INSERT_VECTOR_ANNOTATION) || defined(_INSERT_STRING_ANNOTATION) |
There was a problem hiding this comment.
I think there should be a concentrated controlling macro for these.
There was a problem hiding this comment.
I don't think it's necessary since this check is only done here. I also added defined(_INSERT_OPTIONAL_ANNOTATION) to the check.
stl/inc/optional
Outdated
| constexpr _Optional_destruct_base() noexcept : _Dummy{}, _Has_value{false} {} // initialize an empty optional | ||
| constexpr _Optional_destruct_base() noexcept : _Dummy{}, _Has_value{false} { | ||
| #ifdef _INSERT_OPTIONAL_ANNOTATION | ||
| __asan_poison_memory_region(_STD addressof(_Value), sizeof(_Value)); |
There was a problem hiding this comment.
This invocations probably break constant evaluation. We should wrap them in if (!_STD _Is_constant_evaluated()) { }, IMO.
Also, why not consistently write sizeof(_Ty)?
There was a problem hiding this comment.
Done.
Also, why not consistently write sizeof(_Ty)?
That's how I usually write it. Changed it to sizeof(_Ty) to match the existing style.
297ab67 to
1be4772
Compare
When
optionalis empty the internal storage is poisoned, it's unpoisoned when a value is assigned._ANNOTATE_OPTIONAL,_DISABLE_OPTIONAL_ANNOTATION, etc. to__msvc_sanitizer_annotate_container.hpp._Optional_destruct_baseon empty construction andreset._Optional_construct_base::_Construct.Resolves #5974