RFC: libc: thread-safe newlib

[kaidoho]

Introduction

Extend Zephyr to be able to use the C standard library newlib in a multi-threaded environment.

Problem description

The minimum requirement to use libstdc++ in an RTOS environment is to have a thread-safe C library at hand – which is, from my point of view, regarding Zephyr’s adoption of newlib not the case today.

Proposed change

Implement the hooks provided by newlib to allow its use in a thread-safe manner.

Detailed RFC

Despite the fact that there's a scheduler, none of the requirements to use newlib in a multi-threaded environment have been fullfilled.

Reentrancy

Newlib manages multi-threaded applications with the help of structs of type _reent, which serve as a constant data container between library calls. In addition to that, stubs to allow an RTOS to implement reentrant versions of the standard functions (open_r, write_r, etc.) are provided. Simplified, in a multi-threaded environment each thread should have its own _reent struct. It should then either call the reentrant library functions and pass it's _reent struct or set up the global _reent struct impure_ptr and use the non-reentrant library functions.

Today, the non-reentrant library functions are used and all threads operate on the same global _reent struct impure_ptr defined within newlib.

Malloc Hooks

Issue #17552 aims to provide thread-safety for malloc by adding a semaphore within sbrk. Newlib has addressed this issue by providing the hooks __malloc_lock and __malloc_unlock. The advantage of implementing them is that they guard also the calling code, not only sbrk.

Retarget Locking

Newlib internally uses mutex synchronization quite often. At the moment empty stubs are used as Zephyr does not provide an implementation. Therefore, there’s no synchronization at all. The stubs can be replaced at application build time if newlib was build with --enable-newlib-retargetable-locking. In such a scenario several __retarget_lock_* functions to create, delete, and operate recursive and non-recursive mutex become available for reimplementation. In addition to the mutex created during runtime by newlib one has to provide nine static locks.

GNU ARM Embedded toolchain is build using the switch above, but Zephyr SDK is not doing that yet.

Proposed change (Detailed)

Reentrancy

According to the documentation of newlib there are two ways to achieve reentrancy. Both require each thread of execution control to initialize a unique global variable of type struct _reent:

1. Use the reentrant versions of the library functions, after initializing a global reentrancy structure for each process. Use the pointer to this structure as the extra argument for all library functions.
2. Ensure that each thread of execution control has a pointer to its own unique reentrancy structure in the global variable _impure_ptr, and call the standard library subroutines.

Option 1 would require us to always call the reentrant functions such as malloc_r instead of malloc. On the one hand, this has to be explained to every user to make sure they call the right functions. On the other hand, 3rd party code can not be simply dropped in without modifications. Therefore, I propose to implement option 2.

Proposed change:

• Add reent struct to each k_thread struct
• Initialize thread's reent struct
• Switch impure_ptr to point to thread's reent struct after context switch

Malloc Hooks / Retarget Locking

Proposed change:

