diff mbox

[FFmpeg-devel] configure: reserve a register for gcc before 5 on x86_32 with PIE

Message ID 1fac701c-6989-5e4f-3536-64d458e4e09e@googlemail.com
State Withdrawn
Headers show

Commit Message

Andreas Cadhalpun Nov. 20, 2016, 12:07 a.m. UTC
gcc before gcc-5 reserves a register on x86_32 for the GOT, when PIE is
enabled.

This fixes build failures due to:
error: 'asm' operand has impossible constraints

Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
---

A build log of a failed build with gcc 4.9 is available at:
https://buildd.debian.org/status/fetch.php?pkg=ffmpeg&arch=i386&ver=7%3A3.2-2~bpo8%2B1&stamp=1478791165

---
 configure | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Hendrik Leppkes Nov. 20, 2016, 12:52 a.m. UTC | #1
On Sun, Nov 20, 2016 at 1:07 AM, Andreas Cadhalpun
<andreas.cadhalpun@googlemail.com> wrote:
> gcc before gcc-5 reserves a register on x86_32 for the GOT, when PIE is
> enabled.
>
> This fixes build failures due to:
> error: 'asm' operand has impossible constraints
>
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
> ---
>
> A build log of a failed build with gcc 4.9 is available at:
> https://buildd.debian.org/status/fetch.php?pkg=ffmpeg&arch=i386&ver=7%3A3.2-2~bpo8%2B1&stamp=1478791165
>
> ---
>  configure | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/configure b/configure
> index b5bfad6..8794580 100755
> --- a/configure
> +++ b/configure
> @@ -5364,6 +5364,19 @@ EOF
>      enabled ssse3  && check_inline_asm ssse3_inline  '"pabsw %xmm0, %xmm0"'
>      enabled mmxext && check_inline_asm mmxext_inline '"pmaxub %mm0, %mm1"'
>
> +    case "$toolchain" in
> +        hardened)
> +            if enabled x86_32; then
> +                # When PIE is enabled on x86_32, gcc before gcc-5 reserves a register for the GOT.
> +                case $gcc_basever in
> +                    2*|3*|4*)
> +                        disable ebp_available
> +                    ;;
> +                esac
> +            fi
> +        ;;
> +    esac

Can't we build a test to detect this instead of hard-coding some constraints?

- Hendrik
Ronald S. Bultje Nov. 20, 2016, 3:19 a.m. UTC | #2
Hi,

On Sat, Nov 19, 2016 at 7:07 PM, Andreas Cadhalpun <
andreas.cadhalpun@googlemail.com> wrote:

> gcc before gcc-5 reserves a register on x86_32 for the GOT, when PIE is
> enabled.
>
> This fixes build failures due to:
> error: 'asm' operand has impossible constraints
>
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
> ---
>
> A build log of a failed build with gcc 4.9 is available at:
> https://buildd.debian.org/status/fetch.php?pkg=ffmpeg&
> arch=i386&ver=7%3A3.2-2~bpo8%2B1&stamp=1478791165
>
> ---
>  configure | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/configure b/configure
> index b5bfad6..8794580 100755
> --- a/configure
> +++ b/configure
> @@ -5364,6 +5364,19 @@ EOF
>      enabled ssse3  && check_inline_asm ssse3_inline  '"pabsw %xmm0,
> %xmm0"'
>      enabled mmxext && check_inline_asm mmxext_inline '"pmaxub %mm0, %mm1"'
>
> +    case "$toolchain" in
> +        hardened)
> +            if enabled x86_32; then
> +                # When PIE is enabled on x86_32, gcc before gcc-5
> reserves a register for the GOT.
> +                case $gcc_basever in
> +                    2*|3*|4*)
> +                        disable ebp_available
> +                    ;;
> +                esac
> +            fi
> +        ;;
> +    esac


This doesn't test whether PIE is enabled, it does it unconditionally. I'm
almost 100% sure that's not necessary.

Ronald
James Almer Nov. 20, 2016, 3:40 a.m. UTC | #3
On 11/20/2016 12:19 AM, Ronald S. Bultje wrote:
> Hi,
> 
> On Sat, Nov 19, 2016 at 7:07 PM, Andreas Cadhalpun <
> andreas.cadhalpun@googlemail.com> wrote:
> 
>> gcc before gcc-5 reserves a register on x86_32 for the GOT, when PIE is
>> enabled.
>>
>> This fixes build failures due to:
>> error: 'asm' operand has impossible constraints
>>
>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
>> ---
>>
>> A build log of a failed build with gcc 4.9 is available at:
>> https://buildd.debian.org/status/fetch.php?pkg=ffmpeg&
>> arch=i386&ver=7%3A3.2-2~bpo8%2B1&stamp=1478791165
>>
>> ---
>>  configure | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/configure b/configure
>> index b5bfad6..8794580 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5364,6 +5364,19 @@ EOF
>>      enabled ssse3  && check_inline_asm ssse3_inline  '"pabsw %xmm0,
>> %xmm0"'
>>      enabled mmxext && check_inline_asm mmxext_inline '"pmaxub %mm0, %mm1"'
>>
>> +    case "$toolchain" in
>> +        hardened)
>> +            if enabled x86_32; then
>> +                # When PIE is enabled on x86_32, gcc before gcc-5
>> reserves a register for the GOT.
>> +                case $gcc_basever in
>> +                    2*|3*|4*)
>> +                        disable ebp_available
>> +                    ;;
>> +                esac
>> +            fi
>> +        ;;
>> +    esac
> 
> 
> This doesn't test whether PIE is enabled, it does it unconditionally. I'm
> almost 100% sure that's not necessary.
> 
> Ronald

