search

kleopatra999 / webm

Automatically exported from code.google.com/p/webm
0 stars 0 forks source link

vp9_sub_pixel_variance*_(sse2|ssse3) are broken on 32-bit Darwin/OpenBSD (at least) #924

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
v1.3.0-5291-g7bd5994 (but likely since the introduction of GOT support)

32-bit --enable-pic / --enable-shard builds will fail the tests or cause a 
runtime error (SIGABRT) due to a corrupted stack. These are currently worked 
around by --disable-use-x86inc, but this is not optimal as it disables more 
assembly than necessary.

Related OpenBSD stack smashing issue [1].

[1] https://code.google.com/p/webm/issues/detail?id=808

Original issue reported on code.google.com by jz...@google.com on 15 Jan 2015 at 11:30

GoogleCodeExporter commented 9 years ago 
Yunqing you added the GET_GOT/RESTORE_GOT I believe and other instances appear 
to work, could you take a look please?

Original comment by jz...@google.com on 15 Jan 2015 at 11:31

GoogleCodeExporter commented 9 years ago

Original comment by ya...@google.com on 16 Jan 2015 at 4:19

GoogleCodeExporter commented 9 years ago 
Didn't see problem while doing vpxenc. But did see sub_pixel_variance unit test 
fail. Will look at that.

Original comment by yunqingw...@google.com on 16 Jan 2015 at 10:13

GoogleCodeExporter commented 9 years ago

Original comment by jz...@google.com on 23 Jan 2015 at 4:08

GoogleCodeExporter commented 9 years ago 
Note to be able to reproduce the issues as reported in ticket 808 (in theory) 
you should build libvpx on a 32-bit x86 system running some other OS with 
-fstack-protector-strong (which is the default on OpenBSD).

Original comment by brad.ope...@gmail.com on 23 Jan 2015 at 3:20

GoogleCodeExporter commented 9 years ago 
I built libvpx on mac book using the config options: --target=x86-darwin13-gcc 
--enable-pic --enable-use-x86inc. I saw the issue, and saw failed 
sub_pixel_variance unit test, and also saw encoding artifacts. The 
-fstack-protector (no -fstack-protector-strong for gcc I have) flag didn't help 
more.

The 32bit build with same config options works correctly on Linux.

Original comment by yunqingw...@google.com on 23 Jan 2015 at 6:22

GoogleCodeExporter commented 9 years ago 
Please note, that I'm happy to test diffs on OpenBSD/i386, if you have 
difficulties to repro the issue.

Original comment by mikolaj....@gmail.com on 23 Jan 2015 at 6:36

GoogleCodeExporter commented 9 years ago 
You need either GCC 4.9 or newer or LLVM 3.5 or newer.

Well unless you're using stack protector strong then the Linux build is just 
hiding the issue.

Original comment by brad.ope...@gmail.com on 23 Jan 2015 at 7:29

GoogleCodeExporter commented 9 years ago 
Actually with your older compiler you could probably use -fstack-protector-all 
to expose the issues.

Original comment by brad.ope...@gmail.com on 25 Jan 2015 at 6:45

GoogleCodeExporter commented 9 years ago 
Could you test if the patch:
https://gerrit.chromium.org/gerrit/#/c/73593/2
helps? Thanks.

Original comment by yunqingw...@google.com on 28 Jan 2015 at 12:24

GoogleCodeExporter commented 9 years ago 
Can you make a unified patch with changes? The interface at above gerrit url is 
so not intuitive.

Original comment by mikolaj....@gmail.com on 28 Jan 2015 at 12:34

GoogleCodeExporter commented 9 years ago 
Try attached diff please.

Original comment by yunqingw...@google.com on 28 Jan 2015 at 3:35

Attachments:

GoogleCodeExporter commented 9 years ago 
Attached patch from previous comment applied to libvpx commit fe2439703 makes 
ffmpeg encode webm with vp9 codec without SIGABRT. I've tested this on 
OpenBSD/i386 running on Intel Atom and on KVM-based virtual machine. They both 
work fine. Thanks!

libvpx was compiled with following options to ./configure:

--prefix=/usr/local  --disable-optimizations  --disable-unit-tests 
--disable-examples  --enable-runtime-cpu-detect  --enable-use-x86inc 
--enable-shared

Original comment by mikolaj....@gmail.com on 28 Jan 2015 at 1:30

GoogleCodeExporter commented 9 years ago 
Thank you for the testing.

I modified the patch since the change in vp9_intrapred_sse2.asm is not 
necessary. Here is the new diff file if you could try.

By the way, why is "--disable-optimizations" needed?

Original comment by yunqingw...@google.com on 28 Jan 2015 at 10:26

Attachments:

GoogleCodeExporter commented 9 years ago 
Yup, second patch works too on both machines.

Brad, can you comment on --disable-optimizations?

Original comment by mikolaj....@gmail.com on 30 Jan 2015 at 1:51

GoogleCodeExporter commented 9 years ago 
Great! The patch was already submitted in our Git repository.

Original comment by yunqingw...@google.com on 30 Jan 2015 at 1:56

GoogleCodeExporter commented 9 years ago 
We use --disable-optimizations to override the optimization flag passed in to 
the compiler. Our standard CFLAGS when built via the ports tree is -O2.

Original comment by brad.ope...@gmail.com on 30 Jan 2015 at 9:41

GoogleCodeExporter commented 9 years ago 
Ok. Thanks.

Original comment by yunqingw...@google.com on 30 Jan 2015 at 6:40