Skip to content

Commit 31e0dac

Browse files
committed
Disable allocation pooling when AddressSanitizer is enabled
Allocation pooling obscures whether or not memory is freed or not from AddressSanitizer, making it less useful. With this patch, the game works same as it ever was, but AddressSanitizer now properly finds some errors that previously would only trigger spurious really cryptic UndefinedBehaviorSanitizer errors. An alternative would be to, instead of doing this, use ASan's API to poison/unpoison memory. However, the functions to do that require an allocation size to be provided, and the memory pooling functions don't get those.
1 parent f5fdc24 commit 31e0dac

File tree

2 files changed

+30
-0
lines changed

2 files changed

+30
-0
lines changed

Source/System/Entity.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,11 @@ namespace RTE {
237237
}
238238

239239
void Entity::ClassInfo::FillPool(int fillAmount) {
240+
#ifdef __SANITIZE_ADDRESS__
241+
// If we have ASan, make this a no-op.
242+
(void)(fillAmount); // Silence warning about unused variable.
243+
#else
244+
240245
// Default to the set block allocation size if fillAmount is 0
241246
if (fillAmount <= 0) {
242247
fillAmount = m_PoolAllocBlockCount;
@@ -248,6 +253,7 @@ namespace RTE {
248253
m_AllocatedPool.push_back(m_Allocate());
249254
}
250255
}
256+
#endif
251257
}
252258

253259
bool Entity::ClassInfo::IsClassOrChildClassOf(const ClassInfo* classInfoToCheck) const {
@@ -260,6 +266,13 @@ namespace RTE {
260266
}
261267

262268
void* Entity::ClassInfo::GetPoolMemory() {
269+
#ifdef __SANITIZE_ADDRESS__
270+
// If compiled with ASan, sidestep pooling and just use the allocator normally.
271+
272+
void* foundMemory = m_Allocate();
273+
RTEAssert(foundMemory, "m_Allocate failed! to make memory!");
274+
#else
275+
263276
std::lock_guard<std::mutex> guard(m_Mutex);
264277

265278
RTEAssert(IsConcrete(), "Trying to get pool memory of an abstract Entity class!");
@@ -274,6 +287,7 @@ namespace RTE {
274287
m_AllocatedPool.pop_back();
275288

276289
RTEAssert(foundMemory, "Could not find an available instance in the pool, even after increasing its size!");
290+
#endif
277291

278292
// Keep track of the number of instances passed out
279293
m_InstancesInUse++;
@@ -285,8 +299,14 @@ namespace RTE {
285299
if (!returnedMemory) {
286300
return 0;
287301
}
302+
303+
#ifdef __SANITIZE_ADDRESS__
304+
// If compiled with ASan, sidestep pooling and just use the allocator normally.
305+
m_Deallocate(returnedMemory);
306+
#else
288307
std::lock_guard<std::mutex> guard(m_Mutex);
289308
m_AllocatedPool.push_back(returnedMemory);
309+
#endif
290310

291311
// Keep track of the number of instances passed in
292312
m_InstancesInUse--;

Source/System/Entity.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@
77
#include <list>
88
#include <unordered_set>
99

10+
// Concoction based on:
11+
// https://stackoverflow.com/questions/34813412/how-to-detect-if-building-with-address-sanitizer-when-building-with-gcc-4-8#78444624
12+
#if defined(__has_feature) // MSVC doesn't have this
13+
# if __has_feature(address_sanitizer) // for Clang
14+
# define __SANITIZE_ADDRESS__ // GCC and MSVC already set this
15+
# endif
16+
#endif
17+
1018
namespace RTE {
1119

1220
typedef std::function<void*()> MemoryAllocate; //!< Convenient name definition for the memory allocation callback function.
@@ -173,7 +181,9 @@ namespace RTE {
173181
int m_PoolAllocBlockCount; //!< The number of instances to fill up the pool of this type with each time it runs dry.
174182
int m_InstancesInUse; //!< The number of allocated instances passed out from the pool.
175183

184+
#ifndef __ADDRESS_SANITIZER__ // Unused when ASan is enabled.
176185
std::mutex m_Mutex; //!< Mutex to ensure multiple things aren't grabbing/deallocating memory at the same time
186+
#endif
177187

178188
// Forbidding copying
179189
ClassInfo(const ClassInfo& reference) = delete;

0 commit comments

Comments
 (0)