Re: [avr-gcc-list] COMPILER Frame pointer bug!!!

[Christoph Plattner]

Hello !

No the code is OK, you simple have overseen the interrupt handling.

Using cli/sei is ALWAYS WRONG and many old operating system errors
have to be searched. The correct handling is (used C style to 
explain):

flags = save_cpu_flags ();      /* inlcuding Interrupt Enable bit */
cli ();                         /* disable interrupts */

< critical code >

restore_cpu_flags (flags);      /* set the old IE state !!! */

This method is necessary, if this routines is called under interrupts
OFF, the interrupt MUST NOT switched on with SEI !!

>         in __tmp_reg__,__SREG__
>         cli                         
> DISABLE INTERRUPTS
>         out __SP_H__,r29
>         out __SREG__,__tmp_reg__
>         out __SP_L__,r28

Here you see the "save_cpu_flags" in the first line (save SREG into 
__tmp_reg__), the the `cli', and at the bottom the restore from
__tmp_reg__ in the SREG !  

For optimization for do short interrupt locks, the restoring is not the
last operation, as AVR guarantees to execute one instruction of the
current sequence after enabling interrupts again. So the restoring is
done before the second part of the stack pointer is setup. In this
way, the interrupts off time is shorter.

The protections with locked interrupts is necessary here. The access to
the 16bit stack pointer must be atomic. Otherwise it would be fatal, if
one of the two parts are defined to the new value and suddenly an
interrupt
via an IRQ is done. The stack pointer has an invalid value then.

I hope I could clarify the stuff

With friendly regards
        Christoph P.



dimmy wrote:
> 
> Fellows,
> 
> I found  strange avr-gcc behaviour.
> 
> If you declare a function with a variable number of arguments or the
> number of arguments is over 18 (actually, total number of bytes,
> required for arguments) or you allocate over 18 bytes inside the
> function, then avr-gcc disables interrupts on enter to the function and
> does not enable them anymore.
> 
> This is valid only in case, when frame pointer cannot be eliminated or
> when no optimization enabled.
> For example, the code:
> 
> char *newfmt(int n,  char *fmt, ...)
> {
>         char *p;
>         va_list ap;
>         int ind;
>         int i;
>         char w[3000];    /* will never be used. the same result if the
> size of w is over 16 */
> 
>         va_start(ap, fmt);
> 
>         for (i=0; i<n;i++)
>         {
>                 ind = va_arg(ap, int);
>                 excall(ind);
>         }
> 
>         va_end(ap);
>         return (p);
> }
> 
> will result as (avr-gcc -mmcu=atmega103 -O -S ):
> 
> /* prologue: frame size=3000 */
>         push r10
>         push r11
>         push r12
>         push r13
>         push r14
>         push r15
>         push r16
>         push r17
>         push r28
>         push r29
>         in r28,__SP_L__
>         in r29,__SP_H__
>         subi r28,lo8(3000)
>         sbci r29,hi8(3000)
>         in __tmp_reg__,__SREG__
>         cli                                                ;    <=====
> DISABLE INTERRUPTS
>         out __SP_H__,r29
>         out __SREG__,__tmp_reg__
>         out __SP_L__,r28
> /* prologue end (size=19) */
> .......
> 
> And no 'sei' instruction in the function body!!!
> Actully, in function's epilogue, gcc disables interrupts as well.
> 
> I'll contact the originator of the compiler later.
> 
> If you cannot wait and want to fix this bug right now, you have to
> recompile the gcc sources.
> The patch is here:
> diff -u gcc/config/avr/avr.c.orig gcc/config/avr/avr.c
> --- gcc/config/avr/avr.c.orig   Sat Nov 17 21:18:14 2001
> +++ gcc/config/avr/avr.c        Sat Nov 17 21:24:28 2001
> @@ -511,13 +511,14 @@
>        fprintf (file, AS2 (out, __SREG__, __tmp_reg__) CR_TAB);
>        size++;
>      }
> -  else if (do_sei)
> +
> +  fprintf (file, AS2 (out, __SP_L__, r28) "\n")
> +
> +  if (do_sei)
>      {
>        fprintf (file, "sei" CR_TAB);
>        size++;
>      }
> -
> -  fprintf (file, AS2 (out, __SP_L__, r28) "\n");
> 
>    return size;
>  }
> 
> So, just be carefull -
> 
> 1. make shure, gcc can omit frame pointer within a function call
> 2. declare vars as global or static,
> 3 if you cannot do 1 or 2, apply patch
> 
> Hope it helps,
> Dmitry
> 
> _______________________________________________
> avr-gcc-list mailing list
> address@hidden
> http://avr.jpk.co.nz/mailman/listinfo/avr-gcc-list

-- 
-------------------------------------------------------
private:        address@hidden
company:        address@hidden