Skip to content
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.

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

Add the ability to opt-out of ASan container annotations on a per-allocator basis #5241

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
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
4 changes: 4 additions & 0 deletions stl/inc/vector
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,10 @@ private:
return;
}

if constexpr (!_Is_ASan_enabled_for_allocator<allocator_type>) {
return;
}

const void* const _First = _STD _Unfancy(_First_);
const void* const _End = _STD _Unfancy(_End_);
const void* const _Old_last = _STD _Unfancy(_Old_last_);
Expand Down
4 changes: 4 additions & 0 deletions stl/inc/xmemory
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,10 @@ struct _Simple_types { // wraps types from allocators with simple addressing for
_INLINE_VAR constexpr size_t _Asan_granularity = 8;
_INLINE_VAR constexpr size_t _Asan_granularity_mask = _Asan_granularity - 1;

// Represents whether ASan container annotations are enabled a given allocator.
template <class>
constexpr bool _Is_ASan_enabled_for_allocator = true;
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

I may have missed some conversation about it, but I'm wondering if the name should be changed. The feature is disabling annotations based on the container's underlying allocator.

Since the options for compiler are _DISABLE_X_ANNOTATION should we try to follow suite with Disable in the name?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since the options for compiler are _DISABLE_X_ANNOTATION should we try to follow suite with Disable in the name?

Oh I didn't realize the compiler options / flags followed that naming convention? Do you have a link or example where I can see this?

But yeah, don't feel strongly about naming. If it matches the flags, I'm happy to have Disabled over Enabled in the name. I'll tackle that on my next push.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'm basically commenting on staying in line with these

Copy link
Member Author

Choose a reason for hiding this comment

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

Incorporated: 57e7aff


struct _Asan_aligned_pointers {
const void* _First;
const void* _End;
Expand Down
4 changes: 4 additions & 0 deletions stl/inc/xstring
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,10 @@ private:
return;
}

if constexpr (!_Is_ASan_enabled_for_allocator<allocator_type>) {
return;
}

// Note that `_Capacity`, `_Old_size`, and `_New_size` do not include the null terminator
const void* const _End = _First + _Capacity + 1;
const void* const _Old_last = _First + _Old_size + 1;
Expand Down
35 changes: 35 additions & 0 deletions tests/std/tests/GH_002030_asan_annotate_string/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,26 @@ STATIC_ASSERT(_Container_allocation_minimum_asan_alignment<
basic_string<wchar_t, char_traits<wchar_t>, implicit_allocator<wchar_t>>>
== 2);

// Simple implicit allocator that opts out of ASan annotations (via `_Is_ASan_enabled_for_allocator`)
template <class T, class Pocma = true_type, class Stateless = true_type>
struct implicit_allocator_no_asan_annotations : implicit_allocator<T, Pocma, Stateless> {
implicit_allocator_no_asan_annotations() = default;
template <class U>
constexpr implicit_allocator_no_asan_annotations(const implicit_allocator_no_asan_annotations<U, Pocma, Stateless>&) noexcept {}

T* allocate(size_t n) {
T* mem = new T[n + 1];
return mem + 1;
}

void deallocate(T* p, size_t) noexcept {
delete[] (p - 1);
}
};

template <typename T>
constexpr bool _Is_ASan_enabled_for_allocator<implicit_allocator_no_asan_annotations<T>> = true;

template <class Alloc>
void test_construction() {
using CharType = typename Alloc::value_type;
Expand Down Expand Up @@ -1853,6 +1873,20 @@ void run_tests() {
#endif // ^^^ no workaround ^^^
}

// Tests that ASan analysis can be disabled for a vector with an arena allocator.
template <class CharType>
void run_asan_disablement_test() {

// We'll give the string capacity 100
std::basic_string<CharType, std::char_traits<CharType>, implicit_allocator_no_asan_annotations<CharType>> myString;
myString.reserve(100);

CharType* data = &myString[0]; // Get a mutable pointer to the string's data
data[50] = CharType{'A'}; // TODO: this isn't failing due to bug: https://github.com/microsoft/STL/issues/5251

// TODO: is it possible to add a 'negative' test case here? One where ASan expectedly fails?
}

template <class CharType, template <class, class, class> class Alloc>
void run_custom_allocator_matrix() {
run_tests<Alloc<CharType, true_type, true_type>>();
Expand All @@ -1867,6 +1901,7 @@ void run_allocator_matrix() {
run_custom_allocator_matrix<CharType, aligned_allocator>();
run_custom_allocator_matrix<CharType, explicit_allocator>();
run_custom_allocator_matrix<CharType, implicit_allocator>();
run_asan_disablement_test<CharType>();
}

void test_DevCom_10116361() {
Expand Down
38 changes: 38 additions & 0 deletions tests/std/tests/GH_002030_asan_annotate_vector/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,26 @@ struct implicit_allocator : custom_test_allocator<T, Pocma, Stateless> {
STATIC_ASSERT(_Container_allocation_minimum_asan_alignment<vector<char, implicit_allocator<char>>> == 1);
STATIC_ASSERT(_Container_allocation_minimum_asan_alignment<vector<wchar_t, implicit_allocator<wchar_t>>> == 2);

// Simple implicit allocator that opts out of ASan annotations (via `_Is_ASan_enabled_for_allocator`)
template <class T, class Pocma = true_type, class Stateless = true_type>
struct implicit_allocator_no_asan_annotations : implicit_allocator<T, Pocma, Stateless> {
implicit_allocator_no_asan_annotations() = default;
template <class U>
constexpr implicit_allocator_no_asan_annotations(const implicit_allocator_no_asan_annotations<U, Pocma, Stateless>&) noexcept {}

T* allocate(size_t n) {
T* mem = new T[n + 1];
return mem + 1;
}

void deallocate(T* p, size_t) noexcept {
delete[] (p - 1);
}
};

template <typename T>
constexpr bool _Is_ASan_enabled_for_allocator<implicit_allocator_no_asan_annotations<T>> = false;

template <class Alloc>
void test_push_pop() {
using T = typename Alloc::value_type;
Expand Down Expand Up @@ -1002,12 +1022,30 @@ void run_custom_allocator_matrix() {
run_tests<AllocT<T, false_type, false_type>>();
}

// Tests that ASan analysis can be disabled for a vector with an arena allocator.
template <class T>
void run_asan_disablement_test() {

// We'll give the vector capacity 100
std::vector<T, implicit_allocator_no_asan_annotations<T>> vec;
vec.reserve(100);

// We access position (50) of the vector, which is within the capacity of the vector
// but uninitialized. This would normally trigger an AV because of ASan annotations.
// However, the allocator has opted out of ASan analysis through,
// `_Is_ASan_enabled_for_allocator`, so this should succeed.
vec.data()[50] = T();

// TODO: is it possible to add a 'negative' test case here? One where ASan expectedly fails?
davidmrdavid marked this conversation as resolved.
Show resolved Hide resolved
}

template <class T>
void run_allocator_matrix() {
run_tests<allocator<T>>();
run_custom_allocator_matrix<T, aligned_allocator>();
run_custom_allocator_matrix<T, explicit_allocator>();
run_custom_allocator_matrix<T, implicit_allocator>();
run_asan_disablement_test<T>();
}

int main() {
Expand Down