Skip to content
This repository has been archived by the owner on Nov 15, 2019. It is now read-only.

Commit

Permalink
pkg/tlsf: Early initialization of memory pool.
Browse files Browse the repository at this point in the history
The TLSF allocator needs to be initialized before use. This is an issue when
it is used as a default system allocator since the user expects to be able to
call malloc right away. This is made worse by the fact that the C library uses
malloc internally to create buffers and that may happen before the user's code
has a chance to run.

As a consequence, even doing printf when using USEMODULE=tlsf-malloc will lead
to a crash.

A mechanism is needed to:

1. Initialize the pool early.
2. Determine which memory should be used as a heap and reserve it.

Issue (1) is solved by adding the initializer to the C library's `.preinit_array`,
which is a cross-file array of function pointers that run before the library is
initialized -that is before _init(). See the newlib source code for more details.

Point (2) is important because TLSF dows not support growing the pool, only adding
new ones. We would like to initialize it with a pool as big as possible.

In native (2) is handled by defining a static array of fixed size (given by
TLSF_NATIVE_HEAPSIZE). Memory is plentiful in native and we down't care about
the overhead of zeroing out this array.

On embedded targets using newlib (this may be working on other plaforms, I only
tested ARM) `sbrk()` is used to find the start of the heap and reserve it and
the `_eheap` linker symbol is used to determine the end of the usable heap.
An array is a bad choice here because the size would be board dependent and hard
to determine without build-system magic and because it would be zeroed by default,
making the boot sequence way longer.

sbrk() does nothing more than move a pointer that marks the fraction of the space
between _sheap and _eheap that is reserved. Since we are using the whole heap it
might be tempting to just use the symbols to derive the pool location and size and
to sidestep sbrk(). Especially since the memory allocation functions are expected to
be the only users of such a feature. That "trick" would make the OS impossible to
debug in case the was a mistake and some of the original allocation functions slipped
through non-overriden. If sbrk is used to reserve the entirety of the space then that
rogue function will try to call it and fail as no more heap is available. In fact
this is how I found out that I was overriding the wrong functions (put a breakpoint
int sbrk and show a traceback.) If sbrk is sidestepped one would have nasty and
impossible to debug memory corruption errors.

A third option could be to use the heap space directly and not define sbrk. This
is beyond the scope of this change, but is probably the route to go for platform
that do not define this call (but first do a thoroug investigation of how the libc
works in that platform).

Messing with the global system allocator is not an easy thing to do. I would say
that tslf-malloc is ATM _only_ supported in native and cortex-m.

Testing procedure:

Run `tests/pkg_tlsf_malloc`.

Fixes: RIOT-OS#4490, RIOT-OS#5796.
Closes: RIOT-OS#12021
  • Loading branch information
jcarrano committed Sep 28, 2019
1 parent 93d4c22 commit 6945bba
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 0 deletions.
1 change: 1 addition & 0 deletions pkg/tlsf/contrib/Makefile.include
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
UNDEF += $(BINDIR)/tlsf-malloc/tlsf-malloc.o
13 changes: 13 additions & 0 deletions pkg/tlsf/contrib/native.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,19 @@
#include "tlsf-malloc.h"
#include "tlsf-malloc-internal.h"

/*
* On native the linker dos not define the Heap area.
* It is handled by the C library (it requests memory from the OS ??)
* TODO: make this configurable
*/
static char _sheap[TLSF_NATIVE_HEAPSIZE];

/* Define the initialization function */
void init_tlsf_malloc(void)
{
tlsf_add_global_pool(_sheap, ROUND_DOWN4(TLSF_NATIVE_HEAPSIZE));
}

/* TODO: Add defines for other compilers */
#if defined(__GNUC__) && !defined(__clang__) /* Clang supports __GNUC__ but
* not the alloc_size()
Expand Down
43 changes: 43 additions & 0 deletions pkg/tlsf/contrib/newlib.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,58 @@
*
*/

#define _DEFAULT_SOURCE
#include <string.h>
#include <reent.h>
#include <errno.h>
#include <unistd.h>

