Last Comment Bug 589635 - Crash [@ nsSubDocumentFrame::AttributeChanged] with throwing error and removing type attribute on iframe
: Crash [@ nsSubDocumentFrame::AttributeChanged] with throwing error and removi...
[sg:dos null pointer access] "fixed" ...
: crash, testcase, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: DOM (show other bugs)
: 1.9.2 Branch
: All All
-- critical (vote)
: mozilla2.0b8
Assigned To: Mats Palmgren (:mats)
: Andrew Overholt [:overholt]
Depends on:
  Show dependency treegraph
Reported: 2010-08-22 17:15 PDT by Martijn Wargers [:mwargers]
Modified: 2011-06-13 10:01 PDT (History)
9 users (show)
mats: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (1.40 KB, text/html)
2010-08-22 17:15 PDT, Martijn Wargers [:mwargers]
no flags Details
Fix for 1.9.2 and 1.9.1 (733 bytes, patch)
2010-12-06 17:59 PST, Mats Palmgren (:mats)
roc: review+
dveditz: approval1.9.2.14+
dveditz: approval1.9.1.17+
Details | Diff | Splinter Review
Fix for trunk (757 bytes, patch)
2010-12-06 18:00 PST, Mats Palmgren (:mats)
roc: review+
roc: approval2.0+
Details | Diff | Splinter Review
crashtest.diff (2.52 KB, patch)
2010-12-06 18:27 PST, Mats Palmgren (:mats)
no flags Details | Diff | Splinter Review

Description User image Martijn Wargers [:mwargers] 2010-08-22 17:15:58 PDT
Created attachment 468196 [details]

See testcase, which crashes current trunk build and Firefox3.6.8 after a while (normally within 1 minute or so).

The content of the iframe is this:
<?xml-stylesheet href="chrome://browser/skin/" type="text/css"?>
<window xmlns="" xmlns:html="">
<iframe type="a"></iframe>

setTimeout(function() {
}, 100);

setInterval(function() {
for (var i=0;i < 6; i++) {throw('t');}

var all = document.getElementsByTagName('*');
var iframe = all[1];

function doe(node) {

setTimeout(function() {

0  	xul.dll  	nsSubDocumentFrame::AttributeChanged  	 layout/generic/nsFrameFrame.cpp:788
1 	xul.dll 	nsStyleContext::DoGetStyleDisplay 	layout/style/nsStyleStructList.h:95
2 	mozcrt19.dll 	arena_dalloc_small 	obj-firefox/memory/jemalloc/crtsrc/jemalloc.c:4153
3 	mozcrt19.dll 	arena_dalloc 	obj-firefox/memory/jemalloc/crtsrc/jemalloc.c:4284
4 	xul.dll 	nsDOMAttribute::Release 	content/base/src/nsDOMAttribute.cpp:151
5 	xul.dll 	nsTHashtable<nsBaseHashtableET<nsAttrHashKey,nsRefPtr<nsDOMAttribute> > >::s_ClearEntry 	obj-firefox/dist/include/nsTHashtable.h:397
6 	xul.dll 	nsNodeUtils::AttributeChanged 	content/base/src/nsNodeUtils.cpp:128
7 	xul.dll 	nsXULElement::UnsetAttr 	content/xul/content/src/nsXULElement.cpp:1455
8 	xul.dll 	nsDOMAttributeMap::RemoveNamedItem 	content/base/src/nsDOMAttributeMap.cpp:390
9 	xul.dll 	nsGenericElement::RemoveAttributeNode 	content/base/src/nsGenericElement.cpp:2478
10 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
11 	xul.dll 	js::InvokeCommon<int > 	js/src/jsinterp.cpp:561
12 		@0x11d215b2 	
13 	xul.dll 	nsXPConnect::GetWrapperForObject 	js/src/xpconnect/src/nsXPConnect.cpp:2481
14 	xul.dll 	js_GetPropertyHelper 	js/src/jsobj.cpp:4830
15 	xul.dll 	XPC_WN_JSOp_ThisObject 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1533
Comment 1 User image Jesse Ruderman 2010-08-31 12:39:10 PDT
Stack trace looks corrupt, so I'm calling this [sg:critical].

I'm having trouble reproducing on Mac OS X 10.5.x with Firefox trunk.
Comment 2 User image Jesse Ruderman 2010-08-31 15:25:02 PDT
Couldn't reproduce using Valgrind (on Mac OS X 10.6.x) either :(
Comment 3 User image Marcia Knous [:marcia - use ni] 2010-09-21 13:24:25 PDT
Marcia will try to reproduce this in the QA lab following the meeting.
Comment 4 User image Marcia Knous [:marcia - use ni] 2010-09-21 15:36:42 PDT
I crashed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20100914 Firefox/3.6.10 (.NET CLR 3.5.30729) -

My first attempt at loading the testcase on the trunk, it did not crash. Will try again.
Comment 5 User image Marcia Knous [:marcia - use ni] 2010-09-21 15:39:05 PDT
Not able to crash with testcase using  Mozilla/5.0 (Windows NT 5.1; rv:2.0b7pre) Gecko/20100921 Firefox/4.0b7pre.
Comment 6 User image Marcia Knous [:marcia - use ni] 2010-09-28 13:36:55 PDT
Martijn: Can you try your STR again please? I am unable to get a trunk crash.
Comment 7 User image Daniel Veditz [:dveditz] 2010-09-28 13:37:02 PDT
Did this get fixed along the way with another patch?
Comment 8 User image Martijn Wargers [:mwargers] 2010-09-28 13:41:02 PDT
I'm still crashing in current trunk build.
Comment 9 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2010-09-29 06:26:17 PDT
Couldn't reproduce on linux
Comment 10 User image Martijn Wargers [:mwargers] 2010-09-29 10:08:10 PDT
It doesn't crash on trunk, because xul is disabled by default. You'll have to enable xul for this site (but that's kinda difficult right now, because xul is disabled without the option of turning it back on in about:config or the ui).
Comment 11 User image Damon Sicore (:damons) 2010-10-05 13:42:38 PDT
This probably isn't critical on trunk, but nominating for 1.9.2.
Comment 12 User image christian 2010-10-06 10:14:57 PDT
Does this also affect 1.9.1?
Comment 13 User image Daniel Veditz [:dveditz] 2010-10-08 11:10:45 PDT
On 3.6.11pre I crash at the same location referencing the same 0x0 address as the stacks in comment 0 and comment 4, but my stack didn't look messed up like the other two: bp-04b15f88-f811-414f-b660-0afc72101008
Comment 14 User image Daniel Veditz [:dveditz] 2010-10-08 12:17:13 PDT
Shiretoko has the same crash: bp-f745d95f-1f53-42f6-b356-5f2032101008
Comment 15 User image Damon Sicore (:damons) 2010-10-12 13:49:25 PDT
Mats, are you actually working on this?  Since we can reproduce this on branch, we should just fix this.
Comment 16 User image Mats Palmgren (:mats) 2010-10-12 14:00:18 PDT
I'm busy on bug 571995.  Optimistically, I could get to this in a couple
of days.  When is the next branch release(s) due?
Comment 17 User image Mats Palmgren (:mats) 2010-10-19 12:06:13 PDT
There were a couple of regressions from bug 571995 (bug 604843, bug 605340)
which now has patches for review, so I'll look at this bug after that.
Comment 18 User image Daniel Veditz [:dveditz] 2010-11-09 14:10:04 PST
Turning this into a branch bug since we've disabled content XUL on trunk (at least, for non-whitelisted sites).
Comment 19 User image Johnny Stenback (:jst, 2010-11-23 13:45:53 PST
Mats, got an eta here?
Comment 20 User image Johnny Stenback (:jst, 2010-11-30 13:37:48 PST
Mats, ping?
Comment 21 User image Mats Palmgren (:mats) 2010-12-06 17:50:33 PST
Sorry for the delay.

I've tested this in debug builds on WinXP, OSX and Linux64 and AFAICT
it's a safe null pointer access.  Jesse reported in comment 1 that
he saw evidence of a damaged stack so I guess there could have been
another bug involved earlier that is now gone.  I'm lowering this to
sg:dos until we have new evidence indicating otherwise.
Comment 22 User image Mats Palmgren (:mats) 2010-12-06 17:59:22 PST
Created attachment 495726 [details] [diff] [review]
Fix for 1.9.2 and 1.9.1

BTW, there's a comment that says the code is more or less the same as
in nsFrameLoader::EnsureDocShell.  I've checked that method and it
already has a corresponding null check.

I've stress tested this patch on WinXP, OSX and Linux64 debug 1.9.2 builds
and a Linux64 1.9.1 debug build.

Making a testcase that actually halts seems tricky -- I'll try make one
in a separate patch.
Comment 23 User image Mats Palmgren (:mats) 2010-12-06 18:00:15 PST
Created attachment 495727 [details] [diff] [review]
Fix for trunk
Comment 24 User image Mats Palmgren (:mats) 2010-12-06 18:27:09 PST
Created attachment 495736 [details] [diff] [review]
Comment 25 User image Mats Palmgren (:mats) 2010-12-07 18:27:57 PST
Comment 26 User image Daniel Veditz [:dveditz] 2010-12-08 10:33:50 PST
Comment on attachment 495726 [details] [diff] [review]
Fix for 1.9.2 and 1.9.1

Approved for and, a=dveditz for release-drivers
Comment 28 User image Al Billings [:abillings] 2011-01-04 17:10:33 PST
Verified for 1.9.2 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20110104 Namoroka/3.6.14pre ( .NET CLR 3.5.30729) and the testcase. With, the testcase will crash after a minute or three. It no longer does so with the build.

On 1.9.1, I cannot get a crash with the testcase in Have we seen the crash there?
Comment 29 User image Al Billings [:abillings] 2011-01-04 17:36:12 PST
I was wrong. I was able to get the crash, it just took longer. I've run it in the nightly 1.9.1 build (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20110104 Shiretoko/3.5.17pre ( .NET CLR 3.5.30729)) for a while now and it hasn't crashed so I'm calling this verified for 1.9.1.

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