[FFmpeg-devel] [PATCH] swscale: also save ebx register when using PIE

[Andreas Cadhalpun]

On 20.12.2016 03:04, Michael Niedermayer wrote:
> On Mon, Dec 19, 2016 at 11:28:44PM +0100, Andreas Cadhalpun wrote:
>> On 16.12.2016 04:08, Michael Niedermayer wrote:
>>> On Fri, Dec 16, 2016 at 02:36:53AM +0100, Andreas Cadhalpun wrote:
>>>> Otherwise the build fails when configuring with --toolchain=hardened
>>>> --disable-pic on i386 using gcc 4.8:
>>>> error: PIC register clobbered by '%ebx' in 'asm'
>>>>
>>>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>>>> ---
>>>>  libswscale/x86/hscale_fast_bilinear_simd.c | 20 ++++++++++----------
>>>>  1 file changed, 10 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/libswscale/x86/hscale_fast_bilinear_simd.c b/libswscale/x86/hscale_fast_bilinear_simd.c
>>>> index 2cba5f0..3f0f5f5 100644
>>>> --- a/libswscale/x86/hscale_fast_bilinear_simd.c
>>>> +++ b/libswscale/x86/hscale_fast_bilinear_simd.c
>>>> @@ -199,7 +199,7 @@ void ff_hyscale_fast_mmxext(SwsContext *c, int16_t *dst,
>>>>  #if ARCH_X86_64
>>>>      uint64_t retsave;
>>>>  #else
>>>> -#if defined(PIC)
>>>> +#if defined(PIC) || defined(__PIE__)
>>>
>>> please correct me if iam wrong
>>> our configure adds -DPIC to define PIC when its enabled,
>>> it does not add that in this case but gcc is still generating PIC code
>>> that doesnt seem good
>>
>> gcc does not generate PIC, only PIE, which is subtly different.
> 
> does all the code under PIC work with
> PIE that does not have PIC set ?

It seems so, as the full FATE test passes.

> the identifier seems used a bit in .asm files
> 
>> What's wrong here is that this code in swscale tries to determine, whether
>> or not the ebx register can be used for asm, but doesn't check that correctly.
>> However, configure has a working check for that, the result of which can
>> be used here. Patch doing that is attached.
> 
> i see your argument for this and it seems sound.
> I hope this doesnt break anything as this logic was that way for a
> really long time and worked fine and gcc inline asm can be annoying

I think the regression potential is rather low, as current gcc does not require
saving the ebx register anymore.

> that said, no objections to the patch 

Pushed.

Best regards,
Andreas