Skip to content

cpu/msp430_common: set top of heap for sbrk#10943

Merged
jcarrano merged 1 commit intoRIOT-OS:masterfrom
gschorcht:cpu/mps430_common_heap
Mar 21, 2019
Merged

cpu/msp430_common: set top of heap for sbrk#10943
jcarrano merged 1 commit intoRIOT-OS:masterfrom
gschorcht:cpu/mps430_common_heap

Conversation

@gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Feb 5, 2019

Contribution description

MSP430 uses the module 'oneway_malloc' to provide the malloc function. The implemented sbrk function always checks whether the current break value cur_break is less than the SP. However, since cur_break starts above the .bss section while SP in thread mode is always in thread stacks in .bss section, this check always fails and malloc does not work.

This PR fixes this problem by setting a variable __heap_end in the startup function to the current position of SP immediately before entering the thread mode. sbrk then checks cur_break value against this __heap_end variable.

The changes of this PR require 10 additional bytes of ROM and 2 additional bytes of RAM.

Testing procedure

Use tests/malloc with the current master and the changes in this PR to verify the differences, for example:

make -C tests/malloc BOARD=wsn430-v1_3b flash test

With current master, tests/malloc doesn't work at all and gives no output. With the changes in this PR, malloc function works. free always fails since it is not implemented in oneway_malloc.

Issues/PRs references

This PR is prerequisite for PR #10944.

@gschorcht gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: MSP Platform: This PR/issue effects MSP-based platforms Area: cpu Area: CPU/MCU ports labels Feb 5, 2019
Copy link
Contributor

@jcarrano jcarrano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it in IOTLAB. It works fine. I'd like to have STACK_EXTRA explained a little, though.

@gschorcht
Copy link
Contributor Author

@jcarrano Thank you for testing it.

@jcarrano jcarrano added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 12, 2019
@jcarrano
Copy link
Contributor

Oh, and by the way, I'm working on revamping the malloc tests. It's going to take a while since I'm taking it as a chance to try out some new testing infrastructure PRs.

@jcarrano jcarrano added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Mar 13, 2019
@jcarrano
Copy link
Contributor

jcarrano commented Mar 13, 2019

Squash and merge it.

Set __heap_end to current SP before entering thread mode to make the remaining RAM available as heap for module oneway_malloc.
@gschorcht gschorcht force-pushed the cpu/mps430_common_heap branch from 2daa715 to 55f4331 Compare March 13, 2019 12:32
@gschorcht
Copy link
Contributor Author

@jcarrano Squashed, but merge shouldn't be done by the author.

@jcarrano jcarrano added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 13, 2019
@gschorcht
Copy link
Contributor Author

@jcarrano Don't forget to merge 😉

@jcarrano jcarrano merged commit c391ed4 into RIOT-OS:master Mar 21, 2019
@jcarrano
Copy link
Contributor

@gschorcht sorry, I forgot!!!

@gschorcht
Copy link
Contributor Author

Thanks, no problem.

@gschorcht gschorcht deleted the cpu/mps430_common_heap branch March 21, 2019 13:50
@danpetry danpetry added this to the Release 2019.04 milestone Mar 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: MSP Platform: This PR/issue effects MSP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants