Skip to content

Fix incorrect memory allocation in release builds. #1053

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
akien-mga merged 1 commit into from
Feb 21, 2023
Merged

Fix incorrect memory allocation in release builds. #1053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
14 changes: 9 additions & 5 deletions include/godot_cpp/core/memory.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@

#include <type_traits>

#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
Expand All @@ -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 *) {}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -169,7 +173,7 @@ void memdelete_arr(T *p_class) {
}
}

Memory::free_static(ptr);
Memory::free_static(ptr, true);
}

struct _GlobalNil {
Expand Down
10 changes: 5 additions & 5 deletions include/godot_cpp/templates/cowdata.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ void CowData<T>::_unref(void *p_data) {
}

// free mem
Memory::free_static((uint8_t *)p_data);
Memory::free_static((uint8_t *)p_data, true);
}

template <class T>
Expand All @@ -233,7 +233,7 @@ uint32_t CowData<T>::_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<uint32_t>(1); // refcount
*(mem_new - 1) = current_size; // size
Expand Down Expand Up @@ -286,15 +286,15 @@ Error CowData<T>::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<uint32_t>(1); // refcount

_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<uint32_t>(rc); // refcount

Expand Down Expand Up @@ -324,7 +324,7 @@ Error CowData<T>::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<uint32_t>(rc); // refcount

Expand Down
59 changes: 53 additions & 6 deletions src/core/memory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down