Last Comment Bug 750556 - crash in mozilla::dom::Element::ClearStyleStateLocks
: crash in mozilla::dom::Element::ClearStyleStateLocks
Status: RESOLVED FIXED
: crash, regression
Product: Core
Classification: Components
Component: DOM (show other bugs)
: 15 Branch
: All All
: -- critical (vote)
: mozilla15
Assigned To: Heather Arthur [:harth]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-30 16:56 PDT by Marcia Knous [:marcia - use ni]
Modified: 2012-05-10 07:39 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Check element before dereferencing in pseudo class lock API functions + test (6.17 KB, patch)
2012-05-03 07:49 PDT, Heather Arthur [:harth]
bzbarsky: review+
Details | Diff | Splinter Review

Description Marcia Knous [:marcia - use ni] 2012-04-30 16:56:00 PDT
This bug was filed from the Socorro interface and is 
report bp-70268703-b0ea-4e1b-8e6b-710482120430 .
============================================================= 

Seen while looking at crash stats. Low volume Mac crash which started appearing in crash stats using the 20120427030500 build. https://crash-stats.mozilla.com/report/list?signature=mozilla::dom::Element::ClearStyleStateLocks

Possible regression range based on crash stats: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cc5254f9825f&tochange=450d8cd16316

Frame 	Module 	Signature 	Source
0 	XUL 	mozilla::dom::Element::ClearStyleStateLocks 	nsINode.h:595
1 	XUL 	inDOMUtils::ClearPseudoClassLocks 	inDOMUtils.cpp:457
2 	XUL 	NS_InvokeByIndex_P 	xptcinvoke_x86_64_unix.cpp:195
3 	XUL 	XPCWrappedNative::CallMethod 	XPCWrappedNative.cpp:3114
4 	XUL 	XPC_WN_CallMethod 	XPCWrappedNativeJSOps.cpp:1549
5 	XUL 	js::InvokeKernel 	jscntxtinlines.h:314
6 	XUL 	js::Interpret 	jsinterp.cpp:2757
7 	XUL 	js::RunScript 	jsinterp.cpp:475
8 	XUL 	js::InvokeKernel 	jsinterp.cpp:535
9 	XUL 	array_forEach 	jsinterp.h:172
10 	XUL 	js::InvokeKernel 	jscntxtinlines.h:314
11 	XUL 	js::Interpret 	jsinterp.cpp:2757
12 	XUL 	js::RunScript 	jsinterp.cpp:475
13 	XUL 	js::InvokeKernel 	jsinterp.cpp:535
14 	XUL 	js_fun_apply 	jsinterp.h:172
15 	XUL 	js::InvokeKernel 	jscntxtinlines.h:314
16 	XUL 	js::Interpret 	jsinterp.cpp:2757
17 	XUL 	js::RunScript 	jsinterp.cpp:475
18 	XUL 	js::InvokeKernel 	jsinterp.cpp:535
19 	XUL 	array_forEach 	jsinterp.h:172
20 	XUL 	js::InvokeKernel 	jscntxtinlines.h:314
21 	XUL 	js::Interpret 	jsinterp.cpp:2757
22 	XUL 	js::RunScript 	jsinterp.cpp:475
23 	XUL 	js::InvokeKernel 	jsinterp.cpp:535
24 	XUL 	js_fun_apply 	jsinterp.h:172
25 	XUL 	js::InvokeKernel 	jscntxtinlines.h:314
26 	XUL 	js::Interpret 	jsinterp.cpp:2757
27 	XUL 	js::RunScript 	jsinterp.cpp:475
28 	XUL 	js::InvokeKernel 	jsinterp.cpp:535
29 	XUL 	js::Invoke 	jsinterp.h:172
30 	XUL 	JS_CallFunctionValue 	jsapi.cpp:5416
31 	XUL 	nsXPCWrappedJSClass::CallMethod 	XPCWrappedJSClass.cpp:1509
32 	XUL 	nsXPCWrappedJS::CallMethod 	XPCWrappedJS.cpp:617
33 	XUL 	PrepareAndDispatch 	xptcstubs_x86_64_darwin.cpp:153
34 	XUL 	XUL@0xfb060a 	
35 	XUL 	nsBrowserStatusFilter::OnStateChange 	nsBrowserStatusFilter.cpp:183
36 	XUL 	nsDocLoader::DoFireOnStateChange 	nsDocLoader.cpp:1383
37 	XUL 	nsDocLoader::FireOnStateChange 	nsDocLoader.cpp:1320
38 	XUL 	nsDocLoader::OnStartRequest 	nsDocLoader.cpp:885
39 	XUL 	nsLoadGroup::AddRequest 	nsLoadGroup.cpp:612
40 	XUL 	nsDocShell::BeginRestore 	nsDocShell.cpp:6852
41 	XUL 	nsDocShell::RestorePresentation 	nsDocShell.cpp:6988
42 	XUL 	nsDocShell::InternalLoad 	nsDocShell.cpp:8721
43 	XUL 	nsDocShell::LoadHistoryEntry 	nsDocShell.cpp:10223
44 	XUL 	nsDocShell::LoadURI 	nsDocShell.cpp:1419
45 	XUL 	nsSHistory::InitiateLoad 	nsSHistory.cpp:1760
46 	XUL 	nsSHistory::LoadEntry 	nsSHistory.cpp:1627
47 	XUL 	nsSHistory::GoBack 	nsSHistory.cpp:835
48 	XUL 	nsDocShell::GoBack 	nsDocShell.cpp:3779
49 	XUL 	NS_InvokeByIndex_P 	xptcinvoke_x86_64_unix.cpp:195
50 	XUL 	XPCWrappedNative::CallMethod 	XPCWrappedNative.cpp:3114
51 	XUL 	XPC_WN_CallMethod 	XPCWrappedNativeJSOps.cpp:1549
52 	XUL 	js::InvokeKernel 	jscntxtinlines.h:314
53 	XUL 	js::Interpret 	jsinterp.cpp:2757
54 	XUL 	js::RunScript 	jsinterp.cpp:475
55 	XUL 	js::InvokeKernel 	jsinterp.cpp:535
56 	XUL 	js::Invoke 	jsinterp.h:172
57 	XUL 	JS_CallFunctionValue 	jsapi.cpp:5416
58 	XUL 	nsJSContext::CallEventHandler 	nsJSEnvironment.cpp:1889
59 	XUL 	nsJSEventListener::HandleEvent 	nsJSEventListener.cpp:225
60 	XUL 	nsEventListenerManager::HandleEventInternal 	nsEventListenerManager.cpp:818
61 	XUL 	nsEventTargetChainItem::HandleEventTargetChain 	nsEventListenerManager.h:169
62 	XUL 	nsEventDispatcher::Dispatch 	nsEventDispatcher.cpp:684
63 	XUL 	nsEventDispatcher::DispatchDOMEvent 	nsEventDispatcher.cpp:747
64 	XUL 	nsINode::DispatchEvent 	nsGenericElement.cpp:1165
65 	XUL 	nsContentUtils::DispatchXULCommand 	nsContentUtils.cpp:5844
66 	XUL 	nsXULElement::PreHandleEvent 	nsXULElement.cpp:1672
67 	XUL 	nsEventDispatcher::Dispatch 	nsEventDispatcher.cpp:278
68 	XUL 	nsEventDispatcher::DispatchDOMEvent 	nsEventDispatcher.cpp:747
69 	XUL 	nsINode::DispatchEvent 	nsGenericElement.cpp:1165
70 	XUL 	nsIDOMEventTarget_DispatchEvent 	dom_quickstubs.cpp:10223
71 	XUL 	js::InvokeKernel 	jscntxtinlines.h:314
72 	XUL 	js::Interpret 	jsinterp.cpp:2757
73 	XUL 	js::RunScript 	jsinterp.cpp:475
74 	XUL 	js::InvokeKernel 	jsinterp.cpp:535
75 	XUL 	js::Invoke 	jsinterp.h:172
76 	XUL 	JS_CallFunctionValue 	jsapi.cpp:5416
77 	XUL 	nsXPCWrappedJSClass::CallMethod 	XPCWrappedJSClass.cpp:1509
78 	XUL 	nsXPCWrappedJS::CallMethod 	XPCWrappedJS.cpp:617
79 	XUL 	PrepareAndDispatch 	xptcstubs_x86_64_darwin.cpp:153
80 	XUL 	XUL@0xfb060a 	
81 	XUL 	nsEventListenerManager::HandleEventInternal 	nsEventListenerManager.cpp:818
82 	XUL 	nsEventTargetChainItem::HandleEventTargetChain 	nsEventListenerManager.h:169
83 	XUL 	nsEventDispatcher::Dispatch 	nsEventDispatcher.cpp:684
84 	XUL 	PresShell::HandleEventInternal 	nsPresShell.cpp:6584
85 	XUL 	PresShell::HandleEventWithTarget 	nsPresShell.cpp:6271
86 	XUL 	nsEventStateManager::CheckForAndDispatchClick 	nsEventStateManager.cpp:4429
87 	XUL 	nsEventStateManager::PostHandleEvent 	nsEventStateManager.cpp:3219
88 	XUL 	PresShell::HandleEventInternal 	nsPresShell.cpp:6606
89 	XUL 	PresShell::HandlePositionedEvent 	nsPresShell.cpp:6256
90 	XUL 	PresShell::HandleEvent 	nsPresShell.cpp:6086
91 	XUL 	nsViewManager::DispatchEvent 	nsViewManager.cpp:908
92 	XUL 	HandleEvent 	nsView.cpp:158
93 	XUL 	nsChildView::DispatchEvent 	nsChildView.mm:1509
94 	XUL 	nsChildView::DispatchWindowEvent 	nsChildView.mm:1519
95 	XUL 	-[ChildView mouseUp:] 	nsChildView.mm:3347
96 	AppKit 	AppKit@0xd67a5 	
97 	libsystem_c.dylib 	libsystem_c.dylib@0x4d46f 	
98 	libsystem_c.dylib 	libsystem_c.dylib@0x4d6aa 	
99 	AppKit 	AppKit@0x98e087 	
100 	AppKit 	AppKit@0x98e087 	
103 	AppKit 	AppKit@0x6c4bb 	
104 	AppKit 	AppKit@0x6d18b 	
105 	libobjc.A.dylib 	libobjc.A.dylib@0xb0ff 	
106 	XUL 	-[ToolbarWindow sendEvent:] 	nsCocoaWindow.mm:2660
107 	AppKit 	AppKit@0x6f16c 	
108 	libobjc.A.dylib 	libobjc.A.dylib@0xd2c5 	
109 	libobjc.A.dylib 	libobjc.A.dylib@0xd254 	
110 	CoreFoundation 	CoreFoundation@0x30b98 	
111 	AppKit 	AppKit@0x6d7f7 	
112 	AppKit 	AppKit@0x8bd0
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2012-04-30 18:34:47 PDT
The crash looks like a null deref.

