Skip to content
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

Use newlib-nano headers, FreeRTOS _impure_ptr hooks #2126

Merged
merged 15 commits into from Jun 23, 2020
Merged

Conversation

avtolstoy
Copy link
Member

Problem

When building DeviceOS with ARM GCC toolchain, we link with a variant of newlib (an implementation of C standard library) called nano, which has certain trade-offs to reduce the code/RAM size.

In order to support thread-safety for a number of libc functions, we are allocating a so-called reentrant structure for each thread. This is a FreeRTOS feature which we have enabled on all the platforms because we've seen various crashes due to absence of that.

There is currently a number of problems:

  1. We are only linking with nano version of newlib, however when building we are not specifying that in CFLAGS, which causes us to use incorrect include paths, which increases size of some of the libc structures. One of them is as you would expect the reentrant structure mentioned above. With nano version it's ~100 bytes, with full libc variant it's over a 1000. We are erroneously allocating more than needed and on top of that the nano version of newlib would allocate even more things dynamically as needed
  2. WICED, see particle-iot/photon-wiced#36
  3. Incompatibility between newlibs in WICED and DeviceOS (see particle-iot/photon-wiced#35)
  4. Each DeviceOS module (system-part1,2,3, user module) has its own reentrant structure and any module except for the system part where FreeRTOS resides does not actually have thread safety if some libc function is not being used through dynalib, as the module-local _impure_ptr pointer residing in RAM will not get changed on thread context switches. We need to add hooks into other modules, so that we can do that from the system part with FreeRTOS.

Solution

  1. We are using --specs=nano.specs in CFLAGS now, to ensure that not only we are linking with nano variant of the newlib, but also that we are using the correct include paths when building DeviceOS
  2. See particle-iot/photon-wiced#36
  3. Necessary changes to support particle-iot/photon-wiced#35
  4. We have added hooks into FreeRTOS and necessary dynalib functions to ensure that on FreeRTOS context switches we are modifying _impure_ptr accordingly in all the modules

This PR also adds appropriate tests to wiring/no_fixture and moves SPI tests out of that test suite into a separate wiring/no_fixture_spi in order to fit 128K on Electron.

Steps to Test

  • wiring/no_fixture
  • Test Gen 2 (Photon/P1/Electron) and at least on Gen 3 platform
  • Step-by-step OTA/ymodem update from 1.5.x

Example App

N/A

References

  • [CH56479]
  • particle-iot/photon-wiced#35
  • particle-iot/photon-wiced#36

Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

hal/src/electron/FreeRTOSConfig.h Outdated Show resolved Hide resolved
user/tests/wiring/no_fixture/thread.cpp Show resolved Hide resolved
@avtolstoy avtolstoy merged commit 558e39d into develop Jun 23, 2020
@avtolstoy avtolstoy deleted the fix/newlib-nano branch June 23, 2020 13:34
@avtolstoy avtolstoy added ready to merge PR has been reviewed and tested and removed needs review labels Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ready to merge PR has been reviewed and tested
Projects
None yet
2 participants