Stack allocator and C11's alignment features

Question

I am trying to write a custom stack allocator for a game I am working on. To deal with memory alignment, I am using C11's stdalign.h header; however, since I do not have prior experience with alignment issues, I'd like a expert opinion on whether I am doing it right.

The majority of the alignment stuff happens in alignup and aligndown macros. Specifically, in alignup, I try to go up to the nearest multiple of alignof(max_align_t) (unless we were already a multiple) by shooting past it by adding alignof(max_align_t) - 1 and then coming back using the bitwise AND. In aligndown, I first go below the last multiple of alignof(max_align_t) and then come up using alignup.

NOTE: The stack is in a memory arena and it grows down in memory instead of going up.

#define alignup(p) ((void*)((((char*)(p)) + (alignof(max_align_t) - 1)) & (~ (alignof(max_align_t) - 1))))

#define aligndown(p) ((void*)(alignup((((char*)(p)) - (alignof(max_align_t) - 1)))))

struct Mem_Stack_Header {
    size_t allocation_size;
    size_t total_size;
};

// This contains the "head" of the stack. The stack grows down, instead of up.
extern void *stack_head;


void* memPush (size_t s)
{
    void *head = stack_head;

    void *mem = (char*)head - s;
    mem = aligndown(mem);

    struct Mem_Stack_Header *h = (struct Mem_Stack_Header*)((char*)mem - sizeof(*h));
    h =  aligndown(h);
    h->allocation_size = s;
    h->total_size = (uintptr_t)((char*)head - (char*)h);

    stack_head = h;

    return mem;
}


void memPop (void)
{
    struct Mem_Stack_Header *h = stack_head;

    stack_head = (char*)stack_head + h->total_size;

    return;
}

Is this the correct way of doing this kind of stuff? Any improvements I can make?

Answer 1

Is this the correct way of doing this kind of stuff? Any improvements I can make?

1. The role of member allocation_size is unclear. It appears to represent the raw alignment with h->allocation_size = s;. Later use with stack_head = (char*)stack_head + h->total_size; could then result in an unaligned stack_head. I recommend that void* memPush (size_t s) first take s and increment it as needed to an aligned size and then save that value in allocation_size. This will simplify subsequent code too especially the macro aligndown()

   s = (s + alignof(max_align_t) - 1) & ~(alignof(max_align_t) - 1u);
   

2. There is no protection against pushing too much. I'd expect the size of memory available with stack_head to be provided and memPush() to code against overflow. In any case, OP should provide documentation concerning what happens when too much memory is requested.

3. There is no clear functionality against popping too much. I'd expect either an error return or a perhaps the equivalent of the top allocation having size 0.

4. Some compilers will warn about mixed signed/unsigned math. As the return type from alignof() is some unsigned type, use unsigned constants throughout.

   //alignof(max_align_t) - 1
   alignof(max_align_t) - 1u
   

5. The name stack_head is a global object that could easily collide with other code. Its name does not suggest that is is part of the memPop(), memPush() functions. Recommend a name aligned with the funcitons like mem_stack_head.

6. A use of memPush(0) needs special consideration. I recommend that such usage returns the same as memPush(alignof(max_align_t)) to provide unique allocations. In any case, OP should provide documentation concerning the special case.

7. uintptr_t is an optional type and not the best to use here. (char*)head - (char*)h results in type ptrdiff_t, simply cast it to the destination type. Alternative:

   //h->total_size = (uintptr_t)((char*)head - (char*)h);
   h->total_size = (size_t)((char*)head - (char*)h);
   

8. Code could reduce explicit coding of cast (char *) by using char *head = stack_head;

9. Alternative idea to this line of code:

   // struct Mem_Stack_Header *h = (struct Mem_Stack_Header*)((char*)mem - sizeof(*h));
   struct Mem_Stack_Header *h = (struct Mem_Stack_Header*)mem - 1;
   

10. A simple test harness would help in evaluation of this code.

11. alignof is not valid without #include <stdalign.h>. Other include files missing too.