879172 - crash in mozilla::MediaCacheStream::SetReadMode @ libmmparser.so on Sony Ericsson LT26 running JB 4.1

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Scoobidiver (away)]

It's #44 crasher in 21.0 and #26 in 22.0b3. It first showed up on April 28.

Here is the breakdown per device:
Sony Ericsson LT26i 	130
Sony Ericsson LT26w 	19
Sony Ericsson LT26ii 	11
Sony Ericsson LT28h 	2

Sony devices are blocklisted for Stagefright on JB (bug 845734) but Sony Ericsson ones are not (.Equals instead of .Find). See bp-dc2459be-ec55-4682-b70c-befb72130603.

Signature 	@0x0 | libmmparser.so@0x8cc72 More Reports Search
UUID	3df3b5f1-b326-4139-9f9e-d1edc2130604
Date Processed	2013-06-04 02:53:14
Uptime	224
Last Crash	3.9 minutes before submission
Install Age	4.0 days since version was first installed.
Install Time	2013-05-31 02:21:08
Product	FennecAndroid
Version	22.0
Build ID	20130528175857
Release Channel	beta
OS	Android
OS Version	0.0.0 Linux 3.4.0+1.0.21100-313065-g1ccebb5-00165-g78362b4 #1 SMP PREEMPT Wed Apr 24 10:01:32 2013 armv7l SEMC/LT26w_1266-2032/LT26w:4.1.2/6.2.B.0.200/N7__zg:user/release-keys
Build Architecture	arm
Build Architecture Info	ARMv0
Crash Reason	SIGSEGV
Crash Address	0x0
App Notes 	
AdapterDescription: 'Qualcomm -- Adreno (TM) 220 -- OpenGL ES 2.0 V@6.0 AU@1.04.01.01.06.041 (CL@) -- Model: LT26w, Product: LT26w_1266-2032, Manufacturer: Sony Ericsson, Hardware: semc'
EGL? EGL+ GL Context? GL Context+ GL Layers? GL Layers+ 
Sony Ericsson LT26w
SEMC/LT26w_1266-2032/LT26w:4.1.2/6.2.B.0.200/N7__zg:user/release-keys
Processor Notes 	sp-processor05_phx1_mozilla_com_19323:2012; exploitability tool: ERROR: unable to analyze dump
EMCheckCompatibility	True
Adapter Vendor ID	Qualcomm
Adapter Device ID	Adreno (TM) 220
Device	Sony Ericsson LT26w
Android API Version	16 (REL)
Android CPU ABI	armeabi-v7a

Frame 	Module 	Signature 	Source
0 		@0x0 	
1 	libmmparser.so 	libmmparser.so@0x8cc72 	
2 	linker 	linker@0x4e1d 	
3 	libstagefright.so 	libstagefright.so@0xfe6bf 	
4 	linker 	linker@0x14bc6 	
5 	linker 	linker@0x1489e 	
6 	linker 	linker@0x50e3 	
7 	libc.so 	libc.so@0x15699 	
8 	libmmparser.so 	libmmparser.so@0x4bf43 	
9 	libomxplugin.so (deleted) 	libomxplugin.so @0x23bf 	
10 	libstagefright.so 	libstagefright.so@0x688e7 	
11 	libomxplugin.so (deleted) 	libomxplugin.so @0x23bf 	
12 	libc.so 	libc.so@0x17247 	
13 	libstdc++.so 	libstdc++.so@0x8fd 	
14 	libstagefright.so 	libstagefright.so@0x67d49 	
15 	libstagefright.so 	libstagefright.so@0x67e6d 	
16 	libstagefright.so 	libstagefright.so@0x7e147 	
17 	libxul.so 	mozilla::MediaCacheStream::SetReadMode 	content/media/MediaCache.cpp:2018
18 	libomxplugin.so (deleted) 	libomxplugin.so @0x1dad 	

More reports at:
https://crash-stats.mozilla.com/report/list?signature=%400x0+|+libmmparser.so%400x8cc72

Comment 1

[Robert Kaiser]

This is rising fast in 21 crash stats on Android, #18 in yesterday's data.

Comment 2

[Scoobidiver (away)]

It's #12 crasher in 21.0 and #14 in 22.0b4.

