newlib/malloc locking mechanism to be thread-safe

[mattias norlander]

Several users have over the years rightfully complained about newlib/malloc not being thread-safe. This problem does not present itself until the code is executed on target and is quite difficult to debug.

Below we will show a solution for this, but first let's elaborate on the problem. To describe the problems, lets consider two common Use Cases (UCs):

UC1: Bare-metal or RTOS application with malloc in interrupts

Even though quite bad coding practice, nothing forbids the user from performing dynamic memory allocation from inside an interrupt service routine (ISR).

Imagine that the application is executing FunctionA in (non-ISR-context) application code.

FunctionA makes a call to malloc to allocate some memory and while this happens, the CPU receives an interrupt request and thereby branches to associated ISR. Inside the ISR, malloc is again called to allocate some memory.

This will lead to memory corruption since the first call to malloc performed in FunctionA, is quite likely to be partly or totally overwritten by the second call to malloc from the ISR. FunctionA or some other function sharing the intended global data will likely crash at a later point during application execution.

UC2: RTOS application with malloc in several threads

In this use case an application is running FreeRTOS (or another RTOS). There are two or more threads which both contains calls to malloc or any other function which implicitly uses malloc.

ThreadA makes a call to malloc. The scheduler at this point switches context to ThreadB which is allowed to execute and make a second call to malloc.

As described in UC1 the same problem will now arise. The first malloc call will be corrupted by the second and as a result the application is likely to crash or behave in an unexpected way at a later point in time, making the root-cause problem all the more difficult to identify.

Summary of problems:

newlib/malloc is not thread-safe. Using newlib/malloc in ISRs or in RTOS threads is not safe by default. The user may be further confused by that malloc is also called by some other newlib functions such as strtok.

To support UC1 in the bare-metal case and to support UC2 in the RTOS case, we provide an implementation of locks for newlib/malloc.

CAUTION / DISCLAIMER:

It is very important to state that this implementation of locks for newlib/malloc will temporarily disable interrupts and thereby effect the real-time behavior of your application!

Please carefully consider the implications of disabling interrupts in your application before adopting the lock mechanisms.

The new approach is safe-by-default in terms of re-entrance from ISR and using RTOS threads, at the cost of real-time penalty.

How to use the new implementation of locks for newlib/malloc:

1. Download the zip-file linked with this post.

2. Extract and drop/replace the sysmem.c in a source folder of your project.

3. sysmem.c has a dependency: #include “cmsis_compiler.h�?. This file is typically generated as part of a STM32CubeMX generated projects. It is however not part of the generated code in an Empty project. The CMSIS files can be copied from the MCU corresponding STM32Cube firmware package. Typically located in the User folder on your OS:

           STM32Cube\Repository\STM32Cube_FW_X_Y\Drivers\CMSIS\Core\Include

4. By default the symbol __NEWLIB_SAFE_FROM_INTERRUPT__ is defined in sysmem.c which will disable interrupts while executing malloc. Consider if this is desirable in your application. If not desirable, comment out the symbol.

           If __NEWLIB_SAFE_FROM_INTERRUPT__ is defined, all interrupts are disabled, even in an RTOS use case, when malloc is called.

           If __NEWLIB_SAFE_FROM_INTERRUPT__ is undefined, application will hang if malloc is called from an interrupt and RTOS locks will be used if they are defined (see 6).

5. main.h

           a) If no main.h exists, extract and drop it in some Inc folder of your project.

           b) If main.h exists in your project then simply copy the necessary defines from the downloaded main.h into your main.h

6. If your application uses an RTOS you will need to define RTOS_SUSPEND_ALL_TASKS and RTOS_RESUME_ALL_TASKS in main.h to gain task safe malloc

           For FreeRTOS define them as follows:

                       #define RTOS_SUSPEND_ALL_TASKS vTaskSuspendAll

                       #define RTOS_RESUME_ALL_TASKS xTaskResumeAll

Please download the new sysmem.c attached to this post.

This new sysmem.c is, at this point, only distributed here on the forum. This sysmem.c version has not yet been made part of the code generated by STM32CubeIDE, STM32CubeMX nor is it bundled in the CubeFW pacakages.

A shout-out to Dave Nadler for his vigorous work on helping the community to understand and overcome the problem with newlib/malloc not being thread-safe.

Feel free to submit questions and feedback in this forum thread!

[Dave Nadler]

Thanks @mattias norlander​ .

For anyone else looking at this, you might want to read this first:

http://www.nadler.com/embedded/newlibAndFreeRTOS.html

