From b05c27a27f42a547001781eb2847507dd1c7e5b0 Mon Sep 17 00:00:00 2001 From: est31 Date: Fri, 19 Feb 2016 07:13:16 +0100 Subject: [PATCH] Fix allocation bug if compiled with modern clang or gcc * Add overflow checked intrinsic abstractions that check on overflow. * Use them for memory allocation code. * Use size_t type for memory allocation code to support full platform dependent width. Fixes #3756. --- core/typedefs.h | 30 +++++++++++++++++++ core/vector.h | 35 ++++++++++++++++------ drivers/unix/memory_pool_static_malloc.cpp | 27 +++++++++++++---- 3 files changed, 78 insertions(+), 14 deletions(-) diff --git a/core/typedefs.h b/core/typedefs.h index 700346c3bd6..48acca326e2 100644 --- a/core/typedefs.h +++ b/core/typedefs.h @@ -154,6 +154,23 @@ inline void __swap_tmpl(T &x, T &y ) { ((m_hex>='A' && m_hex<='F')?(10+m_hex-'A'):\ ((m_hex>='a' && m_hex<='f')?(10+m_hex-'a'):0))) +// Macro to check whether we are compiled by clang +// and we have a specific builtin +#if defined(__llvm__) && defined(__has_builtin) + #define _llvm_has_builtin(x) __has_builtin(x) +#else + #define _llvm_has_builtin(x) 0 +#endif + +#if (defined(__GNUC__) && (__GNUC__ >= 5)) || _llvm_has_builtin(__builtin_mul_overflow) +# define _mul_overflow __builtin_mul_overflow +#endif + +#if (defined(__GNUC__) && (__GNUC__ >= 5)) || _llvm_has_builtin(__builtin_add_overflow) +# define _add_overflow __builtin_add_overflow +#endif + + @@ -167,6 +184,19 @@ static _FORCE_INLINE_ unsigned int nearest_power_of_2(unsigned int x) { x |= x >> 4; x |= x >> 8; x |= x >> 16; + + return ++x; +} + +template +static _FORCE_INLINE_ T nearest_power_of_2_templated(T x) { + + --x; + // If the compiler is smart, it unrolls this loop + // If its dumb, this is a bit slow. + for (size_t i = 0; i < sizeof(T); i++) + x |= x >> (1 << i); + return ++x; } diff --git a/core/vector.h b/core/vector.h index 641721f401d..398d7f1bd59 100644 --- a/core/vector.h +++ b/core/vector.h @@ -68,11 +68,26 @@ class Vector { return reinterpret_cast(_ptr); } - - _FORCE_INLINE_ int _get_alloc_size(int p_elements) const { - - return nearest_power_of_2(p_elements*sizeof(T)+sizeof(SafeRefCount)+sizeof(int)); - } + + _FORCE_INLINE_ size_t _get_alloc_size(size_t p_elements) const { + return nearest_power_of_2_templated(p_elements*sizeof(T)+sizeof(SafeRefCount)+sizeof(int)); + } + + _FORCE_INLINE_ bool _get_alloc_size_checked(size_t p_elements, size_t *out) const { +#if defined(_add_overflow) && defined(_mul_overflow) + size_t o; + size_t p; + if (_mul_overflow(p_elements, sizeof(T), &o)) return false; + if (_add_overflow(o, sizeof(SafeRefCount)+sizeof(int), &p)) return false; + *out = nearest_power_of_2_templated(p); + return true; +#else + // Speed is more important than correctness here, do the operations unchecked + // and hope the best + *out = _get_alloc_size(p_elements); + return true; +#endif + } void _unref(void *p_data); @@ -257,19 +272,21 @@ Error Vector::resize(int p_size) { // possibly changing size, copy on write _copy_on_write(); + size_t alloc_size; + ERR_FAIL_COND_V(!_get_alloc_size_checked(p_size, &alloc_size), ERR_OUT_OF_MEMORY); + if (p_size>size()) { if (size()==0) { // alloc from scratch - void* ptr=memalloc(_get_alloc_size(p_size)); + void* ptr=memalloc(alloc_size); ERR_FAIL_COND_V( !ptr ,ERR_OUT_OF_MEMORY); _ptr=(T*)((uint8_t*)ptr+sizeof(int)+sizeof(SafeRefCount)); _get_refcount()->init(); // init refcount *_get_size()=0; // init size (currently, none) } else { - - void *_ptrnew = (T*)memrealloc((uint8_t*)_ptr-sizeof(int)-sizeof(SafeRefCount),_get_alloc_size(p_size)); + void *_ptrnew = (T*)memrealloc((uint8_t*)_ptr-sizeof(int)-sizeof(SafeRefCount), alloc_size); ERR_FAIL_COND_V( !_ptrnew ,ERR_OUT_OF_MEMORY); _ptr=(T*)((uint8_t*)_ptrnew+sizeof(int)+sizeof(SafeRefCount)); } @@ -293,7 +310,7 @@ Error Vector::resize(int p_size) { t->~T(); } - void *_ptrnew = (T*)memrealloc((uint8_t*)_ptr-sizeof(int)-sizeof(SafeRefCount),_get_alloc_size(p_size)); + void *_ptrnew = (T*)memrealloc((uint8_t*)_ptr-sizeof(int)-sizeof(SafeRefCount), alloc_size); ERR_FAIL_COND_V( !_ptrnew ,ERR_OUT_OF_MEMORY); _ptr=(T*)((uint8_t*)_ptrnew+sizeof(int)+sizeof(SafeRefCount)); diff --git a/drivers/unix/memory_pool_static_malloc.cpp b/drivers/unix/memory_pool_static_malloc.cpp index e75b682c198..f89b55de129 100644 --- a/drivers/unix/memory_pool_static_malloc.cpp +++ b/drivers/unix/memory_pool_static_malloc.cpp @@ -48,7 +48,12 @@ void* MemoryPoolStaticMalloc::alloc(size_t p_bytes,const char *p_description) { #else - int total = p_bytes + DEFAULT_ALIGNMENT; + size_t total; + #if defined(_add_overflow) + if (_add_overflow(p_bytes, DEFAULT_ALIGNMENT, &total)) return NULL; + #else + total = p_bytes + DEFAULT_ALIGNMENT; + #endif uint8_t* ptr = (uint8_t*)_alloc(total, p_description); ERR_FAIL_COND_V( !ptr, ptr ); int ofs = (DEFAULT_ALIGNMENT - ((uintptr_t)ptr & (DEFAULT_ALIGNMENT - 1))); @@ -64,11 +69,18 @@ void* MemoryPoolStaticMalloc::_alloc(size_t p_bytes,const char *p_description) { MutexLock lock(mutex); #ifdef DEBUG_MEMORY_ENABLED - void *mem=malloc(p_bytes+sizeof(RingPtr)); /// add for size and ringlist + + size_t total; + #if defined(_add_overflow) + if (_add_overflow(p_bytes, sizeof(RingPtr), &total)) return NULL; + #else + total = p_bytes + sizeof(RingPtr); + #endif + void *mem=malloc(total); /// add for size and ringlist if (!mem) { - printf("**ERROR: out of memory while allocating %i bytes by %s?\n",(int) p_bytes, p_description); - printf("**ERROR: memory usage is %i\n", (int)get_total_usage()); + printf("**ERROR: out of memory while allocating %lu bytes by %s?\n", (unsigned long) p_bytes, p_description); + printf("**ERROR: memory usage is %lu\n", (unsigned long) get_total_usage()); }; ERR_FAIL_COND_V(!mem,0); //out of memory, or unreasonable request @@ -129,7 +141,12 @@ void* MemoryPoolStaticMalloc::realloc(void *p_memory,size_t p_bytes) { if (!p_memory) return alloc(p_bytes); - int total = p_bytes + DEFAULT_ALIGNMENT; + size_t total; + #if defined(_add_overflow) + if (_add_overflow(p_bytes, DEFAULT_ALIGNMENT, &total)) return NULL; + #else + total = p_bytes + DEFAULT_ALIGNMENT; + #endif uint8_t* mem = (uint8_t*)p_memory; int ofs = *(mem-1); mem = mem - ofs;