Skip to content

Commit ccf3afb

Browse files
Tacetcopybara-github
Tacet
authored andcommitted
[ASan][libc++] Annotating std::basic_string with all allocators (#75845)
This commit turns on ASan annotations in `std::basic_string` for all allocators by default. Originally suggested here: https://reviews.llvm.org/D146214 String annotations added here: llvm/llvm-project#72677 This commit is part of our efforts to support container annotations with (almost) every allocator. Annotating `std::basic_string` with default allocator is implemented in llvm/llvm-project#72677. Additionally it removes `__begin != nullptr` because `data()` should never return a nullptr. Support in ASan API exists since llvm/llvm-project@1c5ad6d. This patch removes the check in std::basic_string annotation member function (__annotate_contiguous_container) to support different allocators. You can turn off annotations for a specific allocator based on changes from llvm/llvm-project@2fa1bec. The motivation for a research and those changes was a bug, found by Trail of Bits, in a real code where an out-of-bounds read could happen as two strings were compared via a call to `std::equal` that took `iter1_begin`, `iter1_end`, `iter2_begin` iterators (with a custom comparison function). When object `iter1` was longer than `iter2`, read out-of-bounds on `iter2` could happen. Container sanitization would detect it. If you have any questions, please email: - [email protected] - [email protected] NOKEYCHECK=True GitOrigin-RevId: 60ac394dc9ed617f802b33c3b9ac8881ca6a940c
1 parent 8c1e8ec commit ccf3afb

File tree

4 files changed

+160
-3
lines changed

4 files changed

+160
-3
lines changed

include/string

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1891,8 +1891,7 @@ private:
18911891
#if !defined(_LIBCPP_HAS_NO_ASAN) && defined(_LIBCPP_INSTRUMENTED_WITH_ASAN)
18921892
const void* __begin = data();
18931893
const void* __end = data() + capacity() + 1;
1894-
if (!__libcpp_is_constant_evaluated() && __begin != nullptr &&
1895-
is_same<allocator_type, __default_allocator_type>::value)
1894+
if (__asan_annotate_container_with_allocator<allocator_type>::value && !__libcpp_is_constant_evaluated())
18961895
__sanitizer_annotate_contiguous_container(__begin, __end, __old_mid, __new_mid);
18971896
#endif
18981897
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
// REQUIRES: asan
10+
// UNSUPPORTED: c++03
11+
12+
// Basic test if ASan annotations work for basic_string.
13+
14+
#include <string>
15+
#include <cassert>
16+
#include <cstdlib>
17+
18+
#include "asan_testing.h"
19+
#include "min_allocator.h"
20+
#include "test_iterators.h"
21+
#include "test_macros.h"
22+
23+
extern "C" void __sanitizer_set_death_callback(void (*callback)(void));
24+
25+
void do_exit() { exit(0); }
26+
27+
int main(int, char**) {
28+
{
29+
typedef cpp17_input_iterator<char*> MyInputIter;
30+
// Should not trigger ASan.
31+
std::basic_string<char, std::char_traits<char>, safe_allocator<char>> v;
32+
char i[] = {'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'a', 'b', 'c', 'd', 'e',
33+
'f', 'g', 'h', 'i', 'j', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'};
34+
35+
v.insert(v.begin(), MyInputIter(i), MyInputIter(i + 29));
36+
assert(v[0] == 'a');
37+
assert(is_string_asan_correct(v));
38+
}
39+
40+
__sanitizer_set_death_callback(do_exit);
41+
{
42+
using T = char;
43+
using C = std::basic_string<T, std::char_traits<T>, safe_allocator<T>>;
44+
const T t[] = {'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'a', 'b', 'c', 'd', 'e',
45+
'f', 'g', 'h', 'i', 'j', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'};
46+
C c(std::begin(t), std::end(t));
47+
assert(is_string_asan_correct(c));
48+
assert(__sanitizer_verify_contiguous_container(c.data(), c.data() + c.size() + 1, c.data() + c.capacity() + 1) !=
49+
0);
50+
T foo = c[c.size() + 1]; // should trigger ASAN and call do_exit().
51+
assert(false); // if we got here, ASAN didn't trigger
52+
((void)foo);
53+
54+
return 0;
55+
}
56+
}
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
// REQUIRES: asan
10+
// UNSUPPORTED: c++03
11+
12+
// Test based on: https://bugs.chromium.org/p/chromium/issues/detail?id=1419798#c5
13+
// Some allocators during deallocation may not call destructors and just reuse memory.
14+
// In those situations, one may want to deactivate annotations for a specific allocator.
15+
// It's possible with __asan_annotate_container_with_allocator template class.
16+
// This test confirms that those allocators work after turning off annotations.
17+
//
18+
// A context to this test is a situations when memory is repurposed and destructors are not called.
19+
// Related issue: https://github.com/llvm/llvm-project/issues/60384
20+
//
21+
// That issue appeared in the past and was addressed here: https://reviews.llvm.org/D145628
22+
//
23+
// There was also a discussion, if it's UB.
24+
// Related discussion: https://reviews.llvm.org/D136765#4155262
25+
// Related notes: https://eel.is/c++draft/basic.life#6
26+
// Probably it's no longer UB due a change in CWG2523.
27+
// https://cplusplus.github.io/CWG/issues/2523.html
28+
//
29+
// Therefore we make sure that it works that way, also because people rely on this behavior.
30+
// Annotations are turned off only, if a user explicitly turns off annotations for a specific allocator.
31+
32+
#include <assert.h>
33+
#include <stdlib.h>
34+
#include <string>
35+
#include <new>
36+
37+
// Allocator with pre-allocated (with malloc in constructor) buffers.
38+
// Memory may be freed without calling destructors.
39+
struct reuse_allocator {
40+
static size_t const N = 100;
41+
reuse_allocator() {
42+
for (size_t i = 0; i < N; ++i)
43+
__buffers[i] = malloc(8 * 1024);
44+
}
45+
~reuse_allocator() {
46+
for (size_t i = 0; i < N; ++i)
47+
free(__buffers[i]);
48+
}
49+
void* alloc() {
50+
assert(__next_id < N);
51+
return __buffers[__next_id++];
52+
}
53+
void reset() { __next_id = 0; }
54+
void* __buffers[N];
55+
size_t __next_id = 0;
56+
} reuse_buffers;
57+
58+
template <typename T>
59+
struct user_allocator {
60+
using value_type = T;
61+
user_allocator() = default;
62+
template <class U>
63+
user_allocator(user_allocator<U>) {}
64+
friend bool operator==(user_allocator, user_allocator) { return true; }
65+
friend bool operator!=(user_allocator x, user_allocator y) { return !(x == y); }
66+
67+
T* allocate(size_t n) {
68+
if (n * sizeof(T) > 8 * 1024)
69+
throw std::bad_array_new_length();
70+
return (T*)reuse_buffers.alloc();
71+
}
72+
void deallocate(T*, size_t) noexcept {}
73+
};
74+
75+
// Turn off annotations for user_allocator:
76+
template <class T>
77+
struct std::__asan_annotate_container_with_allocator<user_allocator<T>> {
78+
static bool const value = false;
79+
};
80+
81+
int main(int, char**) {
82+
using S = std::basic_string<char, std::char_traits<char>, user_allocator<char>>;
83+
84+
{
85+
// Create a string with a buffer from reuse allocator object:
86+
S* s = new (reuse_buffers.alloc()) S();
87+
// Use string, so it's poisoned, if container annotations for that allocator are not turned off:
88+
for (int i = 0; i < 40; i++)
89+
s->push_back('a');
90+
}
91+
// Reset the state of the allocator, don't call destructors, allow memory to be reused:
92+
reuse_buffers.reset();
93+
{
94+
// Create a next string with the same allocator, so the same buffer due to the reset:
95+
S s;
96+
// Use memory inside the string again, if it's poisoned, an error will be raised:
97+
for (int i = 0; i < 60; i++)
98+
s.push_back('a');
99+
}
100+
101+
return 0;
102+
}

test/support/asan_testing.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ TEST_CONSTEXPR bool is_string_asan_correct(const std::basic_string<ChrT, TraitsT
7575
return true;
7676

7777
if (!is_string_short(c) || _LIBCPP_SHORT_STRING_ANNOTATIONS_ALLOWED) {
78-
if (std::is_same<Alloc, std::allocator<ChrT>>::value)
78+
if (std::__asan_annotate_container_with_allocator<Alloc>::value)
7979
return __sanitizer_verify_contiguous_container(c.data(), c.data() + c.size() + 1, c.data() + c.capacity() + 1) !=
8080
0;
8181
else

0 commit comments

Comments
 (0)