Closed Bug 844580 Opened 10 years ago Closed 5 years ago

crash in js::ObjectImpl::nativeLookup

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Version:
20 Branch
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED WONTFIX
Milestone:
mozilla22
Tracking Flags:
Tracking Status
firefox19 --- wontfix
firefox20 + affected
firefox21 + affected
firefox22 --- affected

People

(Reporter: scoobidiver, Unassigned)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

turn off pgo for nativeLookup()
1.17 KB, patch
billm
: review+
lsblakk
: approval-mozilla-aurora+
lsblakk
: approval-mozilla-beta+
Details | Diff | Splinter Review 
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
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.
Assignee: general → dvander
tracking-firefox20: ? → +
Keywords: needURLs
It's hard to say. It could be a new PGO bug. I'll take a look this week.
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.
Attachment #723572 - Flags: review?(wmccloskey) → review+
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.
tracking-firefox20: + → ?
Keywords: topcrash
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.
tracking-firefox20: ? → -
We could definitely take this on Aurora to see what the impact is (again, once it's on central).
https://hg.mozilla.org/mozilla-central/rev/5cb34f00f6ae
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
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.
tracking-firefox20: - → ?
Keywords: topcrash
tracking-firefox20: ? → +
tracking-firefox21: --- → +
Flags: needinfo?(dvander)
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.
Attachment #723572 - Flags: approval-mozilla-beta+
Attachment #723572 - Flags: approval-mozilla-aurora+
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?
Flags: needinfo?(dvander)
(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.
(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
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?
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?
Flags: needinfo?
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
Keywords: topcrash
(In reply to lsblakk@mozilla.com 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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Crash Signature: [@ js::ObjectImpl::nativeLookup(JSContext*, int)] → [@ js::ObjectImpl::nativeLookup(JSContext*, int)] [@ js::ObjectImpl::nativeLookup]
Assignee: dvander → nobody
Closing because no crash reported since 12 weeks.
Status: REOPENED → RESOLVED
Closed: 10 years ago → 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.