Hi,

2012/1/9 Måns Rullgård <[email protected]>:
> "Ronald S. Bultje" <[email protected]> writes:
>> 2012/1/9 Måns Rullgård <[email protected]>:
>>> "Ronald S. Bultje" <[email protected]> writes:
>>>> 2012/1/9 Måns Rullgård <[email protected]>:
>>>>> "Ronald S. Bultje" <[email protected]> writes:
>>>>>> Fixes bug 78. I'd normally prefer a -mno-red-zone per-function
>>>>>> attribute, but it seems gcc doesn't support that yet (it's not in the
>>>>>> docs, and trying what should work extrapolating from other -m options
>>>>>> generated compiler errors saying it didn't recognize that attribute).
>>>>>>
>>>>>> Sean confirmed it fixes the crash.
>>>>>>
>>>>>> Ronald
>>>>>>
>>>>>> From 9908ee0200ee3911452f10c6214d9ba0425b1da7 Mon Sep 17 00:00:00 2001
>>>>>> From: Ronald S. Bultje <[email protected]>
>>>>>> Date: Sun, 20 Nov 2011 15:54:15 -0800
>>>>>> Subject: [PATCH] swscale: fix crash in fast_bilinear code when compiled 
>>>>>> with -mred-zone.
>>>>>>
>>>>>> ---
>>>>>>  libswscale/x86/swscale_template.c |   48 
>>>>>> +++++++++++++++++++++++++++++++++++++
>>>>>>  1 files changed, 48 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/libswscale/x86/swscale_template.c 
>>>>>> b/libswscale/x86/swscale_template.c
>>>>>> index 5e7df5c..c6d7e98 100644
>>>>>> --- a/libswscale/x86/swscale_template.c
>>>>>> +++ b/libswscale/x86/swscale_template.c
>>>>>> @@ -1656,10 +1656,22 @@ static void RENAME(hyscale_fast)(SwsContext *c, 
>>>>>> int16_t *dst,
>>>>>>  #if defined(PIC)
>>>>>>      DECLARE_ALIGNED(8, uint64_t, ebxsave);
>>>>>>  #endif
>>>>>> +#if ARCH_X86_64
>>>>>> +    DECLARE_ALIGNED(8, uint64_t, retsave);
>>>>>> +#endif
>>>>>>
>>>>>>      __asm__ volatile(
>>>>>>  #if defined(PIC)
>>>>>>          "mov               %%"REG_b", %5        \n\t"
>>>>>> +#if ARCH_X86_64
>>>>>> +        "mov               -8(%%rsp), %%"REG_a" \n\t"
>>>>>> +        "mov               %%"REG_a", %6        \n\t"
>>>>>> +#endif
>>>>>> +#else
>>>>>> +#if ARCH_X86_64
>>>>>> +        "mov               -8(%%rsp), %%"REG_a" \n\t"
>>>>>> +        "mov               %%"REG_a", %5        \n\t"
>>>>>> +#endif
>>>>>
>>>>> This is broken.  The compiler is perfectly free to allocate "retsave" in
>>>>> the red zone, even in the very location you are trying to save.
>>>>
>>>> But it wouldn't matter.
>>>>
>>>> First of all, we do a call. The called function doesn't call other
>>>> function, is pure assembly and doesn't use stack. So we only worry
>>>> about the ptrsize bytes taken up by the return address in the call
>>>> itself.
>>>>
>>>> Then, for these bytes, three options exist for the ptrsize bytes
>>>> present in the call return address before we do the call:
>>>> A) it is irrelevant memory. We don't care what it does.
>>>> B) it is retsave itself. See A).
>>>> C) it is important memory of a variable we want to save. If so, this
>>>> is not retsave. Thus, we can save it in retsave (regardless of whether
>>>> that's above rsp or somewhere below in the deeper red zone), pop later
>>>> and voila, the memory was restored properly.
>>>>
>>>> Now, I fully agree that it's a hack. It's commented as such. It will
>>>> disappear when ported to yasm. But, porting to yasm takes time and the
>>>> code should work now. So I'd like to apply this as a temp workaround.
>>>
>>> Just disable this nasty code for x86_64 then.
>>
>> It works with the workaround.
>
> But the workaround is horrible.

Most workarounds are.

I'll eventually move this code to yasm and it will disappear. All
inputs are done, only output and this fast-scaler is left, so it won't
be awfully long.

Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to