Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 389323 - media-libs/libmpeg2-0.5.1-r1 crashes in SSE code when compiled with gcc-4.6
Summary: media-libs/libmpeg2-0.5.1-r1 crashes in SSE code when compiled with gcc-4.6
Status: RESOLVED WORKSFORME
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: New packages (show other bugs)
Hardware: All Linux
: Normal major
Assignee: Gentoo Media-video project
URL: https://qa.mandriva.com/show_bug.cgi?...
Whiteboard:
Keywords:
Depends on:
Blocks: gcc-4.6
  Show dependency tree
 
Reported: 2011-11-02 14:09 UTC by Mart Raudsepp
Modified: 2013-01-07 06:59 UTC (History)
10 users (show)

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


Attachments
Fix patch from Mandriva (libmpeg2-0.5.1-gcc4.6.patch,6.56 KB, patch)
2011-11-02 14:19 UTC, Mart Raudsepp
Details | Diff
mark tables as used so they won't be optimized out (mpeg2-fix.diff,7.12 KB, patch)
2011-11-28 17:29 UTC, Luca Barbato
Details | Diff
Fix libmpeg2 inline ASM memory references for GCC-4.6 (libmpeg2_gcc46_mref_lvalue.patch,23.76 KB, patch)
2012-02-21 00:37 UTC, Jan Seiffert
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mart Raudsepp gentoo-dev 2011-11-02 14:09:08 UTC
Starting program: /usr/bin/mpeg2dec -v big_buck_bunny_480p_MPEG2_MP2_25fps_1800K.MPG
libmpeg2-0.5.1 - by Michel Lespinasse <walken@zoy.org> and Aaron Holtzman
     83b SEQUENCE MPEG2 MP@H-14 PROG 864x480 chroma 432x240 fps 25 maxBps 312000 vbv 229376 picture 854x480 display 854x480 pixel 1x1
     843 GOP  0: 0: 0: 0
     854 PICTURE I PROG fields 2 time_ref 0 offset 0/0

Program received signal SIGSEGV, Segmentation fault.
sse2_idct (block=0x60f500) at idct_mmx.c:973
973	idct_mmx.c: No such file or directory.
	in idct_mmx.c
(gdb) bt full
#0  sse2_idct (block=0x60f500) at idct_mmx.c:973
No locals.
#1  mpeg2_idct_copy_sse2 (block=0x60f500, dest=0x7ffff661cc00 "", stride=864) at idct_mmx.c:1228
No locals.
#2  0x00007ffff7478505 in slice_intra_DCT (stride=864, dest=0x7ffff661cc00 "", cc=0, decoder=0x60f3c0) at slice.c:954
No locals.
#3  mpeg2_slice (decoder=0x60f3c0, code=<optimized out>, buffer=<optimized out>) at slice.c:1844
        DCT_offset = 6912
        DCT_stride = 864
        offset = 0
        dest_y = 0x7ffff661cc00 ""
        macroblock_modes = 1
        mba_inc = <optimized out>
        mba = <optimized out>
        cpu_state = {dummy = 6374980}
#4  0x00007ffff746e5c8 in mpeg2_parse (mpeg2dec=0x60f3c0) at decode.c:188
        size_buffer = <optimized out>
        size_chunk = <optimized out>
        copied = <optimized out>
#5  0x0000000000402c6c in decode_mpeg2 (current=<optimized out>, end=<optimized out>) at mpeg2dec.c:279
        info = 0x613640
        state = <optimized out>
        setup_result = {convert = 0}
#6  0x00000000004022b0 in es_loop () at mpeg2dec.c:758
        buffer = 0x613bc0 ""
        end = 0x614bc0 ""
#7  main (argc=<optimized out>, argv=<optimized out>) at mpeg2dec.c:792
No locals.
(gdb)
Comment 1 Mart Raudsepp gentoo-dev 2011-11-02 14:19:29 UTC
Created attachment 291523 [details, diff]
Fix patch from Mandriva

Prior discussions with a few potential patches at
https://qa.mandriva.com/show_bug.cgi?id=63279

Also http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46615 and http://mandriva.598463.n5.nabble.com/Bug-63279-New-crash-in-SSE-code-with-gcc-4-6-0-compiled-mpeg2dec-td4389595.html


