316641 - Crash [@ nsCSSFrameConstructor::FindFrameWithContent() line 11133]

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[Bob Clary [:bc] (inactive)]

Automated RandomStyles testing on WinXP with today's FF trunk:

http://www.netscape.com/
seed=172;skip=234;changesPerInterval=214;interval=244


+	aParentContent	0x03d24360
+	aParentFrame	0x03fe0ca4
+	kidFrame	0x00000000
+	listName	0x00000000
+	this	0x0301be80

oops!

dddddddd()
nsCSSFrameConstructor::FindFrameWithContent(nsFrameManager * 0x0301b97c, nsIFrame * 0x03fe0ca4, nsIContent * 0x03d24360, nsIContent * 0x03cdff00, nsFindFrameHint * 0x00000000) line 11133 + 15 bytes
nsCSSFrameConstructor::FindPrimaryFrameFor(nsFrameManager * 0x0301b97c, nsIContent * 0x03cdff00, nsIFrame * * 0x0012e300, nsFindFrameHint * 0x00000000) line 11241 + 33 bytes
nsFrameManager::GetPrimaryFrameFor(nsIContent * 0x03cdff00) line 408
nsCSSFrameConstructor::FindPrimaryFrameFor(nsFrameManager * 0x0301b97c, nsIContent * 0x03ce00b8, nsIFrame * * 0x0012e35c, nsFindFrameHint * 0x00000000) line 11237 + 17 bytes
nsFrameManager::GetPrimaryFrameFor(nsIContent * 0x03ce00b8) line 408
PresShell::GetPrimaryFrameFor(nsIContent * 0x03ce00b8) line 5307
nsCSSFrameConstructor::AttributeChanged(nsIContent * 0x03ce00b8, int 0x00000000, nsIAtom * 0x00ad4b80, int 0x00000001) line 10484 + 26 bytes
PresShell::AttributeChanged(nsIDocument * 0x02e6e9c8, nsIContent * 0x03ce00b8, int 0x00000000, nsIAtom * 0x00ad4b80, int 0x00000001) line 5106
nsDocument::AttributeChanged(nsIContent * 0x03ce00b8, int 0x00000000, nsIAtom * 0x00ad4b80, int 0x00000001) line 2371
nsHTMLDocument::AttributeChanged(nsIContent * 0x03ce00b8, int 0x00000000, nsIAtom * 0x00ad4b80, int 0x00000001) line 1242
nsGenericElement::SetAttrAndNotify(int 0x00000000, nsIAtom * 0x00ad4b80, nsIAtom * 0x00000000, const nsAString_internal & {...}, nsAttrValue & {...}, int 0x00000001, int 0x00000000, int 0x00000001) line 4137
nsGenericHTMLElement::SetInlineStyleRule(nsGenericHTMLElement * const 0x03ce00b8, nsICSSStyleRule * 0x03b4ac7c, int 0x00000001) line 1865 + 51 bytes
nsDOMCSSAttributeDeclaration::DeclarationChanged() line 91 + 32 bytes
nsDOMCSSDeclaration::ParsePropertyValue(nsCSSProperty eCSSProperty_display, const nsAString_internal & {...}) line 267 + 11 bytes
nsDOMCSSDeclaration::SetPropertyValue(nsDOMCSSDeclaration * const 0x03ff15e8, nsCSSProperty eCSSProperty_display, const nsAString_internal & {...}) line 102
CSS2PropertiesTearoff::SetDisplay(CSS2PropertiesTearoff * const 0x03ff15ec, const nsAString_internal & {...}) line 329 + 27 bytes
XPTC_InvokeByIndex(nsISupports * 0x03ff15ec, unsigned int 0x00000058, unsigned int 0x00000001, nsXPTCVariant * 0x0012ea48) line 102
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode CALL_SETTER) line 2139 + 43 bytes
XPCWrappedNative::SetAttribute(XPCCallContext & {...}) line 1928 + 14 bytes
XPC_WN_GetterSetter(JSContext * 0x02faff88, JSObject * 0x0291f8f0, unsigned int 0x00000001, long * 0x03f2a434, long * 0x0012ed30) line 1468 + 12 bytes
js_Invoke(JSContext * 0x02faff88, unsigned int 0x00000001, unsigned int 0x00000002) line 1177 + 23 bytes
js_InternalInvoke(JSContext * 0x02faff88, JSObject * 0x0291f8f0, long 0x01762178, unsigned int 0x00000000, unsigned int 0x00000001, long * 0x0012f860, long * 0x0012f860) line 1274 + 20 bytes
js_InternalGetOrSet(JSContext * 0x02faff88, JSObject * 0x0291f8f0, long 0x027ae398, long 0x01762178, int 0x00000008, unsigned int 0x00000001, long * 0x0012f860, long * 0x0012f860) line 1333 + 31 bytes
js_SetProperty(JSContext * 0x02faff88, JSObject * 0x0291f8f0, long 0x027ae398, long * 0x0012f860) line 3024 + 53 bytes
js_Interpret(JSContext * 0x02faff88, unsigned char * 0x03cb826d, long * 0x0012fa14) line 3373 + 1981 bytes
js_Invoke(JSContext * 0x02faff88, unsigned int 0x00000001, unsigned int 0x00000002) line 1197 + 19 bytes
js_InternalInvoke(JSContext * 0x02faff88, JSObject * 0x01762008, long 0x03ff71a8, unsigned int 0x00000000, unsigned int 0x00000001, long * 0x04073488, long * 0x0012fb94) line 1274 + 20 bytes
JS_CallFunctionValue(JSContext * 0x02faff88, JSObject * 0x01762008, long 0x03ff71a8, unsigned int 0x00000001, long * 0x04073488, long * 0x0012fb94) line 4157 + 31 bytes
nsJSContext::CallEventHandler(JSObject * 0x01762008, JSObject * 0x03ff71a8, unsigned int 0x00000001, long * 0x04073488, long * 0x0012fb94) line 1422 + 33 bytes
nsGlobalWindow::RunTimeout(nsTimeout * 0x04073410) line 6219
nsGlobalWindow::TimerCallback(nsITimer * 0x04074028, void * 0x04073410) line 6577
nsTimerImpl::Fire() line 400 + 17 bytes
nsTimerManager::FireNextIdleTimer(nsTimerManager * const 0x0173bd08) line 636
nsAppShell::Run(nsAppShell * const 0x00baad68) line 142
nsAppStartup::Run(nsAppStartup * const 0x00baacc8) line 161 + 26 bytes
XRE_main(int 0x00000004, char * * 0x003f6d60, const nsXREAppData * 0x0042101c kAppData) line 2289 + 35 bytes
main(int 0x00000004, char * * 0x003f6d60) line 61 + 18 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 7c81

Comment 1

[Jesse Ruderman]

dddddddd() at the top of the stack means Firefox is jumping to a memory address it got from deleted memory.  (In debug builds, certain kinds of deleted memory get overwritten with 0xDDDDDDDD in order to make crashes more reliable.)  So this is probably a security hole allowing arbitrary code execution.

Comment 2

[Daniel Veditz [:dveditz]]

No sign of a fix (or even an assignee), not realistic for 1.8.0.1

Comment 3

[Mike Schroepfer]

Jonas - can you take a look at this for 1.8.0.2?

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I have a pretty nice list of stirdom bugs on my plate right now, especially with the new mutation fuzzer. I'll give it my best shot, but I can't promise anything :(

Comment 5

[Daniel Veditz [:dveditz]]

Moving out to next release, hoping for a fix by then

Comment 6

[Darin Fisher]

Jonas: Any possibility of a fix for 1.8.0.3?

Comment 7

[Daniel Veditz [:dveditz]]

Still interested in this one...

Comment 8

[Bob Clary [:bc] (inactive)]

I can't reproduce the crash in winxp with a recent trunk build using the current netscape page nor my saved version of it, though I hang on both. This is another of the problem bugs that need to get better assertion reporting of bad pointers and a reduced testcase. I'll try to get to it soon.

Comment 9

[Daniel Veditz [:dveditz]]

No progress, possibly fixed anyway? A hang would still be a problem. Punting on 1.8.0.4... ->1.8.0.5

Comment 10

[Jesse Ruderman]

bc's Automated testing Stir DOM with Valgrind turned up a stack with the first few lines matching the one in this bug.  I can probably make a reduced testcase, but it will take a fair amount of work.  Is it worth reducing now, or is it likely to be a dup of a known bug such as bug 337419 or bug 339651?  Does a testcase belong in this bug or in a new one?

Invalid read of size 4
	nsCSSFrameConstructor::FindFrameWithContent(nsFrameManager*, nsIFrame*, nsIContent*, nsIContent*, nsFindFrameHint*) (/work/mozilla/builds/ff/trunk-test/mozilla/layout/base/nsCSSFrameConstructor.cpp:11304)
	nsCSSFrameConstructor::FindPrimaryFrameFor(nsFrameManager*, nsIContent*, nsIFrame**, nsFindFrameHint*) (/work/mozilla/builds/ff/trunk-test/mozilla/layout/base/nsCSSFrameConstructor.cpp:11414)
	nsFrameManager::GetPrimaryFrameFor(nsIContent*) (/work/mozilla/builds/ff/trunk-test/mozilla/layout/base/nsFrameManager.cpp:400)
	nsCSSFrameConstructor::FindPrimaryFrameFor(nsFrameManager*, nsIContent*, nsIFrame**, nsFindFrameHint*) (/work/mozilla/builds/ff/trunk-test/mozilla/layout/base/nsCSSFrameConstructor.cpp:11411)
	nsFrameManager::GetPrimaryFrameFor(nsIContent*) (/work/mozilla/builds/ff/trunk-test/mozilla/layout/base/nsFrameManager.cpp:400)
	PresShell::GetPrimaryFrameFor(nsIContent*) const (/work/mozilla/builds/ff/trunk-test/mozilla/layout/base/nsPresShell.cpp:5397)
	nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, int, int) (/work/mozilla/builds/ff/trunk-test/mozilla/layout/base/nsCSSFrameConstructor.cpp:9902)
	PresShell::ContentRemoved(nsIDocument*, nsIContent*, nsIContent*, int) (/work/mozilla/builds/ff/trunk-test/mozilla/layout/base/nsPresShell.cpp:5276)
	nsDocument::ContentRemoved(nsIContent*, nsIContent*, int) (/work/mozilla/builds/ff/trunk-test/mozilla/content/base/src/nsDocument.cpp:2357)
	nsHTMLDocument::ContentRemoved(nsIContent*, nsIContent*, int) (/work/mozilla/builds/ff/trunk-test/mozilla/content/html/document/src/nsHTMLDocument.cpp:1229)
	nsGenericElement::doRemoveChildAt(unsigned, int, nsIContent*, nsIContent*, nsIDocument*, nsAttrAndChildArray&) (/work/mozilla/builds/ff/trunk-test/mozilla/content/base/src/nsGenericElement.cpp:2343)
	nsGenericElement::RemoveChildAt(unsigned, int) (/work/mozilla/builds/ff/trunk-test/mozilla/content/base/src/nsGenericElement.cpp:2286)
	nsGenericElement::doReplaceOrInsertBefore(int, nsIDOMNode*, nsIDOMNode*, nsIContent*, nsIDocument*, nsIDOMNode**) (/work/mozilla/builds/ff/trunk-test/mozilla/content/base/src/nsGenericElement.cpp:2940)
	nsGenericElement::InsertBefore(nsIDOMNode*, nsIDOMNode*, nsIDOMNode**) (/work/mozilla/builds/ff/trunk-test/mozilla/content/base/src/nsGenericElement.cpp:2420)
	nsHTMLLIElement::InsertBefore(nsIDOMNode*, nsIDOMNode*, nsIDOMNode**) (/work/mozilla/builds/ff/trunk-test/mozilla/content/html/content/src/nsHTMLLIElement.cpp:57)
	XPTC_InvokeByIndex (in /work/mozilla/builds/ff/trunk-test/mozilla/obj-opt/xpcom/build/libxpcom_core.so)
	XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (/work/mozilla/builds/ff/trunk-test/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:2145)
	XPC_WN_CallMethod(JSContext*, JSObject*, unsigned, long*, long*) (/work/mozilla/builds/ff/trunk-test/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1445)
	js_Invoke (/work/mozilla/builds/ff/trunk-test/mozilla/js/src/jsinterp.c:1328)
	js_InternalInvoke (/work/mozilla/builds/ff/trunk-test/mozilla/js/src/jsinterp.c:1422)
	 Address is inside a block free'd
	free (vg_replace_malloc.c:235)
	PR_Free (/work/mozilla/builds/ff/trunk-test/mozilla/nsprpub/pr/src/malloc/prmem.c:490)
	FrameArena::FreeFrame(unsigned, void*) (/work/mozilla/builds/ff/trunk-test/mozilla/layout/base/nsPresShell.cpp:668)
	PresShell::FreeFrame(unsigned, void*) (/work/mozilla/builds/ff/trunk-test/mozilla/layout/base/nsPresShell.cpp:2008)
	nsFrame::Destroy() (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsFrame.cpp:667)
	nsSplittableFrame::Destroy() (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsSplittableFrame.cpp:73)
	nsImageFrame::Destroy() (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsImageFrame.cpp:263)
	nsFrameList::DestroyFrames() (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsFrameList.cpp:60)
	nsContainerFrame::Destroy() (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsContainerFrame.cpp:157)
	nsFrameList::DestroyFrames() (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsFrameList.cpp:60)
	nsContainerFrame::Destroy() (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsContainerFrame.cpp:157)
	nsFrameList::DestroyFrames() (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsFrameList.cpp:60)
	nsContainerFrame::Destroy() (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsContainerFrame.cpp:157)
	nsFrameList::DestroyFrames() (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsFrameList.cpp:60)
	nsContainerFrame::Destroy() (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsContainerFrame.cpp:157)
	nsLineBox::DeleteLineList(nsPresContext*, nsLineList&) (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsLineBox.cpp:346)
	nsBlockFrame::Destroy() (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsBlockFrame.cpp:300)
	nsBlockFrame::DoRemoveFrame(nsIFrame*, int, int) (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsBlockFrame.cpp:5801)
	nsBlockFrame::RemoveFrame(nsIAtom*, nsIFrame*) (/work/mozilla/builds/ff/trunk-test/mozilla/layout/generic/nsBlockFrame.cpp:5584)
	nsFrameManager::RemoveFrame(nsIFrame*, nsIAtom*, nsIFrame*) (/work/mozilla/builds/ff/trunk-test/mozilla/layout/base/nsFrameManager.cpp:681)

Comment 11

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

That top of stack (recursive FindFrameWithContent leading to a crash) just means that we're operating on a frame tree that has pointers to a deleted frame.  It's hard to know how we got into that state, but I'd lean towards separate bugs unless there's something in common between the testcases or other parts of the stack.

The quick way to tell (with considerably more accuracy than the crash stacks, at least) if they're the same problem might be with valgrind:  flip the macro that makes FrameArena::AllocateFrame and FreeFrame just call through to malloc and free, and then see if the valgrind accessing freed memory report gives you the same **deletion stack**, which is probably more relevant than the access stack.

Comment 12

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(In reply to comment #11)
> means that we're operating on a frame tree that has pointers to a deleted
> frame.

Or that somebody's stomped all over memory, which valgrind will probably tell you about even faster.

Comment 13

[Jesse Ruderman]

The build I'm testing Valgrind with does have frame arenas set to just use malloc/free.  You can see the freeing stack in comment 10.

I tried to make a reduced testcase based on the fuzzing that resulted in comment 10, but gave up after a few hours because the invalid reads are not reliable enough, even in Valgrind.

Comment 14

[Jesse Ruderman]

I suspect I was hitting bug 340733.

Comment 15

[Darin Fisher]

No patch; not going to block FF2 beta1 for this, but we'll keep it on the radar for FF2 beta2.

Comment 16

[Mike Schroepfer]

Would prefer a patch for 1.8.1 - but minusing since we won't hold the release for this.

Comment 17

[Martijn Wargers (dead)]

I don't crash at that url with current trunk build, using the mentioned settings for the random styles bookmarklet.
That said, the site's design might have been changed.

Comment 18

[Jesse Ruderman]

Yeah, netscape.com went through a complete design and purpose change last month.  See http://www.betanews.com/article/Netscapecom_Reborn_as_Digg_Rival/1150317022.

Comment 19

[chris hofmann]

no test case around any more and couple of possible dupe bugs now fixed; is there anything useful in leaving this one open or new ideas for how to research this some more?  If not sounds like we should close it.

Comment 20

[Jesse Ruderman]

I agree.  Marking WFM.