cpu/msp430_common: add real malloc/free functions

[gschorcht]

Contribution description

This PR realizes real malloc/free functions for MSP430 CPUs. For this purpose adapted AVR libc function are used. Thus, the remaining RAM becomes available as heap.

When used, malloc and free functions require 318 additional bytes of ROM and 7 additional bytes of RAM compared to the oneway_malloc module.

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 and free functions work as expected.

Issues/PRs references

Depends on PR #10943

[jcarrano]

If this is not msp430-specific, I'd rather have it as a module in sys (like oneway_malloc) so it can be reused. I started some work long ago on modularizing some parts of the newlib support code (#9258 ).

One problem I see with this implementation is that it is not thread safe.

How does this compare to the implementation in baselibc?

[gschorcht]

@jcarrano

If this is not msp430-specific, I'd rather have it as a module in sys (like oneway_malloc) so it can be reused. I started some work long ago on modularizing some parts of the newlib support code (#9258 ).

Hm, I think it is MSP430 specific since only AVR and MSP430 do not use newlibc. Since AVR uses avrlibc, the only platform which might require such an implementation is MSP430. Therefore, yes it is platform specific.

One problem I see with this implementation is that it is not thread safe.

Since it is a simply a slight adaption of malloc functions of avrlibc, it is as thread safe as the avrlibc is. However, it shouldn't be a big thing to define critical sections disabling interrupts or using guard variables as baselibc does.

How does this compare to the implementation in baselibc?

Concerning the complexity and the code size, it seems to be very similar to baslibc's malloc.

Baslibc seems to be an interesting project. I saw your PR #10292 which is labled as WIP. Once, this is merged, I would be fine to use this package instead of this specialized implementation. But until then, implementation in this PR could help to have real heap capabilities in MSP430. oneway_malloc seems to have no real worth.

[jcarrano]

Ok, then let's leave it inside the msp port. But please add some irq save/restore around critical sections.

It strikes me that the avr version is not thread safe. I guess that's a good reason to use an alternate implementation (or not use avr.)

#10292 should help, yes, but there's this malloc i've been wanting to add for some time now that I think is even better for teeny tiny devices. The problem is riot/newlib integration and the lack of comprehensive tests for malloc. I'm working on this last issue.

[gschorcht]

Ok, then let's leave it inside the msp port. But please add some irq save/restore around critical sections.

I will do that 😄

It strikes me that the avr version is not thread safe. I guess that's a good reason to use an alternate implementation (or not use avr.)

The sources of avr-libc package we use in riotdocker can be found here.
It was probably not intended to use avrlibc in a multithreaded environment. Maybe, we could provide a thread safe version in cpu/atmega_common/avr_libc_extra.

[gschorcht]

But please add some irq save/restore around critical sections.

@kaspar030 Do you know whether MSP430 is able to interrupt an ISR or is it enough to disable interrupts in critical sections?

[kaspar030]

@kaspar030 Do you know whether MSP430 is able to interrupt an ISR or is it enough to disable interrupts in critical sections?

IIRC, both. MSP430 supports nested interrupts, but disabling ISRs protects critical sections from being interrupted.

[gschorcht]

@jcarrano I have added irq_disable/irq_restore arround critical sections. This increases the code size by 44 byte.

Furthermore, I used tests/malloc to measure the time taken by 'malloc' and 'free' to estimate how long interrupts are disabled. Both of them take around 25 us. IMHO, an acceptable time. However, tests/malloc allocates memory of the same size, so the time these functions require might differ as the heap becomes more and more fragmented.

[jcarrano]

Squash and fix the issue reported by coccinelle and we merge.

[gschorcht]

@jcarrano Fixed and finally squashed. Murdock is also satisfied.

[gschorcht]

@jcarrano We forgot to merge it before feature freeze. That's surly not a problem but we should not forget it again. I didn't megre it since it's a bad style to merge own PRs.

[kaspar030]

LGTM!

[gschorcht]

@kaspar030 Thanks 😄