arena icon indicating copy to clipboard operation
arena copied to clipboard

`arena_malloc` doesn't align the returned memory properly.

Open camel-cdr opened this issue 4 years ago • 1 comments
trafficstars

I'm just writing this issue, since I stumbled on this project and saw that arena_malloc doesn't return aligned memory. This can lead to a hardware trap or crashes: https://blog.quarkslab.com/unaligned-accesses-in-cc-what-why-and-solutions-to-do-it-properly.html, https://stackoverflow.com/questions/19352232/unaligned-memory-access

You can try compiling this example program showcasing the problem with sanitization:

// gcc test.c arena.c -fsanitize=address -fsanitize=undefined
int
main()
{
	arena_t *arena = arena_create();
	uint8_t *it = arena_malloc(arena, 1);

	{
		uint8_t *x = arena_malloc(arena, sizeof *x);
		*x = 3;
	}

	{
		uint64_t *x = arena_malloc(arena, sizeof *x);
		*x = 3;
	}

	arena_destroy(arena);
	return 0;
}

camel-cdr avatar Apr 23 '21 19:04 camel-cdr

This may not be the way you'd like to have the API work, but I wrote down the code in a way I'd write it.

Note that I created an extra arena_malloc_aligned since allocating e.g. uint8_t with arena_malloc would now always waste quite a lot of bytes, this can probably be done more elegantly. (That's why I didn't create a pull request)

#include "arena.h"
#include <assert.h>
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>

#define PAGE_SIZE 4095

#define ALIGN_UP(x, n) (((x) + (n) - 1) & ~((n) - 1))

static arena_t *
_arena_create(size_t size) {
  arena_t *arena = (arena_t *) calloc(1, sizeof(arena_t));
  if(!arena) return NULL;
  arena->region = (uint8_t *) calloc(size, sizeof(uint8_t));
  arena->size   = size;
  if(!arena->region) { free(arena); return NULL; }
  return arena;
}

arena_t *
arena_create() {
  return _arena_create(PAGE_SIZE);
}

void *
arena_malloc_aligned(arena_t *arena, size_t size, size_t alignment) {
  arena_t *last = arena;
  assert(alignment && ((alignment & (alignment - 1)) == 0) &&
         "arena_malloc_aligned: alignment must be a power-of-2");

  size_t old = size;
  size = ALIGN_UP(size, alignment);

  do {
    if((arena->size - arena->current) >= size){
      arena->current += size;
      return arena->region + (arena->current - size);
    }
    last = arena;
  } while((arena = arena->next) != NULL);

  size_t asize   = size > PAGE_SIZE ? size : PAGE_SIZE;
  arena_t *next  = _arena_create(asize);
  last->next     = next;
  next->current += size;
  return next->region + (size - old);
}

void *
arena_malloc(arena_t *arena, size_t size) {
  union MaxAlign {
    long double ld;
    long l;
    double d;
    char *p;
    int (*f)(void);
#if __STDC_VERSION__ >= 199901L
    long long ll;
#endif
#if __STDC_VERSION__ >= 201112L
    max_align_t ma;
#endif
  };

  return arena_malloc_aligned(arena, size, sizeof(union MaxAlign));
}

void
arena_destroy(arena_t *arena) {
  arena_t *next, *last = arena;
  do {
    next = last->next;
    free(last->region);
    free(last);
    last = next;
  } while(next != NULL);
}

camel-cdr avatar Apr 23 '21 19:04 camel-cdr