Closed Bug 952321 Opened 6 years ago Closed 6 years ago

crash in mozilla::dom::DocumentBinding::genericMethod

Categories

(Core :: JavaScript Engine, defect, critical)

28 Branch
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla30
Tracking Status
firefox26 --- unaffected
firefox27 --- unaffected
firefox28 + verified
firefox29 + verified
firefox30 + verified
firefox-esr24 --- unaffected
b2g18 --- unaffected
b2g-v1.1hd --- unaffected
b2g-v1.2 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: ashughes, Assigned: efaust)

References

Details

(5 keywords)

Crash Data

Attachments

(2 files)

This bug was filed from the Socorro interface and is 
report bp-15e359e1-be74-4f1a-960a-401f72131216.
=============================================================
> 0 	xul.dll 	mozilla::dom::DocumentBinding::genericMethod 	obj-firefox/dom/bindings/DocumentBinding.cpp
> 1 	xul.dll 	nsStandardURL::SetUserPass(nsACString_internal const &) 	netwerk/base/src/nsStandardURL.cpp
> 2 	mozjs.dll 	js::jit::IonCannon(JSContext *,js::RunState &) 	js/src/jit/Ion.cpp

More reports:
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3Adom%3A%3ADocumentBinding%3A%3AgenericMethod

From bug 864511 comment 18, this signature has reappeared in high volume with Firefox 28. It is currently the #2 signature on Aurora and #5 on Nightly, non-existent for Beta and Release.

Top 20 URLs:
8 	http://www.sympatico.ca/defaultf.aspx?lang=fr-CA&symeid=www2
3 	http://www.nu.nl/achterklap/3655705/miley-cyrus-bleef-angst-bij-liam-hemsworth.html
3 	http://9gag.com/
3 	http://www.nu.nl/tech/3654626/ford-onthult-zelfrijdende-auto.html
3 	http://www.telegraaf.nl/
3 	http://www.tumblr.com/customize/allwayzshine?redirect_to=/dashboard
2 	http://www.nu.nl/net-binnen/
2 	http://www.globaltv.com/themillers/video/carols+parents+are+coming+to+town/video.html?v=88976963538#themillers/video/full+episodes
2 	http://www.meteomedia.com/tendance-meteo-14-jours/canada/quebec/montreal
2 	http://www.esquire.com/features/what-ive-learned/meaning-of-life-2012/gary-oldman-quotes-0112
2 	http://nos.nl/
2 	http://www.wpcentral.com/windows-phone-81-new-features-revealed
2 	http://www.nufoto.nl/
2 	http://www.nu.nl/film/3654944/actrice-joan-fontaine-96-overleden.html
2 	https://www.facebook.com/
2 	http://www.nu.nl/ondernemen/3655522/ryanair-sluit-basis-maastricht-aachen-airport.html
2 	about:blank
2 	http://www.nu.nl/film/3654587/soof-krijgt-gouden-film-veel-vroeg.html
2 	http://www.nu.nl/de-jager/
2 	http://www.rtlnieuws.nl/nieuws/buitenland/laatste-eer-mandela#node_585601