#include "irq.h"
#include "panic.h"
#include "tlsf.h"
#include "tlsf-malloc.h"
#include "tlsf-malloc-internal.h"

/*
* The linker script for embedded target defines _sheap and _eheap:
*
* / * heap section * /
* . = ALIGN(4);
* _sheap = . ;
* _eheap = ORIGIN(ram) + LENGTH(ram);
*
* For some reason, at program startup there is something at _sbreak. so
* initializing the tlsf heap there leads to failure. In fact, sbrk(0)
* points quite some bytes after _sheap.
* The solution here is to request new memory to sbrk, use only that and
* request all memory up to _eheap.
* Keep in mind that this may mean less memory available for growing the
* stack! The way to solve it is to configure a smaller _eheap.
*/
extern char _eheap[]; /* FIXME: what to do with platforms without this symbol? */

/* Define the initialization function */
void init_tlsf_malloc(void)
{
/* Use sbrk(0) instead if _sheap since they may not be equal. */
size_t request_size = ROUND_DOWN4(_eheap - (char*)sbrk(0));
void *mem_start = sbrk(request_size);

if (mem_start == (void *)(-1)) {
/* FIXME: This message does not show. The UART is probably not yet
* initialized.
*/
core_panic(PANIC_GENERAL_ERROR, "Could not enlarge heap");
}

/* Why do we use sbrk instead of _sheap and _eheap?
* Because of the (unlikely) possibility that some other libc procedure is
* using sbrk, which - if we were to bypass it by directly using _{s,e}heap
* would result in weird and hard to debug memory errors.
*/
tlsf_add_global_pool(mem_start, request_size);
}


/* TODO: Add defines for other compilers */
#if defined(__GNUC__) && !defined(__clang__) /* Clang supports __GNUC__ but
Expand Down
16 changes: 16 additions & 0 deletions pkg/tlsf/contrib/tlsf-malloc-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,24 @@
extern "C" {
#endif

#define ROUND_DOWN4(x) (((x)/4)*4) /* Is this necessary??? */

#ifndef TLSF_NATIVE_HEAPSIZE
/**
* Fixed heap size to use in native.
*
* In native there is virtually unlimited memory and no predefined heap area
* in the linker script, so one needs to define it manually.
* The default here is 8MiB which should be plenty for RIOT.
* Note that this has no effect on other platforms.
*/
#define TLSF_NATIVE_HEAPSIZE (0x800000)
#endif

extern tlsf_t tlsf_malloc_gheap;

void init_tlsf_malloc(void);

#ifdef __cplusplus
}
#endif
Expand Down
18 changes: 18 additions & 0 deletions pkg/tlsf/contrib/tlsf-malloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@
* @brief TLSF-based global memory allocator.
* @author Juan I Carrano
*
* # About initialization
*
* Some system (standard library) routines trigger the allocation of buffers
* which results in segmentation faults/ hard faults if using tlsf and the
* heap is not yet allocated. This function can possibly be used super early
* in the boot process, so the TLSF initialization must run earlier still.
*
* On native we define a static array and on embedded platforms we use the
* _sheap and _eheap linker symbols and sbrk.
*
*/

#include <stdio.h>
Expand Down Expand Up @@ -56,6 +66,14 @@ void tlsf_size_walker(void* ptr, size_t size, int used, void* user)
}
}

/* The C library runs the functions in this array before it initializes itself.
* This section constitutes a cross-file array.
* Note that in order for this to work this object file has to be added to
* UNDEF (see ../Makefile.include) otherwise this symbol gets discarded.
*/
void (* const init_tlsf_malloc_p) (void) __attribute__((section (".preinit_array"))) =
init_tlsf_malloc;

/**
* @}
*/
1 change: 1 addition & 0 deletions tests/pkg_tlsf_malloc/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ include ../Makefile.tests_common

# The bug is not fixed yet, so blacklist this test.
TEST_ON_CI_BLACKLIST += all
TEST_ON_CI_WHITELIST += native samr21-xpro

USEMODULE += tlsf-malloc

Expand Down

0 comments on commit 6945bba

Please sign in to comment.