Closed Bug 350267 Opened 18 years ago Closed 18 years ago

[FIX] Branch only crash [@ SetFrameIsSpecial]

Categories

(Core :: Layout, defect, P2)

Product:
Core
Component:
Layout
Version:
1.8 Branch
Platform:
x86
Linux
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.8.1

People

(Reporter: MatsPalmgren_bugz, Assigned: bzbarsky)

References

(
URL
)

Details

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

Crash Data

Attachments

(3 files)

assertions + stack
3.89 KB, text/plain
Details
Patch to fix the null-deref crash here
2.60 KB, patch
roc
: review+
roc
: superreview+
dveditz
: approval1.8.0.9+
dveditz
: approval1.8.1.1+
Details | Diff | Splinter Review
minimal testcase (crashes on load)
76 bytes, text/html
Details 
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.

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

ACTUAL RESULTS
#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...


URL: https://bugzilla.mozilla.org/attachme...

    
    

    
  


  
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 1.8.1.1 and 1.8.0.9.
Status: NEW → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.0.9, 
          
            fixed1.8.1.1
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:1.8.0.9pre) Gecko/20061128 Firefox/1.5.0.9pre 
and 
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.1pre) Gecko/20061128 BonEcho/2.0.0.1pre

No crashes with minimal testcase (does crash FF2.0).
Keywords: fixed1.8.0.9, 
          
            fixed1.8.1.1 → verified1.8.0.9, 
          
            verified1.8.1.1
Group: security
Crash Signature: [@ SetFrameIsSpecial]

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: