Closed Bug 350267 Opened 16 years ago Closed 15 years ago

[FIX] Branch only crash [@ SetFrameIsSpecial]


(Core :: Layout, defect, P2)

1.8 Branch





(Reporter: MatsPalmgren_bugz, Assigned: bzbarsky)




(4 keywords, Whiteboard: [sg:nse?] null-deref)

Crash Data


(3 files)

Followup for the crash in bug 348887 comment 17.

This bug occurs on 1.8/1.8.0 branches with the same stack,
but it does not occur on trunk.

1. load "original testcase" (attachment 234043 [details]) from bug 348887.

#0  0x40e5a0ff in SetFrameIsSpecial (aFrame=0x0, aSpecialSibling=0x86a6d14) at nsCSSFrameConstructor.cpp:514
#1  0x40e6d63c in nsCSSFrameConstructor::CantRenderReplacedElement(nsIFrame*) (this=0x862c248, aFrame=0x8677f38) at nsCSSFrameConstructor.cpp:11077
#2  0x40eb44e5 in CantRenderReplacedElementEvent::HandleEvent() (this=0x86250d8) at nsPresShell.cpp:4133
#3  0x40eb3f9b in HandleCantRenderReplacedElementEvent (aEvent=0x86250d8) at nsPresShell.cpp:4000
#4  0x400fe8f9 in PL_HandleEvent (self=0x86250d8) at plevent.c:688
Attached file assertions + stack
Blocks: 348887
> ###!!! ASSERTION: out of bounds: 'PRInt32(aIndex) >= 0 && aIndex <= length',
> file nsChildIterator.h, line 133
> Break: at file nsChildIterator.h, line 133

This happens because aIndex is 0xffffffff.  Or more precisely, it's -1 but cast to PRUint32.  Which makes sense, because this is anonymous :before content:

(gdb) p ((class nsTextNode*)textContent)->mText
$21 = {{m2b = 0x8a27970, m1b = 0x8a27970 "before text"}, {mAllBits = 89, mState = {
      mInHeap = -1, mIs2b = 0, mIsBidi = 0, mLength = 11}}}

Then the floating first-letter frame ends up with the :before wrapper of the <body> as its parent, and things sorta go to hell...

This fixes the immediate crash, but then I get a deleted-frame deref from nsBlockFrame::CollectFloats...  I do think we might want this patch in general, possibly, since I think there are simpler ways to trigger that null deref.

But what we should really figure out is why we don't get those asserts on trunk; that's the real thing to fix here.
Could really use a minimalish testcase here...
Flags: blocking1.8.1.1?
Keywords: qawanted
I tried to reproduce the crash Boris was seeing with his patch applied, but wasn't able to. With that patch applied to my branch build, I wasn't able to reproduce a crash, with either the original testcase or my minimal testcase.
Comment on attachment 235676 [details] [diff] [review]
Patch to fix the null-deref crash here

OK, looks like current 1.8 branch builds are not crashing once this patch is applied, though there are still tons of wrong float parent asserts...  Looks like one of the other fuzz fixes helped.  ;)
Attachment #235676 - Flags: superreview?(roc)
Attachment #235676 - Flags: review?(roc)
Attachment #235676 - Flags: superreview?(roc)
Attachment #235676 - Flags: superreview+
Attachment #235676 - Flags: review?(roc)
Attachment #235676 - Flags: review+
Attachment #235676 - Flags: approval1.8.0.8?
Assignee: nobody → bzbarsky
Keywords: qawanted
Priority: -- → P2
Summary: Branch only crash [@ SetFrameIsSpecial] → [FIX] Branch only crash [@ SetFrameIsSpecial]
Target Milestone: --- → mozilla1.8.1
Whiteboard: [sg:nse?] null-deref
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1+
Flags: blocking1.8.0.9+
Comment on attachment 235676 [details] [diff] [review]
Patch to fix the null-deref crash here

approved for 1.8/1.8.0 branches, a=dveditz for drivers
Attachment #235676 - Flags: approval1.8.1.1+
Attachment #235676 - Flags: approval1.8.0.9?
Attachment #235676 - Flags: approval1.8.0.9+
Fixed for and
Closed: 15 years ago
Resolution: --- → FIXED
v.fixed on 1.8.0 and 1.8.1 branches with 
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv: Gecko/20061128 Firefox/ 
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv: Gecko/20061128 BonEcho/

No crashes with minimal testcase (does crash FF2.0).
Group: security
Crash Signature: [@ SetFrameIsSpecial]
You need to log in before you can comment on or make changes to this bug.