Fix undefined behavior in arr_free;

Currently there is a single allocator function used in arr_t.  Its
behavior depends on the values for the pointer and size arguments:

- If pointer is NULL, it should allocate new memory.
- If pointer is non-NULL and size is positive, it should resize memory.
- If size is zero, it should free memory.

All instances of arr_t use realloc for this right now.  The problem
is that realloc's behavior is undefined when the size argument is zero.
On Windows and Linux, realloc will free the pointer, but on macOS this
isn't the case.  This means that arr_t leaks memory on macOS.

It's best to not rely on undefined behavior like this, so let's instead
use a helper function that behaves the way we want.
This commit is contained in:
bjorn 2022-03-14 12:27:58 -07:00
parent 1e91eea9b0
commit c9fe026a66
12 changed files with 40 additions and 27 deletions

View File

@ -1581,7 +1581,7 @@ static int l_lovrGraphicsNewComputeShader(lua_State* L) {
static int l_lovrGraphicsNewShaderBlock(lua_State* L) {
arr_uniform_t uniforms;
arr_init(&uniforms, realloc);
arr_init(&uniforms, arr_alloc);
BlockType type = luax_checkenum(L, 1, BlockType, NULL);

View File

@ -59,6 +59,17 @@ void lovrRelease(void* object, void (*destructor)(void*)) {
}
}
// Dynamic Array
// Default malloc-based allocator for arr_t (like realloc except well-defined when size is 0)
void* arr_alloc(void* data, size_t size) {
if (size > 0) {
return realloc(data, size);
} else {
free(data);
return NULL;
}
}
// UTF-8
// https://github.com/starwing/luautf8
size_t utf8_decode(const char *s, const char *e, unsigned *pch) {

View File

@ -72,6 +72,8 @@ typedef void* arr_allocator(void* data, size_t size);
#define arr_splice(a, i, n) memmove((a)->data + (i), (a)->data + ((i) + n), ((a)->length - (i) - (n)) * sizeof(*(a)->data)), (a)->length -= n
#define arr_clear(a) (a)->length = 0
void* arr_alloc(void* data, size_t size);
static inline void _arr_reserve(void** data, size_t n, size_t* capacity, size_t stride, arr_allocator* allocator) {
if (*capacity >= n) return;
if (*capacity == 0) *capacity = 1;

View File

@ -110,16 +110,16 @@ ModelData* lovrModelDataInitObj(ModelData* model, Blob* source, ModelDataIO* io)
arr_t(float) normals;
arr_t(float) uvs;
arr_init(&groups, realloc);
arr_init(&images, realloc);
arr_init(&materials, realloc);
arr_init(&groups, arr_alloc);
arr_init(&images, arr_alloc);
arr_init(&materials, arr_alloc);
map_init(&materialMap, 0);
arr_init(&vertexBlob, realloc);
arr_init(&indexBlob, realloc);
arr_init(&vertexBlob, arr_alloc);
arr_init(&indexBlob, arr_alloc);
map_init(&vertexMap, 0);
arr_init(&positions, realloc);
arr_init(&normals, realloc);
arr_init(&uvs, realloc);
arr_init(&positions, arr_alloc);
arr_init(&normals, arr_alloc);
arr_init(&uvs, arr_alloc);
arr_push(&groups, ((objGroup) { .material = -1 }));

View File

@ -21,7 +21,7 @@ void lovrVariantDestroy(Variant* variant) {
bool lovrEventInit() {
if (state.initialized) return false;
arr_init(&state.events, realloc);
arr_init(&state.events, arr_alloc);
return state.initialized = true;
}

View File

@ -111,7 +111,7 @@ bool lovrFilesystemInit(const char* archive) {
if (state.initialized) return false;
state.initialized = true;
arr_init(&state.archives, realloc);
arr_init(&state.archives, arr_alloc);
arr_reserve(&state.archives, 2);
lovrFilesystemSetRequirePath("?.lua;?/init.lua");
@ -168,7 +168,7 @@ bool lovrFilesystemMount(const char* path, const char* mountpoint, bool append,
}
Archive archive;
arr_init(&archive.strings, realloc);
arr_init(&archive.strings, arr_alloc);
if (!dir_init(&archive, path, mountpoint, root) && !zip_init(&archive, path, mountpoint, root)) {
arr_free(&archive.strings);
@ -593,7 +593,7 @@ static void zip_close(Archive* archive) {
static bool zip_init(Archive* archive, const char* filename, const char* mountpoint, const char* root) {
char path[LOVR_PATH_MAX];
memset(&archive->lookup, 0, sizeof(archive->lookup));
arr_init(&archive->nodes, realloc);
arr_init(&archive->nodes, arr_alloc);
// mmap the zip file, try to parse it, and figure out how many files there are
archive->zip.data = fs_map(filename, &archive->zip.size);

View File

@ -73,7 +73,7 @@ Font* lovrFontCreate(Rasterizer* rasterizer, uint32_t padding, double spread, Fi
font->atlas.width = 256;
font->atlas.height = 256;
font->atlas.padding = atlasPadding;
arr_init(&font->atlas.glyphs, realloc);
arr_init(&font->atlas.glyphs, arr_alloc);
map_init(&font->atlas.glyphMap, 0);
// Set initial atlas size

View File

@ -1350,7 +1350,7 @@ void lovrGpuInit(void (*getProcAddress(const char*))(void), bool debug) {
#endif
for (int i = 0; i < MAX_BARRIERS; i++) {
arr_init(&state.incoherents[i], realloc);
arr_init(&state.incoherents[i], arr_alloc);
}
Image* image = lovrImageCreate(1, 1, NULL, 0xff, FORMAT_RGBA);
@ -2405,7 +2405,7 @@ static void lovrShaderSetupUniforms(Shader* shader) {
lovrAssert((size_t) blockCount <= MAX_BLOCK_BUFFERS, "Shader has too many uniform blocks (%d) the max is %d", blockCount, MAX_BLOCK_BUFFERS);
map_init(&shader->blockMap, blockCount);
arr_block_t* uniformBlocks = &shader->blocks[BLOCK_UNIFORM];
arr_init(uniformBlocks, realloc);
arr_init(uniformBlocks, arr_alloc);
arr_reserve(uniformBlocks, (size_t) blockCount);
for (int i = 0; i < blockCount; i++) {
UniformBlock block = { .slot = i, .source = NULL };
@ -2417,12 +2417,12 @@ static void lovrShaderSetupUniforms(Shader* shader) {
int blockId = (i << 1) + BLOCK_UNIFORM;
map_set(&shader->blockMap, hash64(name, length), blockId);
arr_push(uniformBlocks, block);
arr_init(&uniformBlocks->data[uniformBlocks->length - 1].uniforms, realloc);
arr_init(&uniformBlocks->data[uniformBlocks->length - 1].uniforms, arr_alloc);
}
// Shader storage buffers and their buffer variables
arr_block_t* computeBlocks = &shader->blocks[BLOCK_COMPUTE];
arr_init(computeBlocks, realloc);
arr_init(computeBlocks, arr_alloc);
#ifndef LOVR_WEBGL
if ((GLAD_GL_ARB_shader_storage_buffer_object && GLAD_GL_ARB_program_interface_query) || GLAD_GL_ES_VERSION_3_1) {
@ -2438,7 +2438,7 @@ static void lovrShaderSetupUniforms(Shader* shader) {
#else
glShaderStorageBlockBinding(program, i, block.slot);
#endif
arr_init(&block.uniforms, realloc);
arr_init(&block.uniforms, arr_alloc);
GLsizei length;
char name[LOVR_MAX_UNIFORM_LENGTH];
@ -2480,7 +2480,7 @@ static void lovrShaderSetupUniforms(Shader* shader) {
int imageSlot = 0;
glGetProgramiv(program, GL_ACTIVE_UNIFORMS, &uniformCount);
map_init(&shader->uniformMap, 0);
arr_init(&shader->uniforms, realloc);
arr_init(&shader->uniforms, arr_alloc);
for (uint32_t i = 0; i < (uint32_t) uniformCount; i++) {
Uniform uniform;
GLenum glType;
@ -2904,7 +2904,7 @@ ShaderBlock* lovrShaderBlockCreate(BlockType type, Buffer* buffer, arr_uniform_t
lovrAssert(block, "Out of memory");
block->ref = 1;
arr_init(&block->uniforms, realloc);
arr_init(&block->uniforms, arr_alloc);
map_init(&block->uniformMap, (uint32_t) uniforms->length);
arr_append(&block->uniforms, uniforms->data, uniforms->length);

View File

@ -85,7 +85,7 @@ static ovrInputState *refreshButtons(void) {
static bool oculus_init(float supersample, float offset, uint32_t msaa, bool overlay) {
arr_init(&state.textures, realloc);
arr_init(&state.textures, arr_alloc);
ovrResult result = ovr_Initialize(NULL);
if (OVR_FAILURE(result)) {

View File

@ -53,7 +53,7 @@ Curve* lovrCurveCreate(void) {
Curve* curve = calloc(1, sizeof(Curve));
lovrAssert(curve, "Out of memory");
curve->ref = 1;
arr_init(&curve->points, realloc);
arr_init(&curve->points, arr_alloc);
arr_reserve(&curve->points, 16);
return curve;
}

View File

@ -82,7 +82,7 @@ World* lovrWorldCreate(float xg, float yg, float zg, bool allowSleep, const char
world->space = dHashSpaceCreate(0);
dHashSpaceSetLevels(world->space, -4, 8);
world->contactGroup = dJointGroupCreate(0);
arr_init(&world->overlaps, realloc);
arr_init(&world->overlaps, arr_alloc);
lovrWorldSetGravity(world, xg, yg, zg);
lovrWorldSetSleepingAllowed(world, allowSleep);
for (uint32_t i = 0; i < tagCount; i++) {
@ -322,8 +322,8 @@ Collider* lovrColliderCreate(World* world, float x, float y, float z) {
collider->restitution = 0;
collider->tag = NO_TAG;
dBodySetData(collider->body, collider);
arr_init(&collider->shapes, realloc);
arr_init(&collider->joints, realloc);
arr_init(&collider->shapes, arr_alloc);
arr_init(&collider->joints, arr_alloc);
lovrColliderSetPosition(collider, x, y, z);

View File

@ -21,7 +21,7 @@ Channel* lovrChannelCreate(uint64_t hash) {
Channel* channel = calloc(1, sizeof(Channel));
lovrAssert(channel, "Out of memory");
channel->ref = 1;
arr_init(&channel->messages, realloc);
arr_init(&channel->messages, arr_alloc);
mtx_init(&channel->lock, mtx_plain | mtx_timed);
cnd_init(&channel->cond);
channel->hash = hash;