From 73f6ac6bcec88b490182e874a722ec5ffbd90164 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Fri, 31 Oct 2014 13:24:32 +0100 Subject: [PATCH 1/3] Revert "cpu/cc2538: sbrk() checks if the requested memory is really available." This reverts commit 81dea364a5b372ad1625181eff710c095420735c. --- cpu/cc2538/syscalls.c | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/cpu/cc2538/syscalls.c b/cpu/cc2538/syscalls.c index 84721323a9..d1bb148835 100644 --- a/cpu/cc2538/syscalls.c +++ b/cpu/cc2538/syscalls.c @@ -39,13 +39,6 @@ #ifdef MODULE_UART0 #include "board_uart0.h" #endif - -#ifdef CPU_MODEL_CC2538NF11 -#define SRAM_LENGTH (16 * 1024) /**< The CC2538NF11 has 16 Kb of RAM */ -#else -#define SRAM_LENGTH (32 * 1024) /**< All other existing models of the CC2538 have 32 Kb of RAM */ -#endif - /** * manage the heap */ @@ -120,25 +113,21 @@ void _exit(int n) * @brief Allocate memory from the heap. * * The current heap implementation is very rudimentary, it is only able to allocate - * memory. It does not have any means to free memory again. + * memory. But it does not + * - check if the returned address is valid (no check if the memory very exists) + * - have any means to free memory again * - * @return a pointer to the successfully allocated memory - * @return -1 on error, and errno is set to ENOMEM + * TODO: check if the requested memory is really available + * + * @return [description] */ caddr_t _sbrk_r(struct _reent *r, size_t incr) { unsigned int state = disableIRQ(); - if ((uintptr_t)heap_top + incr > SRAM_BASE + SRAM_LENGTH) { - restoreIRQ(state); - r->_errno = ENOMEM; - return (caddr_t)-1; - } - else { - caddr_t res = heap_top; - heap_top += incr; - restoreIRQ(state); - return res; - } + caddr_t res = heap_top; + heap_top += incr; + restoreIRQ(state); + return res; } /** From 879768397e3e21f91275bb594c1a04206713067d Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Wed, 8 Oct 2014 23:14:59 +0200 Subject: [PATCH 2/3] malloc: check if the requested memory is really available --- cpu/cc2538/cc2538_linkerscript.ld | 4 +++- cpu/cc2538/syscalls.c | 20 +++++++++++++------- cpu/nrf51822/nrf51822qfaa_linkerscript.ld | 4 +++- cpu/nrf51822/syscalls.c | 19 +++++++++++++------ cpu/sam3x8e/sam3x8e_linkerscript.ld | 6 ++++-- cpu/sam3x8e/syscalls.c | 21 +++++++++++++-------- cpu/stm32f0/stm32f051r8_linkerscript.ld | 4 +++- cpu/stm32f0/syscalls.c | 19 +++++++++++++------ cpu/stm32f1/stm32f103cb_linkerscript.ld | 4 +++- cpu/stm32f1/stm32f103re_linkerscript.ld | 4 +++- cpu/stm32f1/syscalls.c | 19 +++++++++++++------ cpu/stm32f3/stm32f303vc_linkerscript.ld | 4 +++- cpu/stm32f3/syscalls.c | 19 +++++++++++++------ cpu/stm32f4/stm32f407vg_linkerscript.ld | 4 +++- cpu/stm32f4/stm32f415rg_linkerscript.ld | 4 +++- cpu/stm32f4/syscalls.c | 19 +++++++++++++------ 16 files changed, 119 insertions(+), 55 deletions(-) diff --git a/cpu/cc2538/cc2538_linkerscript.ld b/cpu/cc2538/cc2538_linkerscript.ld index b8313d33f7..ecede73739 100644 --- a/cpu/cc2538/cc2538_linkerscript.ld +++ b/cpu/cc2538/cc2538_linkerscript.ld @@ -130,8 +130,10 @@ SECTIONS _estack = .; } > ram + /* heap section */ . = ALIGN(4); - _end = . ; + _sheap = . ; + _eheap = ORIGIN(ram) + LENGTH(ram); .flashcca : { diff --git a/cpu/cc2538/syscalls.c b/cpu/cc2538/syscalls.c index d1bb148835..f1cdb8491c 100644 --- a/cpu/cc2538/syscalls.c +++ b/cpu/cc2538/syscalls.c @@ -42,8 +42,9 @@ /** * manage the heap */ -extern uint32_t _end; /* address of last used memory cell */ -caddr_t heap_top = (caddr_t) &_end + 4; +extern char _sheap; /* start of the heap */ +extern char _eheap; /* end of the heap */ +caddr_t heap_top = (caddr_t)&_sheap + 4; #ifndef MODULE_UART0 /** @@ -114,18 +115,23 @@ void _exit(int n) * * The current heap implementation is very rudimentary, it is only able to allocate * memory. But it does not - * - check if the returned address is valid (no check if the memory very exists) * - have any means to free memory again * - * TODO: check if the requested memory is really available - * * @return [description] */ -caddr_t _sbrk_r(struct _reent *r, size_t incr) +caddr_t _sbrk_r(struct _reent *r, ptrdiff_t incr) { unsigned int state = disableIRQ(); caddr_t res = heap_top; - heap_top += incr; + + if (((incr > 0) && ((heap_top + incr > &_eheap) || (heap_top + incr < res))) || + ((incr < 0) && ((heap_top + incr < &_sheap) || (heap_top + incr > res)))) { + r->_errno = ENOMEM; + res = (void *) -1; + } else { + heap_top += incr; + } + restoreIRQ(state); return res; } diff --git a/cpu/nrf51822/nrf51822qfaa_linkerscript.ld b/cpu/nrf51822/nrf51822qfaa_linkerscript.ld index d1358af19c..0c2c037cb7 100644 --- a/cpu/nrf51822/nrf51822qfaa_linkerscript.ld +++ b/cpu/nrf51822/nrf51822qfaa_linkerscript.ld @@ -138,6 +138,8 @@ SECTIONS _estack = .; } > ram + /* heap section */ . = ALIGN(4); - _end = . ; + _sheap = . ; + _eheap = ORIGIN(ram) + LENGTH(ram); } diff --git a/cpu/nrf51822/syscalls.c b/cpu/nrf51822/syscalls.c index 30737628e3..b103c3e697 100644 --- a/cpu/nrf51822/syscalls.c +++ b/cpu/nrf51822/syscalls.c @@ -41,8 +41,9 @@ /** * manage the heap */ -extern uint32_t _end; /* address of last used memory cell */ -caddr_t heap_top = (caddr_t)&_end + 4; +extern char _sheap; /* start of the heap */ +extern char _eheap; /* end of the heap */ +caddr_t heap_top = (caddr_t)&_sheap + 4; /** * @brief use mutex for waiting on incoming UART chars @@ -111,18 +112,24 @@ void _exit(int n) * * The current heap implementation is very rudimentary, it is only able to allocate * memory. But it does not - * - check if the returned address is valid (no check if the memory very exists) * - have any means to free memory again * - * TODO: check if the requested memory is really available - * * @return [description] */ caddr_t _sbrk_r(struct _reent *r, ptrdiff_t incr) { unsigned int state = disableIRQ(); caddr_t res = heap_top; - heap_top += incr; + + if (((incr > 0) && ((heap_top + incr > &_eheap) || (heap_top + incr < res))) || + ((incr < 0) && ((heap_top + incr < &_sheap) || (heap_top + incr > res)))) { + r->_errno = ENOMEM; + res = (void *) -1; + } + else { + heap_top += incr; + } + restoreIRQ(state); return res; } diff --git a/cpu/sam3x8e/sam3x8e_linkerscript.ld b/cpu/sam3x8e/sam3x8e_linkerscript.ld index 078b6efb83..e5cbb3cdc3 100644 --- a/cpu/sam3x8e/sam3x8e_linkerscript.ld +++ b/cpu/sam3x8e/sam3x8e_linkerscript.ld @@ -139,6 +139,8 @@ SECTIONS _estack = .; } > ram + /* heap section */ . = ALIGN(4); - _end = . ; -} \ No newline at end of file + _sheap = . ; + _eheap = ORIGIN(ram) + LENGTH(ram); +} diff --git a/cpu/sam3x8e/syscalls.c b/cpu/sam3x8e/syscalls.c index 06edd0400c..0b5aacdecf 100644 --- a/cpu/sam3x8e/syscalls.c +++ b/cpu/sam3x8e/syscalls.c @@ -37,10 +37,9 @@ /** * manage the heap */ -extern uint32_t _end; /* address of last used memory cell */ -caddr_t heap_top = (caddr_t)&_end + 4; - - +extern char _sheap; /* start of the heap */ +extern char _eheap; /* end of the heap */ +caddr_t heap_top = (caddr_t)&_sheap + 4; /** * @brief Initialize NewLib, called by __libc_init_array() from the startup script */ @@ -77,18 +76,24 @@ void _exit(int n) * * The current heap implementation is very rudimentary, it is only able to allocate * memory. But it does not - * - check if the returned address is valid (no check if the memory very exists) * - have any means to free memory again * - * TODO: check if the requested memory is really available - * * @return [description] */ caddr_t _sbrk_r(struct _reent *r, ptrdiff_t incr) { unsigned int state = disableIRQ(); caddr_t res = heap_top; - heap_top += incr; + + if (((incr > 0) && ((heap_top + incr > &_eheap) || (heap_top + incr < res))) || + ((incr < 0) && ((heap_top + incr < &_sheap) || (heap_top + incr > res)))) { + r->_errno = ENOMEM; + res = (void *) -1; + } + else { + heap_top += incr; + } + restoreIRQ(state); return res; } diff --git a/cpu/stm32f0/stm32f051r8_linkerscript.ld b/cpu/stm32f0/stm32f051r8_linkerscript.ld index c48ffa56e9..2ddc74344a 100644 --- a/cpu/stm32f0/stm32f051r8_linkerscript.ld +++ b/cpu/stm32f0/stm32f051r8_linkerscript.ld @@ -137,6 +137,8 @@ SECTIONS _estack = .; } > ram + /* heap section */ . = ALIGN(4); - _end = . ; + _sheap = . ; + _eheap = ORIGIN(ram) + LENGTH(ram); } diff --git a/cpu/stm32f0/syscalls.c b/cpu/stm32f0/syscalls.c index 1f0787e8eb..ffd03ccf53 100644 --- a/cpu/stm32f0/syscalls.c +++ b/cpu/stm32f0/syscalls.c @@ -43,8 +43,9 @@ /** * manage the heap */ -extern uint32_t _end; /* address of last used memory cell */ -caddr_t heap_top = (caddr_t)&_end + 4; +extern char _sheap; /* start of the heap */ +extern char _eheap; /* end of the heap */ +caddr_t heap_top = (caddr_t)&_sheap + 4; #ifndef MODULE_UART0 /** @@ -114,18 +115,24 @@ void _exit(int n) * * The current heap implementation is very rudimentary, it is only able to allocate * memory. But it does not - * - check if the returned address is valid (no check if the memory very exists) * - have any means to free memory again * - * TODO: check if the requested memory is really available - * * @return [description] */ caddr_t _sbrk_r(struct _reent *r, ptrdiff_t incr) { unsigned int state = disableIRQ(); caddr_t res = heap_top; - heap_top += incr; + + if (((incr > 0) && ((heap_top + incr > &_eheap) || (heap_top + incr < res))) || + ((incr < 0) && ((heap_top + incr < &_sheap) || (heap_top + incr > res)))) { + r->_errno = ENOMEM; + res = (void *) -1; + } + else { + heap_top += incr; + } + restoreIRQ(state); return res; } diff --git a/cpu/stm32f1/stm32f103cb_linkerscript.ld b/cpu/stm32f1/stm32f103cb_linkerscript.ld index 67a41bde44..b4cf5bd9b6 100644 --- a/cpu/stm32f1/stm32f103cb_linkerscript.ld +++ b/cpu/stm32f1/stm32f103cb_linkerscript.ld @@ -137,6 +137,8 @@ SECTIONS _estack = .; } > ram + /* heap section */ . = ALIGN(4); - _end = . ; + _sheap = . ; + _eheap = ORIGIN(ram) + LENGTH(ram); } diff --git a/cpu/stm32f1/stm32f103re_linkerscript.ld b/cpu/stm32f1/stm32f103re_linkerscript.ld index 7582df1719..a35f74230e 100644 --- a/cpu/stm32f1/stm32f103re_linkerscript.ld +++ b/cpu/stm32f1/stm32f103re_linkerscript.ld @@ -137,6 +137,8 @@ SECTIONS _estack = .; } > ram + /* heap section */ . = ALIGN(4); - _end = . ; + _sheap = . ; + _eheap = ORIGIN(ram) + LENGTH(ram); } diff --git a/cpu/stm32f1/syscalls.c b/cpu/stm32f1/syscalls.c index 247caa4bb5..b70f0b5358 100644 --- a/cpu/stm32f1/syscalls.c +++ b/cpu/stm32f1/syscalls.c @@ -43,8 +43,9 @@ /** * manage the heap */ -extern uint32_t _end; /* address of last used memory cell */ -caddr_t heap_top = (caddr_t)&_end + 4; +extern char _sheap; /* start of the heap */ +extern char _eheap; /* end of the heap */ +caddr_t heap_top = (caddr_t)&_sheap + 4; #ifndef MODULE_UART0 /** @@ -114,18 +115,24 @@ void _exit(int n) * * The current heap implementation is very rudimentary, it is only able to allocate * memory. But it does not - * - check if the returned address is valid (no check if the memory very exists) * - have any means to free memory again * - * TODO: check if the requested memory is really available - * * @return [description] */ caddr_t _sbrk_r(struct _reent *r, ptrdiff_t incr) { unsigned int state = disableIRQ(); caddr_t res = heap_top; - heap_top += incr; + + if (((incr > 0) && ((heap_top + incr > &_eheap) || (heap_top + incr < res))) || + ((incr < 0) && ((heap_top + incr < &_sheap) || (heap_top + incr > res)))) { + r->_errno = ENOMEM; + res = (void *) -1; + } + else { + heap_top += incr; + } + restoreIRQ(state); return res; } diff --git a/cpu/stm32f3/stm32f303vc_linkerscript.ld b/cpu/stm32f3/stm32f303vc_linkerscript.ld index a724def1d3..3b7de69c22 100644 --- a/cpu/stm32f3/stm32f303vc_linkerscript.ld +++ b/cpu/stm32f3/stm32f303vc_linkerscript.ld @@ -138,6 +138,8 @@ SECTIONS _estack = .; } > ram + /* heap section */ . = ALIGN(4); - _end = . ; + _sheap = . ; + _eheap = ORIGIN(ram) + LENGTH(ram); } diff --git a/cpu/stm32f3/syscalls.c b/cpu/stm32f3/syscalls.c index 4f89a0e7e3..f78f80f034 100644 --- a/cpu/stm32f3/syscalls.c +++ b/cpu/stm32f3/syscalls.c @@ -44,8 +44,9 @@ /** * @brief manage the heap */ -extern uint32_t _end; /* address of last used memory cell */ -caddr_t heap_top = (caddr_t)&_end + 4; +extern char _sheap; /* start of the heap */ +extern char _eheap; /* end of the heap */ +caddr_t heap_top = (caddr_t)&_sheap + 4; #ifndef MODULE_UART0 /** @@ -114,18 +115,24 @@ void _exit(int n) * * The current heap implementation is very rudimentary, it is only able to allocate * memory. But it does not - * - check if the returned address is valid (no check if the memory very exists) * - have any means to free memory again * - * TODO: check if the requested memory is really available - * * @return [description] */ caddr_t _sbrk_r(struct _reent *r, ptrdiff_t incr) { unsigned int state = disableIRQ(); caddr_t res = heap_top; - heap_top += incr; + + if (((incr > 0) && ((heap_top + incr > &_eheap) || (heap_top + incr < res))) || + ((incr < 0) && ((heap_top + incr < &_sheap) || (heap_top + incr > res)))) { + r->_errno = ENOMEM; + res = (void *) -1; + } + else { + heap_top += incr; + } + restoreIRQ(state); return res; } diff --git a/cpu/stm32f4/stm32f407vg_linkerscript.ld b/cpu/stm32f4/stm32f407vg_linkerscript.ld index 5517745479..857d99db60 100644 --- a/cpu/stm32f4/stm32f407vg_linkerscript.ld +++ b/cpu/stm32f4/stm32f407vg_linkerscript.ld @@ -139,6 +139,8 @@ SECTIONS _estack = .; } > ram + /* heap section */ . = ALIGN(4); - _end = . ; + _sheap = . ; + _eheap = ORIGIN(ram) + LENGTH(ram); } diff --git a/cpu/stm32f4/stm32f415rg_linkerscript.ld b/cpu/stm32f4/stm32f415rg_linkerscript.ld index 5517745479..857d99db60 100644 --- a/cpu/stm32f4/stm32f415rg_linkerscript.ld +++ b/cpu/stm32f4/stm32f415rg_linkerscript.ld @@ -139,6 +139,8 @@ SECTIONS _estack = .; } > ram + /* heap section */ . = ALIGN(4); - _end = . ; + _sheap = . ; + _eheap = ORIGIN(ram) + LENGTH(ram); } diff --git a/cpu/stm32f4/syscalls.c b/cpu/stm32f4/syscalls.c index 2d4508ca3f..67ccff6b48 100644 --- a/cpu/stm32f4/syscalls.c +++ b/cpu/stm32f4/syscalls.c @@ -43,8 +43,9 @@ /** * manage the heap */ -extern uint32_t _end; /* address of last used memory cell */ -caddr_t heap_top = (caddr_t)&_end + 4; +extern char _sheap; /* start of the heap */ +extern char _eheap; /* end of the heap */ +caddr_t heap_top = (caddr_t)&_sheap + 4; #ifndef MODULE_UART0 /** @@ -114,18 +115,24 @@ void _exit(int n) * * The current heap implementation is very rudimentary, it is only able to allocate * memory. But it does not - * - check if the returned address is valid (no check if the memory very exists) * - have any means to free memory again * - * TODO: check if the requested memory is really available - * * @return [description] */ caddr_t _sbrk_r(struct _reent *r, ptrdiff_t incr) { unsigned int state = disableIRQ(); caddr_t res = heap_top; - heap_top += incr; + + if (((incr > 0) && ((heap_top + incr > &_eheap) || (heap_top + incr < res))) || + ((incr < 0) && ((heap_top + incr < &_sheap) || (heap_top + incr > res)))) { + r->_errno = ENOMEM; + res = (void *) -1; + } + else { + heap_top += incr; + } + restoreIRQ(state); return res; } From 39d73d753c9aa064c2ee6c297782c830c8b72676 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Wed, 8 Oct 2014 23:21:13 +0200 Subject: [PATCH 3/3] resurrect malloc test --- tests/malloc/Makefile | 18 ++++++++++ tests/malloc/main.c | 82 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 tests/malloc/Makefile create mode 100644 tests/malloc/main.c diff --git a/tests/malloc/Makefile b/tests/malloc/Makefile new file mode 100644 index 0000000000..ccc0417173 --- /dev/null +++ b/tests/malloc/Makefile @@ -0,0 +1,18 @@ +# name of your application +APPLICATION = test_malloc + +# If no BOARD is found in the environment, use this default: +BOARD ?= native + +# This has to be the absolute path to the RIOT base directory: +RIOTBASE ?= $(CURDIR)/../.. + +# Comment this out to disable code in RIOT that does safety checking +# which is not needed in a production environment but helps in the +# development process: +CFLAGS += -DDEVELHELP + +# Change this to 0 show compiler invocation lines by default: +QUIET ?= 1 + +include $(RIOTBASE)/Makefile.include diff --git a/tests/malloc/main.c b/tests/malloc/main.c new file mode 100644 index 0000000000..0af810345e --- /dev/null +++ b/tests/malloc/main.c @@ -0,0 +1,82 @@ +/* + * Copyright (C) 2013 Benjamin Valentin + * + * This file is subject to the terms and conditions of the GNU Lesser General + * Public License v2.1. See the file LICENSE in the top level directory for more + * details. + */ + +/** + * @ingroup tests + * @{ + * + * @file + * @brief Simple malloc/free test + * + * + * @author Benjamin Valentin + * + * @} + */ + +#include +#include +#include + +#define CHUNK_SIZE 1024 + +struct node { + struct node *next; + void *ptr; +}; + +int total = 0; + +void fill_memory(struct node *head) +{ + while (head && (head->ptr = malloc(CHUNK_SIZE))) { + printf("Allocated %d Bytes at 0x%p, total %d\n", CHUNK_SIZE, head->ptr, total += CHUNK_SIZE); + memset(head->ptr, '@', CHUNK_SIZE); + head = head->next = malloc(sizeof(struct node)); + head->ptr = 0; + head->next = 0; + total += sizeof(struct node); + } +} + +void free_memory(struct node *head) +{ + struct node *old_head; + + while (head) { + if (head->ptr) { + printf("Free %d Bytes at 0x%p, total %d\n", CHUNK_SIZE, head->ptr, total -= CHUNK_SIZE); + free(head->ptr); + } + + if (head->next) { + old_head = head; + head = head->next; + free(old_head); + } + else { + free(head); + head = 0; + } + + total -= sizeof(struct node); + } +} + +int main(void) +{ + while (1) { + struct node *head = malloc(sizeof(struct node)); + total += sizeof(struct node); + + fill_memory(head); + free_memory(head); + } + + return 0; +}