557892 - Crash [@ nsGenericElement::SetAttr] with classList.toggle

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes current trunk build and Firefox3.6 when you allow enhanced privileges. Otherwise, it may crash when going back and forth in history.

http://crash-stats.mozilla.com/report/index/58e07623-1257-43d6-9d0c-ae0e72100407
0  	xul.dll  	nsGenericElement::SetAttr  	 content/base/src/nsGenericElement.h:388
1 	xul.dll 	nsDOMTokenList::AddInternal 	content/base/src/nsDOMTokenList.cpp:168
2 	mozjs.dll 	mozjs.dll@0xedd8f

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

This is a null pointer crash. toggle() and add() may trigger this.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

This is the easiest way to fix this.

Comment 3

[Johnny Stenback (:jst)]

Opening this up, it's a DOS bug, about to get fixed.

Comment 4

[chris hofmann]

might also be know as signatures 
  24 nsGenericElement::SetAttr(int, nsIAtom*, nsIAtom*, nsAString_internal const&, int)
  22 nsGenericElement::SetAttrAndNotify(int, nsIAtom*, nsIAtom*, nsAString_internal const&, nsAttrValue&, int, int, int, nsAString_internal const*)
   8 @0x0 | nsGenericElement::SetAttr(int, nsIAtom*, nsIAtom*, nsAString_internal const&, int)
   3 nsGenericElement::SetAttributeNS(nsAString_internal const&, nsAString_internal const&, nsAString_internal const&)
   2 nsGenericElement::SetAttribute(nsAString_internal const&, nsAString_internal const&)
   2 memmove | nsGenericElement::SetAttr(int, nsIAtom*, nsIAtom*, nsAString_internal const&, int)
   1 nsGenericElement::SetAttributeNode(nsIDOMAttr*, nsIDOMAttr**)
   1 nsGenericElement::SetAttrAndNotify(int, nsIAtom*, nsIAtom*, nsAString_internal const&, nsAttrValue&, int, int, int)
   1 nsCOMPtr_base::assign_assuming_AddRef(nsISupports*) | nsGenericElement::SetAttrAndNotify(int, nsIAtom*, nsIAtom*, nsAString_internal const&, nsAttrValue&, int, int, int, nsAString_internal const*)
   1 _purecall | nsGenericElement::SetAttrAndNotify(int, nsIAtom*, nsIAtom*, nsAString_internal const&, nsAttrValue&, int, int, int, nsAString_internal const*)

Correlation to releases

checking --- nsGenericElement::SetAttr 20100411-crashdata.csv
found in: 3.6.3 3.5.9 3.5.8 3.6 3.5b99 3.5.6 3.0.2 3.0.19 3.0
release total-crashes
              nsGenericElement::SetAttr crashes
                         pct.
all  	309235	65	0.000210196
3.6.3	212615	45	0.00021165
3.5.9	28010	10	0.000357015
3.5.8	3873	3	0.000774593
3.6	16877	2	0.000118504
3.5b99	146	1	0.00684932
3.5.6	905	1	0.00110497
3.0.2	303	1	0.00330033

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

3.5 certainly doesn't have .classList.

Comment 6

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

Comment on attachment 438068 [details] [diff] [review]
patch

Do make sure to check in a testcase.

Thanks for the patch!

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: with test

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/1adcb8d5f504

Comment 9

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100419 Minefield/3.7a5pre