[FFmpeg-devel] [PATCH] fix h264_deblock_sse2.asm segfaults on clang/x86-32

[Alexander Strange]

On Sep 1, 2010, at 1:53 AM, Reimar D?ffinger wrote:

> On Tue, Aug 31, 2010 at 07:06:09PM -0400, Ronald S. Bultje wrote:
>> on fate/clang/freebsd/x86-32 [1], all h264 tests fail with a sigbus.
>> There's some alignment issues in the sse2 asm. The asm appears to
>> assume that the caller function guarantees a certain alignment, which
>> is (IMO) not necessarily true. If the function is called directly from
>> C, stack can have any alignment, depending on the compiler used, so
>> the functions shouldn't assume any specific alignment imo.
> 
> We already use
> attribute_align_arg
> on the functions where it should be necessary since they are called
> from outside code and use optimizations and the compiler
> is supposed to keep sufficient alignment for all functions below.
> If that's not the case we might at least have performance issues
> with several other functions using aligned stack arguments,
> so it depends on that if there's any point in trying to fix that.

There are two issues; the assumed alignment on entry to a function and the alignment it retains when calling something else.

gcc on linux assumes 16-byte alignment for both, and realigns on entry with attribute_align_arg.

clang on linux/freebsd assumes 4-byte alignment, realigns to 16-byte on entry with attribute_align_arg, but only attempts to retain 4-byte alignment. So any callee code needing an aligned stack and using clang on freebsd/linux needs to realign it themselves.

Although they will probably accept gcc compatibility as reason to retain 16-byte stack alignment, the SysV i386 ABI does not actually require it, and I wouldn't be surprised if e.g. openbsd's pcc doesn't do it.

So I think this patch is the right thing to do. If the problem comes up again, we can generate a config.asm and define HOST_GCC_REALLY_IS_GCC if it doesn't need to realign.

Note x86-64 and Darwin x86-32 both do require a 16-byte aligned stack, so these problems don't come up (and can't be reproduced) here. Until AVX comes along.