Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 471756 - media-video/ffmpeg: fails to build on and64 hardened system when CFLAGS+=-fstack-check
Summary: media-video/ffmpeg: fails to build on and64 hardened system when CFLAGS+=-fst...
Status: RESOLVED OBSOLETE
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: media-video herd
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-29 22:16 UTC by Anthony Basile
Modified: 2015-02-18 10:42 UTC (History)
4 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
This patch fixes the issue by using matrix addressing instead (whose cost is negligible anyways). (this.patch,1.76 KB, patch)
2013-05-29 23:00 UTC, Francisco Blas Izquierdo Riera
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anthony Basile gentoo-dev 2013-05-29 22:16:56 UTC
The problem is similar to pic/pie failures in x86 when you run out of registers, only its rare in amd64 because of expanded number of available general regs.

CFLAGS="-fcheck-stack" emerge =media-videoa/ffmepg-0.10.7, we hit the following in libavcodec/x86/mlpdsp.c

x86_64-gentoo-linux-uclibc-gcc -I. -I./ -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600 -DPIC -DHAVE_AV_CONFIG_H -fstack-check -O2 -pipe -fstack-check -O2 -pipe  -std=c99 -fomit-frame-pointer -fPIC -pthread -I/usr/include/p11-kit-1 -I/usr/include/freetype2 -I/usr/include/dirac -D_REENTRANT -I/usr/include/p11-kit-1 -I/usr/include/schroedinger-1.0 -I/usr/include/orc-0.4  -D_GNU_SOURCE=1 -D_REENTRANT -I/usr/include/SDL -Wdeclaration-after-statement -Wall -Wno-parentheses -Wno-switch -Wno-format-zero-length -Wdisabled-optimization -Wpointer-arith -Wredundant-decls -Wno-pointer-sign -Wcast-qual -Wwrite-strings -Wtype-limits -Wundef -Wmissing-prototypes -Wno-pointer-to-int-cast -Wstrict-prototypes -fstack-check -O2 -pipe -fno-math-errno -fno-signed-zeros -fno-tree-vectorize -Werror=implicit-function-declaration -Werror=missing-prototypes -Werror=return-type  -MMD -MF libavcodec/x86/mlpdsp.d -MT libavcodec/x86/mlpdsp.o -c -o libavcodec/x86/mlpdsp.o libavcodec/x86/mlpdsp.c
libavcodec/x86/mlpdsp.c:44:36: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c:44:56: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c:45:36: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c:45:56: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c:46:36: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c:46:56: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c:47:36: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c:47:56: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c:48:36: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c:49:36: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c:49:56: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c:50:36: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c:50:56: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c:51:36: warning: taking address of expression of type 'void' [enabled by default]
libavcodec/x86/mlpdsp.c: In function 'mlp_filter_channel_x86':
libavcodec/x86/mlpdsp.c:123:5: error: can't find a register in class 'GENERAL_REGS' while reloading 'asm'
libavcodec/x86/mlpdsp.c:123:5: error: 'asm' operand has impossible constraints
make: *** [libavcodec/x86/mlpdsp.o] Error 1


Looking at the asm, which I report here without an ifdef, you can see where the problem is.  Around line 150 we see ...

         ...

        "sub  "ACCUM"   ,"RESULT"     \n\t"
        "mov  "RESULT32","IOFFS"(%0)  \n\t"
        "incl              %3         \n\t"
        "js 1b                        \n\t"
        : /* 0*/"+r"(state),
          /* 1*/"+r"(coeff),
          /* 2*/"+r"(sample_buffer),
          /* 3*/"+r"(blocksize)
        : /* 4*/"r"((x86_reg)mask), /* 5*/"r"(firjump),
          /* 6*/"r"(iirjump)      , /* 7*/"c"(filter_shift)
        , /* 8*/"r"((int64_t)coeff[0])
        , /* 9*/"r"((int64_t)coeff[1])
        , /*10*/"r"((int64_t)coeff[2])
        : "rax", "rdx", "rsi"
    );

Line /*10*/ is just one register too many.  I know it breask the code totally, but to convince yourself you're running out of registers, commenting out that line, it compiles.  So to fix this we need to play the sort of games we do in x86 to conserver registers, or compile without -fno-check-stack.






Reproducible: Always

Actual Results:
Comment 1 Francisco Blas Izquierdo Riera gentoo-dev 2013-05-29 23:00:38 UTC
Created attachment 349600 [details, diff]
This patch fixes the issue by using matrix addressing instead (whose cost is negligible anyways).

