Page 1 of 1

MSP430 IAR register clobber crash

PostPosted: Mon Apr 14, 2008 5:43 am
by mchiglinsky
using Salvo 4.1.2 rc0:

In the IAR MSP430 C/C++ Compiler Reference Guide, registers R4 to R11 are preserved registers, but none of these are preserved across a context switch, at least when using OS_Delay().

This caused a crash with some code recently. In a task, any local variables intended to be persistent across context switches were declared static, following the Salvo documentation, but even when this was the case the compiler would still store temporary values in registers, especially when optimization was turned on. The task called one function more than once, and on optimization level "High", the compiler decided to store the address of that function in R11 at the beginning of the task and then call the function using R11 later in the task. Because a context switch did not preserve R11, it ended up calling to the arbitrary address left in R11 once the scheduler returned to the task. It executed arbitrary code, and the system basically crashed and froze.

A workaround for this seems to be to set the optimization level to at most "Medium". In that case, the compiler retrieves the constant address of the function often enough that the value in R11 does not need to be persistent, but it isn't clear whether this behavior is guaranteed or accidental.

Re: MSP430 IAR register clobber crash

PostPosted: Tue Apr 15, 2008 8:13 am
by aek
Thank you for this report.

It seems that in the latest version(s) of the IAR EW430 toolset, this register allocation has been implemented at a relatively low level of optimization, and it is in fact causing problems.

We plan to release a new version of Salvo 4 for MSP430 shortly (Friday?). It will include a note on incompatible optimizations in the RM-IAR430.PDF reference manual, and will likely also need the libraries to be built at a different level of optimization to avoid this issue in the scheduler.

We're also looking at a two-pronged approach to this -- namely, one where the the required registers are fully saved and restored (which will cost more RAM in the tcbs, but will work with any level of optimization), and one where they are not (which will only work with low(er) levels of optimization.


Re: MSP430 IAR register clobber crash

PostPosted: Tue Apr 29, 2008 10:44 am
by aek
OK, it seems that the code motion optimization _sometimes_ causes problems with Salvo code -- specifically, due to the lack of register saves mentioned below.

Testing here has revealed that the "trigger" that causes the problems is only code motion -- all other optimizations -- whether "Balanced", "Speed" or "Size" do not appear to matter as long as code motion is NOT selected.

Also, the problem seems to happen only is MSP430 code and not in MSP430X code, though to be on the safe side we would recommend disabling code motion for both MSP430 and MSP430X.

This information will be added to RM-IAR430.PDF, and the Salvo libraries will be built accordingly. Will appears in 4.2.1 release.


Re: MSP430 IAR register clobber crash

PostPosted: Tue May 20, 2008 8:52 am
by Dave Hohl
I am using the 430X, and am experiencing problems even when only the Medium optimization level is enabled. The problem goes away if I turn off the code motion optimization.

However, if I select High optimization and turn off code motion, I still have problems, so the issue is not localized to just the code motion optimization.

I would really like to see the option mentioned earlier (about saving the additional registers) implemented. Even though it increases code size a bit, it seems to be a more reliable solution. Perhaps there could be a config option to enable/disable the register saving?

Re: MSP430 IAR register clobber crash

PostPosted: Wed May 21, 2008 7:47 am
by aek
Well, it's not the code size that grows much, it's the RAM requirements ...

It's on our to-do list. It has far-reaching implications in terms of how libraries are built and used, and we're working those out now Salvo-wide so that we can implement this in a future release.


[This message has been edited by aek (edited May 21, 2008).]