Add-on Correlations:
100% (43/43) vs.  89% (1268/1419) {972ce4c6-7e08-4474-a285-3208198ce6fd} (Default, https://addons.mozilla.org/addon/8150)
16% (7/43) vs.  10% (135/1419) {b9db16a4-6edc-47ec-a1f4-b86292ed211d} (Video DownloadHelper, https://addons.mozilla.org/addon/3006)
7% (3/43) vs.   1% (9/1419) {b6b1a201-b252-484f-b9fe-68efbb273fbd}

DLL Correlations:
95% (41/43) vs.  54% (762/1419) icm32.dll
58% (25/43) vs.  30% (422/1419) api-ms-win-downlevel-shlwapi-l2-1-0.dll
58% (25/43) vs.  30% (424/1419) api-ms-win-downlevel-advapi32-l2-1-0.dll
58% (25/43) vs.  33% (468/1419) api-ms-win-downlevel-ole32-l1-1-0.dll
58% (25/43) vs.  34% (484/1419) api-ms-win-downlevel-normaliz-l1-1-0.dll
58% (25/43) vs.  34% (484/1419) api-ms-win-downlevel-version-l1-1-0.dll
58% (25/43) vs.  34% (484/1419) api-ms-win-downlevel-user32-l1-1-0.dll
58% (25/43) vs.  34% (485/1419) api-ms-win-downlevel-advapi32-l1-1-0.dll
58% (25/43) vs.  34% (485/1419) api-ms-win-downlevel-shlwapi-l1-1-0.dll
86% (37/43) vs.  62% (883/1419) normaliz.dll
98% (42/43) vs.  76% (1075/1419) secur32.dll
33% (14/43) vs.  11% (160/1419) aticfx32.dll
100% (43/43) vs.  79% (1123/1419) iertutil.dll
81% (35/43) vs.  61% (871/1419) explorerframe.dll
81% (35/43) vs.  61% (871/1419) dui70.dll
67% (29/43) vs.  48% (676/1419) cscapi.dll
81% (35/43) vs.  62% (879/1419) duser.dll
100% (43/43) vs.  81% (1150/1419) rsaenh.dll
100% (43/43) vs.  81% (1152/1419) nssckbi.dll
100% (43/43) vs.  81% (1153/1419) softokn3.dll
100% (43/43) vs.  81% (1153/1419) nssdbm3.dll
100% (43/43) vs.  81% (1154/1419) freebl3.dll
28% (12/43) vs.   9% (134/1419) atidxx32.dll
100% (43/43) vs.  82% (1158/1419) rasadhlp.dll
28% (12/43) vs.  10% (137/1419) atiuxpag.dll
81% (35/43) vs.  63% (900/1419) FWPUCLNT.DLL
98% (42/43) vs.  80% (1132/1419) wbemcomn.dll
81% (35/43) vs.  64% (904/1419) apphelp.dll
70% (30/43) vs.  52% (740/1419) RpcRtRemote.dll
100% (43/43) vs.  82% (1170/1419) winrnr.dll
84% (36/43) vs.  67% (948/1419) NapiNSP.dll
84% (36/43) vs.  67% (949/1419) pnrpnsp.dll
98% (42/43) vs.  81% (1151/1419) fastprox.dll
95% (41/43) vs.  79% (1122/1419) ntmarta.dll
98% (42/43) vs.  82% (1157/1419) wbemsvc.dll
100% (43/43) vs.  84% (1191/1419) wininet.dll
86% (37/43) vs.  70% (995/1419) ntdsapi.dll
98% (42/43) vs.  82% (1164/1419) wbemprox.dll
72% (31/43) vs.  57% (805/1419) WSHTCPIP.DLL
72% (31/43) vs.  57% (806/1419) wship6.dll
81% (35/43) vs.  66% (940/1419) nlaapi.dll
100% (43/43) vs.  85% (1205/1419) dnsapi.dll
81% (35/43) vs.  66% (943/1419) cryptsp.dll
100% (43/43) vs.  86% (1216/1419) browsercomps.dll
100% (43/43) vs.  86% (1216/1419) icuuc50.dll
100% (43/43) vs.  86% (1216/1419) icudt50.dll
51% (22/43) vs.  37% (527/1419) slc.dll
100% (43/43) vs.  86% (1226/1419) wintrust.dll
100% (43/43) vs.  87% (1240/1419) mswsock.dll
30% (13/43) vs.  18% (251/1419) WLIDNSP.DLL
72% (31/43) vs.  60% (849/1419) Wldap32.dll
84% (36/43) vs.  72% (1015/1419) DWrite.dll
58% (25/43) vs.  46% (654/1419) mf.dll
58% (25/43) vs.  46% (654/1419) mfreadwrite.dll
60% (26/43) vs.  48% (687/1419) linkinfo.dll
58% (25/43) vs.  46% (659/1419) mfplat.dll
100% (43/43) vs.  89% (1258/1419) firefox.exe
60% (26/43) vs.  49% (702/1419) ntshrui.dll
84% (36/43) vs.  73% (1034/1419) AudioSes.dll
58% (25/43) vs.  47% (673/1419) dxva2.dll
81% (35/43) vs.  71% (1003/1419) sspicli.dll
84% (36/43) vs.  73% (1040/1419) MMDevAPI.dll
81% (35/43) vs.  72% (1023/1419) profapi.dll
9% (4/43) vs.   0% (4/1419) synrgyhk.dll
81% (35/43) vs.  73% (1030/1419) wkscli.dll
81% (35/43) vs.  73% (1030/1419) samcli.dll
81% (35/43) vs.  73% (1030/1419) netutils.dll
81% (35/43) vs.  73% (1030/1419) srvcli.dll
100% (43/43) vs.  91% (1295/1419) mscms.dll
91% (39/43) vs.  82% (1169/1419) msctf.dll
53% (23/43) vs.  45% (642/1419) atl.dll
100% (43/43) vs.  92% (1304/1419) crypt32.dll
100% (43/43) vs.  92% (1304/1419) msasn1.dll
58% (25/43) vs.  50% (711/1419) dxgi.dll
35% (15/43) vs.  27% (384/1419) msdmo.dll
53% (23/43) vs.  46% (649/1419) d2d1.dll
100% (43/43) vs.  92% (1311/1419) dbghelp.dll
53% (23/43) vs.  46% (651/1419) d3d10_1core.dll
53% (23/43) vs.  46% (651/1419) d3d10_1.dll
9% (4/43) vs.   2% (26/1419) ADvdDiscHlp1.dll
26% (11/43) vs.  18% (258/1419) devenum.dll
100% (43/43) vs.  93% (1314/1419) userenv.dll
44% (19/43) vs.  37% (523/1419) d3d11.dll
58% (25/43) vs.  51% (722/1419) avrt.dll
84% (36/43) vs.  77% (1086/1419) propsys.dll
7% (3/43) vs.   0% (3/1419) hmpalert.dll
77% (33/43) vs.  70% (994/1419) lpk.dll
21% (9/43) vs.  14% (205/1419) bcrypt.dll
84% (36/43) vs.  77% (1098/1419) IPHLPAPI.DLL
84% (36/43) vs.  77% (1098/1419) winnsi.dll
28% (12/43) vs.  22% (307/1419) winsta.dll
16% (7/43) vs.  10% (145/1419) davhlpr.dll
12% (5/43) vs.   6% (80/1419) RTWorkQ.dll
81% (35/43) vs.  76% (1072/1419) devobj.dll
28% (12/43) vs.  22% (315/1419) mdnsNSP.dll
12% (5/43) vs.   6% (84/1419) msxml6.dll
81% (35/43) vs.  76% (1074/1419) sechost.dll
81% (35/43) vs.  76% (1074/1419) CRYPTBASE.dll
81% (35/43) vs.  76% (1074/1419) KERNELBASE.dll
12% (5/43) vs.   6% (87/1419) evr.dll
12% (5/43) vs.   6% (88/1419) ondemandconnroutehelper.dll
9% (4/43) vs.   4% (55/1419) msmpeg2adec.dll
102% (44/43) vs.  97% (1378/1419) msvcr100.dll
16% (7/43) vs.  11% (158/1419) igdumd32.dll
81% (35/43) vs.  76% (1082/1419) cfgmgr32.dll
12% (5/43) vs.   7% (93/1419) igdumdx32.dll
Looking at the list of crash reports it looks like this first appeared in Firefox 28.0a1 on 20131208030204 which assumes the following pushlog:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=42b2a2adda8f&tochange=edac8cba9f78
Flagging this for QAWANTED to see if we can find steps to reproduce and narrow the regression window further. Andrei, can you please have your team take a look at this overnight tonight? Browsing various pages on nu.nl with Video Download Helper in Firefox 28.0a1 20131208030204 on Windows 7 32-bit is probably a good place to start.
Flags: needinfo?(andrei.vaida)
The vast majority of crashes are on address 0xffffffffffffff87.

Unfortunately, for generated files breakpad seems to not give us useful line numbers.  :(

So what's special about the value 0xffffffffffffff87?  At first glance, on 32-bit 0xFFFFFF87 is JSVAL_TAG_OBJECT.  But why would we ever dereference that?

Steps to reproduce would be amazingly awesome here.
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #2)
> Flagging this for QAWANTED to see if we can find steps to reproduce and
> narrow the regression window further. Andrei, can you please have your team
> take a look at this overnight tonight? Browsing various pages on nu.nl with
> Video Download Helper in Firefox 28.0a1 20131208030204 on Windows 7 32-bit
> is probably a good place to start.

Assigning this issue to Petruta for further investigations.
Flags: needinfo?(andrei.vaida)
QA Contact: petruta.rasa
I tried for several hours to reproduce this following the guidelines from comment 2 and comment 0 using both Nightly 28.0a1 and Aurora 28.0a2.
I reproduced the crash only once on Aurora under Win 7 x86 when I performed a video quick download using the Video Download Helper on the following page:
http://mavie.sympatico.ca/Videos/metier-ambulancier/?bclid=2653897211001&bctid=2686476728001

Here is the crash id:
https://crash-stats.mozilla.com/report/index/6cce3e9e-be0a-4f9f-b91a-181712131220

Unfortunately the steps to reproduce aren't reliable. 

I had no crashes using the same builds under Win XP, Win 8 and Win 8.1 x86.

Please let us know if we can help further.
QA Contact: petruta.rasa
(In reply to petruta.rasa from comment #5)
> I reproduced the crash only once on Aurora under Win 7 x86 when I performed
> a video quick download using the Video Download Helper.

Interestingly a lot of user reviews for this add-on report that it stopped working correctly on Dec 9th.
https://addons.mozilla.org/en-US/firefox/addon/video-downloadhelper/reviews/522294/

> I had no crashes using the same builds under Win XP, Win 8 and Win 8.1 x86.

That's not too surprising since >53% of the crashes are reported against Windows 7 x86.

Lukas, do we have any contact with the developer of this add-on? Would they be able to assist in the investigation? We are having a hard time finding reliable steps to reproduce.
Flags: needinfo?(lsblakk)
Passing the question in comment 6 to Jorge since he has better contacts with add-on developers.
Flags: needinfo?(lsblakk) → needinfo?(jorge)
Is the correlation strong enough? Video DownloadHelper is one of the most popular extensions out there so it might be just noise. I'm CCing the developer to bring this to his attention anyway.
Flags: needinfo?(jorge)
(In reply to Jorge Villalobos [:jorgev] from comment #8)
> Is the correlation strong enough? Video DownloadHelper is one of the most
> popular extensions out there so it might be just noise. I'm CCing the
> developer to bring this to his attention anyway.

It's not highly correlated but it is somewhat correlated:
> 100% (43/43) vs.  89% (1268/1419) {972ce4c6-7e08-4474-a285-3208198ce6fd} (Default, https://addons.mozilla.org/addon/8150)
> 16% (7/43) vs.  10% (135/1419) {b9db16a4-6edc-47ec-a1f4-b86292ed211d} (Video DownloadHelper, https://addons.mozilla.org/addon/3006)
> 7% (3/43) vs.   1% (9/1419) {b6b1a201-b252-484f-b9fe-68efbb273fbd}

Also the only time this reproduced in QA so far was when performing a video quick download using the Video Download Helper on http://mavie.sympatico.ca/Videos/metier-ambulancier/?bclid=2653897211001&bctid=2686476728001

It's not a strong correlation but it's the best/only lead we have at this point.
Current Rank:
> Firefox 29: No longer visible
> Firefox 28: #2 @ 3.2% (-1.14%)
> Firefox 27: N/A
> Firefox 26: N/A

Based on this it seems like the issue is dropping off. Unfortunately it's still a top crash for Firefox 28 and we don't have solid steps to reproduce this crash.
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #10)
> Current Rank:
> > Firefox 29: No longer visible
> > Firefox 28: #2 @ 3.2% (-1.14%)
> > Firefox 27: N/A
> > Firefox 26: N/A
> 
> Based on this it seems like the issue is dropping off. Unfortunately it's
> still a top crash for Firefox 28 and we don't have solid steps to reproduce
> this crash.

I'm seeing it as #4 on 29. Am I looking at the right numbers?
(In reply to David Major [:dmajor] from comment #11)
> I'm seeing it as #4 on 29. Am I looking at the right numbers?

Not sure why it wasn't showing up yesterday in the Nightly report for me but it's there now.

Firefox 29: #5 @ 1.81% (-1.02%)
Firefox 28: #2 @ 3.20% (-1.17%)
Firefox 27: N/A
Firefox 26: N/A
Tracking based on comment 12's crash positions - Naveen, does comment 9 help at all with ideas of what's new in FF28 JS engine that we could investigate here for a potential fix?
Flags: needinfo?(nihsanullah)
If I'm reading things correctly, the crash is at this code, inlined several levels into genericMethod:

inline const DOMClass*
GetDOMClass(JSObject* obj)
{
  const js::Class* clasp = js::GetObjectClass(obj);
  if (IsDOMClass(clasp)) {

6408ec54 8b46f8          mov     eax,dword ptr [esi-8]     ; obj
6408ec57 89442420        mov     dword ptr [esp+20h],eax 
6408ec5b 8b4804          mov     ecx,dword ptr [eax+4]     ; obj->type
6408ec5e 8b09            mov     ecx,dword ptr [ecx]       ; obj->type->clasp   *CRASH HERE*
6408ec60 f6410410        test    byte ptr [ecx+4],10h      ; IsDOMClass

So it looks like some object has a type pointer of 0xFFFFFF87 (JSVAL_TAG_OBJECT) which must be some kind of corruption.

The Video DownloadHelper correlation doesn't seem strong enough to be suspicious to me. What is suspicious is that the majority of reports have locale set to NL and/or were on a .nl site, frequently www.nu.nl. I tried visiting several of the pages but couldn't crash locally.
> which must be some kind of corruption.

Is this on 32-bit systems?  Does this ever happen on 64-bit?

Because this could conceivably happen if we somehow grabbed the wrong half of a Value on a 32-bit system when unboxing...  On the other hand, genericMethod does its own unboxing, in C++ code, not via the JIT, so it'd be pretty weird for that to just happen.
> Is this on 32-bit systems?  Does this ever happen on 64-bit?

Only 32-bit. (All those extra F's in the addresses must be a sign-extend somewhere in the reporting)
The request was answered in Comment 5, so I am dropping the qawanted keyword. Please add the keyword back if something more specific is needed.
Keywords: qawanted
This signature has dropped 0.64% on Nightly to #38 with 91 reports in the last week.
This signature has dropped 0.46% on Aurora to #3 with 241 reports in the last week.
As such this still remains a topcrash on Aurora but not on Nightly.

Here are some correlations just in case it's helpful.

Modules
=======
96% (46/48) vs.  63% (1001/1590) nlaapi.dll
92% (44/48) vs.  59% (938/1590) icm32.dll
96% (46/48) vs.  63% (1005/1590) pnrpnsp.dll
92% (44/48) vs.  59% (939/1590) dui70.dll
92% (44/48) vs.  59% (940/1590) explorerframe.dll
96% (46/48) vs.  63% (1007/1590) NapiNSP.dll
92% (44/48) vs.  60% (948/1590) duser.dll
92% (44/48) vs.  62% (981/1590) FWPUCLNT.DLL
83% (40/48) vs.  55% (868/1590) linkinfo.dll
81% (39/48) vs.  53% (836/1590) cscapi.dll
96% (46/48) vs.  67% (1073/1590) DWrite.dll
48% (23/48) vs.  21% (326/1590) mdnsNSP.dll
92% (44/48) vs.  64% (1025/1590) cryptsp.dll
83% (40/48) vs.  56% (894/1590) ntshrui.dll
77% (37/48) vs.  50% (801/1590) mf.dll
77% (37/48) vs.  50% (801/1590) mfreadwrite.dll
77% (37/48) vs.  51% (807/1590) mfplat.dll
71% (34/48) vs.  46% (725/1590) dxgi.dll
77% (37/48) vs.  52% (825/1590) dxva2.dll
96% (46/48) vs.  72% (1151/1590) AudioSes.dll
58% (28/48) vs.  35% (555/1590) d3d11.dll
96% (46/48) vs.  73% (1154/1590) MMDevAPI.dll
77% (37/48) vs.  54% (857/1590) avrt.dll
96% (46/48) vs.  73% (1165/1590) propsys.dll
98% (47/48) vs.  76% (1201/1590) ntmarta.dll
65% (31/48) vs.  43% (683/1590) d2d1.dll
65% (31/48) vs.  43% (690/1590) d3d10_1core.dll
65% (31/48) vs.  43% (690/1590) d3d10_1.dll
96% (46/48) vs.  75% (1191/1590) winnsi.dll
96% (46/48) vs.  75% (1195/1590) IPHLPAPI.DLL
92% (44/48) vs.  71% (1131/1590) profapi.dll
92% (44/48) vs.  71% (1131/1590) samcli.dll
92% (44/48) vs.  71% (1132/1590) wkscli.dll
92% (44/48) vs.  71% (1132/1590) netutils.dll
92% (44/48) vs.  71% (1132/1590) srvcli.dll
73% (35/48) vs.  53% (838/1590) wship6.dll
96% (46/48) vs.  76% (1213/1590) powrprof.dll
96% (46/48) vs.  77% (1218/1590) dwmapi.dll
96% (46/48) vs.  77% (1218/1590) nsi.dll
35% (17/48) vs.  17% (271/1590) devenum.dll
73% (35/48) vs.  55% (870/1590) Wldap32.dll
92% (44/48) vs.  74% (1172/1590) devobj.dll
98% (47/48) vs.  80% (1275/1590) msctf.dll
58% (28/48) vs.  41% (646/1590) slc.dll
35% (17/48) vs.  18% (282/1590) WLIDNSP.DLL
100% (48/48) vs.  82% (1309/1590) nssckbi.dll
92% (44/48) vs.  74% (1177/1590) sechost.dll
92% (44/48) vs.  74% (1177/1590) CRYPTBASE.dll
92% (44/48) vs.  74% (1177/1590) KERNELBASE.dll
71% (34/48) vs.  53% (850/1590) WSHTCPIP.DLL
100% (48/48) vs.  83% (1314/1590) freebl3.dll
67% (32/48) vs.  49% (784/1590) RpcRtRemote.dll
100% (48/48) vs.  83% (1315/1590) nssdbm3.dll
100% (48/48) vs.  83% (1315/1590) softokn3.dll
98% (47/48) vs.  81% (1288/1590) rsaenh.dll
100% (48/48) vs.  83% (1325/1590) winrnr.dll
92% (44/48) vs.  75% (1193/1590) cfgmgr32.dll
63% (30/48) vs.  46% (731/1590) ksuser.dll
83% (40/48) vs.  67% (1063/1590) sspicli.dll
100% (48/48) vs.  84% (1329/1590) rasadhlp.dll
56% (27/48) vs.  41% (654/1590) dhcpcsvc6.DLL
63% (30/48) vs.  48% (766/1590) atl.dll
94% (45/48) vs.  80% (1274/1590) fastprox.dll
94% (45/48) vs.  81% (1284/1590) wbemsvc.dll
100% (48/48) vs.  87% (1384/1590) dnsapi.dll
94% (45/48) vs.  81% (1290/1590) wbemprox.dll
100% (48/48) vs.  87% (1391/1590) browsercomps.dll
100% (48/48) vs.  87% (1391/1590) icudt50.dll
100% (48/48) vs.  87% (1391/1590) icuuc50.dll
100% (48/48) vs.  88% (1396/1590) wintrust.dll
79% (38/48) vs.  68% (1076/1590) apphelp.dll
100% (48/48) vs.  89% (1411/1590) firefox.exe
17% (8/48) vs.   6% (95/1590) ncrypt.dll
100% (48/48) vs.  89% (1421/1590) mswsock.dll
58% (28/48) vs.  48% (765/1590) dhcpcsvc.dll
15% (7/48) vs.   4% (70/1590) igd10iumd32.dll
27% (13/48) vs.  17% (270/1590) bcrypt.dll
15% (7/48) vs.   5% (72/1590) igdusc32.dll
38% (18/48) vs.  28% (440/1590) msdmo.dll
10% (5/48) vs.   1% (13/1590) sysfer.dll
100% (48/48) vs.  91% (1441/1590) dbghelp.dll
25% (12/48) vs.  16% (249/1590) winhttp.dll
33% (16/48) vs.  24% (383/1590) wshbth.dll
88% (42/48) vs.  78% (1248/1590) wbemcomn.dll
23% (11/48) vs.  14% (222/1590) aticfx32.dll
25% (12/48) vs.  16% (256/1590) SHCore.dll
25% (12/48) vs.  16% (256/1590) bcryptPrimitives.dll
25% (12/48) vs.  16% (256/1590) WINMMBASE.dll
25% (12/48) vs.  16% (256/1590) combase.dll
21% (10/48) vs.  12% (192/1590) atidxx32.dll
35% (17/48) vs.  27% (424/1590) api-ms-win-downlevel-advapi32-l2-1-0.dll
60% (29/48) vs.  52% (823/1590) icuin50.dll
13% (6/48) vs.   4% (63/1590) ntasn1.dll
35% (17/48) vs.  27% (431/1590) api-ms-win-downlevel-shlwapi-l2-1-0.dll
13% (6/48) vs.   4% (71/1590) atiumdva.dll
21% (10/48) vs.  13% (205/1590) atiuxpag.dll
17% (8/48) vs.   9% (140/1590) twinapi.dll
13% (6/48) vs.   5% (76/1590) atiumdag.dll
15% (7/48) vs.   7% (113/1590) ondemandconnroutehelper.dll
13% (6/48) vs.   5% (82/1590) atiu9pag.dll
17% (8/48) vs.   9% (149/1590) kernel.appcore.dll
25% (12/48) vs.  18% (282/1590) tiptsf.dll
100% (48/48) vs.  93% (1480/1590) mscms.dll
38% (18/48) vs.  31% (491/1590) d3d9.dll
15% (7/48) vs.   8% (133/1590) Bcp47Langs.dll
75% (36/48) vs.  69% (1094/1590) ntdsapi.dll
6% (3/48) vs.   0% (3/1590) katrack.dll
6% (3/48) vs.   0% (3/1590) sizerhook32.dll
6% (3/48) vs.   0% (3/1590) cham_ex32.dll
13% (6/48) vs.   6% (103/1590) RTWorkQ.dll
10% (5/48) vs.   4% (70/1590) DropboxExt.22.dll
6% (3/48) vs.   0% (4/1590) ScreenshotDll.dll
6% (3/48) vs.   0% (4/1590) wh_hook.dll
100% (48/48) vs.  94% (1501/1590) crypt32.dll
100% (48/48) vs.  94% (1501/1590) msasn1.dll
100% (48/48) vs.  95% (1506/1590) userenv.dll
6% (3/48) vs.   1% (16/1590) DXGIDebug.dll
13% (6/48) vs.   7% (116/1590) evr.dll

Add-ons
=======
35% (17/48) vs.  19% (308/1590) testpilot@labs.mozilla.com (Mozilla Labs - Test Pilot, https://addons.mozilla.org/addon/13661)
10% (5/48) vs.   1% (10/1590) {AE93811A-5C9A-4d34-8462-F7B864FC4696} (StumbleUpon, https://addons.mozilla.org/addon/138)
8% (4/48) vs.   1% (9/1590) {5F590AA2-1221-4113-A6F4-A4BB62414FAC} (SmoothWheel, https://addons.mozilla.org/addon/357)
8% (4/48) vs.   2% (25/1590) isreaditlater@ideashower.com (Read It Later, https://addons.mozilla.org/addon/7661)
98% (47/48) vs.  92% (1455/1590) {972ce4c6-7e08-4474-a285-3208198ce6fd} (Default, https://addons.mozilla.org/addon/8150)
8% (4/48) vs.   2% (33/1590) personas@christopher.beard (Personas, https://addons.mozilla.org/addon/10900)
6% (3/48) vs.   0% (3/1590) jid0-ODIKJS9b4IT3H1NYlPKr0NDtLuE@jetpack
6% (3/48) vs.   0% (3/1590) addtopicasa@victor.coustenoble (AddToPicasa, https://addons.mozilla.org/addon/5699)
6% (3/48) vs.   0% (3/1590) mediahint@jetpack
6% (3/48) vs.   0% (3/1590) donottrack@checkpoint.com
6% (3/48) vs.   0% (3/1590) cutyfox@apps.metzweb.net
6% (3/48) vs.   0% (3/1590) {65e41d20-f092-41b7-bb83-c6e8a9ab0f57}
6% (3/48) vs.   0% (3/1590) {3c6e1eed-a07e-4c80-9cf3-66ea0bf40b37} (Dust-Me Selectors, https://addons.mozilla.org/addon/5392)
6% (3/48) vs.   0% (3/1590) shortly@aloshbennett.in
6% (3/48) vs.   0% (3/1590) {E2883E8F-472F-4fb0-9522-AC9BF37916A7} (Adobe Flash Extension, https://addons.mozilla.org/addon/13845)
6% (3/48) vs.   0% (3/1590) Restart@schuzak.jp
6% (3/48) vs.   0% (4/1590) {394DCBA4-1F92-4f8e-8EC9-8D2CB90CB69B}
6% (3/48) vs.   0% (4/1590) {99210d54-6321-41e8-bd1b-2b4c55874efb} (Tumblr Post, https://addons.mozilla.org/addon/5867)
6% (3/48) vs.   0% (5/1590) gmailadsremover@florian.bersier
6% (3/48) vs.   0% (5/1590) {04426594-bce6-4705-b811-bcdba2fd9c7b} (Firesizer, https://addons.mozilla.org/addon/5792)
6% (3/48) vs.   0% (5/1590) {53A03D43-5363-4669-8190-99061B2DEBA5} (ScrapBook, https://addons.mozilla.org/addon/427)
6% (3/48) vs.   0% (6/1590) firefox-extension@shareaholic.com (Shareaholic for Firefox, https://addons.mozilla.org/addon/5457)
6% (3/48) vs.   0% (7/1590) {0545b830-f0aa-4d7e-8820-50a4629a56fe} (ColorfulTabs, https://addons.mozilla.org/addon/1368)
6% (3/48) vs.   0% (7/1590) {77b819fa-95ad-4f2c-ac7c-486b356188a9} (IE Tab, https://addons.mozilla.org/addon/1419)
6% (3/48) vs.   1% (8/1590) tiletabs@DW-dev
6% (3/48) vs.   1% (9/1590) nosquint@urandom.ca (NoSquint, https://addons.mozilla.org/addon/2592)
6% (3/48) vs.   1% (11/1590) jid0-GjwrPchS3Ugt7xydvqVK4DQk8Ls@jetpack
6% (3/48) vs.   1% (14/1590) {37fa1426-b82d-11db-8314-0800200c9a66} (WebMail Notifier, https://addons.mozilla.org/addon/4490)


Naveed's needinfo request has been outstanding for a couple weeks now. David, does this information give you any leads?
Flags: needinfo?(dmajor)
I don't have enough domain expertise on this one... efaust, any ideas about comment 14?
Flags: needinfo?(dmajor) → needinfo?(efaustbmo)
Group: core-security
OK, I should start by saying I can't prove any of this, and it may well end up being a wild goose chase, but I have/had the following thought:

> So what's special about the value 0xffffffffffffff87?  At first glance, on
> 32-bit 0xFFFFFF87 is JSVAL_TAG_OBJECT.  But why would we ever dereference
> that?

What if the value genericMethod() unboxed was bogusly generated by the JIT, and contained not a JSObject*, but a Value* erroneously tagged JSVAL_TAG_OBJECT because of bad TI? This would mean that the checks in genericMethod for object-ness would all pass on the value, since it had the right tag, and the unboxed "object" would still be a Value*. This means that on 32 bit systems the "obj->type" dereference:
> 6408ec5b 8b4804          mov     ecx,dword ptr [eax+4]     ; obj->type

loads not "obj->type" but the *tag* from the value, giving 0xFFFFFF87 and thus our bad dereference.

This incorrect behavior would be the result of bad MIRType tagging of the PassArg's argument on Aurora or the calls argument directly on Nightly, which implies bad TI. For this reason, I recommended marking the bug s-s until we can figure out if the theory is likely fact or fiction.

If this seems plausible (needifo bhackett for thoughts), then we should also consider marking bug 963316 as s-s, as it appears to be the same failure mode manifesting in a different place.

Unfortunately, without steps to reproduce, this one is gonna be very messy to prove, let alone fix.
Flags: needinfo?(efaustbmo) → needinfo?(bhackett1024)
(In reply to Eric Faust [:efaust] from comment #20)
> Unfortunately, without steps to reproduce, this one is gonna be very messy
> to prove, let alone fix.

Based on the information so far could you give us any leads we might follow to find steps to reproduce? Our last blind attempt (comment 5) was unsuccessful after several hours of testing.
Chris - can you help move this forward? It's been sitting a while with no assignee yet.
Flags: needinfo?(nihsanullah) → needinfo?(cpeterson)
jandem: efaust says bhackett is the best person to debug this crash, but bhackett is on PTO until "early February". This topcrash will have been merged from Aurora to Beta 28 by then. Do you have any hunches about TI changes in Nightly 28 that might have caused this crash? Or can you recommend another JS engineer to debug this until bhackett returns?
Flags: needinfo?(cpeterson) → needinfo?(jdemooij)
Chris, I can take a look. Maybe we can add some extra sanity checks in some strategic places to make it easier to reproduce.

dmajor is right, looking at comment 0 I see *a lot* of NL websites. Mostly popular Dutch news websites, that's suspicious. These websites are also mostly unrelated to each other so it's unlikely to be some JS snippet used by all of them.

I wonder if either a single user is crashing frequently, or if it's some add-on that's popular here in NL. I'll take a look at crash-stats to get some more info.
Bug 963316 definitely looks related. There we're crashing in GetElementIC::update -> IsOptimizableArgumentsObjectForGetElem -> IsOptimizableArgumentsObjectForLength -> JSObject::is<js::ArgumentsObject>() and also have an object type tag.

The first thing GetElementIC::update does with the object is call IsOptimizableArgumentsObjectForGetElem on it, that's probably why we crash there.

Unfortunately this bug and bug 963316 are both Windows-only as far as I can tell, so we can't rule out MSVC PGO issues etc.
Assigning to Jan while this is underway since we try not to track bug assigned to 'nobody'.
Assignee: nobody → jdemooij
So, Shu had an interesting realization that I failed to have. If the TI failure theory is right, it is not necessarily the case that we treated a Value like an Object. It's also possible that we treated an Object like a Value, and tagged into it, smashing the type field.
Two things we can do:

(1) Get a Try build that (a) Ion-compiles a bit more eagerly (b) has extra sanity checks that we have in debug builds also in opt builds and (c) is a PGO build in case it's an MSVC bug.

(2) Add some instrumentation to Ion GetElementIC::update (see comment 25) to recognize this case (bogus object pointer) and dump some info somewhere on the stack, so that we can analyze the minidumps and get it. Unfortunately I'm afraid we'd need to save a lot of info to fix this, but maybe the script's filename, lineno, sourceStart and pc could tell us something about the kind of code we're executing.
Pushed a patch to Try that lowers the Ion compile threshold to 50 and enables extra checks in opt builds:

Non-PGO: https://tbpl.mozilla.org/?tree=Try&rev=0e52afd8751d
Win32 PGO: https://tbpl.mozilla.org/?tree=Try&rev=a6455fae8f73

Also requested tests on all platforms in case that uncovers anything interesting.
Hmm, the Try build shows some Mochitest failures. Some look pretty interesting, although nothing seems related to this bug. I'll investigate and maybe do a Try push with even lower use count thresholds.
Filed bug 966665 for the M-1 orange. More next week.
Petruta, would you mind giving this build a shot?

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jandemooij@gmail.com-a6455fae8f73/try-win32/

It makes us JIT more aggressively so maybe it'll be easier to trigger the crash. I'll also try this build :)
Flags: needinfo?(petruta.rasa)
Jan, I had several crashes on this try build by just scrolling down on some of the http://www.nu.nl/achterklap and http://nu.nl/achterklap articles. (eg http://www.nu.nl/achterklap/3655705/miley-cyrus-bleef-angst-bij-liam-hemsworth.html; http://www.nu.nl/ondernemen/3655522/ryanair-sluit-basis-maastricht-aachen-airport.html)

The other websites noted on Comment 0 (nu.nl also) didn't crashed.
I also didn't encounter crashes while using the Video DownloadHelper add-on.

This is the crash report https://crash-stats.mozilla.com/report/index/bp-e12888c4-e72f-45ea-b5b5-324672140203.
The crash address is the same as for the initial issue, but the signature is different: 
@ xul.dll@0x2ad751
Flags: needinfo?(petruta.rasa)
(In reply to Petruta Rasa [QA] [:petruta] from comment #33)
> The crash address is the same as for the initial issue, but the signature is
> different: 
> @ xul.dll@0x2ad751

That's probably because we don't have debug symbols for the Try build.

Crash address of 0xffffffffffffff87 likely means it's the same bug, that's really great news! Thank you so much, I'll also try to repro.
I was able to reproduce this on OS X 32-bit with my patch to Ion-compile more. In debug builds the problem is obvious:

Assertion failure: hasValue(), at jit/RegisterSets.h:171

    frame #0: 0x076b68ae XUL`js::jit::TypedOrValueRegister::dataValue(this=0xbfff6e28) const + 110 at RegisterSets.h:171
    frame #1: 0x076b66c5 XUL`js::jit::TypedOrValueRegister::valueReg(this=0xbfff6e28) const + 21 at RegisterSets.h:210
    frame #2: 0x07423493 XUL`js::jit::GetPropertyIC::tryAttachDOMProxyUnshadowed(...) + 1699 at IonCaches.cpp:1455
    frame #3: 0x074252bd XUL`js::jit::GetPropertyIC::tryAttachProxy(...) + 829 at IonCaches.cpp:1520

So the problem is that GetPropertyIC::tryAttachDOMProxyUnshadowed does output().valueReg() but output is typed in this case (MIRType_Object).
Flags: needinfo?(jdemooij)
Flags: needinfo?(bhackett1024)
Eric, can you take this from here? You're probably most familiar with the DOM/proxy ICs.
Flags: needinfo?(efaustbmo)
I think the problem is this:

    // Make sure we observe our invariants if we're gonna deoptimize.
    if (!holder && (idempotent() || !output().hasValue()))
        return true;

In this case holder is non-null so we don't get to the !output().hasValue() part.

Based on "hg annotate" that'd make it a regression from bug 922493, but it was backported to 26 and 27 so I'm not sure why we're seeing these crashes only since 28.
Assignee: jdemooij → efaustbmo
This affects beta too so we should fix this ASAP.
This is topcrash #6 on Firefox 28 Beta.
OK, so, some stuff we learned while tooling around inside the crash this afternoon (many thanks to bz, who played remote-hands!)

The crash comes from an idempotent cache with only one site, in this function:

>    function getFlashObj(assetId) {
>        try {
>            this.assetID = assetId ? assetId : this.assetID;
>            var flashObj = (!this.fIsStd) ? this.getElementById(this.assetID) : document.getElementById(this.assetID);
>            return flashObj;
>        } catch (e) {}
>    }

This comes from http://ds.serving-sys.com/BurstingCachedScripts//SBTemplates_2_8_0_0/StdBannerEx.js?ai=18581874

The difference between this site and a standard testcase using getElementById is that no barrier is required in the crashing case, because a specific object is known, despite the |thisv| at the site being an HTMLDocument. In a standard testcase, we get TI for AnyObject.

The lookup of the global name gets barriered in the crashing case, where it does not get barriered in the standard case.

There are two things I still don't understand: How can we not barrier the getprop, since we have a DOMProxy, which must have unknownProperties(), and why do we require a barrier when doing the name lookup on the global?

It's like TI thinks it's looking at the this case, but the IC gets the document case? We should take a look at what object TI is so sure is coming through that site.
Flags: needinfo?(efaustbmo)
Attached patch FixSplinter Review
Attachment #8374507 - Flags: review?(jdemooij)
Attachment #8374503 - Flags: review?(jdemooij) → review+
Comment on attachment 8374507 [details] [diff] [review]
Fix

Review of attachment 8374507 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, looks good.

::: js/src/jit/IonCaches.cpp
@@ +1363,4 @@
>      JS_ASSERT(canAttachStub());
>      JS_ASSERT(!*emitted);
>      JS_ASSERT(IsCacheableDOMProxy(obj));
> +    JS_ASSERT(output().hasValue());

Can you also JS_ASSERT(monitoredResult()) here?

The caller checks this, but it can't hurt to assert in case something else will call this later (and it's good to be reminded of it when reading the code).

@@ +1408,4 @@
>      JS_ASSERT(canAttachStub());
>      JS_ASSERT(!*emitted);
>      JS_ASSERT(IsCacheableDOMProxy(obj));
> +    JS_ASSERT(output().hasValue());

And here.

@@ +1549,4 @@
>      JS_ASSERT(canAttachStub());
>      JS_ASSERT(!*emitted);
>      JS_ASSERT(obj->is<ProxyObject>());
> +    JS_ASSERT(output().hasValue());

And here.
Attachment #8374507 - Flags: review?(jdemooij) → review+
Comment on attachment 8374507 [details] [diff] [review]
Fix

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Not. We can't even get our STR to crash half the time. There is no additional minimized test in the patch.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

No. You could figure out roughly what was going on, but getting it to happen would be hard.

Which older supported branches are affected by this flaw?

Aurora 29, Beta 28.

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Yes, also in bug.

How likely is this patch to cause regressions; how much testing does it need?

Very unlikely. It keeps the jit from generating code in a specific circumstance.
Attachment #8374507 - Flags: sec-approval?
We need to get a security rating on this for approval. Any suggestions here? It is a crash but how exploitable is it?
(In reply to Al Billings [:abillings] from comment #45)
> We need to get a security rating on this for approval. Any suggestions here?
> It is a crash but how exploitable is it?

I think we should treat it as sec-critical. It's causing topcrashes because we treat type tags as object pointers, but it may be possible for an attacker to do something worse.
Keywords: sec-critical
Comment on attachment 8374507 [details] [diff] [review]
Fix

sec-approval+ for trunk. Please nominate Aurora and Beta patches once it is cleanly on mozilla-central so we can avoid shipping this problem.
Attachment #8374507 - Flags: sec-approval? → sec-approval+
https://hg.mozilla.org/integration/mozilla-inbound/rev/437b91c18b03

Landed. Will nominate for uplift when this hits trunk
Whiteboard: [leave open]
Sigh. I still don't understand bug management, apparently.
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/437b91c18b03
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Comment on attachment 8374507 [details] [diff] [review]
Fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Proxy IC stubs (901100)
User impact if declined: topcrash
Testing completed (on m-c, etc.): landed on inbound several days ago 
Risk to taking this patch (and alternatives if risky): Low, as it lowers the amount of situations that jitcode is generated.
String or IDL/UUID changes made by this patch: None

Beta will need to land with the other patch in this bug (already reviewed) folded in.
Attachment #8374507 - Flags: approval-mozilla-beta?
Attachment #8374507 - Flags: approval-mozilla-aurora?
Attachment #8374507 - Flags: approval-mozilla-beta?
Attachment #8374507 - Flags: approval-mozilla-beta+
Attachment #8374507 - Flags: approval-mozilla-aurora?
Attachment #8374507 - Flags: approval-mozilla-aurora+
Flagging for verification based on crash-stats. The most recent report of this signature on Nightly is 20140205030203. I think we'll need to give it a few more days before calling this for Aurora and Beta.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Duplicate of this bug: 957006
Eric asked me to look into whether this needs uplift into any b2g builds.

This bug is present and not fixed in b2g 26 (1.2):
http://mxr.mozilla.org/mozilla-b2g26_v1_2/source/js/src/jit/IonCaches.cpp#1536 

Are we still uplifting sec-crits to that train?
Yes, until EOL.
https://wiki.mozilla.org/Release_Management/B2G_Landing#v1.2.0

I assume that what landed on beta should uplift to b2g26 OK?
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #57)

> I assume that what landed on beta should uplift to b2g26 OK?

Sure looks that way. It will need the beta pre-patch, but it should apply.
OS: Windows NT → All
Hardware: x86 → All
Based on crash-stats it seems like this is fixed 100% on Firefox 30.0a1 and 29.0a2 with a couple of lingering crashes on Firefox 28.0b4. The two crashes which remain on Firefox 28.0b4 appear to have different stacks than what was reported in comment 0 so I think they may be different than this bug.

Source:
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Adom%3A%3ADocumentBinding%3A%3AgenericMethod&product=Firefox&query_type=contains&range_unit=weeks&process_type=any&version=Firefox%3A28.0b4&hang_type=any&date=2014-02-24+21%3A00%3A00&range_value=1#tab-reports
Duplicate of this bug: 963316
Group: core-security
You need to log in before you can comment on or make changes to this bug.