Bug#813858: gcc-5: [mips] regression: miscompilation caused by -fexpensive-optimizations

[Aurelien Jarno <aurelien@aurel32.net>]

On 2016-02-06 02:38, Andreas Cadhalpun wrote:
> Package: gcc-5
> Version: 5.3.1-7
> Severity: serious
> Justification: causes ffmpeg to FTBFS
> Control: affects -1 ffmpeg
> X-Debbugs-Cc: debian-mips@lists.debian.org
> 
> Dear Maintainer,
> 
> ffmpeg 7:2.8.6-1 failed to build on mips due to test failures.
> 
> I investigated the problem and it turns out to be caused by a compiler bug
> that can be avoided by using '-fno-expensive-optimizations'.
> (Somehow this feels like Déjà vu... #800318)
> 
> Attached is a reduced test case:
> $ ls
> Makefile  main.c  test.c
> $  make
> cc -fPIC -g -O2 -fno-expensive-optimizations -c -o working.o test.c
> cc -shared -o libworking.so working.o
> cc -fPIC -g -o working main.c -L. -lworking
> cc -fPIC -g -O2 -c -o broken.o test.c
> cc -shared -o libbroken.so broken.o
> cc -fPIC -g -o broken main.c -L. -lbroken
> LD_LIBRARY_PATH=. ./working || true
> chunk_size: 11760
> working
> LD_LIBRARY_PATH=. ./broken || true
> chunk_size: 0
> broken
> 
> This works correctly with gcc-5 5.3.1-6, so it is a regression in 5.3.1-7.

In my case I am able to reproduce the issue with both version 5.3.1-6
and 5.3.1-7, as well as 5.3.1-8. That said I don't think that GCC is
broken here, instead the source code is wrong.

> static uint32_t av_bswap32(uint32_t x)
> {
>     return ((((x) << 8 & 0xff00) | ((x) >> 8 & 0x00ff)) << 16 | ((((x) >> 16) << 8 & 0xff00) | (((x) >> 16) >> 8 & 0x00ff)));
> }

Note that this doesn't really produce optimized code. <endian.h> has a
better code which calls __builtin_bswap32 on a (not that) recent GCC.
Anyway that's not the problem here.

> int roq_read_packet(AVFormatContext *s, AVPacket *pkt)
> {
>     unsigned char preamble[8];
>     int i;
>     for (i = 0; i < 8; i += 1) {
>         preamble[i] = s->pb->buf_ptr[0];
>         s->pb->buf_ptr += 1;
>     }
>     return av_bswap32(*((uint32_t *) (&preamble[2])));

This is dereferencing a type-punned pointer, as reported by using -Wall.
This means that GCC can now assume that the value to fetch in preamble is
32-bit aligned, and that's why it generates code to bswap the aligned
32-bit code starting at position 4 instead of the one starting at position
2.

Note that this undefined behavior can be reproduced on amd64 by using
the -fsanitize=undefined option (ubsan):

| test.c:32:12: runtime error: load of misaligned address 0xffdf50da for type 'uint32_t', which requires 4 byte alignment
| 0xffdf50da: note: pointer points here
|  04 08  21 10 f0 2d 00 00 00 00  48 d1 df ff 60 c1 75 f7  eb 15 74 f7 d4 99 04 08  00 00 00 00 c0 84
| 
|               ^ 

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net