Skip to content

Commit 11a049c

Browse files
derek-gerstmannDerek Gerstmann
derek-gerstmann
and
Derek Gerstmann
authored
#6863 - Fixes to make address sanitizer happy for internal runtime classes (#6880)
* Fixes to make address sanitizer happy. Fixed initialization defects in StringStorage that could cause buffer overruns Fixed memory leaks within RegionAllocator and BlockAllocator Added system memory allocation tracking to all internal runtime tests. * Clang Tidy / Format pass * Fix formatting to use braces around if statements Co-authored-by: Derek Gerstmann <[email protected]>
1 parent 4770495 commit 11a049c

File tree

12 files changed

+251
-110
lines changed

12 files changed

+251
-110
lines changed

‎src/runtime/internal/block_allocator.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ void BlockAllocator::destroy(void *user_context) {
231231
destroy_block_entry(user_context, block_entry);
232232
block_entry = prev_entry;
233233
}
234+
block_list.destroy(user_context);
234235
}
235236

236237
MemoryRegion *BlockAllocator::reserve_memory_region(void *user_context, RegionAllocator *allocator, const MemoryRequest &request) {
@@ -331,7 +332,6 @@ void BlockAllocator::destroy_region_allocator(void *user_context, RegionAllocato
331332
<< "region_allocator=" << (void *)(region_allocator) << ")...\n";
332333
#endif
333334
if (region_allocator == nullptr) { return; }
334-
region_allocator->destroy(user_context);
335335
RegionAllocator::destroy(user_context, region_allocator);
336336
}
337337

‎src/runtime/internal/memory_arena.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,12 @@ void MemoryArena::initialize(void *user_context,
128128
}
129129

130130
void MemoryArena::destroy(void *user_context) {
131-
for (size_t i = blocks.size(); i--;) {
132-
Block *block = lookup_block(user_context, i);
133-
halide_abort_if_false(user_context, block != nullptr);
134-
destroy_block(user_context, block);
131+
if (!blocks.empty()) {
132+
for (size_t i = blocks.size(); i--;) {
133+
Block *block = lookup_block(user_context, i);
134+
halide_abort_if_false(user_context, block != nullptr);
135+
destroy_block(user_context, block);
136+
}
135137
}
136138
blocks.destroy(user_context);
137139
}

‎src/runtime/internal/memory_resources.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ enum class AllocationStatus {
1313
InvalidStatus,
1414
InUse,
1515
Available,
16+
Purgeable,
1617
Dedicated
1718
};
1819

‎src/runtime/internal/region_allocator.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ void RegionAllocator::release_block_region(void *user_context, BlockRegion *bloc
336336
<< "user_context=" << (void *)(user_context) << " "
337337
<< "block_region=" << (void *)(block_region) << ") ...\n";
338338
#endif
339-
free_block_region(user_context, block_region);
339+
block_region->status = AllocationStatus::Available;
340340
}
341341

342342
void RegionAllocator::destroy_block_region(void *user_context, BlockRegion *block_region) {
@@ -375,6 +375,7 @@ void RegionAllocator::free_block_region(void *user_context, BlockRegion *block_r
375375
MemoryRegion *memory_region = &(block_region->memory);
376376
allocators.region.deallocate(user_context, memory_region);
377377
block->reserved -= block_region->memory.size;
378+
block_region->memory.size = 0;
378379
}
379380
block_region->status = AllocationStatus::Available;
380381
}
@@ -422,9 +423,11 @@ void RegionAllocator::destroy(void *user_context) {
422423
destroy_block_region(user_context, prev_region);
423424
}
424425
}
425-
block->regions = nullptr;
426426
block->reserved = 0;
427-
arena->destroy(user_context);
427+
block->regions = nullptr;
428+
block->allocator = nullptr;
429+
MemoryArena::destroy(user_context, arena);
430+
arena = nullptr;
428431
}
429432