This patch fixes DVD and MPEG2 playing for me with gst-plugins-mpeg2dec, without this I can't view or dvdrip anything. The independent from libmpeg2 standalone implementation in mplayer/ffmpeg/libav doesn't appear affected
Comment 2 Mart Raudsepp gentoo-dev 2011-11-02 14:20:31 UTC
I have a local revision bump for this, just need ack on the attached patch if you want me to commit it to tree. I wasn't able to quickly find any upstream revision control for this to see what they are up to though and if this is the best solution, or if it's a potential problem in GCC.
Comment 3 Ryan Hill (RETIRED) gentoo-dev 2011-11-18 01:00:54 UTC
I say add it if no one from video speaks up soon.  It's hard to get an answer from them lately.  It'd be nice to see what upstream does though.
Comment 4 Luca Barbato gentoo-dev 2011-11-28 17:29:29 UTC
Created attachment 294081 [details, diff]
mark tables as used so they won't be optimized out

The patch is untested but should work properly, please test.
Comment 5 Mart Raudsepp gentoo-dev 2011-11-30 03:35:43 UTC
(In reply to comment #4)
> Created attachment 294081 [details, diff] [details, diff]
> mark tables as used so they won't be optimized out
> 
> The patch is untested but should work properly, please test.

$ mpeg2dec -v big_buck_bunny_480p_MPEG2_MP2_25fps_1800K.MPG 
libmpeg2-0.5.1 - by Michel Lespinasse <walken@zoy.org> and Aaron Holtzman
     83b SEQUENCE MPEG2 MP@H-14 PROG 864x480 chroma 432x240 fps 25 maxBps 312000 vbv 229376 picture 854x480 display 854x480 pixel 1x1
     843 GOP  0: 0: 0: 0
     854 PICTURE I PROG fields 2 time_ref 0 offset 0/0
Segmentation fault (core dumped)



(gdb) bt full
#0  sse2_idct_col (col=<optimized out>) at idct_mmx.c:475
No locals.
#1  sse2_idct (block=0x13cb500) at idct_mmx.c:972
No locals.
#2  mpeg2_idct_copy_sse2 (block=0x13cb500, dest=0x7f1851462c00 "", stride=864) at idct_mmx.c:1227
No locals.
#3  0x00007f18522be505 in slice_intra_DCT (stride=864, dest=0x7f1851462c00 "", cc=0, decoder=0x13cb3c0) at slice.c:954
No locals.
#4  mpeg2_slice (decoder=0x13cb3c0, code=<optimized out>, buffer=<optimized out>) at slice.c:1844
        DCT_offset = 6912
        DCT_stride = 864
        offset = 0
        dest_y = 0x7f1851462c00 ""
        macroblock_modes = 1
        mba_inc = <optimized out>
        mba = <optimized out>
        cpu_state = {dummy = 20776516}
#5  0x00007f18522b45c8 in mpeg2_parse (mpeg2dec=0x13cb3c0) at decode.c:188
        size_buffer = <optimized out>
        size_chunk = <optimized out>
        copied = <optimized out>
#6  0x0000000000402c6c in decode_mpeg2 (current=<optimized out>, end=<optimized out>) at mpeg2dec.c:279
        info = 0x13cf640
        state = <optimized out>
        setup_result = {convert = 0}
#7  0x00000000004022b0 in es_loop () at mpeg2dec.c:758
        buffer = 0x13cfbc0 ""
        end = 0x13d0bc0 ""
#8  main (argc=<optimized out>, argv=<optimized out>) at mpeg2dec.c:792
No locals.


-----

Note that now the crashing function is sse2_idct_col instead of sse2_idct. Not 100% sure the bitstream of that same file didn't change meanwhile, but 99% confident it's the same.
Comment 6 Mart Raudsepp gentoo-dev 2011-11-30 03:37:57 UTC
Oh, and this MPEG2 encoding is available at http://samplemedia.linaro.org/MPEG2/ for testing. Your apparent knowledge of ASM here is giving confidence on tracking down the real problem than what seems to be rather a trial-error hack in the other distros patches with random commenting out of constness. In particular in making sure this isn't a mishandling of code by GCC
Comment 7 Jan Seiffert 2012-02-21 00:37:36 UTC
Created attachment 302651 [details, diff]
Fix libmpeg2 inline ASM memory references for GCC-4.6
Comment 8 Jan Seiffert 2012-02-21 01:04:16 UTC
The Problem is not GCC, but the typical "Developer does not understand GCC inline ASM".
GCC does not understand your ASM text and can not infer from some magic heuristic what you want to do. But GCC also does not transform into some kind of obedient pet dog in the presence of inline ASM. You have to "talk" to GCC by the constraints at the end of the inline ASM. If you lie there to GCC, or simply do not fully understand the implications, things go booom. Maybe not in this version of GCC but the next.

The problems in libmpeg2 are two fold:
1) they try to pass arrays into inline ASM as a memory reference.
static const int16_t[] some_constants[] = {...}
asm volatile ("movdqa %0, %%xmm0" : : "m" (*some_constant));

