malloc unexpected behaviour on GCC_ARM compiler

[maciejbocianski]

Description

• Type: Bug
• Priority: Major

---

Bug

Target
all

Toolchain:
GCC_ARM

Toolchain version:
gcc version 6.3.1 20170620 (release) [ARM/embedded-6-branch revision 249437] (GNU Tools for ARM Embedded Processors 6-2017-q2-update)

mbed-cli version:
1.2.0

mbed-os sha:
38ba693

Expected behavior
When try to allocate more memory than heap can handle we get NULL from malloc and further allocations (within heap size) work fine

Actual behavior
When try to allocate more memory than heap can handle we get NULL from malloc and any further allocation (within heap size) return NULL

Steps to reproduce

Compile and run below code
Program find maximum memory which malloc can allocate

extern unsigned char *mbed_heap_start;
extern uint32_t mbed_heap_size;
uint32_t max_mbed_heap_size = mbed_heap_size;

int main()
{
	void *data = NULL;

	while(true)
	{
		data = malloc(max_mbed_heap_size);
		if(data || max_mbed_heap_size == 0) {
			break;
		} else {
			max_mbed_heap_size--;
		}
	}

	printf("max_mbed_heap_size: %lu \r\n", max_mbed_heap_size);
	printf("mbed_heap_start: %X \r\n", mbed_heap_start);
	printf("mbed_heap_size: %lu \r\n", mbed_heap_size);
	printf("data: %p \r\n\r\n", data);
}

Output from NUCLEO_F070RB

GCC_ARM
max_mbed_heap_size: 0
mbed_heap_start: 2000275C
mbed_heap_size: 5284
data: 0x0

ARM
max_mbed_heap_size: 6556
mbed_heap_start: 2000224C
mbed_heap_size: 6580
data: 20002260

IAR
max_mbed_heap_size: 4044
mbed_heap_start: 200006E8
mbed_heap_size: 4096
data: 200006f0

Test program 2

extern unsigned char *_sbrk_ret_1;
extern unsigned char *_sbrk_ret_2;
extern int _sbrk_incr;
extern uint32_t _sbrk__end__;
extern unsigned char *_sbrk_prev_heap;
extern unsigned char *_sbrk_new_heap;

extern unsigned char *mbed_heap_start;
extern uint32_t mbed_heap_size;
const uint32_t size = 100;
uint32_t max_mbed_heap_size = 0;

int main()
{
	void *data = NULL;

	printf("*** main begin ***\r\n");
	printf("\r\n");
	printf("_sbrk_incr: %i\r\n", _sbrk_incr);
	printf("_sbrk__end__: %lu\r\n", _sbrk__end__);
	printf("prev_heap: %X\r\n", _sbrk_prev_heap);
	printf("new_heap: %X\r\n\r\n", _sbrk_new_heap);

	printf("*** test begin ***\r\n");
	while(max_mbed_heap_size <= mbed_heap_size)
	{
		_sbrk_ret_1 = 0;
		_sbrk_ret_2 = 0;
		_sbrk_incr = 12345;
		_sbrk__end__ = 12345;
		_sbrk_prev_heap = NULL;
		_sbrk_new_heap = NULL;

		void *tmp_data = malloc(size);

		if(_sbrk_incr != 12345)
			printf("_sbrk_incr: %i\r\n", _sbrk_incr);
		if(_sbrk__end__ != 12345)
			printf("_sbrk__end__: %lu\r\n", _sbrk__end__);
		if(_sbrk_prev_heap != NULL)
			printf("prev_heap: %X\r\n", _sbrk_prev_heap);
		if(_sbrk_new_heap != NULL)
			printf("new_heap: %X\r\n", _sbrk_new_heap);
		if(_sbrk_ret_1 != NULL)
			printf("_sbrk_ret_1: %X\r\n", _sbrk_ret_1);
		if(_sbrk_ret_2 != NULL)
			printf("_sbrk_ret_2: %X\r\n", _sbrk_ret_2);

		if(NULL == tmp_data) {
			break;
		} else {
			data = tmp_data;
			max_mbed_heap_size += size;
			printf("allocated: %lu\r\n", size);
			printf("allocated total: %lu\r\n", max_mbed_heap_size);
			printf("\r\n");
		}
	}
	printf("*** test end ***\r\n\r\n");

	printf("max_mbed_heap_size: %lu \r\n", max_mbed_heap_size);
	printf("mbed_heap_start: %X \r\n", mbed_heap_start);
	printf("mbed_heap_size: %lu \r\n", mbed_heap_size);
	printf("data: %p \r\n", data);
	printf("__get_MSP(): %X \r\n", __get_MSP());
	printf("*** main end ***\r\n");
	printf("\r\n");
}

Test program 2 additional changes in platform/mbed_retarget.cpp

diff --git a/platform/mbed_retarget.cpp b/platform/mbed_retarget.cpp
index 6c5f12031..5d5b323a8 100644
--- a/platform/mbed_retarget.cpp
+++ b/platform/mbed_retarget.cpp
@@ -744,24 +744,37 @@ extern "C" caddr_t _sbrk(int incr) {
 }
 #else
 // Linker defined symbol used by _sbrk to indicate where heap should start.
