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

MCUXpresso: Update ARM linker files to eliminate reserving RAM for stack & heap #4137

Merged
merged 1 commit into from Apr 20, 2017

Conversation

Projects
None yet
6 participants
@mmahadevan108
Contributor

mmahadevan108 commented Apr 7, 2017

Description

Update ARM linker files to eliminate reserving RAM for stack & heap. Heap and stack size is determined via the RTOS. This should fix the below issue:
#3763

Status

**READY

  • Tests
    Ran & passed mbed-os tests.
@mmahadevan108

This comment has been minimized.

Contributor

mmahadevan108 commented Apr 7, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 10, 2017

@mmahadevan108 Again, this is fine for mbed 5 ,but some of these targets that you are changing are enabled for mbed 2 (non rtos applications).

@mmahadevan108

This comment has been minimized.

Contributor

mmahadevan108 commented Apr 10, 2017

Is there a way to do this for mbed 5 only. We are wasting memory as this is handled in the RTOS as well.

@sg-

This comment has been minimized.

Member

sg- commented Apr 10, 2017

What about using MBED_CONF_RTOS_PRESENT given the linkers now support preprocessing?

@theotherjimmy @0xc0170 @c1728p9

@c1728p9

This comment has been minimized.

Contributor

c1728p9 commented Apr 10, 2017

This change is relevant to both mbed 2 and 5. Even when the rtos is not present the heap and stack are determined at runtime, similar to with the rtos. These sizes can be decreased, but it would probably be good to set them to non-zero values so you get a compile time error, rather than runtime, on both 2 and 5 if your program is too big. Something like ~1k for both should be sufficient.

@sg-

This comment has been minimized.

Member

sg- commented Apr 10, 2017

Even when the rtos is not present the heap and stack are determined at runtime, similar to with the rto

What happens to the symbols defined for the heap in the linker scripts, are they not used ie wasted space?

@c1728p9

This comment has been minimized.

Contributor

c1728p9 commented Apr 10, 2017

These defines aren't used to set the size of a dedicated heap region. Instead they are subtracted off the max size of the RW data section so you have at least __stack_size__ + __heap_size__ of free space. At run-time both mbed 2 and 5 setup the stack and heap based on this free space.

@mmahadevan108

This comment has been minimized.

Contributor

mmahadevan108 commented Apr 10, 2017

Thanks Russ. Why do we need this free space?

@c1728p9

This comment has been minimized.

Contributor

c1728p9 commented Apr 10, 2017

This space is for the stack and the heap. If it isn't big enough then the stack will grow into valid data and clobber it leading to a crash.

@mmahadevan108

This comment has been minimized.

Contributor

mmahadevan108 commented Apr 10, 2017

I am confused. I thought the stack & heap space is reserved in the RTOS file for both mbed 2 & 5.

@c1728p9

This comment has been minimized.

Contributor

c1728p9 commented Apr 10, 2017

With the ARM compiler the free space starts at the end of RW_IRAM1 and goes to the end of the RAM bank. This is the case both for mbed 2 and mbed 5.

  • mbed 5 uses this space for the interrupt stack and for the heap. The code to do this is located here
  • mbed 2 uses this space for the main stack and for the heap. The code to do this for ARM is target specific at the moment and can be found here for the K66F.
@mmahadevan108

This comment has been minimized.

Contributor

mmahadevan108 commented Apr 10, 2017

Thanks Russ. Some of the platform linker files for ARM compiler do not have __stack & __heap defined and no space is not reserved. Setting these to 0 would result in a similar environment.

@c1728p9

This comment has been minimized.

Contributor

c1728p9 commented Apr 10, 2017

It works with the size set to zero, you just get a runtime error rather than a compile time error if your program is too big. If the scatter file has support for this extra level of checking why not use it?

@mmahadevan108

This comment has been minimized.

Contributor

mmahadevan108 commented Apr 10, 2017

I understand. I will reduce the stack & heap size but not make them 0.

@0xc0170 0xc0170 added needs: work and removed needs: review labels Apr 11, 2017

MCUXpresso: Update ARM linker files to reduce RAM reserved for stack …
…& heap

Heap and stack size is determined via the RTOS.

Signed-off-by: Mahadevan Mahesh <Mahesh.Mahadevan@nxp.com>

@mmahadevan108 mmahadevan108 force-pushed the NXPmicro:Kinetis_Fix_ARM_Linker_Scripts branch to 5b866b7 Apr 14, 2017

@mmahadevan108

This comment has been minimized.

Contributor

mmahadevan108 commented Apr 14, 2017

@0xc0170 @sg- @c1728p9 I have pushed in an updated patch. Space reserved for stack & heap has been reduced but is not 0.

@c1728p9

This comment has been minimized.

Contributor

c1728p9 commented Apr 14, 2017

Thanks for the update @mmahadevan108. This PR looks good to me.

@sg-

This comment has been minimized.

Member

sg- commented Apr 19, 2017

/morph test-nightly

@sg- sg- added needs: CI and removed needs: work labels Apr 19, 2017

@mbed-bot

This comment has been minimized.

mbed-bot commented Apr 19, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 34

All builds and test passed!

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Apr 20, 2017

@adbridge adbridge merged commit 5b86a10 into ARMmbed:master Apr 20, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test-nightly Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mmahadevan108 mmahadevan108 deleted the NXPmicro:Kinetis_Fix_ARM_Linker_Scripts branch Apr 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment