639733 - Crash [@ nsIsIndexFrame::RestoreState]

Status: RESOLVED FIXED

(Core :: Layout, defect, critical)

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox when loaded)

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Mats Palmgren (:mats)]

Null-pointer crash trying to restore a saved <embed> state on a <isindex>.
Tracing frame state save/restore leading up to the crash:

No state to save for HTMLScroll(html)(-1)@0x7fffe1b57448
No state to save for HTMLScroll(html)(-1)@0x7fffdae18448
No state to save for HTMLScroll(html)(-1)@0x7fffda3a1448
No state '0>1' to restore for HTMLScroll(html)(-1)@0x7fffda3a1448
No state '0>0>o>1>3>0' to restore for HTMLScroll(embed)(1)@0x7fffda3a9ab0
No state '0>0>o>3>3>0' to restore for IsIndex(isindex)(3)@0x7fffd91057c0
AddState '0>0>o>1>3>0' = 0x7fffd91a9040 for HTMLScroll(embed)(1)@0x7fffda3a9ab0
No state to save for IsIndex(isindex)(2)@0x7fffd91057c0
RestoreState '0>0>o>1>3>0' = 0x7fffd91a9040 for IsIndex(isindex)(1)@0x7fffd91057c0
[0x7fffd91057c0]RestoreState: aState=0x7fffd91a9040 GetStateProperty stateString=(nil)
###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr != 0', file ../../dist/include/nsCOMPtr.h, line 819

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff603fd0f in nsIsIndexFrame::RestoreState (this=0x7fffd91057c0, aState=0x7fffd91a9040) at layout/forms/nsIsIndexFrame.cpp:571
571       stateString->GetData(data);

Comment 3

[Mats Palmgren (:mats)]

Attachment: fix

Include the tag name in the frame state key, instead of "o".
Make nsIsIndexFrame::RestoreState null safe, just in case.

Comment 4

[Mats Palmgren (:mats)]

Here's what the trace looks like with the fix:

No state to save for HTMLScroll(html)(-1)@0x7fffe187a448
No state to save for HTMLScroll(html)(-1)@0x7fffdaeb9448
No state to save for HTMLScroll(html)(-1)@0x7fffda272448
No state '0>1' to restore for HTMLScroll(html)(-1)@0x7fffda272448
No state '0>0>embed>1>3>0' to restore for HTMLScroll(embed)(1)@0x7fffda285ab0
No state '0>0>isindex>3>3>0' to restore for IsIndex(isindex)(3)@0x7fffd90d27c0
AddState '0>0>embed>1>3>0' = 0x7fffd8c0a060 for HTMLScroll(embed)(1)@0x7fffda285ab0
No state to save for IsIndex(isindex)(2)@0x7fffd90d27c0
No state '0>0>isindex>1>3>0' to restore for IsIndex(isindex)(1)@0x7fffd90d27c0

Comment 5

[Mats Palmgren (:mats)]

Attachment: crashtest

Comment 6

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Comment on attachment 524003 [details] [diff] [review]
fix

Why not just:

  KeyAppendString(nsDependentAtomString(aContent->Tag()), aKey);

?

r=me with that.  Don't forget to check in the crashtest.

Comment 7

[Mats Palmgren (:mats)]

Much better, thanks.

Fixed in Cedar:
http://hg.mozilla.org/projects/cedar/rev/1652e3d8dc1c
http://hg.mozilla.org/projects/cedar/rev/80dc22b6c3f6

Comment 9

[:ehsan akhgari]

http://hg.mozilla.org/mozilla-central/rev/80dc22b6c3f6
http://hg.mozilla.org/mozilla-central/rev/1652e3d8dc1c

Comment 10

[Cameron Kaiser [:spectre]]

Per security group discussion, requesting landing on mozilla-2.0.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 524003 [details] [diff] [review]
fix

minus on long past 2.0 approval