1087359 - Crash on WebM playback (@ vp9_setup_mask)

Status: RESOLVED DUPLICATE of bug 1085599

(Core :: Audio/Video, defect)

Description

[Cristina Madaras, QA [:CristinaM]]

Environment:
Build: Firefox for Android 36.0a1 (2014-10-22)
Devices:
Asus Transformer pad TF101 (Android 4.0.3)
Samsung galaxy tab 10.1 (Android 4.0.4)

Steps to reproduce:
1. Go to http://people.mozilla.com/~nhirata/html_tp/elephants-dream.webm;

Expected results:
Video starts playing.

Actual results:
Fennec crashes.

Notes: No Crash Report.

Comment 1

[Cristina Madaras, QA [:CristinaM]]

In about:crashes is displayed this Report ID:
https://crash-stats.mozilla.com/report/index/dfbc0ef9-9f5c-456e-9c1e-c134a2141022
but I can't open it.

Comment 2

[Aaron Train [:aaronmt]]

Please find a regression-window and use inbound.

Comment 3

[Cristina Madaras, QA [:CristinaM]]

Inbound regression:
Last good: 1413561588  
First bad: 1413561827  

Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=61df477b1505&tochange=5dc1be48edc3

Comment 4

[Aaron Train [:aaronmt]]

[Tracking Requested - why for this release]: Unable to play WebM video in Firefox for Android without crashing

Comment 5

[Gian-Carlo Pascutto [:gcp]]

Asus Transformer pad TF101 (Android 4.0.3)
Samsung galaxy tab 10.1 (Android 4.0.4)

As mentioned in the original bug, these are Tegra 2 devices, i.e. ARMv7 without NEON.

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

I think bug 1063356 is the culprit here, specifically:

Ralph Giles — Bug 1063356 - Pass -mfpu=neon when building libvpx. r=kinetik,mshal This is required to compile the new intrinsic-based simd acceleration code for arm.

I think -mfpu=neon means the compiler can use neon instructions as it sees fit, which is not going to work on devices without neon (like the Tegra2). Ralph can you back that out or fix some other way?

Comment 7

[Gian-Carlo Pascutto [:gcp]]

The GCC manuals disagree: https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html

"If the selected floating-point hardware includes the NEON extension (e.g. -mfpu=‘neon’), note that floating-point operations are not generated by GCC's auto-vectorization pass unless -funsafe-math-optimizations is also specified."

So this is different from for example -mfpu=sse vs -msse.

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #7)
> The GCC manuals disagree: https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html
> 
> "If the selected floating-point hardware includes the NEON extension (e.g.
> -mfpu=‘neon’), note that floating-point operations are not generated by
> GCC's auto-vectorization pass unless -funsafe-math-optimizations is also
> specified."
> 
> So this is different from for example -mfpu=sse vs -msse.

Yeah, I guess libvpx actually has inline neon asm causing this problem.

Comment 9

[Ralph Giles (:rillian)]

What :gcp said. At least, I thought we were safe on armv7 based on that manual text; I didn't verify there are no neon instructions in object files without neon intrinsics.

There are no neon intrinsics in the media/libvpx/vp9/common/vp9_loopfilter.c source where the reported crash occurs. Maybe something is sneaking in without runtime cpu detection protection.

I think it will take debugging the actual sigill location to figure this out. I can take a look tomorrow if no one beats me to it; I don't have my tegra2 device with me today.

Comment 10

[Ralph Giles (:rillian)]

Android ndk r10c release notes:

> Updated GCC 4.9 by rebasing to the google branch of the GCC repository. Major differences from the upstream version of GCC 4.9 include:
>
>    The -O2 option now turns on vectorization, without loop peeling but with more aggressive unrolling.

IIRC official builds are still on r8, but maybe we shouldn't trust the manpage. :/

Comment 11

[Ralph Giles (:rillian)]

My debug build crashes with an illegal instruction on media/libvpx/vp8/vp8_dx_iface.c:530.

   0x627db796 <+22>:	add	r7, sp, #24
   0x627db798 <+24>:	movs	r2, #28
=> 0x627db79a <+26>:	vmov.i32	d16, #0	; 0x00000000
   0x627db79e <+30>:	vstr	d16, [sp, #8]
   0x627db7a2 <+34>:	mov	r0, r7
   0x627db7a4 <+36>:	vstr	d16, [sp, #16]

vmov and vstr are part of both neon and vfp instructions. The d16 register is part of the vfpv3d32 extension, but tegra 2 only supports vfpv3d16. So this isn't an issue with neon specifically, but is a compiler flag issue.

Now to check whether bug 1085599 resolves the issue.

Comment 12

[Ralph Giles (:rillian)]

Yep, the patch from bug 1085599 fixes it. Should be resolved as soon as it makes its way into nightly. Sorry for the inconvenience.