1058168 - crash in js::PerThreadData::runtimeIfOnOwnerThread()

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Liz Henry (:lizzard) (relman/hg->git project)]

This bug was filed from the Socorro interface and is 
report bp-e418d0a8-e3ea-4ceb-8c2f-aaa5d2140823.
=============================================================

We started seeing a lot of crashes with this signature around the 2014060517 build. It looks pretty significant in Firefox 32 Beta. It's the #16 topcrash for Beta with 856/118069 crashes in the last 7 days.  he signature spiked hugely on the 2014071618 build, then went down to almost nothing, then started picking up again between 2014072100 and 2014072209. 7/21 was merge day and I'm not sure how to figure out the regression range there. 

More reports: https://crash-stats.mozilla.com/report/list?signature=js%3A%3APerThreadData%3A%3AruntimeIfOnOwnerThread%28%29&product=Firefox&query_type=contains&range_unit=weeks&process_type=any&version=Firefox%3A32.0b9&hang_type=any&date=2014-08-25+17%3A00%3A00&range_value=1#tab-sigsummary

Crashing thread:
0 	mozjs.dll 	js::PerThreadData::runtimeIfOnOwnerThread() 	js/src/vm/Runtime.h
1 	mozjs.dll 	RuntimeForCurrentThread 	js/src/jit/AsmJSSignalHandlers.cpp
2 	mozjs.dll 	HandleException 	js/src/jit/AsmJSSignalHandlers.cpp
3 	mozjs.dll 	AsmJSExceptionHandler 	js/src/jit/AsmJSSignalHandlers.cpp
4 	ntdll.dll 	NormBuffer__AppendEx 	
5 	ntdll.dll 	RtlCallVectoredExceptionHandlers 	
6 	ntdll.dll 	RtlDispatchException 	
7 	ntdll.dll 	KiUserExceptionDispatcher 	
8 	mozjs.dll 	js::PerThreadData::runtimeIfOnOwnerThread() 	js/src/vm/Runtime.h

Comment 1

[(Away)]

EXCEPTION_STACK_OVERFLOW from endless AsmJSExceptionHandler.

The failing piece of runtimeIfOnOwnerThread+0xa is:
cmp     dword ptr [esi+100h],eax ds:0023:00000100=????????

Looks like a null runtime_?

Comment 2

[Brian Hackett [Laid off!]]

Attachment: handle null runtime

If the runtime is null here we will crash in the signal handler, yeah, then keep recursing until the stack is eventually blown.  It's possible for PerThreadData to have a null runtime, for helper threads operating outside an AutoEnterRuntime.  That can happen in particular during background sweeping, which happens on these threads as of bug 966646.

The attached patch fixes runtimeIfOnOwnerThread() to handle the null runtime case.  This should fix the signal handler recursion, though we did already crash somewhere else in order to enter the signal handler at all (which could be due to random corruption; background sweeping touches a lot of memory).

Comment 3

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43704ad752ef

Comment 4

[Brian Hackett [Laid off!]]

Comment on attachment 8479284 [details] [diff] [review]
handle null runtime

Approval Request Comment
[User impact if declined]: This patch will improve crash diagnostics by avoiding an infinite recursion in the signal handler used for asm.js and Ion code.
[Risks and why]: none

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/43704ad752ef

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/55c9365701e5

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Safe to say that we're not doing anything else here (and crash-stats indicates that this is gone in 33+).