430433
bool RegionAllocator::is_compatible_block_region(const BlockRegion *block_region, const MemoryProperties &properties) const {

‎src/runtime/internal/string_storage.h‎

Lines changed: 79 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,28 @@ struct StringUtils {
3131
return count;
3232
}
3333

34-
static size_t count_length(const char *str) {
34+
// retuns true if s1 contains s2 (within n characters)
35+
static bool contains(const char *s1, const char *s2, size_t n) {
36+
if (is_empty(s2)) { return true; } // s2 is empty ... return true to match strstr
37+
char starts_with = *s2;
38+
for (size_t length = strlen(s2); length <= n; n--, s1++) {
39+
if (*s1 == starts_with) {
40+
for (size_t i = 1; i <= length; i++) {
41+
if (i == length) {
42+
return true;
43+
}
44+
if (s1[i] != s2[i]) {
45+
break;
46+
}
47+
}
48+
}
49+
}
50+
return false;
51+
}
52+
53+
static size_t count_length(const char *str, size_t max_chars) {
3554
const char *ptr = str;
36-
while (!StringUtils::is_empty(ptr)) {
55+
while (!StringUtils::is_empty(ptr) && ((size_t(ptr - str)) < max_chars)) {
3756
++ptr;
3857
}
3958
return size_t(ptr - str);
@@ -50,6 +69,10 @@ class StringStorage {
5069
StringStorage(const StringStorage &other) = default;
5170
~StringStorage();
5271

72+
// Factory methods for creation / destruction
73+
static StringStorage *create(void *user_context, const SystemMemoryAllocatorFns &ma);
74+
static void destroy(void *user_context, StringStorage *string_storage);
75+
5376
void initialize(void *user_context, uint32_t capacity = 0, const SystemMemoryAllocatorFns &sma = default_allocator());
5477
void destroy(void *user_context);
5578

@@ -89,6 +112,28 @@ StringStorage::~StringStorage() {
89112
destroy(nullptr);
90113
}
91114

115+
StringStorage *StringStorage::create(void *user_context, const SystemMemoryAllocatorFns &system_allocator) {
116+
halide_abort_if_false(user_context, system_allocator.allocate != nullptr);
117+
StringStorage *result = reinterpret_cast<StringStorage *>(
118+
system_allocator.allocate(user_context, sizeof(StringStorage)));
119+
120+
if (result == nullptr) {
121+
halide_error(user_context, "StringStorage: Failed to create instance! Out of memory!\n");
122+
return nullptr;
123+
}
124+
125+
result->initialize(user_context, 32, system_allocator);
126+
return result;
127+
}
128+
129+
void StringStorage::destroy(void *user_context, StringStorage *instance) {
130+
halide_abort_if_false(user_context, instance != nullptr);
131+
const SystemMemoryAllocatorFns &system_allocator = instance->current_allocator();
132+
instance->destroy(user_context);
133+
halide_abort_if_false(user_context, system_allocator.deallocate != nullptr);
134+
system_allocator.deallocate(user_context, instance);
135+
}
136+
92137
StringStorage &StringStorage::operator=(const StringStorage &other) {
93138
if (&other != this) {
94139
assign(nullptr, other.data(), other.length());
@@ -97,14 +142,17 @@ StringStorage &StringStorage::operator=(const StringStorage &other) {
97142
}
98143

99144
bool StringStorage::contains(const char *str) const {
145+
if (contents.empty()) { return false; }
100146
const char *this_str = static_cast<const char *>(contents.data());
101-
return strstr(this_str, str) != nullptr;
147+
return StringUtils::contains(this_str, str, contents.size());
102148
}
103149

104150
bool StringStorage::contains(const StringStorage &other) const {
151+
if (contents.empty()) { return false; }
152+
if (other.contents.empty()) { return false; }
105153
const char *this_str = static_cast<const char *>(contents.data());
106154
const char *other_str = static_cast<const char *>(other.contents.data());
107-
return strstr(this_str, other_str) != nullptr;
155+
return StringUtils::contains(this_str, other_str, contents.size());
108156
}
109157

110158
bool StringStorage::operator==(const StringStorage &other) const {
@@ -125,72 +173,80 @@ void StringStorage::reserve(void *user_context, size_t length) {
125173
}
126174

127175
void StringStorage::assign(void *user_context, char ch) {
128-
contents.resize(user_context, 1);
176+
reserve(user_context, 1);
129177
char *ptr = static_cast<char *>(contents[0]);
130178
(*ptr) = ch;
179+
terminate(user_context, 1);
131180
}
132181

133182
void StringStorage::assign(void *user_context, const char *str, size_t length) {
134183
if (StringUtils::is_empty(str)) { return; }
135184
if (length == 0) { length = strlen(str); }
136-
char *this_str = static_cast<char *>(contents.data());
137185
reserve(user_context, length);
138-
memcpy(this_str, str, length);
186+
contents.replace(user_context, 0, str, length);
139187
terminate(user_context, length);
140188
}
141189

142190
void StringStorage::append(void *user_context, const char *str, size_t length) {
143191
if (StringUtils::is_empty(str)) { return; }
144192
if (length == 0) { length = strlen(str); }
145-
const size_t old_size = contents.size();
146-
size_t new_length = old_size + length;
147-
char *this_str = static_cast<char *>(contents[old_size]);
148-
reserve(user_context, length);
149-
memcpy(this_str, str, length);
193+
const size_t old_length = StringUtils::count_length(data(), contents.size());
194+
size_t new_length = old_length + length;
195+
reserve(user_context, new_length);
196+
contents.insert(user_context, old_length, str, length);
150197
terminate(user_context, new_length);
151198
}
152199

153200
void StringStorage::append(void *user_context, char ch) {
154-
contents.append(user_context, &ch);
201+
const size_t old_length = StringUtils::count_length(data(), contents.size());
202+
size_t new_length = old_length + 1;
203+
reserve(user_context, new_length);
204+
contents.insert(user_context, old_length, &ch, 1);
205+
terminate(user_context, new_length);
155206
}
156207

157208
void StringStorage::prepend(void *user_context, const char *str, size_t length) {
158209
if (StringUtils::is_empty(str)) { return; }
159210
if (length == 0) { length = strlen(str); }
160-
const size_t old_size = contents.size();
161-
size_t new_length = old_size + length;
162-
char *this_str = static_cast<char *>(contents.data());
211+
const size_t old_length = StringUtils::count_length(data(), contents.size());
212+
size_t new_length = old_length + length;
163213
reserve(user_context, new_length);
164-
memcpy(this_str + length, this_str, old_size);
165-
memcpy(this_str, str, length);
214+
contents.insert(user_context, 0, str, length);
166215
terminate(user_context, new_length);
167216
}
168217

169218
void StringStorage::prepend(void *user_context, char ch) {
219+
const size_t old_length = StringUtils::count_length(data(), contents.size());
220+
size_t new_length = old_length + 1;
221+
reserve(user_context, new_length);
170222
contents.prepend(user_context, &ch);
223+
terminate(user_context, new_length);
171224
}
172225

173226
void StringStorage::terminate(void *user_context, size_t length) {
174-
char *end_ptr = static_cast<char *>(contents[length]);
175-
(*end_ptr) = '\0';
227+
if (contents.data() && (length < contents.size())) {
228+
char *end_ptr = static_cast<char *>(contents[length]);
229+
(*end_ptr) = '\0';
230+
}
176231
}
177232

178233
void StringStorage::clear(void *user_context) {
179234
contents.clear(user_context);
180-
if (contents.data()) { terminate(user_context, 0); }
235+
terminate(user_context, 0);
181236
}
182237

183238
void StringStorage::initialize(void *user_context, uint32_t capacity, const SystemMemoryAllocatorFns &sma) {
184239
contents.initialize(user_context, {sizeof(char), 32, 32}, sma);
185-
if (capacity) { contents.reserve(user_context, capacity); }
240+
reserve(user_context, capacity);
241+
terminate(user_context, 0);
186242
}
187243

188244
void StringStorage::destroy(void *user_context) {
189245
contents.destroy(user_context);
190246
}
191247

192248
size_t StringStorage::length() const {
193-
return StringUtils::count_length(data());
249+
return StringUtils::count_length(data(), contents.size());
194250
}
195251

196252
const char *StringStorage::data() const {

0 commit comments

Comments
 (0)