This is quick and dirty patch that should do the trick, it basically removes the hack of caching some coefficients letting the L1 cache handle that instead. The impact should be negligible or even a small win for the smaller register pressure.
Comment 2 Alexis Ballier gentoo-dev 2013-05-30 16:28:41 UTC
(In reply to Francisco Blas Izquierdo Riera from comment #1)
> Created attachment 349600 [details, diff] [details, diff]
> This patch fixes the issue by using matrix addressing instead (whose cost is
> negligible anyways).
> 
> This is quick and dirty patch that should do the trick, it basically removes
> the hack of caching some coefficients letting the L1 cache handle that
> instead. The impact should be negligible or even a small win for the smaller
> register pressure.

please submit it upstream; see http://ffmpeg.org/developer.html#Submitting-patches-1
they will probably ask you for a benchmark before & after the patch
Comment 3 Francisco Blas Izquierdo Riera gentoo-dev 2013-05-31 00:32:43 UTC
Taking into account that "The main priority in FFmpeg is simplicity and small code size in order to minimize the bug count." and the hack my patch reverted goes against the concept of simplicity I don't feel very compelled to send them the patch right now.
Comment 4 Anthony Basile gentoo-dev 2013-05-31 08:13:15 UTC
(In reply to Francisco Blas Izquierdo Riera from comment #3)
> Taking into account that "The main priority in FFmpeg is simplicity and
> small code size in order to minimize the bug count." and the hack my patch
> reverted goes against the concept of simplicity I don't feel very compelled
> to send them the patch right now.

I would be okay with a local patch for our hardened users when 4.8.1 brings in -fstack-check.  Something contingent on USE="hardened" so we don't burden other people with needs.

But!  Did we at least test that that code works?  I'm not sure how to trigger that code.
Comment 5 Alexis Ballier gentoo-dev 2013-05-31 15:43:50 UTC
(In reply to Anthony Basile from comment #4)
> But!  Did we at least test that that code works?  I'm not sure how to
> trigger that code.

that code seems to be used for decoding the following audio files:
MLP (Meridian Lossless Packing)
TrueHD


not sure where to find mlp samples but these seem to be truehd samples:
http://samples.mplayerhq.hu/A-codecs/TrueHD/
Comment 6 Magnus Granberg gentoo-dev 2014-03-22 20:08:32 UTC
ffmepg 1.2.6 build fine for me.
Can some one more test?
And what useflags?
Comment 7 Anthony Basile gentoo-dev 2014-03-22 23:12:16 UTC
(In reply to Magnus Granberg from comment #6)
> ffmepg 1.2.6 build fine for me.
> Can some one more test?
> And what useflags?

I am not able to reproduce with 1.2.6 using gcc-4.8.2 with -fstack-check.  I'm not sure what changed.  Here are my USE flags:

USE="3dnow 3dnowext X aac alsa amr bluray bzip2 cdio cpudetection encode gnutls gsm hardcoded-tables iconv jack jpeg2k libass libcaca libv4l mmx mmxext modplug mp3 network openal oss pic pulseaudio rtmp schroedinger sdl speex ssse3 static-libs theora threads v4l vaapi vorbis vpx xvid zlib -aacplus -bindist -debug -doc -examples -faac -fdk -flite -fontconfig -frei0r -iec61883 -ieee1394 -libsoxr -openssl -opus -truetype -twolame"

FFTOOLS="aviocat cws2fws ffeval fourcc2pixfmt graph2dot ismindex pktdumper qt-faststart trasher"
Comment 8 Anthoine Bourgeois 2014-04-04 19:53:21 UTC
See Also:
https://bugs.gentoo.org/show_bug.cgi?id=471756
Comment 9 Anthoine Bourgeois 2014-04-04 19:54:16 UTC
(In reply to Anthoine Bourgeois from comment #8)
> See Also:
> https://bugs.gentoo.org/show_bug.cgi?id=471756

https://bugs.gentoo.org/show_bug.cgi?id=506206
Sorry :-/
Comment 10 Anthony Basile gentoo-dev 2014-10-17 21:16:09 UTC
Rather than trying to fix the asm which will get no where upstream (and we're not even sure will work as intened), let's go with the same solution as for media-video/libav in bug #482258.

Basically we need to add something like

+	if [[ ${ABI} == amd64 ]] && gcc-specs-stack-check; then
+		append-flags $(test-flags -fno-stack-check)
+	fi
+

before we actually compile.
Comment 11 Alexis Ballier gentoo-dev 2015-01-27 09:23:29 UTC
I'm a bit lost: what versions of ffmpeg currently in tree are actually affected by this ?
Comment 12 Alexis Ballier gentoo-dev 2015-02-17 18:27:59 UTC
tried again with upstream's help:
gcc-4.7.4 fails, 4.8.4 & 4.9.2 are fine. This seems a gcc bug. Since gcc 4.8.3 is stable, maybe we can close this bug as obsolete?
Comment 13 Anthony Basile gentoo-dev 2015-02-17 18:32:24 UTC
(In reply to Alexis Ballier from comment #12)
> tried again with upstream's help:
> gcc-4.7.4 fails, 4.8.4 & 4.9.2 are fine. This seems a gcc bug. Since gcc
> 4.8.3 is stable, maybe we can close this bug as obsolete?

It is possible that 4.8 & 4.9 are making better use of the registers so ffmpeg's aggressive use doesn't exhaust them.  I haven't retested, but I do remember hitting this with 4.7, so I believe you of course that its fixed in 4.8.  Go ahead and close.
Comment 14 Alexis Ballier gentoo-dev 2015-02-18 10:42:49 UTC
(In reply to Anthony Basile from comment #13)
> (In reply to Alexis Ballier from comment #12)
> > tried again with upstream's help:
> > gcc-4.7.4 fails, 4.8.4 & 4.9.2 are fine. This seems a gcc bug. Since gcc
> > 4.8.3 is stable, maybe we can close this bug as obsolete?
> 
> It is possible that 4.8 & 4.9 are making better use of the registers so
> ffmpeg's aggressive use doesn't exhaust them.  I haven't retested, but I do
> remember hitting this with 4.7, so I believe you of course that its fixed in
> 4.8.  Go ahead and close.

yep, i should have phrased it as such instead of 'gcc bug' since inline asm spec is defined by what gcc accepts :)

closing as obsolete