Bug#813858: gcc-5: [mips] regression: miscompilation caused by -fexpensive-optimizations
Control: forwarded -1 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69714
Control: tags -1 patch
Hi Aurelien,
On 08.02.2016 23:59, Aurelien Jarno wrote:
> On 2016-02-06 02:38, Andreas Cadhalpun wrote:
>> 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.
This is very strange, as ffmpeg 7:2.8.5-1 built successfully with gcc-5 5.3.1-6
on the mips buildd.
> That said I don't think that GCC is broken here, instead the source code is wrong.
I beg to differ.
>> 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.
ffmpeg has it's own optimized versions for more common architectures.
>> 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
> |
> | ^
These warnings are a red herring, as they are only an artifact from reducing
the testcase. They don't happen with the ffmpeg code and avoiding them in
the testcase, e.g. with the following diff, doesn't fix the problem:
--- a/test.c
+++ b/test.c
@@ -16,6 +16,8 @@
void *a;
} AVPacket;
+union unaligned_32 { uint32_t l; } __attribute__((packed)) __attribute__((may_alias));
+
static uint32_t av_bswap32(uint32_t x)
{
return ((((x) << 8 & 0xff00) | ((x) >> 8 & 0x00ff)) << 16 | ((((x) >> 16) << 8 & 0xff00) | (((x) >> 16) >> 8 & 0x00ff)));
@@ -29,5 +31,5 @@
preamble[i] = s->pb->buf_ptr[0];
s->pb->buf_ptr += 1;
}
- return av_bswap32(*((uint32_t *) (&preamble[2])));
+ return av_bswap32(((const union unaligned_32 *) (&preamble[2]))->l);
}
On the other hand, gcc-5 5.3.1-7 also regressed on hppa, causing different test failures
of ffmpeg. I've reported this to debian-hppa, where it was analyzed and forwarded
upstream. The patch [1] fixing that also fixes the regression on mips.
Best regards,
Andreas
1: https://gcc.gnu.org/bugzilla/attachment.cgi?id=37655&action=diff
Reply to: