785634 - crash in JSContext::saveFrameChain mainly with Roboform

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Scoobidiver (away)]

It first appeared in 17.0a1/20120825. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1c0ac073dc65&tochange=f077de66e52d

Many crash reports contain the {22119944-ED35-4ab1-910B-E619EA06A115} extension ID which is Roboform Toolbar.

Signature 	JSContext::saveFrameChain() More Reports Search
UUID	e147cfc9-0cdb-4e02-a3de-e11a72120825
Date Processed	2012-08-25 16:58:45
Uptime	36
Last Crash	1.1 minutes before submission
Install Age	50.7 minutes since version was first installed.
Install Time	2012-08-25 16:07:54
Product	Firefox
Version	17.0a1
Build ID	20120825030541
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 15 stepping 6
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x0
App Notes 	
AdapterVendorID: 0x10de, AdapterDeviceID: 0x0398, AdapterSubsysID: 30bb103c, AdapterDriverVersion: 7.15.11.7967
D3D10 Layers? D3D10 Layers- D3D9 Layers? D3D9 Layers- 
Processor Notes 	This dump is too long and has triggered the automatic truncation routine
EMCheckCompatibility	True
Adapter Vendor ID	0x10de
Adapter Device ID	0x0398
Total Virtual Memory	2147352576
Available Virtual Memory	1654087680
System Memory Use Percentage	64
Available Page File	2443055104
Available Physical Memory	771772416

Bugzilla - Report this bug in Firefox, Core, Plug-Ins, or Toolkit
Crashing Thread
Frame 	Module 	Signature 	Source
0 	mozjs.dll 	JSContext::saveFrameChain 	js/src/jscntxt.cpp:1192
1 	mozjs.dll 	JS_SaveFrameChain 	js/src/jsapi.cpp:5935
2 	xul.dll 	XPCJSContextStack::Push 	js/xpconnect/src/XPCJSContextStack.cpp:104
3 	xul.dll 	nsXPConnect::Push 	js/xpconnect/src/nsXPConnect.cpp:2430
4 	xul.dll 	nsCxPusher::DoPush 	content/base/src/nsContentUtils.cpp:3071
5 	xul.dll 	nsImageFrame::Init 	layout/generic/nsImageFrame.cpp:252
6 	xul.dll 	nsCSSFrameConstructor::InitAndRestoreFrame 	layout/base/nsCSSFrameConstructor.cpp:4545
7 	xul.dll 	nsCSSFrameConstructor::ConstructFrameFromItemInternal 	layout/base/nsCSSFrameConstructor.cpp:3648
8 	xul.dll 	nsCSSFrameConstructor::ConstructFramesFromItem 	layout/base/nsCSSFrameConstructor.cpp:5552
9 	xul.dll 	nsCSSFrameConstructor::ConstructFramesFromItemList 	layout/base/nsCSSFrameConstructor.cpp:9817
10 	xul.dll 	nsCSSFrameConstructor::ProcessChildren 	layout/base/nsCSSFrameConstructor.cpp:9962
11 	xul.dll 	nsCSSFrameConstructor::ConstructBlock 	layout/base/nsCSSFrameConstructor.cpp:11009
12 	xul.dll 	nsCSSFrameConstructor::ConstructFrameFromItemInternal 	layout/base/nsCSSFrameConstructor.cpp:3612
...
38 	xul.dll 	nsCSSFrameConstructor::CreateNeededFrames 	layout/base/nsCSSFrameConstructor.cpp:6400
39 	xul.dll 	PresShell::FlushPendingNotifications 	layout/base/nsPresShell.cpp:3858
40 	xul.dll 	nsDocument::FlushPendingNotifications 	content/base/src/nsDocument.cpp:6356
41 	xul.dll 	nsGenericElement::GetPrimaryFrame 	content/base/src/nsGenericElement.cpp:1794
42 	xul.dll 	nsGenericElement::GetStyledFrame 	content/base/src/nsGenericElement.cpp:519
43 	xul.dll 	nsGenericHTMLElement::GetOffsetRect 	content/html/content/src/nsGenericHTMLElement.cpp:471
44 	xul.dll 	nsGenericHTMLElement::GetOffsetWidth 	content/html/content/src/nsGenericHTMLElement.cpp:602
45 	xul.dll 	nsHTMLInputElement::GetOffsetWidth 	content/html/content/src/nsHTMLInputElement.h:78
46 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:70
47 	xul.dll 	XPCWrappedNative::GetAttribute 	js/xpconnect/src/xpcprivate.h:2821
48 	xul.dll 	XPC_WN_GetterSetter 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1526
49 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:352
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=JSContext%3A%3AsaveFrameChain%28%29

