gcc 3.3: miscompilation with -fnew-ra

[Michel LESPINASSE <walken at zoy dot org>]

Hi,

I've been playing with gcc 3.3 a bit, and I found my mpeg2 decoder
gets miscompiled with the new register allocator. Trying to make a
minimal test case, this is what I ended up with:

------------------------------- foo.c ----------------------------
typedef unsigned char uint8_t;
typedef signed short int16_t;
typedef unsigned int uint32_t;

typedef struct {
    int16_t DCTblock[64];

    uint32_t bitstream_buf;
    int bitstream_bits;
    const uint8_t * bitstream_ptr;

    const uint8_t * scan;
} mpeg2_decoder_t;

#define likely(x) __builtin_expect ((x) != 0, 1)
#define unlikely(x) __builtin_expect ((x) != 0, 0)

#define GETWORD(bit_buf,shift,bit_ptr)				\
do {								\
    bit_buf |= ((bit_ptr[0] << 8) | bit_ptr[1]) << (shift);	\
    bit_ptr += 2;						\
} while (0)

/* make sure that there are at least 16 valid bits in bit_buf */
#define NEEDBITS_(bit_buf,bits,bit_ptr)		\
do {						\
    if (bits > 0) {				\
	GETWORD (bit_buf, bits, bit_ptr);	\
	bits -= 16;				\
    }						\
} while (0)

#define NEEDBITS(bit_buf,bits,bit_ptr)		\
do {						\
    if (unlikely (bits > 0)) {			\
	GETWORD (bit_buf, bits, bit_ptr);	\
	bits -= 16;				\
    }						\
} while (0)

/* remove num valid bits from bit_buf */
#define DUMPBITS(bit_buf,bits,num)	\
do {					\
    bit_buf <<= (num);			\
    bits += (num);			\
} while (0)

/* take num bits from the high part of bit_buf and zero extend them */
#define UBITS(bit_buf,num) (((uint32_t)(bit_buf)) >> (32 - (num)))

typedef struct {
    uint8_t run;
    uint8_t level;
    uint8_t len;
} DCTtab;

static const DCTtab DCT_B15_8 [] = {{129, 0, 4}};

static void get_intra_block_B15 (mpeg2_decoder_t * const decoder)
{
    int i;
    int j;
    int val;
    const uint8_t * scan = decoder->scan;
    int mismatch;
    const DCTtab * tab;
    uint32_t bit_buf;
    int bits;
    const uint8_t * bit_ptr;
    int16_t * dest;

    dest = decoder->DCTblock;
    i = 0;
    mismatch = ~dest[0];

    bit_buf = decoder->bitstream_buf;
    bits = decoder->bitstream_bits;
    bit_ptr = decoder->bitstream_ptr;

    NEEDBITS (bit_buf, bits, bit_ptr);

    while (1) {
	tab = DCT_B15_8 + (UBITS (bit_buf, 8) - 4);

	i += tab->run;
	if (i < 64) {

	    j = scan[i];
	    bit_buf <<= tab->len;
	    bits += tab->len + 1;
	    val = tab->level;
	    //dest[j] = val;

	    bit_buf <<= 1;
	    NEEDBITS (bit_buf, bits, bit_ptr); /* XXXXXX */

	} else {

	    i += UBITS (bit_buf << 6, 6) - 64;
	    if (i >= 64)
		break;

	    DUMPBITS (bit_buf, bits, 12);
	    NEEDBITS (bit_buf, bits, bit_ptr);
	    DUMPBITS (bit_buf, bits, 12);
	    NEEDBITS (bit_buf, bits, bit_ptr);
	}
    }
    dest[63] ^= mismatch & 1;
    DUMPBITS (bit_buf, bits, 4);
    decoder->bitstream_buf = bit_buf;
    decoder->bitstream_bits = bits;
    decoder->bitstream_ptr = bit_ptr;
}

#include <stdio.h>

int main (void)
{
    static mpeg2_decoder_t decoder;
    static uint8_t buf[2];

    decoder.bitstream_buf = 0x04000000;
    decoder.bitstream_bits = 6;
    decoder.bitstream_ptr = buf;

    get_intra_block_B15 (&decoder);

    printf ("%s\n", (decoder.bitstream_ptr == buf) ? "FAIL" : "PASS");

    return 0;
}
------------------------------------ end foo.c ---------------------

expected behaviour:

% gcc-3.3 foo.c; ./a.out
PASS

miscompilation with -fnew-ra:

% gcc-3.3 -O2 -fomit-frame-pointer -mcpu=pentiumpro -fnew-ra foo.c; ./a.out
FAIL

in the line marked XXXXXX, I replaced NEEDBITS with NEEDBITS_ - which
is exactly the same macro except that it does not use
__builtin_expect. Somehow this fixes my issue:

% gcc-3.3 -O2 -fomit-frame-pointer -mcpu=pentiumpro -fnew-ra foo.c; ./a.out
PASS

However it pops up again if I dont use -mcpu=pentiumpro:

% gcc-3.3 -O2 -fomit-frame-pointer -fnew-ra foo.c; ./a.out 
FAIL

And it goes away again if I dont use -fomit-frame-pointer:

% gcc-3.3 -O2 -fnew-ra foo.c; ./a.out 
PASS

More information about the gcc snapshot I've been using:

% gcc-3.3 -v
Reading specs from /usr/lib/gcc-lib/i386-linux/3.3/specs
Configured with: ../src/configure -v --enable-languages=c,c++,java,f77,pascal,objc,ada,treelang --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-gxx-include-dir=/usr/include/c++/3.3 --enable-shared --with-system-zlib --enable-nls --without-included-gettext --enable-__cxa_atexit --enable-clocale=gnu --enable-debug --enable-java-gc=boehm --enable-java-awt=xlib --enable-objc-gc i386-linux
Thread model: posix
gcc version 3.3 20030412 (Debian prerelease)

Please advise if you need any additional information. This routine
seems to be the only one that gets miscompiled in my mpeg2 decoder, so
I suppose -fnew-ra is not that far from being useable... hope you guys
can fix it :)

Cheers,

-- 
Michel "Walken" LESPINASSE
"In this time of war against Osama bin Laden and the oppressive
Taliban regime, we are thankful that OUR leader isn't the spoiled son
of a powerful politician from a wealthy oil family who is supported by
religious fundamentalists, operates through clandestine organizations,
has no respect for the democratic electoral process, bombs innocents,
and uses war to deny people their civil liberties." --The Boondocks