Bug: `arena_expand()` is unsafe

[Rax-x]

The arena_expand() function is unsafe to use because, when realloc() is invoked, there is no guarantee that arena->region will continue to point to the same memory area. This behavior is implementation-dependent; for instance, when referencing the stdlib.h realloc implementation. If realloc() cannot expand the existing area in place, it typically allocates a new memory area of the specified size, copies the data to the new location, and then frees the old one (the last one could be implementation-dependent). If this occurs, any old pointers that were pointing to the original memory area will now be dangling. While this may not immediately lead to a segmentation fault, because they're pointing to memory addresses associated to the process, but these old addresses could be marked as free and subsequently chosen by future malloc() calls, leading to potential bugs or data corruption.

I've modified the test code for the arena_expand() in the following way:

...

void test_arena_expand(void)
{
    Arena *arena = arena_create(6);
    char *ptr = arena_alloc(arena, 6);

    memcpy(ptr, "Hello\0", 6);

    void* ptr1 = malloc(1024); // +++
    memset(ptr1, 'H', 1024); // +++

    TEST_EQUAL(arena->region, ptr); // +++

    arena = arena_expand(arena, 1024);
 
    char* ptr2 = malloc(6); // +++
    memset(ptr2, 0, 6); // +++

    TEST_EQUAL(arena->region, ptr); // +++

    TEST_EQUAL(arena->size, 1024);
    TEST_EQUAL(arena->index, 6); 
    TEST_ARRAY_EQUAL(arena->region, "Hello\0", 6);
    TEST_ARRAY_EQUAL(ptr, "Hello\0", 6); // +++

    TEST_NULL(arena_expand(NULL, 0));
    TEST_NULL(arena_expand(arena, 0));
    TEST_NULL(arena_expand(NULL, 13));
    TEST_NULL(arena_expand(arena, 12));

    arena_destroy(arena);

    free(ptr1);
    free(ptr2);
}
...

Without running it with valgrind the output is the following:

Passed 5/5 tests in 'test_arena_create'
Passed 9/10 tests in 'test_arena_expand'
  FAILURE: 'arena->region does not equal ptr' at test.c:72
  FAILURE: 'ptr does not equal "Hello\0"' at test.c:77
Passed 7/7 tests in 'test_arena_alloc'
Passed 7/7 tests in 'test_arena_alloc_aligned'
Passed 8/8 tests in 'test_arena_copy'
Passed 1/1 tests in 'test_arena_clear'
Passed 5/5 tests in 'test_arena_get_allocation_struct'
Passed 9/9 tests in 'test_arena_add_allocation'
Passed 2/2 tests in 'test_arena_delete_allocation_list'

Finished. Passed 53/54 tests.

Using valgrind the output is the following:

Passed 9/10 tests in 'test_arena_expand'
  FAILURE: 'arena->region does not equal ptr' at test.c:72
Passed 7/7 tests in 'test_arena_alloc'
Passed 7/7 tests in 'test_arena_alloc_aligned'
Passed 8/8 tests in 'test_arena_copy'
Passed 1/1 tests in 'test_arena_clear'
Passed 5/5 tests in 'test_arena_get_allocation_struct'
Passed 9/9 tests in 'test_arena_add_allocation'
Passed 2/2 tests in 'test_arena_delete_allocation_list'

Finished. Passed 53/54 tests.

As you can see, the problems I've highlighted are shown in the test results. To solve this bug you can: deprecate arena_extend() and discourage its use, use a list of chunks of memory, or consider some OS-specific system calls like mremap().

GCC version used to compile the code: gcc (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0

References:

• https://www.yodaiken.com/wp-content/uploads/2021/05/ansi-iso-9899-1990-1.pdf
• https://pubs.opengroup.org/onlinepubs/009696899/functions/realloc.html
• https://en.cppreference.com/w/c/memory/realloc
• https://sourceware.org/glibc/wiki/MallocInternals

[ccgargantua]

Thanks for highlighting this. Your issue has been reviewed and I appreciate your thorough analysis.

I have evolved quite a bit as a programmer since I made this project, and as of a few weeks ago a re-write is in progress. Keep an eye on the project, I'll update you when a change occurs that either directly or indirectly addresses this problem :)