"-fPIE" is added unconditionally to cflags somewhere else in configure
if the hardened toolchain is used.

In any case, there's a test for ebp_available long after "-fPIE" is
added to cflags and it evidently passes on gcc < 5, so the proper way
to do this is to either fix/adapt the ebp_available check  (assuming
gcc < 5 x86_32 reserves that specific register when -fPIE is used), or
adding a different check that can detect this behavior and make it
alter HAVE_7REGS and HAVE_6REGS, in a similar fashion ebp_available
and ebx_available do.
Carl Eugen Hoyos Nov. 20, 2016, 12:38 p.m. UTC | #4
2016-11-20 1:07 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>:
> gcc before gcc-5 reserves a register on x86_32 for the GOT, when PIE is
> enabled.
>
> This fixes build failures due to:
> error: 'asm' operand has impossible constraints

Not reproducible with vanilla gcc 4.9.1 and gcc 4.4.6 so the patch is
not ok as-is afaict.

The tested (32bit) configure line here was:
 --enable-gpl --toolchain=hardened --disable-stripping --enable-shared

> A build log of a failed build with gcc 4.9 is available at:
> https://buildd.debian.org/status/fetch.php?pkg=ffmpeg&arch=i386&ver=7%3A3.2-2~bpo8%2B1&stamp=1478791165

Please consider to remove the following options from the configure line,
they do lead to more user questions and make support more difficult:
--disable-libtesseract
--disable-libebur128
--enable-sdl2

Is --disable-stripping really useful? Doesn't it only add symbols to
yasm-generated
files that cannot be read by gdb?
(Sorry if I misremember)

Carl Eugen
Nicolas George Nov. 20, 2016, 12:46 p.m. UTC | #5
Le decadi 30 brumaire, an CCXXV, Carl Eugen Hoyos a écrit :
> Is --disable-stripping really useful? Doesn't it only add symbols to
> yasm-generated
> files that cannot be read by gdb?
> (Sorry if I misremember)

Stripping means removing the static symbol tables from all libraries and
binaries. It also removes all debug information at the same time by
default.

For reference, libavcodec is about 12 Mo code (with my devel config on
x86_64) plus 46 Mo debug symbols.

Debian has the habit of shipping -dbg versions of library packages with
debug symbols included, to allow people to debug programs using these
libraries. I suppose the Debian framework has provisions to create both
the stripped and the debug packages from the installed files.

(I do not see the -dbg version of the ffmpeg libraries in the
repositories right now. Also, I do not understand why the debug
information cannot be shipped as a separate file.)

Regards,
Carl Eugen Hoyos Nov. 20, 2016, 12:50 p.m. UTC | #6
2016-11-20 13:46 GMT+01:00 Nicolas George <george@nsup.org>:
> Le decadi 30 brumaire, an CCXXV, Carl Eugen Hoyos a écrit :
>> Is --disable-stripping really useful? Doesn't it only add symbols to
>> yasm-generated
>> files that cannot be read by gdb?
>> (Sorry if I misremember)
>
> Stripping means removing the static symbol tables from all libraries and
> binaries. It also removes all debug information at the same time by
> default.

Of course.

My point is that without "--disable-stripping", the libraries are still
not stripped except for the additional yasm symbols that no
debugger can read (iiuc).

Carl Eugen
Nicolas George Nov. 20, 2016, 12:55 p.m. UTC | #7
Le decadi 30 brumaire, an CCXXV, Carl Eugen Hoyos a écrit :
> My point is that without "--disable-stripping", the libraries are still
> not stripped except for the additional yasm symbols that no
> debugger can read (iiuc).

I think you are wrong:

/tmp/i/usr/local/lib/libavcodec.so.57.66.102: ELF 64-bit LSB shared
object, x86-64, version 1 (SYSV), dynamically linked,
BuildID[sha1]=6973b96ca16916c88b0e42d660c18a60ae00a0f3, stripped

Notice: "stripped" at the end. Most build systems, starting with the
ones based on GNU autoconf/autobreak/autohell, do not strip by default,
but ffmpeg's does.
Carl Eugen Hoyos Nov. 20, 2016, 1:25 p.m. UTC | #8
2016-11-20 13:55 GMT+01:00 Nicolas George <george@nsup.org>:
> Le decadi 30 brumaire, an CCXXV, Carl Eugen Hoyos a écrit :
>> My point is that without "--disable-stripping", the libraries are still
>> not stripped except for the additional yasm symbols that no
>> debugger can read (iiuc).
>
> I think you are wrong:
>
> /tmp/i/usr/local/lib/libavcodec.so.57.66.102: ELF 64-bit LSB shared
> object, x86-64, version 1 (SYSV), dynamically linked,
> BuildID[sha1]=6973b96ca16916c88b0e42d660c18a60ae00a0f3, stripped

Thank you for explaining, I never install.

Does Debian use "make install"?

Carl Eugen
Andreas Cadhalpun Nov. 20, 2016, 6:48 p.m. UTC | #9
On 20.11.2016 13:38, Carl Eugen Hoyos wrote:
> 2016-11-20 1:07 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>:
>> gcc before gcc-5 reserves a register on x86_32 for the GOT, when PIE is
>> enabled.
>>
>> This fixes build failures due to:
>> error: 'asm' operand has impossible constraints
> 
> Not reproducible with vanilla gcc 4.9.1 and gcc 4.4.6 so the patch is
> not ok as-is afaict.

That's interesting. I just tried with gcc 4.8.4-1 and it works fine, too.
So this is probably a bug in gcc 4.9.2-10, which means we can't do anything
about it.

Consider this patch dropped.

> The tested (32bit) configure line here was:
>  --enable-gpl --toolchain=hardened --disable-stripping --enable-shared
> 
>> A build log of a failed build with gcc 4.9 is available at:
>> https://buildd.debian.org/status/fetch.php?pkg=ffmpeg&arch=i386&ver=7%3A3.2-2~bpo8%2B1&stamp=1478791165
> 
> Please consider to remove the following options from the configure line,
> they do lead to more user questions and make support more difficult:

Can you elaborate on how this makes support more difficult?

> --disable-libtesseract
> --disable-libebur128

These could be dropped. They were just added to the jessie-backports version,
as those libraries are too old/unavailable there.

> --enable-sdl2

I'd rather not remove this. The thing is that sdl2 is autodetected, like sdl
used to be, but ffplay depends on it. When 3.2 switched to sdl2 the build
silently worked, but didn't produce ffplay, which I found annoying.

> Is --disable-stripping really useful?

Yes.

> Doesn't it only add symbols to yasm-generated files that cannot be read by gdb?
> (Sorry if I misremember)

It prevents the removal of all symbols when installing.

On 20.11.2016 14:25, Carl Eugen Hoyos wrote:
> Does Debian use "make install"?

Yes, like this:
dh_auto_install --sourcedirectory=debian/standard
	make -j4 install DESTDIR=/«PKGBUILDDIR»/debian/tmp AM_UPDATE_INFO_DIR=no

The files from debian/tmp are then put into the different packages.

Best regards,
Andreas
Andreas Cadhalpun Nov. 20, 2016, 6:51 p.m. UTC | #10
On 20.11.2016 13:46, Nicolas George wrote:
> Debian has the habit of shipping -dbg versions of library packages with
> debug symbols included, to allow people to debug programs using these
> libraries. I suppose the Debian framework has provisions to create both
> the stripped and the debug packages from the installed files.
> 
> (I do not see the -dbg version of the ffmpeg libraries in the
> repositories right now. Also, I do not understand why the debug
> information cannot be shipped as a separate file.)

The *-dbg packages typically contained only the debug symbols as separated
files. I say contained, because the fancy new thing are automatically
created *-dbgsym packages (with the same content) distributed in a
separate *-debug suite. The wiki [1] has more details.

Best regards,
Andreas


1: https://wiki.debian.org/AutomaticDebugPackages
Carl Eugen Hoyos Nov. 20, 2016, 8:27 p.m. UTC | #11
2016-11-20 19:48 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>:
> On 20.11.2016 13:38, Carl Eugen Hoyos wrote:
>> 2016-11-20 1:07 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>:
>>> gcc before gcc-5 reserves a register on x86_32 for the GOT, when PIE is
>>> enabled.
>>>
>>> This fixes build failures due to:
>>> error: 'asm' operand has impossible constraints
>>
>> Not reproducible with vanilla gcc 4.9.1 and gcc 4.4.6 so the patch is
>> not ok as-is afaict.
>
> That's interesting. I just tried with gcc 4.8.4-1 and it works fine, too.
> So this is probably a bug in gcc 4.9.2-10, which means we can't do anything
> about it.
>
> Consider this patch dropped.

