844580 - crash in js::ObjectImpl::nativeLookup

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Scoobidiver (away)]

It's #74 top browser crasher in 19.0, #9 in 20.0b1, and #68 in 21.0a2.

Stack traces are various and look similar to the ones in bug 670603 and bug 682573.

Frame 	Module 	Signature 	Source
0 		@0x8afbf775 	
1 	mozjs.dll 	js::ObjectImpl::nativeLookup 	js/src/vm/ObjectImpl.cpp:267
2 	mozjs.dll 	js::mjit::JaegerShot 	js/src/methodjit/MethodJIT.cpp:1117
3 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:2421
...

Frame 	Module 	Signature 	Source
0 	mozjs.dll 	js::ObjectImpl::nativeLookup 	js/src/vm/ObjectImpl.cpp:267
1 	mozjs.dll 	js::GetPropertyOperation 	js/src/jsinterpinlines.h:290
2 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:2233
3 	mozjs.dll 	js::analyze::ScriptAnalysis::analyzeBytecode 	js/src/jsanalyze.cpp:142
4 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:404
5 	mozjs.dll 	js::Invoke 	js/src/jsinterp.cpp:437
...

Frame 	Module 	Signature 	Source
0 	mozjs.dll 	js::ObjectImpl::nativeLookup 	js/src/vm/ObjectImpl.cpp:267
1 	mozjs.dll 	js::mjit::CallCompiler::generateNativeStub 	js/src/methodjit/MonoIC.cpp:1078
2 	mozjs.dll 	js::types::TypeScript::Monitor 	js/src/jsinferinlines.h:895
3 	mozjs.dll 	DefinePropertyOnObject 	js/src/jsobj.cpp:583
4 	mozjs.dll 	js::mjit::ic::NativeCall 	js/src/methodjit/MonoIC.cpp:1331
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=js%3A%3AObjectImpl%3A%3AnativeLookup%28JSContext*%2C+int%29

Comment 1

[Lukas Blakk [:lsblakk] use ?needinfo]

David - does this look to be along similar lines as bug 670603? If so then would the work being targeted for FF24 as mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=670603#c30 be applicable here.  If not, then any additional insight you can glean here on where to focus investigation and who to assign would be appreciated.

Comment 2

[Robert Kaiser]

Top URLs:

112 	http://www.facebook.com/
102 	about:blank
67 	https://www.facebook.com/
16 	about:sessionrestore
16 	http://msn.foxsports.com/nascar/shakeandbake/50-cent-takes-a-liking-to-erin-andrews/
12 	http://www.mmajunkie.com/news/2013/02/ufc-157-full-fight-video-highlights-including-ronda-rousey-vs-liz-carmouche
11 	https://www.facebook.com/login.php?login_attempt=1
10 	http://www.facebook.com/?ref=tn_tnmn
9 	https://mail.google.com/mail/u/0/?shva=1#inbox
9 	http://msn.foxsports.com/nascar/shakeandbake/50-cent-takes-a-liking-to-erin-andrews/?ocid=ansfox11
7 	https://www.facebook.com/home.php
7 	about:home
6 	http://movies.msn.com/academy-awards/best-and-worst/
6 	about:newtab
6 	http://www.mmafighting.com/2013/2/24/4023134/ronda-rousey-vs-liz-carmouche-full-fight-video-highlights-ufc-157-dan-henderson-vs-lyoto-machida
6 	https://www.facebook.com/?ref=tn_tnmn
5 	http://deadspin.com/5986500/heres-a-wonder-goal-from-newcastles-papiss-cisse
5 	https://twitter.com/

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

It's hard to say. It could be a new PGO bug. I'll take a look this week.

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

I took a look at five random crash reports. In all cases, it looks like the |this| pointer to ShapeTable is somehow wrong or its contents are corrupted. There's a scary comment above the crashing function explaining that it has been miscompiled in the past by MSVC9. Some digging reveals bug 718541.

It's a shot in the dark but let's see what happens disabling PGO for this function.

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: turn off pgo for nativeLookup()

Comment 6

[Scoobidiver (away)]

It's only #31 top browser crasher in 20.0b4 and #34 in 20.0b3 while it was #10 in 20.0b2. The improving range is:
http://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=f45f4b3cba11&tochange=64a66423dbd3
It's also lower in 21.0a2 at #126 but without a clear improving range.

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

Go ahead and nominate the patch for uplift after landing it to trunk, if it's low risk and we think the reward for uplift is high enough but we don't need to track this if it's going down.

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

We could definitely take this on Aurora to see what the impact is (again, once it's on central).

Comment 9

[David Anderson [:dvander] - inactive, e-mail if emergency]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5cb34f00f6ae

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/5cb34f00f6ae

Comment 11

[Scoobidiver (away)]

Again, it spiked in 20.0b5 where it's currently #8 top browser crasher. The new regression range is:
http://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=a853b233420d&tochange=163304f85fc1

Those ups and downs make me think to a PGO issue as assumed by the patch.

Note that it's only #109 browser crasher in 21.0a2 and #333 in 22.0a1.

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 723572 [details] [diff] [review]
turn off pgo for nativeLookup()

[Triage Comment]
We think this is low risk enough that we should just uplift to aurora/beta and get this into tomorrow's fifth week beta and collect crash stats with this landed to see if it helps.

Comment 13

[David Anderson [:dvander] - inactive, e-mail if emergency]

Sorry, it's not clear by comment #11 - did this patch actually do anything? I'm having trouble figuring it out from crash-stats. Or is it that we want to speculatively try it on aurora/beta, where it matters most?

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

(In reply to David Anderson [:dvander] from comment #13)
> Sorry, it's not clear by comment #11 - did this patch actually do anything?
> I'm having trouble figuring it out from crash-stats. Or is it that we want
> to speculatively try it on aurora/beta, where it matters most?

The latter - at this point we'd rather just take it in the hopes that it will do something since this seems to be a PGO spike and we don't have a way to generate that intentionally.

Comment 15

[Scoobidiver (away)]

(In reply to David Anderson [:dvander] from comment #13)
> did this patch actually do anything?
There have been no crashes since the patch landed but there were six consecutive builds without crashes before: https://crash-stats.mozilla.com/report/list?version=Firefox%3A22.0a1&range_value=4&range_unit=weeks&signature=js%3A%3AObjectImpl%3A%3AnativeLookup%28JSContext*%2C%20int%29

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/853deb8dc20a
https://hg.mozilla.org/releases/mozilla-beta/rev/3c6edb87ca4e

Comment 17

[Ada [:adalucinet]]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20100101 Firefox/20.0

By using the top urls from comment 2, I get 3 crashes on Firefox 20 beta 6 (Build ID: 20130320062118) with new signatures:

2 crashes with signature  [@ mozilla::dom::NodeBinding::genericGetter ]:
https://crash-stats.mozilla.com/report/index/bp-f899d310-2d16-4415-82c4-f4e192130321

1 crash with signature [@ js::mjit::JaegerShot(JSContext*, bool) ]:
https://crash-stats.mozilla.com/report/index/bp-e533fa73-82c4-4f6b-bfd4-892b42130321

Could anyone please verify if the new signatures are related with this issue?

Comment 18

[Ada [:adalucinet]]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20100101 Firefox/21.0

I was able to reproduce this issue with Firefox 21 beta 3 (20130416200523) by using the top urls from comment 2.

In Socorro, there are crash reports for Firefox 20 and 21 with [@ js::ObjectImpl::nativeLookup(JSContext*, int)] signature:
https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&query=js%3A%3AObjectImpl%3A%3AnativeLookup%28JSContext%2A%2C%20int%29&reason_type=contains&date=04%2F17%2F2013%2013%3A24%3A26&range_value=4&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=js%3A%3AObjectImpl%3A%3AnativeLookup%28JSContext%2A%2C%20int%29#reports

Could anyone please take a look?

Comment 19

[Mihai Morar, (:MihaiMorar)]

This is not fixed yet. There are hundreds crashes in last week, most of them, 51 % on FF 20.0.1

https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=exact&query=js%3A%3AObjectImpl%3A%3AnativeLookup%28JSContext%2A%2C%20int%29&reason_type=contains&date=05%2F09%2F2013%2012%3A05%3A15&range_value=1&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=js%3A%3AObjectImpl%3A%3AnativeLookup%28JSContext%2A%2C%20int%29

Comment 20

[Mihai Morar, (:MihaiMorar)]

In addition, it still can be found in topcrashes list for FF 20.0.1 but in not in top 100.

Crash signature: [@ js::ObjectImpl::nativeLookup(JSContext*, int)]
FF 20.0.1 TopCrash list:
https://crash-stats.mozilla.com/topcrasher/byversion/Firefox/20.0.1/28/browser

Comment 21

[Mihai Morar, (:MihaiMorar)]

(In reply to [e-mail] from comment #14)
> >did this patch actually do anything?

> The latter - at this point we'd rather just take it in the hopes that it
> will do something since this seems to be a PGO spike and we don't have a way
> to generate that intentionally.

Based on the above comments, I guess it still doesn't do anything so I think we should reopen it. There are still to many crashes in FF 20.0.1 and FF 21 Beta.

Comment 22

[Sylvestre Ledru [:Sylvestre]]

Closing because no crash reported since 12 weeks.