From ba4b50118d26642a4e3603a5687b0cc1ca093396 Mon Sep 17 00:00:00 2001 From: bruvzg <7645683+bruvzg@users.noreply.github.com> Date: Tue, 21 Feb 2023 12:47:40 +0200 Subject: [PATCH] Fix incorrect memory allocation in release builds. Co-authored-by: lightyears --- .github/workflows/ci.yml | 10 ++--- include/godot_cpp/core/memory.hpp | 14 +++--- include/godot_cpp/templates/cowdata.hpp | 10 ++--- src/core/memory.cpp | 59 ++++++++++++++++++++++--- 4 files changed, 72 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4215a9c0..062cefa7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,14 +18,14 @@ jobs: matrix: include: - name: 🐧 Linux (GCC) - os: ubuntu-18.04 + os: ubuntu-20.04 platform: linux artifact-name: godot-cpp-linux-glibc2.27-x86_64-release artifact-path: bin/libgodot-cpp.linux.template_release.x86_64.a cache-name: linux-x86_64 - name: 🐧 Linux (GCC, Double Precision) - os: ubuntu-18.04 + os: ubuntu-20.04 platform: linux artifact-name: godot-cpp-linux-glibc2.27-x86_64-double-release artifact-path: bin/libgodot-cpp.linux.template_release.double.x86_64.a @@ -56,7 +56,7 @@ jobs: cache-name: macos-universal - name: 🤖 Android (arm64) - os: ubuntu-18.04 + os: ubuntu-20.04 platform: android artifact-name: godot-cpp-android-arm64-release artifact-path: bin/libgodot-cpp.android.template_release.arm64.a @@ -133,7 +133,7 @@ jobs: linux-cmake: name: 🐧 Build (Linux, GCC, CMake) - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 steps: - name: Checkout uses: actions/checkout@v3 @@ -157,7 +157,7 @@ jobs: linux-cmake-ninja: name: 🐧 Build (Linux, GCC, CMake Ninja) - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 steps: - name: Checkout uses: actions/checkout@v3 diff --git a/include/godot_cpp/core/memory.hpp b/include/godot_cpp/core/memory.hpp index 25f87ec5..55cdb8b4 100644 --- a/include/godot_cpp/core/memory.hpp +++ b/include/godot_cpp/core/memory.hpp @@ -40,6 +40,10 @@ #include +#ifndef PAD_ALIGN +#define PAD_ALIGN 16 //must always be greater than this at much +#endif + void *operator new(size_t p_size, const char *p_description); ///< operator new that takes a description and uses MemoryStaticPool void *operator new(size_t p_size, void *(*p_allocfunc)(size_t p_size)); ///< operator new that takes a description and uses MemoryStaticPool void *operator new(size_t p_size, void *p_pointer, size_t check, const char *p_description); ///< operator new that takes a description and uses a pointer to the preallocated memory @@ -64,9 +68,9 @@ class Memory { Memory(); public: - static void *alloc_static(size_t p_bytes); - static void *realloc_static(void *p_memory, size_t p_bytes); - static void free_static(void *p_ptr); + static void *alloc_static(size_t p_bytes, bool p_pad_align = false); + static void *realloc_static(void *p_memory, size_t p_bytes, bool p_pad_align = false); + static void free_static(void *p_ptr, bool p_pad_align = false); }; _ALWAYS_INLINE_ void postinitialize_handler(void *) {} @@ -140,7 +144,7 @@ T *memnew_arr_template(size_t p_elements, const char *p_descr = "") { same strategy used by std::vector, and the Vector class, so it should be safe.*/ size_t len = sizeof(T) * p_elements; - uint64_t *mem = (uint64_t *)Memory::alloc_static(len); + uint64_t *mem = (uint64_t *)Memory::alloc_static(len, true); T *failptr = nullptr; // Get rid of a warning. ERR_FAIL_COND_V(!mem, failptr); *(mem - 1) = p_elements; @@ -169,7 +173,7 @@ void memdelete_arr(T *p_class) { } } - Memory::free_static(ptr); + Memory::free_static(ptr, true); } struct _GlobalNil { diff --git a/include/godot_cpp/templates/cowdata.hpp b/include/godot_cpp/templates/cowdata.hpp index 38f92777..1753687e 100644 --- a/include/godot_cpp/templates/cowdata.hpp +++ b/include/godot_cpp/templates/cowdata.hpp @@ -217,7 +217,7 @@ void CowData::_unref(void *p_data) { } // free mem - Memory::free_static((uint8_t *)p_data); + Memory::free_static((uint8_t *)p_data, true); } template @@ -233,7 +233,7 @@ uint32_t CowData::_copy_on_write() { /* in use by more than me */ uint32_t current_size = *_get_size(); - uint32_t *mem_new = (uint32_t *)Memory::alloc_static(_get_alloc_size(current_size)); + uint32_t *mem_new = (uint32_t *)Memory::alloc_static(_get_alloc_size(current_size), true); new (mem_new - 2) SafeNumeric(1); // refcount *(mem_new - 1) = current_size; // size @@ -286,7 +286,7 @@ Error CowData::resize(int p_size) { if (alloc_size != current_alloc_size) { if (current_size == 0) { // alloc from scratch - uint32_t *ptr = (uint32_t *)Memory::alloc_static(alloc_size); + uint32_t *ptr = (uint32_t *)Memory::alloc_static(alloc_size, true); ERR_FAIL_COND_V(!ptr, ERR_OUT_OF_MEMORY); *(ptr - 1) = 0; // size, currently none new (ptr - 2) SafeNumeric(1); // refcount @@ -294,7 +294,7 @@ Error CowData::resize(int p_size) { _ptr = (T *)ptr; } else { - uint32_t *_ptrnew = (uint32_t *)Memory::realloc_static(_ptr, alloc_size); + uint32_t *_ptrnew = (uint32_t *)Memory::realloc_static(_ptr, alloc_size, true); ERR_FAIL_COND_V(!_ptrnew, ERR_OUT_OF_MEMORY); new (_ptrnew - 2) SafeNumeric(rc); // refcount @@ -324,7 +324,7 @@ Error CowData::resize(int p_size) { } if (alloc_size != current_alloc_size) { - uint32_t *_ptrnew = (uint32_t *)Memory::realloc_static(_ptr, alloc_size); + uint32_t *_ptrnew = (uint32_t *)Memory::realloc_static(_ptr, alloc_size, true); ERR_FAIL_COND_V(!_ptrnew, ERR_OUT_OF_MEMORY); new (_ptrnew - 2) SafeNumeric(rc); // refcount diff --git a/src/core/memory.cpp b/src/core/memory.cpp index d1d7b596..8feda578 100644 --- a/src/core/memory.cpp +++ b/src/core/memory.cpp @@ -34,16 +34,63 @@ namespace godot { -void *Memory::alloc_static(size_t p_bytes) { - return internal::gde_interface->mem_alloc(p_bytes); +void *Memory::alloc_static(size_t p_bytes, bool p_pad_align) { +#ifdef DEBUG_ENABLED + bool prepad = false; // Alredy pre paded in the engine. +#else + bool prepad = p_pad_align; +#endif + + void *mem = internal::gde_interface->mem_alloc(p_bytes + (prepad ? PAD_ALIGN : 0)); + ERR_FAIL_COND_V(!mem, nullptr); + + if (prepad) { + uint8_t *s8 = (uint8_t *)mem; + return s8 + PAD_ALIGN; + } else { + return mem; + } } -void *Memory::realloc_static(void *p_memory, size_t p_bytes) { - return internal::gde_interface->mem_realloc(p_memory, p_bytes); +void *Memory::realloc_static(void *p_memory, size_t p_bytes, bool p_pad_align) { + if (p_memory == nullptr) { + return alloc_static(p_bytes, p_pad_align); + } else if (p_bytes == 0) { + free_static(p_memory, p_pad_align); + return nullptr; + } + + uint8_t *mem = (uint8_t *)p_memory; + +#ifdef DEBUG_ENABLED + bool prepad = false; // Alredy pre paded in the engine. +#else + bool prepad = p_pad_align; +#endif + + if (prepad) { + mem -= PAD_ALIGN; + mem = (uint8_t *)internal::gde_interface->mem_realloc(mem, p_bytes + PAD_ALIGN); + ERR_FAIL_COND_V(!mem, nullptr); + return mem + PAD_ALIGN; + } else { + return (uint8_t *)internal::gde_interface->mem_realloc(mem, p_bytes); + } } -void Memory::free_static(void *p_ptr) { - internal::gde_interface->mem_free(p_ptr); +void Memory::free_static(void *p_ptr, bool p_pad_align) { + uint8_t *mem = (uint8_t *)p_ptr; + +#ifdef DEBUG_ENABLED + bool prepad = false; // Alredy pre paded in the engine. +#else + bool prepad = p_pad_align; +#endif + + if (prepad) { + mem -= PAD_ALIGN; + } + internal::gde_interface->mem_free(mem); } _GlobalNil::_GlobalNil() {