Comment 1

[Scoobidiver (away)]

(In reply to Scoobidiver from comment #0)
> Many crash reports contain the {22119944-ED35-4ab1-910B-E619EA06A115}
> extension ID which is Roboform Toolbar.
It's confirmed by correlations per extension:
     97% (75/77) vs.   1% (76/6565) {22119944-ED35-4ab1-910B-E619EA06A115} (Roboform Toolbar for Firefox, https://addons.mozilla.org/addon/750)
          3% (2/77) vs.   0% (2/6565) 7.6.6
          1% (1/77) vs.   0% (1/6565) 7.7.0
          4% (3/77) vs.   0% (3/6565) 7.7.6
          6% (5/77) vs.   0% (5/6565) 7.7.8.128
          1% (1/77) vs.   0% (2/6565) 7.7.8.8
          3% (2/77) vs.   0% (2/6565) 7.7.9.9
         65% (50/77) vs.   1% (50/6565) 7.8.0.5
         14% (11/77) vs.   0% (11/6565) 7.8.1.5

Comment 2

[Scoobidiver (away)]

It's likely a regression from bug 625199.

Comment 3

[Luke Wagner [:luke]]

Attachment: maybe fix

It looks like Roboform is calling into JSAPI through ctypes and the crash is calling JS_SaveFrameChain on a JSContext whose defaultCompartmentObject_ is NULL.  This suggests the quick-fix of just testing for NULL in JSContext::saveFrameChain.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 656149 [details] [diff] [review]
maybe fix

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

::: js/src/jscntxt.cpp
@@ +1189,5 @@
>          return false;
>      }
>  
> +    if (defaultCompartmentObject_)
> +        compartment = defaultCompartmentObject_->compartment();

Shouldn't we set compartment to null if there is no defaultCompartmentObject_? r=me with that addressed.

Comment 5

[Luke Wagner [:luke]]

Let's see what this does to the crashes on nightly:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d854193424d

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1d854193424d

Comment 7

[Scoobidiver (away)]

There are no crashes in 18.0a1/20120831.

Comment 8

[Luke Wagner [:luke]]

Comment on attachment 656149 [details] [diff] [review]
maybe fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 625199
User impact if declined: roboform crashes
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): very low

Comment 9

[Alex Keybl [:akeybl]]

Comment on attachment 656149 [details] [diff] [review]
maybe fix

[Triage Comment]
Thanks for the quick fix, Luke. Approving for Aurora 17 uplift.

Comment 10

[Luke Wagner [:luke]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b9ad1f78533e

Comment 11

[u279076]

We didn't have an internally reproducible testcase for this crash. Please check Socorro to verify this bug.

Comment 12

[Ioana (away)]

No crashes on builds post-fix:
https://crash-stats.mozilla.com/report/list?query_search=signature&query_type=contains&reason_type=contains&range_value=4&range_unit=weeks&hang_type=any&process_type=any&signature=JSContext%3A%3AsaveFrameChain%28%29

https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&query=JSContext%3A%3AsaveFrameChain&reason_type=contains&date=10%2F17%2F2012%2015%3A00%3A24&range_value=4&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=JSContext%3A%3AsaveFrameChain

Comment 13

[Tracy Walker [:tracy]]

mass remove verifyme requests greater than 4 months old