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

2016-12-20 Thread 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 
 ---
  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

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-12-19 Thread Michael Niedermayer 
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 
> >> ---
> >>  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 ?
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
that said, no objections to the patch 


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-12-19 Thread Andreas Cadhalpun 
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 
>> ---
>>  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.

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.

Best regards,
Andreas
>From 7b5d0714482a0ec42d317fa1e9fa095180e39ccd Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun 
Date: Fri, 16 Dec 2016 02:29:56 +0100
Subject: [PATCH] swscale: save ebx register when it is not available

Configure checks if the ebx register can be used for asm and it has to
be saved if and only if this is not the case.
Without this the build fails when configuring with --toolchain=hardened
--disable-pic on i386 using gcc 4.8:
error: PIC register clobbered by '%ebx' in 'asm'

In that case gcc 4.8 reserves the ebx register for the GOT needed for
PIE, so it can't be used in asm directly.

Signed-off-by: Andreas Cadhalpun 
---
 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 2cba5f0a1c..60a2cbfc50 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 !HAVE_EBX_AVAILABLE
 uint64_t ebxsave;
 #endif
 #endif
@@ -209,7 +209,7 @@ void ff_hyscale_fast_mmxext(SwsContext *c, int16_t *dst,
 "mov   -8(%%rsp), %%"FF_REG_a"\n\t"
 "mov%%"FF_REG_a", %5  \n\t"  // retsave
 #else
-#if defined(PIC)
+#if !HAVE_EBX_AVAILABLE
 "mov%%"FF_REG_b", %5  \n\t"  // ebxsave
 #endif
 #endif
@@ -255,7 +255,7 @@ void ff_hyscale_fast_mmxext(SwsContext *c, int16_t *dst,
 "mov  %5, %%"FF_REG_a" \n\t"
 "mov%%"FF_REG_a", -8(%%rsp)\n\t"
 #else
-#if defined(PIC)
+#if !HAVE_EBX_AVAILABLE
 "mov  %5, %%"FF_REG_b" \n\t"
 #endif
 #endif
@@ -264,12 +264,12 @@ void ff_hyscale_fast_mmxext(SwsContext *c, int16_t *dst,
 #if ARCH_X86_64
   ,"m"(retsave)
 #else
-#if defined(PIC)
+#if !HAVE_EBX_AVAILABLE
   ,"m" (ebxsave)
 #endif
 #endif
 : "%"FF_REG_a, "%"FF_REG_c, "%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_D
-#if ARCH_X86_64 || !defined(PIC)
+#if ARCH_X86_64 || HAVE_EBX_AVAILABLE
  ,"%"FF_REG_b
 #endif
 );
@@ -289,7 +289,7 @@ void ff_hcscale_fast_mmxext(SwsContext *c, int16_t *dst1, int16_t *dst2,
 #if ARCH_X86_64
 DECLARE_ALIGNED(8, uint64_t, retsave);
 #else
-#if defined(PIC)
+#if !HAVE_EBX_AVAILABLE
 DECLARE_ALIGNED(8, uint64_t, ebxsave);
 #endif
 #endif
@@ -298,7 +298,7 @@ void ff_hcscale_fast_mmxext(SwsContext *c, int16_t *dst1, int16_t *dst2,
 "mov  -8(%%rsp), %%"FF_REG_a"\n\t"
 "mov   %%"FF_REG_a", %7  \n\t"  // retsave
 #else
-#if defined(PIC)
+#if !HAVE_EBX_AVAILABLE
 "mov   %%"FF_REG_b", %7  \n\t"  // ebxsave
 #endif
 #endif
@@ -332,7 +332,7 @@ void ff_hcscale_fast_mmxext(SwsContext *c, int16_t *dst1, int16_t *dst2,
 "mov%7, %%"FF_REG_a" \n\t"
 "mov  %%"FF_REG_a", -8(%%rsp)\n\t"
 #else
-#if defined(PIC)
+#if !HAVE_EBX_AVAILABLE
 "mov %7, %%"FF_REG_b"\n\t"
 #endif
 #endif
@@ -341,12 +341,12 @@ void ff_hcscale_fast_mmxext(SwsContext *c, int16_t *dst1, int16_t *dst2,
 #if ARCH_X86_64
   ,"m"(retsave)
 #else
-#if defined(PIC)
+#if !HAVE_EBX_AVAILABLE
   ,"m" (ebxsave)
 #endif
 #endif
 : "%"FF_REG_a,

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

2016-12-15 Thread Michael Niedermayer 
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 
> ---
>  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


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-12-15 Thread Andreas Cadhalpun 
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 
---
 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__)
 uint64_t ebxsave;
 #endif
 #endif
@@ -209,7 +209,7 @@ void ff_hyscale_fast_mmxext(SwsContext *c, int16_t *dst,
 "mov   -8(%%rsp), %%"FF_REG_a"\n\t"
 "mov%%"FF_REG_a", %5  \n\t"  // retsave
 #else
-#if defined(PIC)
+#if defined(PIC) || defined(__PIE__)
 "mov%%"FF_REG_b", %5  \n\t"  // ebxsave
 #endif
 #endif
@@ -255,7 +255,7 @@ void ff_hyscale_fast_mmxext(SwsContext *c, int16_t *dst,
 "mov  %5, %%"FF_REG_a" \n\t"
 "mov%%"FF_REG_a", -8(%%rsp)\n\t"
 #else
-#if defined(PIC)
+#if defined(PIC) || defined(__PIE__)
 "mov  %5, %%"FF_REG_b" \n\t"
 #endif
 #endif
@@ -264,12 +264,12 @@ void ff_hyscale_fast_mmxext(SwsContext *c, int16_t *dst,
 #if ARCH_X86_64
   ,"m"(retsave)
 #else
-#if defined(PIC)
+#if defined(PIC) || defined(__PIE__)
   ,"m" (ebxsave)
 #endif
 #endif
 : "%"FF_REG_a, "%"FF_REG_c, "%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_D
-#if ARCH_X86_64 || !defined(PIC)
+#if ARCH_X86_64 || !(defined(PIC) || defined(__PIE__))
  ,"%"FF_REG_b
 #endif
 );
@@ -289,7 +289,7 @@ void ff_hcscale_fast_mmxext(SwsContext *c, int16_t *dst1, 
int16_t *dst2,
 #if ARCH_X86_64
 DECLARE_ALIGNED(8, uint64_t, retsave);
 #else
-#if defined(PIC)
+#if defined(PIC) || defined(__PIE__)
 DECLARE_ALIGNED(8, uint64_t, ebxsave);
 #endif
 #endif
@@ -298,7 +298,7 @@ void ff_hcscale_fast_mmxext(SwsContext *c, int16_t *dst1, 
int16_t *dst2,
 "mov  -8(%%rsp), %%"FF_REG_a"\n\t"
 "mov   %%"FF_REG_a", %7  \n\t"  // retsave
 #else
-#if defined(PIC)
+#if defined(PIC) || defined(__PIE__)
 "mov   %%"FF_REG_b", %7  \n\t"  // ebxsave
 #endif
 #endif
@@ -332,7 +332,7 @@ void ff_hcscale_fast_mmxext(SwsContext *c, int16_t *dst1, 
int16_t *dst2,
 "mov%7, %%"FF_REG_a" \n\t"
 "mov  %%"FF_REG_a", -8(%%rsp)\n\t"
 #else
-#if defined(PIC)
+#if defined(PIC) || defined(__PIE__)
 "mov %7, %%"FF_REG_b"\n\t"
 #endif
 #endif
@@ -341,12 +341,12 @@ void ff_hcscale_fast_mmxext(SwsContext *c, int16_t *dst1, 
int16_t *dst2,
 #if ARCH_X86_64
   ,"m"(retsave)
 #else
-#if defined(PIC)
+#if defined(PIC) || defined(__PIE__)
   ,"m" (ebxsave)
 #endif
 #endif
 : "%"FF_REG_a, "%"FF_REG_c, "%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_D
-#if ARCH_X86_64 || !defined(PIC)
+#if ARCH_X86_64 || !(defined(PIC) || defined(__PIE__))
  ,"%"FF_REG_b
 #endif
 );
-- 
2.10.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel