Closed Bug 514425 Opened 13 years ago Closed 13 years ago

[HTML5] Crash [@ nsHTMLFormElement::RemoveElement] with form and input

Categories

(Core :: DOM: HTML Parser, defect)

x86
Windows XP
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: martijn.martijn, Assigned: bzbarsky)

References

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(2 files)

Attached file zipped up testcase
I'm sometimes getting this crash while browsing too (it seems, when I press the stop button, while the page is loading).
Anyway, I'm getting this crash with the zipped up testcase too.
To reproduce:
- Unzip the zipped up testcase, open the file named 'parentframe2.htm' and allow the enhanced privilege prompt. It usually crashes within 10 seconds or so.

http://crash-stats.mozilla.com/report/index/3218ccdc-e66a-4374-93ef-08a9b2090903?p=1
0  	ntdll.dll  	KiFastSystemCallRet  	
1 	ntdll.dll 	ZwWaitForSingleObject 	
2 	kernel32.dll 	WaitForSingleObjectEx 	
3 	kernel32.dll 	WaitForSingleObject 	
4 	xul.dll 	google_breakpad::ExceptionHandler::WriteMinidumpOnHandlerThread 	toolkit/crashreporter/google-breakpad/src/client/windows/handler/exception_handler.cc:562
5 	xul.dll 	google_breakpad::ExceptionHandler::HandlePureVirtualCall 	toolkit/crashreporter/google-breakpad/src/client/windows/handler/exception_handler.cc:506
6 	mozcrt19.dll 	_purecall 	obj-firefox/memory/jemalloc/crtsrc/purevirt.c:47
7 	xul.dll 	nsHTMLFormElement::RemoveElement 	content/html/content/src/nsHTMLFormElement.cpp:1463
8 	xul.dll 	nsGenericHTMLFormElement::ClearForm 	content/html/content/src/nsGenericHTMLElement.cpp:2325
9 	mozcrt19.dll 	arena_dalloc 	obj-firefox/memory/jemalloc/crtsrc/jemalloc.c:4548
10 	xul.dll 	nsHTMLInputElement::`vector deleting destructor' 	
11 	xul.dll 	nsNodeUtils::LastRelease 	content/base/src/nsNodeUtils.cpp:259
12 	xul.dll 	nsGenericElement::Release 	content/base/src/nsGenericElement.cpp:4159
13 	xul.dll 	nsRefPtr<nsIDOMEventListener>::~nsRefPtr<nsIDOMEventListener> 	obj-firefox/xpcom/build/nsCOMPtr.cpp:81
14 	xul.dll 	nsHtml5TreeOperation::~nsHtml5TreeOperation 	parser/html/nsHtml5TreeOperation.cpp:60
15 	xul.dll 	nsHtml5TreeOperation::`scalar deleting destructor' 	
16 	xul.dll 	nsTArray<nsHtml5TreeOperation>::DestructRange 	obj-firefox/dist/include/nsTArray.h:862
17 	xul.dll 	nsTArray<nsHtml5TreeOperation>::RemoveElementsAt 	obj-firefox/dist/include/nsTArray.h:663
18 	xul.dll 	nsTArray<nsHtml5TreeOperation>::Clear 	obj-firefox/dist/include/nsTArray.h:674
19 	xul.dll 	nsHtml5TreeBuilder::end 	parser/html/nsHtml5TreeBuilderCppSupplement.h:280
20 	xul.dll 	nsHtml5TreeBuilder::endTokenization 	parser/html/nsHtml5TreeBuilder.cpp:555
21 	xul.dll 	nsHtml5Tokenizer::end 	parser/html/nsHtml5Tokenizer.cpp:3283
22 	xul.dll 	nsHtml5Parser::DidBuildModel 	parser/html/nsHtml5Parser.cpp:778
23 	xul.dll 	nsHtml5Parser::Terminate 	parser/html/nsHtml5Parser.cpp:440
24 	xul.dll 	xul.dll@0x353f0b 	
25 	xul.dll 	nsDocShell::Stop 	docshell/base/nsDocShell.cpp:3964
26 	xul.dll 	nsDocShell::Stop 	docshell/base/nsDocShell.cpp:3992
27 	xul.dll 	nsGlobalWindow::Stop 	dom/base/nsGlobalWindow.cpp:4437
28 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
29 	xul.dll 	XPCWrappedNative::CallMethod 	js/src/xpconnect/src/xpcwrappednative.cpp:2710
You might want to try and crash it online, but I haven't been able to: (not sure why not)
jar:https://bugzilla.mozilla.org/attachment.cgi?id=398391!/parentframe2.htm
Blocks: 507864
not crashing on linux with the bugzilla testcase.
got assertion:
###!!! ASSERTION: need to implement cancel when downloading: '!mIsPending', file /opt/pub/firefox-35/src/modules/libjar/nsJARChannel.cpp, line 397
I was pointed here from Bug 514589. Just received the following crash @ http://lite.facebook.com/geeknik with the 9/13 build of Minefield:

http://crash-stats.mozilla.com/report/index/808b0019-fc10-4b54-86a9-e11bc2090913
Depends on: 516436
FWIW I've had this crash twice on trunk. Both crashes occurred after I closed (by middle-clicking) a tab that was still loading.

http://crash-stats.mozilla.com/report/index/bp-e14a8781-163f-4fe0-8130-5adf32090909
http://crash-stats.mozilla.com/report/index/80a3fc6b-fe6e-47c4-9075-b5fff2090921
No longer depends on: 516436
Duplicate of this bug: 516436
Attached patch Possible fixSplinter Review
Martijn, does this fix the crash for you?  I tried reproducing over here using the testcase and couldn't....
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #402881 - Flags: review?(jst)
Comment on attachment 402881 [details] [diff] [review]
Possible fix

Why is ClearForm virtual? It doesn't seem to be overridden anywhere. Though given the amount of code it calls out to it might be safest to still not call it from the dtor.
Attachment #402881 - Flags: review?(jst) → review+
Oh, I guess it lives on nsIFormControl so it sort of makes sense to keep it virtual.
> given the amount of code it calls out to it might be safest to still not call it from the dtor

if destructors calling too much code are a realistic threat, probably static analysis can search for them.
(In reply to comment #6)
> Martijn, does this fix the crash for you?  I tried reproducing over here using
> the testcase and couldn't....

Yeah, it seems like it is already fixed somehow, I can't also reproduce anymore, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090922 Minefield/3.7a1pre (.NET CLR 3.5.30729)
(In reply to comment #9)
> > given the amount of code it calls out to it might be safest to still not call it from the dtor
> 
> if destructors calling too much code are a realistic threat, probably static
> analysis can search for them.

Depends on what you mean by "threat". I can't think of a way to exploit a pure virtual function call into a security problem. But it certainly can be a stability problem.
OK.  I guess I'll check this in and see whether I stop crashing with this stack....
Fwiw, I did some more testing, but I don't seem to crash anymore with this stack trace.
Pushed http://hg.mozilla.org/mozilla-central/rev/9f3d99c80da2
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment on attachment 402881 [details] [diff] [review]
Possible fix

It might be worth taking this on 1.9.2 as part of our crash effort, though we know of no obvious way to trigger the bug there.  The code is clearly wrong...
Attachment #402881 - Flags: approval1.9.2?
Comment on attachment 402881 [details] [diff] [review]
Possible fix

Definitely!
Attachment #402881 - Flags: approval1.9.2? → approval1.9.2+
martijn, can you verify this on branch and trunk?   thanks.
No, I couldn't reproduce this crash anymore, as I mentioned in comment 10.
Crash Signature: [@ nsHTMLFormElement::RemoveElement]
You need to log in before you can comment on or make changes to this bug.