Skip to content

GCC - Add support to split heap across 2-RAM banks #9944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions TESTS/mbedmicro-rtos-mbed/heap_and_stack/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ extern uint32_t mbed_heap_size;
extern uint32_t mbed_stack_isr_start;
extern uint32_t mbed_stack_isr_size;

#if defined(TOOLCHAIN_GCC_ARM) && defined(MBED_SPLIT_HEAP)
extern uint32_t __mbed_sbrk_start_0;
extern uint32_t __mbed_krbs_start_0;
unsigned char *mbed_heap_start_0 = (unsigned char *) &__mbed_sbrk_start_0;;
uint32_t mbed_heap_size_0 = (uint32_t) &__mbed_krbs_start_0 - (uint32_t) &__mbed_sbrk_start_0;
#endif

struct linked_list {
linked_list *next;
Expand Down Expand Up @@ -121,7 +127,11 @@ static void allocate_and_fill_heap(linked_list *&head)
break;
}
bool result = rangeinrange((uint32_t) temp, sizeof(linked_list), mbed_heap_start, mbed_heap_size);

#if defined(TOOLCHAIN_GCC_ARM) && defined(MBED_SPLIT_HEAP)
if (false == result) {
result = rangeinrange((uint32_t) temp, sizeof(linked_list), (uint32_t)mbed_heap_start_0, mbed_heap_size_0);
}
#endif
TEST_ASSERT_TRUE_MESSAGE(result, "Memory allocation out of range");

// Init
Expand Down Expand Up @@ -169,7 +179,11 @@ void test_heap_in_range(void)
TEST_ASSERT_NOT_NULL(initial_heap);

bool result = inrange((uint32_t) initial_heap, mbed_heap_start, mbed_heap_size);

#if defined(TOOLCHAIN_GCC_ARM) && defined(MBED_SPLIT_HEAP)
if (false == result) {
result = inrange((uint32_t) initial_heap, (uint32_t)mbed_heap_start_0, mbed_heap_size_0);
}
#endif
TEST_ASSERT_TRUE_MESSAGE(result, "Heap in wrong location");
free(initial_heap);
}
Expand Down
41 changes: 40 additions & 1 deletion TESTS/mbedmicro-rtos-mbed/malloc/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,44 @@ void test_multithread_allocation(void)
TEST_ASSERT_FALSE(thread_alloc_failure);
}

/** Test for multiple heap alloc and free calls */
#define ALLOC_ARRAY_SIZE 100
#define ALLOC_LOOP 20
#define SIZE_INCREMENTS 1023
#define SIZE_MODULO 31

void test_alloc_and_free(void)
{
void *array[ALLOC_ARRAY_SIZE];
void *data = NULL;
long total_allocated = 0;
int count = 0;
int size = SIZE_INCREMENTS;
int loop = ALLOC_LOOP;
while (loop) {
data = malloc(size);
if (NULL != data) {
array[count++] = data;
memset((void *)data, 0xdeadbeef, size);
total_allocated += size;
size += SIZE_INCREMENTS;
if (size > 10000) {
size %= SIZE_MODULO;
}
} else {
for (int i = 0; i < count; i++) {
free(array[i]);
array[i] = NULL;
}
loop--;
printf("Total size dynamically allocated: %luB\n", total_allocated);
total_allocated = 0;
count = 0;
continue;
}
}
}

/** Test for large heap allocation

Given a heap of size mbed_heap_size
Expand Down Expand Up @@ -167,7 +205,8 @@ Case cases[] = {
Case("Test 0 size allocation", test_zero_allocation),
Case("Test NULL pointer free", test_null_free),
Case("Test multithreaded allocations", test_multithread_allocation),
Case("Test large allocation", test_big_allocation)
Case("Test large allocation", test_big_allocation),
Case("Test multiple alloc and free calls", test_alloc_and_free)
};

utest::v1::status_t greentea_test_setup(const size_t number_of_cases)
Expand Down
41 changes: 41 additions & 0 deletions platform/mbed_retarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1240,6 +1240,46 @@ extern "C" WEAK void __cxa_pure_virtual(void)
// SP. This make it compatible with RTX RTOS thread stacks.
#if defined(TOOLCHAIN_GCC_ARM)

#if defined(MBED_SPLIT_HEAP)

// Default RAM memory used for heap
extern uint32_t __mbed_sbrk_start;
extern uint32_t __mbed_krbs_start;
/* Additional RAM memory used for heap - please note this
* address should be lower address then the previous default address
*/
extern uint32_t __mbed_sbrk_start_0;
extern uint32_t __mbed_krbs_start_0;

