Fixed last region dereferencing in arena

chinookfile

@@ -30,8 +30,8 @@ tests:
 - name: malunal.allocators.allinone
   type: program
   deps:
-  - malunal.allocators
   - malunal.microtest
+  - malunal.allocators
   srcs:
   - ./tests/libc_allocator.c
   - ./tests/platform_allocator.c

sources/arena.c

@@ -23,10 +23,41 @@ _Static_assert(
   "Arena allocator must be the size of its implementation"
 );
 
+// Every allocation is rounded to this alignment so that any object may be stored
+// within it. The region header is also this size, so offsets stay aligned.
+#define ARENA_ALIGNMENT 16
+
+// Region sizes are tracked in 32 bits, so nothing larger may be allocated.
+#define ARENA_MAX_REQUEST ((malunal_uint32_t)-1 - sizeof(region_t))
+
+static
 error_t
-create_region(region_mptr_t* region) {
+arena_out_of_memory() {
+  return (error_t) {
+    .domain = &ERROR_DOMAIN_ALLOCATOR_T,
+    .code   = ALLOCATOR_ERROR_OUT_OF_MEMORY
+  };
+}
+
+static
+error_t
+create_region(
+  malunal_size_t minimum,
+  region_mptr_t* region
+) {
+  if (minimum > ARENA_MAX_REQUEST)
+    return arena_out_of_memory();
+
+  // Requests larger than the default region size get a region of their own.
+  malunal_size_t wanted = sizeof(region_t) + minimum;
+  if (wanted < MALUNAL_ARENA_REGION_SIZE)
+    wanted = MALUNAL_ARENA_REGION_SIZE;
+
+  malunal_size_t rounded = align_to_page(wanted);
+  if (rounded < wanted || rounded > (malunal_uint32_t)-1)
+    return arena_out_of_memory();
+
   allocator_mptr_t allocator = platform_allocator();
-  malunal_size_t   rounded   = align_to_page(MALUNAL_ARENA_REGION_SIZE);
   malunal_mptr_t*  address   = (malunal_mptr_t*)region;
   error_t          result    = allocator_acquire(allocator, rounded, address);
 
@@ -39,6 +70,7 @@ create_region(region_mptr_t* region) {
   return NO_ERROR;
 }
 
+static
 error_t
 delete_region(region_mptr_t region) {
   if (region == null)
@@ -53,6 +85,7 @@ delete_region(region_mptr_t region) {
   return allocator_dispose(allocator, region, region->size);
 }
 
+static
 error_t
 region_acquire(
   region_mptr_t   region,
@@ -130,7 +163,7 @@ arena_allocator_init(
     };
 
   region_mptr_t region = null;
-  error_t       result = create_region(&region);
+  error_t       result = create_region(capacity, &region);
   if (result.domain != null)
     return result;
 
@@ -152,18 +185,30 @@ arena_allocator_acquire(
       .code   = ALLOCATOR_ERROR_NULL_ALLOCATOR
     };
 
+  if (size > ARENA_MAX_REQUEST)
+    return arena_out_of_memory();
+  size = align_to(size, ARENA_ALIGNMENT);
+
   impl_mptr_t   self   = (impl_mptr_t)allocator;
   region_mptr_t region = self->context;
+  region_mptr_t last   = null;
   while (region != null) {
     if (region->size - region->used >= size)
       return region_acquire(region, size, out);
+    last   = region;
     region = region->next;
   }
 
-  error_t result = create_region((region_mptr_t*)&region->next);
+  region_mptr_t created = null;
-  return result.domain == null
+  error_t       result  = create_region(size, &created);
-    ? region_acquire(region->next, size, out)
+  if (result.domain != null)
-    : result;
+    return result;
+
+  if (last == null)
+    self->context = created;
+  else
+    last->next = created;
+  return region_acquire(created, size, out);
 }
 
 error_t
@@ -178,25 +223,21 @@ arena_allocator_dispose(
       .code   = ALLOCATOR_ERROR_NULL_ALLOCATOR
     };
 
-  impl_mptr_t      self = (impl_mptr_t)allocator;
+  // Arenas never release individual allocations, disposing only verifies that
-  region_mptr_t    reg  = self->context;
+  // the address belongs to one of the regions.
+  impl_iptr_t      self = (impl_iptr_t)allocator;
   malunal_uint8_t* addr = address;
-  malunal_uint8_t* beg  = reg->bytes;
+  for (region_iptr_t reg = self->context; reg != null; reg = reg->next) {
-  malunal_uint8_t* end  = reg->bytes + reg->size - sizeof(region_t);
+    const malunal_uint8_t* beg = reg->bytes;
-  while (true) {
+    const malunal_uint8_t* end = reg->bytes + reg->size - sizeof(region_t);
-    if (addr < beg || addr > end)
+    if (addr >= beg && addr <= end)
-      return (error_t) {
+      return NO_ERROR;
-        .domain = &ERROR_DOMAIN_ALLOCATOR_T,
-        .code   = ALLOCATOR_ERROR_NOT_MY_ADDRESS
-      };
-
-    reg = reg->next;
-    if (reg == null)
-      break;
-    beg = reg->bytes;
-    end = reg->bytes + reg->size - sizeof(region_t);
   }
-  return NO_ERROR;
+
+  return (error_t) {
+    .domain = &ERROR_DOMAIN_ALLOCATOR_T,
+    .code   = ALLOCATOR_ERROR_NOT_MY_ADDRESS
+  };
 }
 
 error_t
@@ -215,6 +256,8 @@ arena_allocator_reset(
     region->used = sizeof(region_t);
     region = region->next;
   }
+
+  return NO_ERROR;
 }
 
 error_t

tests/arena_allocator.c

@@ -76,3 +76,100 @@ MICROTEST(arena_allocator, can_reset_allocator) {
   MICROTEST_EXPECT_EQ(temporary, 16);
   arena_allocator_free(&allocator);
 }
+
+MICROTEST(arena_allocator, can_acquire_beyond_first_region) {
+  arena_allocator_t allocator = {};
+  malunal_mptr_t    addresses[256];
+  malunal_uint32_t  temporary = 0;
+
+  arena_allocator_init(500, &allocator);
+  for (malunal_size_t index = 0; index < 256; index++) {
+    error_t result = arena_allocator_acquire(&allocator, 64, &addresses[index]);
+    MICROTEST_ASSERT_NULL(result.domain);
+    memset(addresses[index], (int)index, 64);
+  }
+
+  for (malunal_size_t index = 0; index < 256; index++) {
+    malunal_uint8_t* bytes = addresses[index];
+    MICROTEST_EXPECT_EQ(bytes[63], (malunal_uint8_t)index);
+  }
+
+  arena_allocator_size(&allocator, &temporary);
+  MICROTEST_EXPECT_GT(temporary, platform_page_size());
+  arena_allocator_free(&allocator);
+}
+
+MICROTEST(arena_allocator, can_acquire_larger_than_region) {
+  arena_allocator_t allocator = {};
+  malunal_mptr_t    address   = null;
+  malunal_size_t    size      = platform_page_size() * 3;
+
+  arena_allocator_init(500, &allocator);
+  error_t result = arena_allocator_acquire(&allocator, size, &address);
+  MICROTEST_EXPECT_NULL(result.domain);
+  MICROTEST_ASSERT_NOT_NULL(address);
+
+  memset(address, 0xAB, size);
+  arena_allocator_free(&allocator);
+}
+
+MICROTEST(arena_allocator, oversized_request_reports_out_of_memory) {
+  arena_allocator_t allocator = {};
+  malunal_mptr_t    address   = null;
+
+  arena_allocator_init(500, &allocator);
+  error_t result = arena_allocator_acquire(&allocator, (malunal_size_t)-1, &address);
+  MICROTEST_EXPECT_NOT_NULL(result.domain);
+  MICROTEST_EXPECT_EQ(result.code, (malunal_int32_t)ALLOCATOR_ERROR_OUT_OF_MEMORY);
+  arena_allocator_free(&allocator);
+}
+
+MICROTEST(arena_allocator, acquisitions_are_aligned) {
+  arena_allocator_t allocator = {};
+
+  arena_allocator_init(500, &allocator);
+  for (malunal_size_t size = 1; size <= 33; size += 3) {
+    malunal_mptr_t address = null;
+    arena_allocator_acquire(&allocator, size, &address);
+    MICROTEST_EXPECT_EQ((malunal_uintptr_t)address % 16, (malunal_uintptr_t)0);
+  }
+
+  arena_allocator_free(&allocator);
+}
+
+MICROTEST(arena_allocator, can_dispose_from_later_region) {
+  arena_allocator_t allocator = {};
+  malunal_mptr_t    address   = null;
+
+  arena_allocator_init(500, &allocator);
+  for (malunal_size_t index = 0; index < 128; index++)
+    arena_allocator_acquire(&allocator, 64, &address);
+
+  error_t result = arena_allocator_dispose(&allocator, address, 64);
+  MICROTEST_EXPECT_NULL(result.domain);
+  arena_allocator_free(&allocator);
+}
+
+MICROTEST(arena_allocator, dispose_rejects_foreign_address) {
+  arena_allocator_t allocator = {};
+  malunal_uint8_t   foreign[64];
+
+  arena_allocator_init(500, &allocator);
+  error_t result = arena_allocator_dispose(&allocator, foreign, sizeof(foreign));
+  MICROTEST_EXPECT_NOT_NULL(result.domain);
+  MICROTEST_EXPECT_EQ(result.code, (malunal_int32_t)ALLOCATOR_ERROR_NOT_MY_ADDRESS);
+  arena_allocator_free(&allocator);
+}
+
+MICROTEST(arena_allocator, can_acquire_after_free) {
+  arena_allocator_t allocator = {};
+  malunal_mptr_t    address   = null;
+
+  arena_allocator_init(500, &allocator);
+  arena_allocator_free(&allocator);
+
+  error_t result = arena_allocator_acquire(&allocator, 64, &address);
+  MICROTEST_EXPECT_NULL(result.domain);
+  MICROTEST_ASSERT_NOT_NULL(address);
+  arena_allocator_free(&allocator);
+}