Hi,
I've followed this mailing list time enough to know that is uncommon to find a
bug in the compiler and that the bugs are normally in the developer's code. I
think this is not a bug in GCC (the compiler). This is a bug in the use of
assembler inline to implement a function very specific for the MSP430
microcontroller. Sorry for my English. I usually don't write in English.
My team has find a bug in _BIC_SR_IRQ() (and all the functions that modify data
in the stack using assembler inline) when compiling with optimizations
activated (-O2, -Os) and when we call a function with five or more parameters
in the interrupt service routine where we later call _BIC_SR_IRQ().
Under this circumstances we see that the macro _BIC_SR_IRQ() have overwrite the
incorrect data in the stack instead of the Status Register stored. This is a
simplified version of code that expose the error:
-------------------------------- BEGIN sr_irq.c ------------------------------
int add_all(int a, int b, int c, int d, int e, int f)
{
return a + b + c + d + e + f;
}
volatile int g;
interrupt(TIMERA0_VECTOR) isr(void)
{
g = add_all(1,2,3,4,5,6);
_BIC_SR_IRQ(LPM4_bits);
}//isr
int main()
{
return 0;
}//main
--------------------------------- END sr_irq.c -------------------------------
I compile with this parameters:
$ msp430-gcc -mmcu=msp430x149 -o sr_irq.msp sr_irq.c -Wall -O2
sr_irq.c: In function `isr':
sr_irq.c:17: warning: concatenation of string literals with __FUNCTION__
is deprecated
I've seen in 'iomacros.h' that the macro '_BIC_SR_IRQ(LPM4_bits)' uses
'.L__FrameOffset_isr' to get the position of the Status Register based on the
stack needed by the function 'isr'. But the call 'add_all()' have 6 parameters
and the last two (5 and 6) are pushed in the stack too (4 bytes more). With
optimizations activated, the stack is set to the correct value (the value
coherent with '.L__FrameOffset_isr') after the instruction _BIC_SR_IRQ is
executed (too late). So, the instruction _BIC_SR_IRQ is modifying the incorrect
value in the stack. This is the generated code in assembler for the function 'isr':
.p2align 1,0
.global isr
.global vector_ffec
.type isr,@function
/***********************
* Interrupt Service Routine `isr' at 0xffec
***********************/
vector_ffec:
isr:
/* prologue: frame size = 0 */
.L__FrameSize_isr=0x0
.L__FrameOffset_isr=0x8
push r15
push r14
push r13
push r12
/* prologue end (size=4) */
push #llo(6)
push #llo(5)
mov #llo(4), r12
mov #llo(3), r13
mov #llo(2), r14
mov #llo(1), r15
call #add_all
mov r15, &g
/* #APP */
bic #llo(240), .L__FrameOffset_isr(r1)
/* #NOAPP */
add #llo(4), r1
/* epilogue: frame size=0 */
pop r12
pop r13
pop r14
pop r15
reti
/* epilogue end (size=5) */
/* function isr size 25 (16) */
.Lfe2:
.size isr,.Lfe2-isr
/********* End of function ******/
Here we can see that '.L__FrameOffset_isr' is 0x8 (4 push instructions in the
stack, needed for 'isr'), but the call to 'add_all' insert two push
instructions more. This is not counted in '.L__FrameOffset_isr'. If we don't
use -O2, after the "call #add_all' the compiler insert the 'add #llo(4), r1'
and the stack have again 8 bytes, and everything works ok. But with -O2 the
'add' instruction is inserted later and the macro _BIC_SR_IRQ, expanding to
"bic #llo(240), .L__FrameOffset_isr(r1)", is overwriting the incorrect data in
the stack.
I think that maybe the function _BIC_SR_IRQ() must be an intrinsic function
that can count the correct number of bytes in the stack when using
optimizations, because with assembler inline and '.L__FrameOffset_isr' with a
fixed value the compiler can put the 'ADD' of the stack pointer 'r1' in an
inconvenient place.
I know is unusual call functions inside interrupt service routines and very
unusual to have functions with a lot of parameters, but it is not incorrect
code. We have solved our particular problem reducing the number of parameters
in our function and sometimes using a macro expansion, but I think that is
better to have a correct implementation of _BIC_SR_IRQ() for all the situations
(including calling functions with a lot of parameters inside the interrupt
service routine) like I can see in other compiler implementations for MSP430.
I've tried, but I do not have sufficient skill to correct the code myself. I
hope someone here can do it. Thanks and greetings,
Mario Palomo Torrero
