Enabling small C library option and deprecating uARM toolchain #12068

rajkan01 · 2019-12-10T13:28:46Z

Summary of changes

In Mbed-OS 2-region memory model, ARM Std and Microlib scatter files are common
more info on https://github.com/ARMmbed/mbed-os/blob/master/docs/design-documents/platform/memory-model/ram_memory_model.md,
so replaced ARM Std scatter file 1-region memory model with Microlib scatter file 2 region memory model.
Moved the "__aeabi_assert" function from Microlib boot code file to mbed_retarget file so that it can be available for both RTOS Microlib and bare metal to resolve the undefined symbol linker issue.
Copied ARM Microlib boot code into ARM std boot code and guarded with __MICROLIB. This is required in order to build a RTOS application with Microlib.
Added a new target config "supported_c_libs" in targets.json and build tools to check if the selected "default_lib" is supported for the selected toolchain. If not the build tool will raise an exception.
Added the new test suite for testing the build tool changes for default_lib and supported_c_libs.
PR changes tested with blinky and blinky bare metal examples and greentea bare metal tests run locally.

Impact of changes

With these changes, uARM toolchain is being deprecated.

Migration actions required

As uARM toolchain is deprecated, the user has to redefine a supported_c_libs configuration with "small" for arm toolchain on the target in targets.json to enable Microlib support like

 "supported_c_libs": {
      "arm": ["std", "small"],
      "gcc_arm": ["std", "small"],
      "iar": ["std"]
  }

and change the scatter file to the 2-region memory model see more info.
Instead of -t uARM, use default_lib with "small" in mbed_app.json configuration for ARMC6 toolchain.

Documentation

ARMmbed/mbed-os-5-docs#1178

Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[x] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@evedon @hugueskamba @jamesbeyond @graham.hammond @mark.edgeworth @adrien.cabarbaye

ciarmcom · 2019-12-10T14:00:32Z

@rajkan01, thank you for your changes.
@evedon @jamesbeyond @hugueskamba @ARMmbed/mbed-os-core @ARMmbed/mbed-os-tools @ARMmbed/mbed-os-test @ARMmbed/mbed-os-maintainers please review.

bulislaw · 2019-12-11T15:25:15Z

By default, Mbed OS build tools use standard C library for all supported toolchains. It is possible to use smaller C libraries by overriding the "target.default_lib" option with "small". This option is only currently supported for the GCC_ARM toolchain. This override config option is now extended in the build tool for ARM toolchain.

default_lib was also renamed to c_lib right?

rajkan01 · 2019-12-11T15:27:34Z

By default, Mbed OS build tools use standard C library for all supported toolchains. It is possible to use smaller C libraries by overriding the "target.default_lib" option with "small". This option is only currently supported for the GCC_ARM toolchain. This override config option is now extended in the build tool for ARM toolchain.

default_lib was also renamed to c_lib right?

We will be doing this renaming from default_lib to c_lib in IOTCORE-1450 after this PR merged.

targets/targets.json

tools/toolchains/mbed_toolchain.py

tools/test/toolchains/test_toolchains.py

tools/toolchains/gcc.py

tools/toolchains/mbed_toolchain.py

TESTS/configs/baremetal.json

tools/test/toolchains/test_toolchains.py

tools/toolchains/arm.py

tools/toolchains/gcc.py

bulislaw

@ARMmbed/team-st-mcd could you have a look at changes to the linker scripts?

bulislaw · 2019-12-12T13:21:23Z

TESTS/configs/baremetal.json