extern "C" WEAK caddr_t _sbrk(int incr)
{
static uint32_t heap = (uint32_t) &__mbed_sbrk_start_0;
static bool once = true;
uint32_t prev_heap = heap;
uint32_t new_heap = heap + incr;

/**
* If the new address is outside the first region, start allocating from the second region.
* Jump to second region is done just once, and `static bool once` is used to keep track of that.
*/
if (once && (new_heap > (uint32_t) &__mbed_krbs_start_0)) {
once = false;
Copy link
Contributor

@0xc0170 0xc0170 Apr 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does once do - either changing the name to be more clear about the intention or adding a comment. Or at least 1258 comment could include this boolean - it touches only address check

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once - is indicator of allowing to enter just once. I can update the comment to add more clarity

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant by sbrk behavior is that if you try to allocate here more memory than is available in first region, the code moves to another region and you end up nothing being allocated from first region in the worst case. And you can't come back here either.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, given the current behaviour of newlib's malloc, I don't think the once flag is needed.

The regions must be in memory address order, or newlib malloc would get confused. (Is that known/documented for this feature? I spotted it by inspection of newlib source, and it makes sense given the normal POSIX function of sbrk).

So the test could be if (heap <= (uint32_t) &__mbed_krbs_start_0 && new_heap > (uint32_t) &_mbed_krbs_start_0).

prev_heap = (uint32_t) &__mbed_sbrk_start;
new_heap = prev_heap + incr;
} else if (new_heap > (uint32_t) &__mbed_krbs_start) {
/**
* If the new address is outside the second region, return out-of-memory.
*/
errno = ENOMEM;
return (caddr_t) - 1;
}

heap = new_heap;
return (caddr_t) prev_heap;
}

#else

extern "C" uint32_t __end__;
extern "C" uint32_t __HeapLimit;

Expand All @@ -1264,6 +1304,7 @@ extern "C" WEAK caddr_t _sbrk(int incr)
return (caddr_t) prev_heap;
}
#endif
#endif

#if defined(TOOLCHAIN_GCC_ARM)
extern "C" void _exit(int return_code)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ STACK_SIZE = MBED_BOOT_STACK_SIZE;