Thank you for doing the additional test.

>> The tested (32bit) configure line here was:
>>  --enable-gpl --toolchain=hardened --disable-stripping --enable-shared
>>
>>> A build log of a failed build with gcc 4.9 is available at:
>>> https://buildd.debian.org/status/fetch.php?pkg=ffmpeg&arch=i386&ver=7%3A3.2-2~bpo8%2B1&stamp=1478791165
>>
>> Please consider to remove the following options from the configure line,
>> they do lead to more user questions and make support more difficult:
>
> Can you elaborate on how this makes support more difficult?

My experience is that users copy the effect-less configure options
from distributions and ask on -users why they don't work.

>> --disable-libtesseract
>> --disable-libebur128
>
> These could be dropped. They were just added to the jessie-backports
> version, as those libraries are too old/unavailable there.

Please do.

>> --enable-sdl2
>
> I'd rather not remove this. The thing is that sdl2 is autodetected, like sdl
> used to be, but ffplay depends on it.

How is that different from before?

> When 3.2 switched to sdl2 the build silently worked, but didn't
> produce ffplay, which I found annoying.

But --enable-sdl2 made no difference at all, or do I misunderstand?

>> Is --disable-stripping really useful?
>
> Yes.

Thank you for explaining this!

Carl Eugen
Andreas Cadhalpun Nov. 21, 2016, 11:20 p.m. UTC | #12
On 20.11.2016 21:27, Carl Eugen Hoyos wrote:
> 2016-11-20 19:48 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>:
>> On 20.11.2016 13:38, Carl Eugen Hoyos wrote:
>>> --enable-sdl2
>>
>> I'd rather not remove this. The thing is that sdl2 is autodetected, like sdl
>> used to be, but ffplay depends on it.
> 
> How is that different from before?

Not at all.

>> When 3.2 switched to sdl2 the build silently worked, but didn't
>> produce ffplay, which I found annoying.
> 
> But --enable-sdl2 made no difference at all, or do I misunderstand?

Indeed, I just thought it would. Isn't that a bug?
Anyway, thanks for pointing this out.
Is there any way to make sure that configure does not disable ffplay?

Best regards,
Andreas
Carl Eugen Hoyos Nov. 22, 2016, 12:41 a.m. UTC | #13
2016-11-22 0:20 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>:
> On 20.11.2016 21:27, Carl Eugen Hoyos wrote:

>> But --enable-sdl2 made no difference at all, or do I misunderstand?
>
> Indeed, I just thought it would. Isn't that a bug?

It is neither a bug nor not a bug, it is just how FFmpeg's configure
works for approximately a decade.

Please remove --enable-sdl2 because it makes people think it
works the way you expected it to work.

> Anyway, thanks for pointing this out.

> Is there any way to make sure that configure does not disable ffplay?

I would expect something like (untested):
Requires: sdl-devel >= 2.0.1

Carl Eugen
Andreas Cadhalpun Nov. 22, 2016, 10:05 p.m. UTC | #14
On 22.11.2016 01:41, Carl Eugen Hoyos wrote:
> 2016-11-22 0:20 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>:
>> On 20.11.2016 21:27, Carl Eugen Hoyos wrote:
> 
>>> But --enable-sdl2 made no difference at all, or do I misunderstand?
>>
>> Indeed, I just thought it would. Isn't that a bug?
> 
> It is neither a bug nor not a bug, it is just how FFmpeg's configure
> works for approximately a decade.

Just that a bug existed for a long time does not mean it is not a bug.

> Please remove --enable-sdl2 because it makes people think it
> works the way you expected it to work.

The problem is really not that people think it works that way, but
rather that it actually works in a very confusing way.
Using --enable-sdl2 while libsdl2-dev is not installed does not cause
a configure failure, as it should, but instead causes the build to
fail later. Thus it achieves the desired end result, but in a very
confusing way. I really think this should be fixed.

Best regards,
Andreas
diff mbox

Patch

diff --git a/configure b/configure
index b5bfad6..8794580 100755
--- a/configure
+++ b/configure
@@ -5364,6 +5364,19 @@  EOF
     enabled ssse3  && check_inline_asm ssse3_inline  '"pabsw %xmm0, %xmm0"'
     enabled mmxext && check_inline_asm mmxext_inline '"pmaxub %mm0, %mm1"'
 
+    case "$toolchain" in
+        hardened)
+            if enabled x86_32; then
+                # When PIE is enabled on x86_32, gcc before gcc-5 reserves a register for the GOT.
+                case $gcc_basever in
+                    2*|3*|4*)
+                        disable ebp_available
+                    ;;
+                esac
+            fi
+        ;;
+    esac
+
     if ! disabled_any asm mmx yasm; then
         if check_cmd $yasmexe --version; then
             enabled x86_64 && yasm_extra="-m amd64"