The patch consist to replace:
          cManufacturer.Equals("Sony", nsCaseInsensitiveCStringComparator());
by
          cManufacturer.Find("Sony", true) != -1;

Comment 3

[Alex Keybl [:akeybl]]

(In reply to Robert Kaiser (:[e-mail]) [away until early June] from comment #1)
> This is rising fast in 21 crash stats on Android, #18 in yesterday's data.

This is due to http://au.ibtimes.com/articles/476496/20130609/sony-xperia-s-new-official-android-4.htm

Comment 4

[Alex Keybl [:akeybl]]

bjacob - does Scoobidiver's suggestion in Comment 2 sound like the right path forward?

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

Yup, it's correct, even correctly has the != -1 that I mistakenly forgot recently causing a lot of trouble :)

r+!

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

...except for one thing: we want to keep using a case-insensitive comparison. I just grepped through some crash reports, and a very small minority have 'sony' all lowercase, even in the Manufacturer field.

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: match Sony as a substring, case-insensitive

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/90f4975ba986

Scoobidiver: modulo the case-insensitive part, this is basically your patch. Next time don't hesitate to submit a patch and/or ask for help with that (say, on IRC #developers or #gfx).

Comment 9

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 761067 [details] [diff] [review]
match Sony as a substring, case-insensitive

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a, driver bug we were not yet blacklisting
User impact if declined: topcrasher on android
Testing completed (on m-c, etc.): m-i
Risk to taking this patch (and alternatives if risky): no risk thanks to having carefully read and re-read this one-line patch
String or IDL/UUID changes made by this patch: none

Comment 10

[Scoobidiver (away)]

(In reply to Benoit Jacob [:bjacob] from comment #8)
> Next time don't hesitate to submit a patch and/or ask for help with that
It's not easy to submit a patch on Windows according to https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Comment 11

[Benoit Jacob [:bjacob] (mostly away)]

The only Windows-specific issue is line endings; it is just a matter of telling your text editor to generate Unix-style line endings. There is no other windows-specific issue that I know (and I have generated patches under Windows). If a wiki page suggests otherwise, it may need to be clarified.

Comment 12

[Alex Keybl [:akeybl]]

We'll want to do some spot checking of this change to make sure we didn't disable more than we intended, Aaron.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Backed out for Android bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/bcf3b603a662

https://tbpl.mozilla.org/php/getParsedLog.php?id=24021356&tree=Mozilla-Inbound

Comment 14

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: patch that actually compiles (nsCString is crazy)

it turns out that while nsCString::Find on a nsCString takes a Comparator, nsCString::Find on a const char * wants a |bool ignoreCase| !!

Comment 15

[Benoit Jacob [:bjacob] (mostly away)]

As a result of this stupid compiler error, the tree has been closed, and I am not allowed to re-land on CLOSED TREE. This is the last time I rush a last minute blocklist fix. Future ones will go on tryserver no matter how urgent they may be.

Comment 16

[Benoit Jacob [:bjacob] (mostly away)]

Which means don't even bother asking me for a same-day fix.

Comment 17

[Benoit Jacob [:bjacob] (mostly away)]

pushed on CLOSED TREE with permission:

https://hg.mozilla.org/integration/mozilla-inbound/rev/34d230cab71c

Comment 18

[Benoit Jacob [:bjacob] (mostly away)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fff3cbfd6857
http://hg.mozilla.org/releases/mozilla-beta/rev/d172de3a2173

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/34d230cab71c

Comment 20

[Scoobidiver (away)]

I've updated https://wiki.mozilla.org/Blocklisting/Blocked_Graphics_Drivers accordingly.

Comment 21

[Aaron Train [:aaronmt]]

This was filed against Android 4.1, patched against Android 4.2 (http://mxr.mozilla.org/mozilla-beta/source/widget/android/GfxInfo.cpp#464) and documented against Android 4.1 - is this a mistake, should the check be in the if/else block above?

Comment 22

[Aaron Train [:aaronmt]]

nevermind I see how CompareVersions()'s works here in this case

Comment 23

[cajbir (:cajbir)]

Do those approvals mean I can just land?

Comment 24

[cajbir (:cajbir)]

Oops, wrong bug.

Comment 25

[Tracy Walker [:tracy]]

mass remove verifyme requests greater than 4 months old