| Summary: | media-libs/libmpeg2-0.5.1-r1 crashes in SSE code when compiled with gcc-4.6 | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Mart Raudsepp <leio> |
| Component: | New packages | Assignee: | Gentoo Media-video project <media-video> |
| Status: | RESOLVED WORKSFORME | ||
| Severity: | major | CC: | ainsaar, alex, b.brachaczek, esigra, gcc-porting, mziab, pacho, paolo.pedroni, quantheory, tdalman |
| Priority: | Normal | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| URL: | https://qa.mandriva.com/show_bug.cgi?id=63279 | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 346809 | ||
| Attachments: |
Fix patch from Mandriva
mark tables as used so they won't be optimized out Fix libmpeg2 inline ASM memory references for GCC-4.6 |
||
|
Description
Mart Raudsepp
2011-11-02 14:09:08 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 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. 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. 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.
(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. 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 Created attachment 302651 [details, diff]
Fix libmpeg2 inline ASM memory references for GCC-4.6
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).
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. 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? 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. (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. (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. Any progress on this? 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').
What is the status of this with gcc-4.6.3? Compiled with gcc 4.6.3, I have no problems even without a patch. 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. I guess this is fixed then? Please reopen if not. |