• Remove the semaphore from sbrk
• If newlib was compiled for retargetable locking, a define _RETARGETABLE_LOCKING is present. Depending on this define make either
  • __malloc_lock and __malloc_unlock or
  • the __retarget_lock_* functions visible and add the static locks (there's one for malloc, too)
• Some memory is required to store the locks created during runtime
  • Add a CONFIG_ to adjust the size of the memory to be reserverd
  • If the size is set to 0 (default), fall back and use only the __malloc_lock and __malloc_unlock
  • Area is defined as sys_mem_pool

Dependencies

• Newlib build with --enable-newlib-retargetable-locking
• Optional --enable-newlib-reent-small (AFAIK GNU ARM Embedded + Zephyr SDK have that set)

Concerns and Unresolved Questions

General

• I am wondering why this hasn't been addessed already. Either simply nobody knew / cared, or I've taken a terribly wrong road somewhere.

Implementation

• I haven’t dived into the Zephyr Kernel before, so I might have placed code at places where it doesnt belong. Is arch_swap the right place for setting up impure_ptr after context switch? (Done only for ARM until there’s some feedback.)
• Carrying struct reent around within k_thread has obviously some impact on memory consumption, but I have no better idea (except saying "Well, use reent_small")
• The non recursive mutex are implemented using a semaphore, is there something lightweight which could be used for the recursive mutex?
• Haven't looked into errno - think there's a per thread errno within reent

Implementation

PR #21518

Relevant Links

Documentation

Newlib Documentation on __malloc_lock

Newlib Documentation on Reentrancy

Mailing Lists

Newlib discussion on --enable-newlib-retargetable-locking

[PATCH, newlib] Allow locking routine to be retargeted

What are the __retarget_lock functions?

Alternatives

Go the RTEMS - way and extend newlib / GCC with a port for Zephyr. That would also give us the opportunity to have OpenMP and a thread-safe libstdc++, which would be striking.

[stephanosio]

For the build options used by the GNU ARM Embedded newlib, refer to the following comment:
zephyrproject-rtos/sdk-ng#152 (comment)

[stephanosio]

• Newlib build with --enable-newlib-retargetable-locking

I see that crosstool-ng, which is used to build newlib for the Zephyr SDK, does not provide a Kconfig option to enable --enable-newlib-retargetable-locking. An upstream patch to crosstool-ng will be necessary for this. => DONE crosstool-ng/crosstool-ng#1284, zephyrproject-rtos/sdk-ng#166

https://github.com/crosstool-ng/crosstool-ng/blob/master/config/libc/newlib.in

• Optional --enable-newlib-reent-small (AFAIK GNU ARM Embedded + Zephyr SDK have that set)

It looks like the GNU ARM Embedded enables --enable-newlib-reent-small for the newlib nano variant only.

This option is specified through crosstool-ng config LIBC_NEWLIB_REENT_SMALL and it defaults to n, so the current Zephyr SDK (0.10.3) should not have this enabled.

For the Zephyr SDK 0.11.0 release, it will have the same config as the GNU ARM Embedded (i.e. --enable-newlib-reent-small enabled for the nano variant only).

[andrewboie]

@kaidoho

I am wondering why this hasn't been addessed already. Either simply nobody knew / cared

These bindings are old, I've touched this file a lot and the basic implementation pre-dates my involvement in the project, this is code from when the OS was closed source and known as Viper. So file under "nobody knew" probably.

Carrying struct reent around within k_thread has obviously some impact on memory consumption

People who are on really constrained systems will usually use minimal libc anyway.

Haven't looked into errno - think there's a per thread errno within reent

We currently do errno in a thread-local storage area carved out of the stack buffer. The os function is z_errno(). This gets mapped onto newlib hook __errno. See kernel/errno.c and newlib libc-hooks. We need a better thread-local storage implementation in Zephyr, right now to get at it you have to make a system call and we don't support __thread.

The non recursive mutex are implemented using a semaphore, is there something lightweight which could be used for the recursive mutex?

A one-count semaphore is the right thing to do if you don't need recursive entry or priority inheritance.

Today, the non-reentrant library functions are used and all threads operate on the same global _reent struct impure_ptr defined within newlib.

This doesn't seem SMP-safe at first glance.

Is arch_swap the right place for setting up impure_ptr after context switch? (Done only for ARM until there’s some feedback.)

Yeah this has to be in the arch code if we want this updated on context switch. For example if you take an interrupt, do something in it which causes a new thread to be runnable at IRQ exit arch_swap gets called directly from the arch code, no calls are made into common code (although perhaps we can change this, we could move the z_check_stack_sentinel invocations there too)

For arches that use CONFIG_SWITCH, you might be able to put the hook in z_get_next_switch_handle().

[andrewboie]

I feel like this should be reclassified as a bug, @nashif do you agree?

[stephanosio]

The newlib thread safety issue will be fully addressed by the PR #36201, which implements the retargetable locking interface to provide locks for the newlib, and this issue can be closed once that PR is merged.

As for the reentrancy, it is really a non-issue at the moment because the current Zephyr-to-newlib integration scheme does not support any features that actually require reentrancy support, and multiple threads can safely call the newlib functions without it.

The reentrancy support will be required in the future when we implement more advanced form of newlib integration (e.g. newlib-provided POSIX functions, newlib-provided errno, ...). That requires implementing native Zephyr RTOS support in the newlib and will be addressed as part of zephyrproject-rtos/sdk-ng#350.