/* Linker script to configure memory regions. */
MEMORY
{
{
FLASH (rx) : ORIGIN = MBED_APP_START, LENGTH = MBED_APP_SIZE
SRAM2 (rwx) : ORIGIN = 0x10000188, LENGTH = 32k - 0x188
SRAM1 (rwx) : ORIGIN = 0x20000000, LENGTH = 96k
Expand All @@ -24,7 +24,7 @@ MEMORY
* with other linker script that defines memory regions FLASH and RAM.
* It references following symbols, which must be defined in code:
* Reset_Handler : Entry of reset handler
*
*
* It defines following symbols, which code can use without definition:
* __exidx_start
* __exidx_end
Expand Down Expand Up @@ -92,81 +92,100 @@ SECTIONS
__etext = .;
_sidata = .;

/* .stack section doesn't contains any symbols. It is only
* used for linker to reserve space for the isr stack section
* WARNING: .stack should come immediately after the last secure memory
* section. This provides stack overflow detection. */
.stack (NOLOAD):
{
__StackLimit = .;
*(.stack*);
. += STACK_SIZE - (. - __StackLimit);
} > SRAM2

/* Set stack top to end of RAM, and stack limit move down by
* size of stack_dummy section */
__StackTop = ADDR(.stack) + SIZEOF(.stack);
_estack = __StackTop;
__StackLimit = ADDR(.stack);
PROVIDE(__stack = __StackTop);

/* Place holder for additional heap */
.heap_0 (COPY):
{
__mbed_sbrk_start_0 = .;
. += (ORIGIN(SRAM2) + LENGTH(SRAM2) - .);
__mbed_krbs_start_0 = .;
} > SRAM2

/* Check if heap exceeds SRAM2 */
ASSERT(__mbed_krbs_start_0 <= (ORIGIN(SRAM2)+LENGTH(SRAM2)), "Heap is too big for SRAM2")

.data : AT (__etext)
{
__data_start__ = .;
_sdata = .;
*(vtable)
*(.data*)

. = ALIGN(4);
. = ALIGN(8);
/* preinit data */
PROVIDE_HIDDEN (__preinit_array_start = .);
KEEP(*(.preinit_array))
PROVIDE_HIDDEN (__preinit_array_end = .);

. = ALIGN(4);
. = ALIGN(8);
/* init data */
PROVIDE_HIDDEN (__init_array_start = .);
KEEP(*(SORT(.init_array.*)))
KEEP(*(.init_array))
PROVIDE_HIDDEN (__init_array_end = .);


. = ALIGN(4);
. = ALIGN(8);
/* finit data */
PROVIDE_HIDDEN (__fini_array_start = .);
KEEP(*(SORT(.fini_array.*)))
KEEP(*(.fini_array))
PROVIDE_HIDDEN (__fini_array_end = .);

KEEP(*(.jcr*))
. = ALIGN(4);
. = ALIGN(8);
/* All data end */
__data_end__ = .;
_edata = .;

} > SRAM1

/* Check if bss exceeds SRAM1 */
ASSERT(__data_end__ <= (ORIGIN(SRAM1)+LENGTH(SRAM1)), ".data is too big for SRAM1")

.bss :
{
. = ALIGN(4);
. = ALIGN(8);
__bss_start__ = .;
_sbss = .;
*(.bss*)
*(COMMON)
. = ALIGN(4);
. = ALIGN(8);
__bss_end__ = .;
_ebss = .;
} > SRAM1

/* Check if bss exceeds SRAM1 */
ASSERT(__bss_end__ <= (ORIGIN(SRAM1)+LENGTH(SRAM1)), "BSS is too big for SRAM1")

/* Placeholder for default single heap */
.heap (COPY):
{
__end__ = .;
end = __end__;
__mbed_sbrk_start = .;
*(.heap*)
. += (ORIGIN(SRAM1) + LENGTH(SRAM1) - .);
__mbed_krbs_start = .;
__HeapLimit = .;
} > SRAM1
PROVIDE(__heap_size = SIZEOF(.heap));
PROVIDE(__mbed_sbrk_start = ADDR(.heap));
PROVIDE(__mbed_krbs_start = ADDR(.heap) + SIZEOF(.heap));
/* Check if data + heap exceeds RAM1 limit */
ASSERT((ORIGIN(SRAM1)+LENGTH(SRAM1)) >= __HeapLimit, "SRAM1 overflow")
/* .stack_dummy section doesn't contains any symbols. It is only
* used for linker to calculate size of stack sections, and assign
* values to stack symbols later */
.stack_dummy (COPY):
{
*(.stack*)
} > SRAM2

/* Set stack top to end of RAM, and stack limit move down by
* size of stack_dummy section */
__StackTop = ORIGIN(SRAM2) + LENGTH(SRAM2);
_estack = __StackTop;
__StackLimit = __StackTop - STACK_SIZE;
PROVIDE(__stack = __StackTop);
/* Check if stack exceeds RAM2 limit */
ASSERT((ORIGIN(SRAM2)+LENGTH(SRAM2)) >= __StackLimit, "SRAM2 overflow")
}
/* Check if heap exceeds SRAM1 */
ASSERT(__HeapLimit <= (ORIGIN(SRAM1)+LENGTH(SRAM1)), "Heap is too big for SRAM1")
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,35 @@ SECTIONS
__etext = .;
_sidata = .;

/* .stack section doesn't contains any symbols. It is only
* used for linker to reserve space for the isr stack section
* WARNING: .stack should come immediately after the last secure memory
* section. This provides stack overflow detection. */
.stack (NOLOAD):
{
__StackLimit = .;
*(.stack*);
. += STACK_SIZE - (. - __StackLimit);
} > SRAM2

/* Set stack top to end of RAM, and stack limit move down by
* size of stack_dummy section */
__StackTop = ADDR(.stack) + SIZEOF(.stack);
_estack = __StackTop;
__StackLimit = ADDR(.stack);
PROVIDE(__stack = __StackTop);

/* Place holder for additional heap */
.heap_0 (COPY):
{
__mbed_sbrk_start_0 = .;
. += (ORIGIN(SRAM2) + LENGTH(SRAM2) - .);
__mbed_krbs_start_0 = .;
} > SRAM2

/* Check if heap exceeds SRAM2 */
ASSERT(__mbed_krbs_start_0 <= (ORIGIN(SRAM2)+LENGTH(SRAM2)), "Heap is too big for SRAM2")

.data : AT (__etext)
{
__data_start__ = .;
Expand Down Expand Up @@ -129,6 +158,9 @@ SECTIONS

} > SRAM1

/* Check if bss exceeds SRAM1 */
ASSERT(__data_end__ <= (ORIGIN(SRAM1)+LENGTH(SRAM1)), ".data is too big for SRAM1")

.bss :
{
. = ALIGN(8);
Expand All @@ -141,30 +173,21 @@ SECTIONS
_ebss = .;
} > SRAM1

/* Check if bss exceeds SRAM1 */
ASSERT(__bss_end__ <= (ORIGIN(SRAM1)+LENGTH(SRAM1)), "BSS is too big for SRAM1")

/* Placeholder for default single heap */
.heap (COPY):
{
__end__ = .;
end = __end__;
__mbed_sbrk_start = .;
*(.heap*)
. = ORIGIN(SRAM1) + LENGTH(SRAM1) - STACK_SIZE;
. += (ORIGIN(SRAM1) + LENGTH(SRAM1) - .);
__mbed_krbs_start = .;
__HeapLimit = .;
} > SRAM1

/* .stack_dummy section doesn't contains any symbols. It is only
* used for linker to calculate size of stack sections, and assign
* values to stack symbols later */
.stack_dummy (COPY):
{
*(.stack*)
} > SRAM1

/* Set stack top to end of RAM, and stack limit move down by
* size of stack_dummy section */
__StackTop = ORIGIN(SRAM1) + LENGTH(SRAM1);
_estack = __StackTop;
__StackLimit = __StackTop - STACK_SIZE;
PROVIDE(__stack = __StackTop);

/* Check if data + heap + stack exceeds RAM limit */
ASSERT(__StackLimit >= __HeapLimit, "region RAM overflowed with stack")
/* Check if heap exceeds SRAM1 */
ASSERT(__HeapLimit <= (ORIGIN(SRAM1)+LENGTH(SRAM1)), "Heap is too big for SRAM1")
}
Loading