This used to work, till GCC became to clever for this. Remember the dreaded "C-pointer-array-duality", they are the same and not the same. If you dereference an array you get the first element, because an array-name is a pointer to the first element (after it gets lowered), but the array name is also a place holder for the array if it is scope (sizeof(array_name)). Now GCC can see trough it and sees he does not have an lvalue, which is needed for an memory ref.

No amounts of __attribute__((used)) or whatever will fix this, only by accident by confusing the compiler.

The right solution is to pass a structure into the ASM, as byproduct this will also tell GCC the size of the access.

static const struct {int16_t[8];} some_constants = {{...}};
asm volatile ("movdqa %0, %%xmm0" : : "m" (some_constant));

2) It tries to stitch instruction sequences together by single instruction ASMs, which have no dependencies on one another. The only thing that prevents things go belly up is the volatile keyword, but that is only that good...

Normally all instructions have to be put into one single ASM statement if you do not (want to) describe how the register (xmm & mm) move between the ASM.

Basically this needs some major rewrite...


I wiped up a patch for 1) (see comment #7), it compiles here (x86_64, gcc 4.5 & gcc 4.6) and i don't get any SIGSEGV. (but don't know if it works, i only get an XError).
Comment 9 Tolga Dalman 2012-03-16 11:43:48 UTC
Am I right to assume that this is not a GCC bug but an application bug (with approaches to solve) ?

For the record: libmpeg2 works fine here with gcc-4.6.2.
Comment 10 Mart Raudsepp gentoo-dev 2012-03-16 11:46:31 UTC
The explanation in comment #8 sounds solid to me. Perhaps you don't end up in the SSE2 code paths?
Luca, could you check and comment on comment #8 and perhaps even get something in-tree?
Comment 11 Ryan Hill (RETIRED) gentoo-dev 2012-03-17 00:34:50 UTC
I want to unmask gcc-4.6.3 next weekend.  I don't consider this bug a blocker, but I sure would like it fixed first.
Comment 12 Samuli Suominen (RETIRED) gentoo-dev 2012-03-17 05:34:04 UTC
(In reply to comment #11)
> I want to unmask gcc-4.6.3 next weekend.  I don't consider this bug a
> blocker, but I sure would like it fixed first.

just if it wasn't clear already. Ryan, feel free to commit any patch for the issue in tree you are satisfied with.
Comment 13 Samuli Suominen (RETIRED) gentoo-dev 2012-06-01 04:35:13 UTC
(In reply to comment #7)
> Created attachment 302651 [details, diff] [details, diff]
> Fix libmpeg2 inline ASM memory references for GCC-4.6

Looks much more comprehensive than these others[1][2]

[1] http://projects.archlinux.org/svntogit/packages.git/tree/trunk/libmpeg2-0.5.1-gcc4.6.patch?h=packages/libmpeg2
http://bugs.archlinux.org/task/25370

[2] http://qa.mandriva.com/show_bug.cgi?id=63279#c5

Will wait for some feedback before pushing anything... 
so many people in CC list to test it.
Comment 14 Ryan Hill (RETIRED) gentoo-dev 2012-06-02 05:09:18 UTC
Any progress on this?
Comment 15 Alastair Murray 2012-07-03 04:00:04 UTC
I wanted to upgrade GCC so I thought I would test this patch -- but it turns out libmpeg2 works for me when compiled with GCC 4.6.3.

I tested the patch in attachment 302651 [details, diff] anyway, mpeg2dec still worked.  So the patch doesn't break decoding for one test video, I can't say if it fixes anything though.

I tested 0.5.1 (stable), 0.5.1-r1 (unstable) and 0.5.1-r1+patch with the commands:
mpeg2dec -s -o x11 big_buck_bunny_480p_MPEG2_MP2_25fps_1800K.MPG
mplayer -vc mpeg12 big_buck_bunny_480p_MPEG2_MP2_25fps_1800K.MPG

It worked identically for all libmpeg2 versions whether compiled with GCC 4.5.3 or 4.6.3.

By the way, upstream does not seem to have made any commits in over two years (based on 'svn log svn://svn.videolan.org/libmpeg2/trunk').
Comment 16 Pacho Ramos gentoo-dev 2012-09-23 14:18:14 UTC
What is the status of this with gcc-4.6.3?
Comment 17 Sean Santos 2012-11-01 04:12:13 UTC
Compiled with gcc 4.6.3, I have no problems even without a patch.
Comment 18 Sean Santos 2012-11-01 04:15:59 UTC
Er, I seem to have deleted a paragraph in the last comment. I used a similar command to comment 0, and this was with 0.5.1-r1.
Comment 19 Ryan Hill (RETIRED) gentoo-dev 2013-01-07 05:31:41 UTC
I guess this is fixed then?  Please reopen if not.