Severe code generator bug in avr-gcc 3.0.3

[Zoltan Kocsi <zoltan at bendor dot com dot au>]

Hi,

There is a quite severe bug in the 3.0.3 gcc AVR code generator. 

The environment and compiler info, with the command line is here:

---------------------------------------------------------------
[zoltan tade]~>uname -a
Linux tade 2.2.14 #4 Tue Aug 22 10:45:02 EST 2000 i686 unknown

[zoltan tade]~>avr-gcc -v -O2 -S -Wall avrbug.c 
Reading specs from /usr/local/lib/gcc-lib/avr/3.0.3/specs
Configured with: ./configure --target=avr
Thread model: single
gcc version 3.0.3
 /usr/local/lib/gcc-lib/avr/3.0.3/cc1 -lang-c -v -D__GNUC__=3 -D__GNUC_MINOR__=0 -D__GNUC_PATCHLEVEL__=3 -DAVR -D__AVR__ -D__AVR -D__OPTIMIZE__ -D__STDC_HOSTED__=1 -Wall -D__AVR_ARCH__=2 -D__SIZE_TYPE__=unsigned int -D__PTRDIFF_TYPE__=int -D__INT_MAX__=32767 avrbug.c -quiet -dumpbase avrbug.c -O2 -Wall -version -o avrbug.s
GNU CPP version 3.0.3 (cpplib) (GNU assembler syntax)
GNU C version 3.0.3 (avr)
        compiled by GNU C version 2.95.2 19991024 (release).
ignoring nonexistent directory "/usr/local/avr/sys-include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/local/lib/gcc-lib/avr/3.0.3/include
 /usr/local/avr/include
End of search list.
[zoltan tade]~>
---------------------------------------------------------------

The test code is very simple, does not need any #include files
so I don't attach the preprocesor output. Since the test code 
is very short, I just insert it here:

---------------------------------------------------------------
typedef    unsigned char ubyte;

typedef    struct {

    ubyte    some_data[ 10 ];  /* Some data of a comms. packet             */

} SOMEDATA;

ubyte    check_sum( SOMEDATA * );

/*
* Tests if the sum of all data bytes within the structure 
* is 0 or not, returns 1 if yes, 0 otherwise. 
*/

ubyte    check_sum( SOMEDATA *data )
{
ubyte    *ptr;
ubyte    cnt;
ubyte    chs;

    chs = 0; 
    ptr = (ubyte *) data; 
    cnt = sizeof( SOMEDATA ); 
    
    while ( cnt-- ) chs += *ptr++;
    
    if ( chs ) 
    
        return( 0 );
    else
        return( 1 );
}
---------------------------------------------------------------

The generated assembly code is this (with my comments):

---------------------------------------------------------------
	.file	"avrbug.c"
	.arch avr2
__SREG__ = 0x3f
__SP_H__ = 0x3e
__SP_L__ = 0x3d
__tmp_reg__ = 0
__zero_reg__ = 1
_PC_ = 2
	.text
.global	check_sum
	.type	check_sum,@function
check_sum:
/* prologue: frame size=0 */
/* prologue end (size=0) */
	ldi r18,lo8(0)                    // chs = 0
	mov r31,r25                       // ptr = data
	mov r30,r24
	ldi r25,lo8(10)                   // cnt = sizeof(SOMEDATA) - 1
.L4:                                  // loop:
	ld r24,Z+                         // tmp = *ptr++
	add r18,r24                       // chs += tmp   SETS THE Z FLAG
	subi r25,1                        // cnt--        DESTROYS THE Z FLAG!!!
	brcc .L4                          // if ( cnt >= 0 ) goto loop
	breq .L6                          // TESTS THE Z FLAG WHICH IS WRONG!!!!
	ldi r24,lo8(0)
	ldi r25,hi8(0)
	rjmp .L1
.L6:
	ldi r24,lo8(1)
	ldi r25,hi8(1)
.L1:
/* epilogue: frame size=0 */
	ret
/* epilogue end (size=1) */
/* function check_sum size 17 (16) */
.Lfe1:
	.size	check_sum,.Lfe1-check_sum
/* File avrbug.c: code   17 = 0x0011 (  16), prologues   0, epilogues   1 */
---------------------------------------------------------------

As you can see from the comments, when it leaves the loop it does not
test the 'chs' value but assumes that the flags contain the last
add's result. This is incorrect, since the loop counter's decrement
will change the flags. Consequently, this function will *always*
return 0 (since Z will not be set when the loop exits).

If there is anything else to be done with the value of chs, such as
storing it in a global variable:

    ...
    while ( cnt-- ) chs += *ptr++;
    
    x = chs;     // static ubyte x; declared outside of the function
    
    if ( chs )
    ...

then the test will be done, as expected:

   ...
.L4:
    ld r24,Z+
    add r18,r24
    subi r25,1
    brcc .L4            // loop test & jump
    sts x,r18           // x = chs
    tst r18             // if ( chs == 0 ) IT SHOULD ALWAYS BE DONE!
    breq .L6
    ldi r24,lo8(0)
    ldi r25,hi8(0)
   ...
   
Also note that the bug occurs with either -O1, -O2 or -O3.

I checked it with gcc 2.95.2 with AVR patches as well and the bug 
is *not* present, it properly tests 'chs' after leaving the loop.
I have not checked 3.0.3 against any other targets.

Best Regards,

Zoltan