-        "*": {
+        "K64F": {
+            "target.default_lib": "small",
+            "mbed-trace.fea-ipv6": false


What is that and why does it need to be set?

I'll let rajkumar answer. But mbed-trace.fea-ipv6 should have been overridden for all targets.

"mbed-trace.fea-ipv6": false added to disable IPV6 tracing code in mbed_trace and we need mbed_trace due to dependency with the device key. More information on PR #11875

TESTS/mbed_drivers/c_strings/main.cpp

TESTS/mbed_platform/minimal-printf/compliance/main.cpp

tools/test/toolchains/test_toolchains.py

TESTS/mbed_platform/minimal-printf/compliance/main.cpp

jeromecoutant

Should you also remove all TOOLCHAIN_ARM_MICRO directory ?

and harmonize TOOLCHAIN_ARM_STD and TOOLCHAIN_ARMC6

jeromecoutant · 2019-12-16T14:47:49Z

TESTS/configs/baremetal.json

+            "target.default_lib": "small"
+        },
+        "NUCLEO_F411RE": {
+            "target.default_lib": "small"


Why target.default_lib is not set to small for all targets,
instead of adding a line for each 1...?

It has only been set for targets that have been tested so far.

rajkan01 · 2019-12-16T14:58:07Z

Should you also remove all TOOLCHAIN_ARM_MICRO directory ?
and harmonize TOOLCHAIN_ARM_STD and TOOLCHAIN_ARMC6

@evedon please comment

evedon · 2019-12-16T15:06:41Z

Should you also remove all TOOLCHAIN_ARM_MICRO directory ?
and harmonize TOOLCHAIN_ARM_STD and TOOLCHAIN_ARMC6

@evedon please comment

We are deprecating TOOLCHAIN_ARM_MICRO but not removing it at this stage. This will be done in a future Mbed OS release.

bulislaw · 2019-12-16T16:38:19Z

TESTS/mbed_platform/minimal-printf/compliance/main.cpp

@@ -51,6 +51,8 @@ static control_t test_printf_d(const size_t call_count)

    /*************************************************************************/
    /*************************************************************************/
+#if !defined(__NEWLIB_NANO)
+    // The format specifier %hhd is not supported by Newlib-Nano


That doesn't make sense to me. These tests are for minimal-printf not for standard printf. I'm assuming they are being run in all cases even if the minimal-printf is not present and that's the problem. The thing is this change will possibly hide issues as we won't execute tests in usecase with minimal-printf and minimal clibs. I think the correct solution is to exclude all minimal-printf tests if minimal-printf is not used, but run them with newlib-nano + minimal-printf as the newlib-nano limitation won't be hit.

The user has the choice to swap the standard printf by minimal-printf ; mbed_printf is a test API that is always present (it calls mbed_minimal_formatted_string which is the minimal printf implementation).
But you are right that the limitation is a newlib-nano limitation and the test could check against a predefined value to avoid calling printf.

It's also a waste of time in the CI (even if not huge one) to test the stdlib. Sure it possible to find bugs there but it's rather rare.

- By default, Mbed OS build tools use standard C library for all supported toolchains. It is possible to use smaller C libraries by overriding the "target.default_lib" option with "small". This option is only currently supported for the GCC_ARM toolchain. This override config option is now extended in the build tool for ARM toolchain. - Add configuration option to specify libraries supported for each toolchain per targets. - Move __aeabi_assert function from rtos to retarget code so it’s available for bare metal. - Use 2 memory region model for ARM toolchain scatter file for the following targets: NUCLEO_F207ZG, STM32F411xE, STM32F429xI, NUCLEO_L073RZ, STM32F303xE - Add a warning message in the build tools to deprecate uARM toolchain. - NewLib-Nano C library is not supporting floating-point and printf with %hhd,%hhu,%hhX,%lld,%llu,%llX format specifier so skipping those green tea test cases.

bulislaw · 2019-12-23T15:55:24Z

My comments around disabling tests weren't addressed. It's not a huge problem, but with minimal amount of work we can do things properly. So unless I'm missing something could you please make sure the minimal-printf tests are not executed when minimal-printf is not used?

rajkan01 · 2019-12-23T16:42:43Z

My comments around disabling tests weren't addressed. It's not a huge problem, but with minimal amount of work we can do things properly. So unless I'm missing something could you please make sure the minimal-printf tests are not executed when minimal-printf is not used?

Minimal printf green tea test has changed to test without using std library printf and snprintf functions, please review.

adbridge · 2019-12-24T11:14:40Z

CI started

mbed-ci · 2019-12-24T12:55:23Z

Test run: FAILED

Summary: 2 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-IAR
jenkins-ci/mbed-os-ci_build-ARM

0Grit · 2019-12-27T19:53:21Z

What are the effects if any on program memory / ram consumption?

0xc0170 · 2020-01-02T12:07:02Z

CI restarted

mbed-ci · 2020-01-02T14:42:06Z

Test run: FAILED

Summary: 1 of 12 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

0xc0170 · 2020-01-02T15:07:21Z

test restarted

0xc0170 · 2020-01-02T15:20:51Z

I'll review this one as well.

0xc0170

Once "Release Notes" are provided, LGTM

0xc0170 · 2020-01-03T10:48:07Z

Once "Release Notes" are provided, LGTM

Oh, wait, reviewing the first template above (was it using older one) ? I'm checking now

adbridge · 2020-02-17T13:49:20Z

@bulislaw does a deprecation (even of a compiler) count as a breaking change ? I've marked it as such for now, pending confirmation.

ciarmcom requested review from evedon, hugueskamba, jamesbeyond and a team December 10, 2019 14:00

ciarmcom added the needs: review label Dec 10, 2019

rajkan01 force-pushed the feature_bare_metal branch from f17280e to 8ee62e9 Compare December 10, 2019 15:50

evedon requested changes Dec 12, 2019

View reviewed changes

TESTS/configs/baremetal.json Outdated Show resolved Hide resolved

hugueskamba reviewed Dec 12, 2019

View reviewed changes

bulislaw reviewed Dec 12, 2019

View reviewed changes

rajkan01 force-pushed the feature_bare_metal branch from 73fdc50 to cb9b6d5 Compare December 12, 2019 17:36

hugueskamba reviewed Dec 13, 2019

View reviewed changes

TESTS/mbed_platform/minimal-printf/compliance/main.cpp Outdated Show resolved Hide resolved

tools/test/toolchains/test_toolchains.py Outdated Show resolved Hide resolved

hugueskamba reviewed Dec 13, 2019

View reviewed changes

TESTS/mbed_platform/minimal-printf/compliance/main.cpp Outdated Show resolved Hide resolved

rajkan01 force-pushed the feature_bare_metal branch from a7b0cf1 to 1116c18 Compare December 13, 2019 11:32

evedon approved these changes Dec 13, 2019

View reviewed changes

rajkan01 force-pushed the feature_bare_metal branch from d0e09b0 to c4b1a65 Compare December 13, 2019 12:11

hugueskamba approved these changes Dec 13, 2019

View reviewed changes

evedon mentioned this pull request Dec 16, 2019

NUCLEO_L073RZ: wake up from deep sleep test fails when linking with microlib #12116

Closed

jeromecoutant reviewed Dec 16, 2019

View reviewed changes

bulislaw reviewed Dec 16, 2019

View reviewed changes

hugueskamba mentioned this pull request Dec 19, 2019

NRF52840: Use 2 region memory model for the ARM toolchain #12144

Merged

adbridge added needs: work and removed needs: review labels Dec 23, 2019

rajkan01 force-pushed the feature_bare_metal branch from 8f7171d to 4cd6236 Compare December 23, 2019 16:29

Minimal-printf: Fix system test without using standard library printf

3697815

rajkan01 force-pushed the feature_bare_metal branch from 4cd6236 to 3697815 Compare December 23, 2019 16:31

bulislaw approved these changes Dec 24, 2019

View reviewed changes

adbridge added needs: CI and removed needs: work labels Dec 24, 2019

Fix the CI build issue

6a8c432

0xc0170 added ready for merge and removed needs: CI labels Jan 2, 2020

0xc0170 added the release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 label Jan 3, 2020

0xc0170 requested changes Jan 3, 2020

View reviewed changes

0xc0170 added needs: work and removed ready for merge labels Jan 3, 2020

0xc0170 removed the needs: work label Jan 3, 2020

0xc0170 approved these changes Jan 3, 2020

View reviewed changes

0xc0170 merged commit fc2a710 into ARMmbed:master Jan 3, 2020

adbridge added the BREAKING-CHANGE label Feb 17, 2020

Enabling small C library option and deprecating uARM toolchain #12068

Enabling small C library option and deprecating uARM toolchain #12068

Uh oh!

Conversation

rajkan01 commented Dec 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

ciarmcom commented Dec 10, 2019

Uh oh!

bulislaw commented Dec 11, 2019

Uh oh!

rajkan01 commented Dec 11, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bulislaw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeromecoutant left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajkan01 Dec 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajkan01 commented Dec 16, 2019

Uh oh!

evedon commented Dec 16, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bulislaw commented Dec 23, 2019

Uh oh!

rajkan01 commented Dec 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adbridge commented Dec 24, 2019

Uh oh!

mbed-ci commented Dec 24, 2019

Uh oh!

0Grit commented Dec 27, 2019

rajkan01 commented Dec 10, 2019 •

edited

Loading

rajkan01 Dec 16, 2019 •

edited

Loading

rajkan01 commented Dec 23, 2019 •

edited

Loading

adbridge commented Feb 17, 2020 •

edited

Loading