+unsigned char * _sbrk_ret_1 = 0;
+unsigned char * _sbrk_ret_2 = 0;
+int _sbrk_incr = 12345;
+uint32_t _sbrk__end__ = 12345;
+unsigned char *_sbrk_prev_heap = (unsigned char*)0x12345;
+unsigned char *_sbrk_new_heap = (unsigned char*)0x12345;
 extern "C" uint32_t __end__;
 extern "C" caddr_t _sbrk(int incr) {
     static unsigned char* heap = (unsigned char*)&__end__;
     unsigned char*        prev_heap = heap;
     unsigned char*        new_heap = heap + incr;

+    _sbrk_incr = incr;
+    _sbrk__end__ = __end__;
+    _sbrk_prev_heap = prev_heap;
+    _sbrk_new_heap = new_heap;
+
 #if defined(TARGET_CORTEX_A)
     if (new_heap >= (unsigned char*)&__HeapLimit) {     /* __HeapLimit is end of heap section */
 #else
     if (new_heap >= (unsigned char*)__get_MSP()) {
 #endif
         errno = ENOMEM;
+        _sbrk_ret_1 = new_heap;
         return (caddr_t)-1;
     }

     // Additional heap checking if set
     if (mbed_heap_size && (new_heap >= mbed_heap_start + mbed_heap_size)) {
         errno = ENOMEM;
+        _sbrk_ret_2 = new_heap;
         return (caddr_t)-1;
     }

[0xc0170]

@maciejbocianski we implement sbrk for gcc in the retarget code file. What sbrk returns if you pass 0 (returns the current program break value) before the failing and after?

cc @bulislaw @c1728p9
[Mirrored to Jira]

[maciejbocianski]

@0xc0170 @bulislaw @c1728p9
Regarding our friday conversation
I did some test with malloc and _sbrk

Below output from "Test program 2" run on NUCLEO_F070RB (program code in issue description)

We can see that on main enter there is 1140B already requested by _sbrk
During the test we allocate 10x100B and then at 11th loop iteration malloc request another memory _sbrk_incr: 4096 and this request fails since new_heap(20004000) >= __get_MSP(20003FC8)

The question is if we can force malloc to request memory in smaller chunks than 4kB at least for platforms with small RAM?

*** main begin ***

_sbrk_incr: 1140
_sbrk__end__: 521
prev_heap: 20002B8C
new_heap: 20003000

*** test begin ***
allocated: 100
allocated total: 100

allocated: 100
allocated total: 200

allocated: 100
allocated total: 300

allocated: 100
allocated total: 400

allocated: 100
allocated total: 500

allocated: 100
allocated total: 600

allocated: 100
allocated total: 700

allocated: 100
allocated total: 800

allocated: 100
allocated total: 900

allocated: 100
allocated total: 1000

_sbrk_incr: 4096
_sbrk__end__: 521
prev_heap: 20003000
new_heap: 20004000
_sbrk_ret_1: 20004000
*** test end ***

max_mbed_heap_size: 1000
mbed_heap_start: 20002774
mbed_heap_size: 5260
data: 0x20002f30
__get_MSP(): 20003FC8
*** main end ***

[Mirrored to Jira]

[c1728p9]

The question is if we can force malloc to request memory in smaller chunks than 4kB at least for platforms with small RAM?

The behavior I saw when I looked at this a while back was that calls to _sbrk make requests up to a 4k boundary. In the above case:

0x20002B8C + 1140-> 0x20003000
0x20003000 + 0x1000 -> 0x20004000

If the interrupt stack was moved to come before the heap then the end of the heap would be on a 4k boundary, and in theory you would make use of the whole heap. I'm not sure what it would take to make this change though.
[Mirrored to Jira]

[maciejbocianski]

other question is why first allocation likely performed by printf is not 4kB but 1140B?
[Mirrored to Jira]

[c1728p9]

@maciejbocianski the first allocation of 1140B brings the heap limit to a 4k boundary 0x20002B8C + 1140 -> 0x20003000.
[Mirrored to Jira]

[maciejbocianski]

Looks like by this 4kB alignment we lose 3kB of heap, what is huge problem for platforms with small RAM (we can utilize not much more than 2kB at least for NUCLEO_F070RB)

Regarding interrupt stack as it can be read here #1561 it was intentionally put on the RAM end.
CC @adamgreen
[Mirrored to Jira]

[jeromecoutant]

Any update about this issue ?
Thx
[Mirrored to Jira]

[bulislaw]

Right now I don't think we have a path forward, more investigation is needed.
[Mirrored to Jira]

[cmonr]

@bulislaw @jeromecoutant @maciejbocianski @c1728p9 Has anyone continued to look at this? Is this still a major bug?
[Mirrored to Jira]

[adamgreen]

You could upgrade to GCC ARM Embedded 6-2017-q1-update or newer and link with newlib-nano instead of the full newlib Standard C Library. That version of newlib-nano synchronizes multithreaded access to the heap. newlib-nano doesn't do this 4k paging as it is the version of newlib meant for small memory devices like this.
[Mirrored to Jira]

[jeromecoutant]

That version of newlib-nano synchronizes multithreaded access

This means that the restriction to forbid thread with nano gcc lib doesn't exist anymore ?
See #4885

[Mirrored to Jira]

[bulislaw]

@cmonr I have a task on me to look into it.
[Mirrored to Jira]