Some clarifications to the above post are in order.

newlib malloc memory is absolutely not unsafe as Mattias states above.

ST's implementation is unsafe because it fails to wrap newlib as required, and clearly explained in newlib's documentation as well as my web page.

newlib's malloc family management family family are completely thread-safe if and only if:

1) newlib is built with the correct options.

Check newlib.h must define _RETARGETABLE_LOCKING (ST distribution is OK)

2) the malloc locking protocol is implemented by implementing __malloc_lock/unlock.

This is all well-described in the newlib documentation here:

https://sourceware.org/newlib/libc.html#g_t_005f_005fmalloc_005flock

Alternatively (from 3.0 newlib?) newlib's default malloc_lock uses the lock API defined in sys/lock.h;

one can implement that protocol and get locking for malloc, env, and other newlib subsystems.

ST failed here.

3) sbrk is correctly implemented

ST failed at this as well.

From a quick look at the code Mattias has posted, there are some issues:

1. For god's sake, please improve the commentary, and explain the purpose of the routines you are implementing, not just the function. And include references to the supporting documentation in newlib etc.
2. sbrk as you implemented is not going to work: You are returning memory in the same area used by the ISR stack (MSP) in any interrupt routine!!! That is going to crash pretty quickly in any non-trivial application, right?? I gather this has not been tested??
3. If you implement the API in sys/lock.h, you don't need to re-implement each of the malloc/env/etc lock/unlock pairs (I didn't do this).
4. I'm not sure why you are re-implementing critical sections. You've missed some important bits regarding priority; please don't incorrectly re-implement the wheel. Please see FreeRTOS taskENTER_CRITICAL and the ISR versions which implement critical sections correctly.

@mattias norlander​ - I really do not understand why you are trying to re-implement the wheel (and doing it incorrectly).

I provided ST a working implementation something like 9 months ago.

Please just use it!

Thanks,

Best Regards, Dave

PS: CC @Amel NASRI​, @Camilo LEMOS​ 

PPS: Anyone else, please use the code from my web site; the code provided above will crash.

[Dave Nadler]

PS: @Herve PIERROT​ , @Ethan HUANG​ , @Pavel A.​ - sorry I forgot to CC you as well.

[Dave Nadler]

@Piranha​ - You might want to review this as well.

Best Regards, Dave

[Piranha]

I completely agree to all that Dave said and can add that this issue is directly related to this one:

https://community.st.com/s/question/0D50X0000C5Tns8SQC/bug-stm32-hal-driver-lock-mechanism-is-not-interrupt-safe

HAL should implement critical sections for non-RTOS environment and use RTOS provided ones for RTOS environment. And then Newlib glue file must be implemented based on that HAL critical section abstraction.

[Pavel A.]

> HAL should implement critical sections for non-RTOS environment and use RTOS provided ones for RTOS environment.

This would be nice but please consider compatibility issues for existing users who designed their workarounds long ago.

At least, all significant or breaking changes in the libraries should be #ifdef'ed and documented.

ST already has AN on FreeRTOS use, it is a prefect place to document these issues.

Can anybody comment on changes in multi-threading support between newlib v3.x (distributed with CubeIDE toolchain) compared to 2.3 (Atollic tolchain)?

Regards,

-- pa

[mattias norlander]

Thanks for the feedback! It is being reviewed.

This takes time because there are several teams involved providing several different tools and we try to make sure that the solution works well in the full ecosystem.

[Nikita91]

The proposed implementation is specific to FreeRTOS: somes RTOS can't suspend/resume all tasks.

The universal solution is to use a reentrant mutex (for large functions like malloc) and a critical section for very small one.

LWIP use lightweight OS independant routines in sys_arch.c. The interface is so simple, it can be implemented by any OS.

ST could be inspired by this architecture.

Please don't use suspend/resume all tasks, the RTOS we use doesn't allow this!

But our malloc/free is thread safe.

Please don't reimplement critical sections: all OS and many bare metal application already have theirs.

It can be difficult to make two mechanisms of critical sections coexist.

Again, the LWIP solution is better: let the user implement these functions.

[wolfgang wayalnd]

For what it's worth mattias norlander "newlib/malloc locking mechanism to be thread-safe"

FreeRTOS printf/floats solution WORKS!

THANKS!!!

[Dave Nadler]

@wolfgang wayalnd​ - Be careful with this; please note the caveats and bugs discussed above.

See: https://community.st.com/s/question/0D50X0000BB1eL7SQJ/bug-cubemx-freertos-projects-corrupt-memory