Closed Bug 1048330 Opened 5 years ago Closed 5 years ago

crash in JSAutoCompartment::JSAutoCompartment(JSContext*, JSObject*) | mozilla::dom::WrapNativeParent<nsISupports>

Categories

(Core :: XPConnect, defect, critical)

32 Branch
x86
Windows NT
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla34
Tracking Status
firefox32 --- verified
firefox33 --- verified
firefox34 --- verified
firefox-esr31 --- fixed

People

(Reporter: lizzard, Assigned: bholley)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-13273c72-cc38-46d0-bd26-646292140801.
=============================================================


This crash signature showed up as explosive on 2014-08-01 for Firefox 32, with a jump from a few crashes per day to 960 crashes on the 2014072812 build. 


Crashing thread:
0 	mozjs.dll 	JSAutoCompartment::JSAutoCompartment(JSContext*, JSObject*) 	js/src/jsapi.cpp
1 	xul.dll 	mozilla::dom::WrapNativeParent<nsISupports> 	obj-firefox/dist/include/mozilla/dom/BindingUtils.h
2 	xul.dll 	mozilla::dom::XULElementBinding::Wrap(JSContext*, nsXULElement*, nsWrapperCache*) 	obj-firefox/dom/bindings/XULElementBinding.cpp
3 	xul.dll 	nsXULElement::WrapNode(JSContext*) 	content/xul/content/src/nsXULElement.cpp
4 	xul.dll 	mozilla::dom::Element::WrapObject(JSContext*) 	content/base/src/Element.cpp
5 	xul.dll 	XPCConvert::NativeInterface2JSObject(JS::MutableHandle<JS::Value>, nsIXPConnectJSObjectHolder**, xpcObjectHelper&, nsID const*, XPCNativeInterface**, bool, tag_nsresult*) 	js/xpconnect/src/XPCConvert.cpp
6 	xul.dll 	nsXPConnect::WrapNativeToJSVal(JSContext*, JSObject*, nsISupports*, nsWrapperCache*, nsID const*, bool, JS::MutableHandle<JS::Value>) 	js/xpconnect/src/nsXPConnect.cpp
7 	xul.dll 	nsContentUtils::WrapNative(JSContext*, nsISupports*, nsWrapperCache*, nsID const*, JS::MutableHandle<JS::Value>, bool) 	content/base/src/nsContentUtils.cpp
8 	xul.dll 	nsXBLProtoImpl::InitTargetObjects(nsXBLPrototypeBinding*, nsIContent*, JS::MutableHandle<JSObject*>, bool*) 	dom/xbl/nsXBLProtoImpl.cpp
9 	xul.dll 	nsXBLProtoImpl::InstallImplementation(nsXBLPrototypeBinding*, nsXBLBinding*) 	dom/xbl/nsXBLProtoImpl.cpp
10 	xul.dll 	nsXBLBinding::InstallImplementation() 	dom/xbl/nsXBLBinding.cpp
Looks like a null-deref due to this line:

  JSAutoCompartment ac(cx, xblScope);

in WrapNativeParent.  Bobby, should xpc::GetXBLScope be managing to return null here???
Component: JavaScript Engine → XPConnect
Flags: needinfo?(bobbyholley)
(In reply to On vacation Aug 5-18.  Please do not request review. from comment #1)
> in WrapNativeParent.  Bobby, should xpc::GetXBLScope be managing to return
> null here???

Per bug 858642 it can. So the fix here is to add a null check. I'll attach a patch.
Assignee: nobody → bobbyholley
Comment on attachment 8467411 [details] [diff] [review]
Null-check the XBL scope in more places. v1

Not sure why that NS_ERROR_OUT_OF_MEMORY is right.
I'd just use NS_ENSURE_STATE(xblScope)

We need this on branches, right?
Attachment #8467411 - Flags: review?(bugs) → review+
Comment on attachment 8467411 [details] [diff] [review]
Null-check the XBL scope in more places. v1

This is basically a couple of missing null checks. Flagging for uplifting.

Approval Request Comment
[Feature/regressing bug #]: unknown
[User impact if declined]: crashes
[Describe test coverage new/current, TBPL]: No automated test. Just pushed to m-i.
[Risks and why]: Very low risk - null checks. 
[String/UUID change made/needed]: None
Attachment #8467411 - Flags: approval-mozilla-beta?
Attachment #8467411 - Flags: approval-mozilla-aurora?
Comment on attachment 8467411 [details] [diff] [review]
Null-check the XBL scope in more places. v1

Can't hurt to get this on esr either.
Attachment #8467411 - Flags: approval-mozilla-esr31?
https://hg.mozilla.org/mozilla-central/rev/1dae22380205
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Attachment #8467411 - Flags: approval-mozilla-beta?
Attachment #8467411 - Flags: approval-mozilla-beta+
Attachment #8467411 - Flags: approval-mozilla-aurora?
Attachment #8467411 - Flags: approval-mozilla-aurora+
Attachment #8467411 - Flags: approval-mozilla-esr31? → approval-mozilla-esr31+
QA Whiteboard: [qa+]
Verified that no crashes are recorded in Socorro using that signature on Firefox 32 beta 5 and later:
https://crash-stats.mozilla.com/query/?product=Firefox&version=Firefox%3A32.0b&range_value=1&range_unit=weeks&date=08%2F20%2F2014+09%3A00%3A00&query_search=signature&query_type=contains&query=JSAutoCompartment%3A%3AJSAutoCompartment%28JSContext*%2C+JSObject*%29+%7C+mozilla%3A%3Adom%3A%3AWrapNativeParent%3CnsISupports%3E&reason=&release_channels=&build_id=20140807212602%2C+20140811180644%2C+20140814150857%2C+20140818191513&process_type=any&hang_type=any

Aurora 4 crashes in the last 2 weeks, BUT on builds before the fix:
https://crash-stats.mozilla.com/report/list?signature=JSAutoCompartment%3A%3AJSAutoCompartment%28JSContext%2A%2C+JSObject%2A%29+%7C+mozilla%3A%3Adom%3A%3AWrapNativeParent%3CnsISupports%3E&product=Firefox&query_type=contains&range_unit=weeks&process_type=any&version=Firefox%3A33.0a2&hang_type=any&date=2014-08-20+09%3A00%3A00&range_value=2#tab-reports

Nightly 1 crash in the last 2 weeks, BUT on build before the fix:
https://crash-stats.mozilla.com/report/list?signature=JSAutoCompartment%3A%3AJSAutoCompartment%28JSContext%2A%2C+JSObject%2A%29+%7C+mozilla%3A%3Adom%3A%3AWrapNativeParent%3CnsISupports%3E&product=Firefox&query_type=contains&range_unit=weeks&process_type=any&version=Firefox%3A34.0a1&hang_type=any&date=2014-08-20+09%3A00%3A00&range_value=2#tab-reports

Based on the results from above, I`m marking this as Verified fixed.
You need to log in before you can comment on or make changes to this bug.