This _could_ happen if some random (chrome!) caller passes an object that's not actually an element (and is in fact not a DOM object at all) as the argument to inDOMUtils::ClearPseudoClassLocks.

Heather, does anything in the regression range look like it might do that?
Comment 2 Heather Arthur [:harth] 2012-05-02 05:36:18 PDT
(In reply to Boris Zbarsky (:bz) from comment #1)
> This _could_ happen if some random (chrome!) caller passes an object that's
> not actually an element (and is in fact not a DOM object at all) as the
> argument to inDOMUtils::ClearPseudoClassLocks.

I'll investigate how this could happen. Is there any check we can do inside ClearPsuedoClassLocks() to prevent this?

> Heather, does anything in the regression range look like it might do that?

Nothing in the range looks like a culprit. This is most likely from the original code and happened because the UI just got exposed recently.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2012-05-02 07:10:48 PDT
> Is there any check we can do inside ClearPsuedoClassLocks() to prevent this?

You can move the null-check to after the QI (so you're null-checking element, not aElement).  That should fix things.
Comment 5 Heather Arthur [:harth] 2012-05-03 07:49:13 PDT
Created attachment 620699 [details] [diff] [review]
Check element before dereferencing in pseudo class lock API functions + test

Can reproduce by passing in an empty object. This patch fixes by adding the element check after the QI.

My editor removes trailing whitespace on save...let me know if I should remove the whitespace changes.
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2012-05-03 14:24:16 PDT
Comment on attachment 620699 [details] [diff] [review]
Check element before dereferencing in pseudo class lock API functions + test

r=me
Comment 8 Ed Morley [:emorley] 2012-05-10 07:39:33 PDT
https://hg.mozilla.org/mozilla-central/rev/d59106f03fa0

Note You need to log in before you can comment on or make changes to this bug.