diff --git a/src/arena.c b/src/arena.c index 45d420d..9e91082 100644 --- a/src/arena.c +++ b/src/arena.c @@ -2,29 +2,22 @@ #include "base_core.h" #include "platform.h" -typedef struct MemoryBlockOptions MemoryBlockOptions; -struct MemoryBlockOptions { - U32 size; - U32 commit_size; - U32 commit_now_size; - U32 page_size; -}; -static MemoryBlock *memoryblock_allocate(const MemoryBlockOptions *opts) -{ - U32 aligned_size = AlignUpPow2(opts->size, opts->page_size); - U32 aligned_commit_size = AlignUpPow2(opts->commit_size, opts->page_size); - U32 aligned_commit_now_size = AlignUpPow2(opts->commit_now_size, opts->page_size); - MemoryBlock *block = memory_reserve(aligned_size); +static MemoryBlock *memoryblock_allocate(U32 size, U32 commit_size, U32 page_size) +{ + Assert(IsAlignedTo(size, page_size)); + Assert(IsAlignedTo(commit_size, page_size)); + + MemoryBlock *block = memory_reserve(size); Assert(block != NULL); - memory_commit(block, aligned_commit_now_size); + memory_commit(block, commit_size); block->base = (U8 *)block; - block->size = aligned_size; + block->size = size; block->first_free = MemoryBlock_Header_Size; block->arena_offset = 0; block->prev = NULL; - block->committed_end = aligned_commit_now_size; + block->committed_end = commit_size; ASAN_POISON_MEMORY_REGION(block->base, block->size); ASAN_UNPOISON_MEMORY_REGION(block, MemoryBlock_Header_Size); @@ -37,22 +30,19 @@ void arena_init(Arena *arena, const ArenaOptions *opts) Assert(opts->size > MemoryBlock_Header_Size); Assert(opts->size < (U32_Max - page_size)); // Reserving last piece so our math doesn't overflow Assert(opts->commit_size > 0); - Assert(opts->commit_size <= opts->size); - MemoryBlockOptions mb_opts; - mb_opts.page_size = page_size; - mb_opts.commit_size = opts->commit_size; - mb_opts.size = opts->size; - mb_opts.commit_now_size = opts->commit_size; - MemoryBlock *block = memoryblock_allocate(&mb_opts); + U32 aligned_commit_size = AlignUp(opts->commit_size, page_size); + U32 aligned_size = AlignUp(opts->size, page_size); + Assert(aligned_commit_size <= aligned_size); + + MemoryBlock *block = memoryblock_allocate(aligned_size, aligned_commit_size, page_size); arena->flags = opts->flags; arena->checkpoint_level = 0; arena->current = block; - arena->block_size = opts->size; - arena->commit_size = opts->commit_size; + arena->block_size = aligned_size; + arena->commit_size = aligned_commit_size; arena->page_size = page_size; - } void arena_free(Arena *arena) @@ -69,35 +59,32 @@ void arena_free(Arena *arena) void *arena_push(Arena *arena, U32 size, U32 align) { - Assert(size < (U32_Max - platform_page_size())); Assert(IsPow2(align)); + MemoryBlock *block = arena->current; - U32 start_pos = AlignUpPow2(block->first_free, align); // might align past block->size, handled in below check + U32 start_pos = AlignUp(block->first_free, align); // might align past block->size, handled in below check U32 available = block->size - start_pos; if (start_pos > block->size || available < size) { - U32 block_size = arena->block_size; // original user requested size + U32 pad = AlignUp(MemoryBlock_Header_Size, align); + Assert(size <= (U32_Max - arena->page_size) - pad); + U32 needed = pad + size; + U32 new_block_size = arena->block_size; U32 new_commit_size = arena->commit_size; - if (size > block_size - MemoryBlock_Header_Size) { - block_size = AlignUpPow2(size + MemoryBlock_Header_Size,align); - new_commit_size = block_size; + if (needed > new_block_size) { + new_block_size = AlignUp(needed, arena->page_size); + new_commit_size = new_block_size; } - U32 commit_now_size = Max(new_commit_size, size); - MemoryBlockOptions mb_opts; - mb_opts.page_size = arena->page_size; - mb_opts.commit_size = new_commit_size; - mb_opts.size = block_size; - mb_opts.commit_now_size = commit_now_size; - MemoryBlock *new_block = memoryblock_allocate(&mb_opts); + MemoryBlock *new_block = memoryblock_allocate(new_block_size, new_commit_size, arena->page_size); // arena_offset tracks the virtual absolute position within an arena // This is not a contiguous space, we use it to free blocks when popping the arena new_block->arena_offset = block->arena_offset + block->size; new_block->prev = block; arena->current = new_block; block = new_block; - start_pos = AlignUpPow2(block->first_free, align); + start_pos = AlignUp(block->first_free, align); } U32 end_pos = start_pos + size; @@ -106,7 +93,7 @@ void *arena_push(Arena *arena, U32 size, U32 align) U8 *mem = &block->base[start_pos]; if (block->committed_end < end_pos) { // Commit more memory - U32 aligned_end_pos = AlignUpPow2(start_pos + size, arena->page_size); + U32 aligned_end_pos = AlignUp(start_pos + size, arena->page_size); U32 aligned_size = RoundUpToMultiple(aligned_end_pos - block->committed_end, arena->commit_size); aligned_size = Min(aligned_size, block->size - block->committed_end); diff --git a/src/base_core.h b/src/base_core.h index ac68e3e..d1542d9 100644 --- a/src/base_core.h +++ b/src/base_core.h @@ -104,11 +104,12 @@ typedef unsigned __int128 U128; (const type *)((const void *)(const unsigned char *)__mptr - offsetof(const type, member)); \ }) -#define AlignUpPow2(val, align) (((val) + (align) - 1UL) & ~((align) - 1UL)) -#define AlignDownPow2(val, align) ((val) & ~((align) - 1UL)) +// align needs to be pow2 +#define AlignUp(val, align) (((val) + (align) - 1UL) & ~((align) - 1UL)) +#define AlignDown(val, align) ((val) & ~((align) - 1UL)) #define IsPow2(x) ((x) && !((x) & ((x) - 1UL))) #define IsPow2OrZero(x) (!((x) & ((x) - 1UL))) -#define IsAlignedToPow2(x, y) (IsPow2(y) && !((x) & ((y) - 1UL))) +#define IsAlignedTo(x, align) (IsPow2(align) && !((x) & ((align) - 1UL))) #define GetBit(val, idx) (((val) >> (idx)) & 1) // Get n_bits starting at idx going left diff --git a/src/main.c b/src/main.c index 1656626..ccd3f8f 100644 --- a/src/main.c +++ b/src/main.c @@ -1,8 +1,10 @@ #include "base_core.h" #include "arena.h" +#include "platform.h" #include #include + void arena_dump(const Arena *arena) { return; @@ -51,8 +53,8 @@ static void arena_test_2(void) TRACEF("\n"); ArenaOptions arena_params = {}; - U32 arena_size = MB(1) + MemoryBlock_Header_Size; - U32 commit_size = KB(16); + U32 arena_size = MB(3) + MemoryBlock_Header_Size; + U32 commit_size = KB(72); U32 push_size = KB(32); arena_params.size = arena_size; arena_params.commit_size = commit_size; @@ -66,7 +68,7 @@ static void arena_test_2(void) arena_dump(&arena); ArenaCheckpoint checkpoint = arena_temp_begin(&arena); - ptr = arena_push(&arena, MB(2), 8); + ptr = arena_push(&arena, MB(4)+3, 8); MemoryZero(ptr, MB(2)); ptr = arena_push(&arena, commit_size * 3, 8); @@ -92,8 +94,10 @@ static void arena_test_2(void) arena_reset_to(&arena, 1089); arena_dump(&arena); arena_reset_to(&arena, 0); - ptr = arena_push(&arena, MB(1), 8); - MemoryZero(ptr, MB(1)); + for (U64 i = 0; i < KB(1); i++) { + ptr = arena_push(&arena, MB(1) + 3, 8); + MemoryZero(ptr, MB(1)); + } arena_free(&arena); } @@ -120,9 +124,49 @@ static void arena_test_3(void) arena_free(&arena); } +// Exercises the alignment-padding edge case: a fresh block has first_free at +// MemoryBlock_Header_Size (64), so a large alignment pushes the user pointer +// well past the header. Requesting (block_size - 64) bytes with page alignment +// previously tripped the end_pos <= block->size assert because the alignment +// gap was not accounted for. After the fix this should allocate a larger block. +static void arena_test_4(void) +{ + TRACEF("\n"); + + U32 page_size = platform_page_size(); + ArenaOptions arena_params = {}; + U32 arena_size = MB(1) + MemoryBlock_Header_Size; + U32 commit_size = KB(64); + arena_params.size = arena_size; + arena_params.commit_size = commit_size; + Arena arena; + arena_init(&arena, &arena_params); + + // block_size is the page-aligned arena_size. Request nearly the whole block + // but with page alignment, so the alignment gap (a full page) forces a grow. + U32 block_size = arena.block_size; + U32 push_size = block_size - MemoryBlock_Header_Size; + + void *ptr = arena_push(&arena, push_size, page_size); + Assert(IsAlignedTo((uintptr_t)ptr, page_size)); + MemoryZero(ptr, push_size); + printf("test_4: allocated %u bytes page-aligned at %p\n", push_size, ptr); + arena_dump(&arena); + + // A second page-aligned push to make sure the cursor math still holds. + ptr = arena_push(&arena, page_size, page_size); + Assert(IsAlignedTo((uintptr_t)ptr, page_size)); + MemoryZero(ptr, page_size); + printf("test_4: allocated %u bytes page-aligned at %p\n", page_size, ptr); + + arena_free(&arena); +} + int main(int argc, char *argv[]) { - arena_test_1(); + // arena_test_1(); arena_test_2(); - arena_test_3(); + // arena_test_3(); + arena_test_4(); } + diff --git a/src/platform.c b/src/platform.c index 118c739..4c1b8a8 100644 --- a/src/platform.c +++ b/src/platform.c @@ -6,7 +6,7 @@ #include -#define IsPageAligned(x) (IsAlignedToPow2(x, platform_page_size())) +#define IsPageAligned(x) (IsAlignedTo(x, platform_page_size())) U32 platform_page_size(void) {