You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The STL "annotates" the vector and basic_string containers so that ASan will report container-overflow whenever allocated but un-initialized data is accessed.
A simple repro is the container overflow error fired in this sample program (assuming it's /fsanitize=address'ed):
// Compile with: cl /EHsc /fsanitize=address /Zi
#include<vector>intmain() {
std::vector<int> v(10);
v.reserve(20); // we've allocated 20 entries, but only initialized only 10// Accessing the 10th entry (0-indexed, naturally) triggers an AV
v[10] = 1;
}
This is sensible behavior in most cases.
One case where it does not bode well is when an arena allocator is used as the custom allocator of the container. Arena allocators often tamper with the entire allocated memory at once (e.g. they commonly deallocate their entire 'arena' at once) which would trigger ASan AVs when the capacity of the container exceeds it's size.
We encountered one such bug in the msvc front end.
This PR:
This PR introduces the ability for custom allocators to opt-out of vector and basic_string's ASan annotations. This is controlled by the newly introduced template variable: _Disable_ASan_container_annotations_for_allocator<...some allocator type...>.
Testing:
For the new annotation opt-out feature: a simple test case was added for basic_string and vector respectively
davidmrdavid
changed the title
[WIP] Add naive first implementation of per-allocator disablement of ASan
Add per-allocator disablement of ASan
Jan 23, 2025
davidmrdavid
changed the title
Add per-allocator disablement of ASan
Add the ability to opt-out of ASan container annotations on a per-allocator basis
Jan 30, 2025
Is the plan to hold on merge while testing the FE?
My current plan is to work on getting the FE to test this by Friday one way or another: either by manually porting this internally on a private branch, or by consuming this 'as normal' by merging this PR and mirror'ing it on ADO's default branch.
So I think it depends on how quickly we can STL's final approval here. One way or another, I'll be working towards enabling this to be validated by FE starting tomorrow :) . Put another way: I don't think it's strictly necessary to block the PR merge on it - the feature and tests seems up to spec to me. But I'll defer to more experienced owners on that decision.
Thanks for the feedback, I've taken notes on the common feedback items (don't shadow, don't add std::, remember the null terminator is initialized, etc.). Hoping to retain that for next time around.
New diff seems sound to me, thanks. Good to merge afaict.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context:
The STL "annotates" the
vector
andbasic_string
containers so that ASan will reportcontainer-overflow
whenever allocated but un-initialized data is accessed.A simple repro is the
container overflow
error fired in this sample program (assuming it's/fsanitize=address
'ed):This is sensible behavior in most cases.
One case where it does not bode well is when an arena allocator is used as the custom allocator of the container. Arena allocators often tamper with the entire allocated memory at once (e.g. they commonly deallocate their entire 'arena' at once) which would trigger ASan AVs when the capacity of the container exceeds it's size.
We encountered one such bug in the
msvc
front end.This PR:
This PR introduces the ability for custom allocators to opt-out of
vector
andbasic_string
's ASan annotations. This is controlled by the newly introduced template variable:_Disable_ASan_container_annotations_for_allocator<...some allocator type...>
.Testing:
basic_string
andvector
respectivelybasic_string
, this replaces the recently addedtest_gh_5251
test (from ASan should detect writing to abasic_string
's reserved but uninitialized memory #5252) to avoid repetition.