Closed Bug 68265 Opened 24 years ago Closed 8 years ago

UMR in nsBoxToBlockAdaptor::DoLayout(...)

Categories

(Core :: XUL, defect, P2)

x86
Windows 2000
defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: bratell, Unassigned)

References

()

Details

I got UMR:s when visiting http://www.mozilla.org/. While not dangerous in 
itself, Unintialized Memory Reads are often the cause of crashes or faulty 
behaviour. I get the UMR in the following places:

    nsBoxToBlockAdaptor::DoLayout(nsBoxLayoutState&) 
[nsBoxToBlockAdaptor.cpp:533]
            if (currentSize) {
              desiredSize.maxElementSize = nsnull;
        
     =>       if (size.width > currentSize->width)
                 currentSize->width = size.width;
        
              if (mCachedMaxElementHeight > currentSize->height) {



    nsBoxToBlockAdaptor::DoLayout(nsBoxLayoutState&) 
[nsBoxToBlockAdaptor.cpp:536]
              if (size.width > currentSize->width)
                 currentSize->width = size.width;
        
     =>       if (mCachedMaxElementHeight > currentSize->height) {
                currentSize->height = mCachedMaxElementHeight;
              }
            }




I don't know if it's the member variable or the "currentsize" variables that are 
uninitialized, but Purify describes it as being "the local variable 
'innerMaxElementSize' in nsLineLayout::ReflowFrame()" and 4 bytes past that 
variable. 

A typical top stack looks like:

    nsBoxToBlockAdaptor::DoLayout(nsBoxLayoutState&) 
[nsBoxToBlockAdaptor.cpp:533]

    nsBox::Layout(nsBoxLayoutState&) [nsBox.cpp:987]

    nsScrollBoxFrame::DoLayout(nsBoxLayoutState&) [nsScrollBoxFrame.cpp:375]

    nsBox::Layout(nsBoxLayoutState&) [nsBox.cpp:987]

    nsGfxScrollFrameInner::LayoutBox(nsBoxLayoutState&,nsIBox *,nsRect const&) 
[nsGfxScrollFrame.cpp:1024]

    nsGfxScrollFrame::DoLayout(nsBoxLayoutState&) [nsGfxScrollFrame.cpp:1033]

    nsBox::Layout(nsBoxLayoutState&) [nsBox.cpp:987]

    nsContainerBox::DoLayout(nsBoxLayoutState&) [nsContainerBox.cpp:551]

    nsBoxFrame::DoLayout(nsBoxLayoutState&) [nsBoxFrame.cpp:982]

    nsGfxTextControlFrame2::Reflow(nsIPresContext 
*,nsHTMLReflowMetrics&,nsHTMLReflowState const&,UINT&) 
[nsGfxTextControlFrame2.obj:2001]

    nsLineLayout::ReflowFrame(nsIFrame *,nsIFrame * *,UINT&,nsHTMLReflowMetrics 
*,int&) [nsLineLayout.cpp:919]

    nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&,nsLineLayout&,nsLineBox 
*,nsIFrame *,BYTE *) [nsBlockFrame.cpp:4385]

  nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&,nsLineLayout&,nsLineBox 
*,int *,BYTE *,int,int) [nsBlockFrame.cpp:4269]

    nsBlockFrame::DoReflowInlineFramesAuto(nsBlockReflowState&,nsLineBox *,int 
*,BYTE *,int,int) [nsBlockFrame.cpp:4203]

    nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&,nsLineBox *,int 
*,int,int) [nsBlockFrame.cpp:4148]

    nsBlockFrame::ReflowLine(nsBlockReflowState&,nsLineBox *,int *,int) 
[nsBlockFrame.cpp:3282]

    nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) [nsBlockFrame.cpp:2971]

    nsBlockFrame::Reflow(nsIPresContext *,nsHTMLReflowMetrics&,nsHTMLReflowState 
const&,UINT&) [nsBlockFrame.cpp:1743]

  nsBlockReflowContext::DoReflowBlock(nsHTMLReflowState&,nsReflowReason,nsIFrame 
*,nsRect const&,int,int,int,nsMargin&,UINT&) [nsBlockReflowContext.cpp:566]

    nsBlockReflowContext::ReflowBlock(nsIFrame *,nsRect 
const&,int,int,int,nsMargin&,UINT&) [nsBlockReflowContext.cpp:334]
Triaging karnaze's bugs.
Assignee: karnaze → attinasi
Priority: -- → P2
Target Milestone: --- → mozilla1.0
Looks like a XUL thang --> hyatt
Assignee: attinasi → hyatt
Component: Layout → XP Toolkit/Widgets: XUL
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0 → mozilla1.0.1
605  
606     if (currentSize) {
607       if (maxElementSize.width > currentSize->width)
608          currentSize->width = maxElementSize.width;
609   
610       if (mCachedMaxElementHeight > currentSize->height) {
611         currentSize->height = mCachedMaxElementHeight;
612       }
613     }
614 

currentSize->width (and currentSize->height) are uninitialized.
It's currentSize->width (and height) that are uninitialized, and 
they get that from this call:

void 
nsBoxLayoutState::GetMaxElementSize(nsSize** aMaxElementSize)
{
  *aMaxElementSize = nsnull;
  if (mReflowState)
     *aMaxElementSize = mMaxElementSize;
}

and nsBoxLayoutState::mMaxElementSize->width is uninitialized, since
the constructor was (presumably) passed 'aDesiredSize' that wasn't 
initialized. 

*** Bug 119544 has been marked as a duplicate of this bug. ***
Brian, would you mind helping out with this?
sure.
Assignee: hyatt → bryner
Status: ASSIGNED → NEW
What seems to be happening is that nsBoxFrame::Reflow does not initialize
aDesiredSize's width and height before calling Layout().  If Layout() results in
a call to nsBoxToBlockAdaptor::DoLayout(), it will pull an uninitialized max
size from the box layout state.

I'm unclear about what an appropriate value is to initialize aDesiredSize with
before we call Layout().  After we call Layout() we fill in aDesiredSize with
the box's bounds.  Should we also do that prior to calling Layout()?  Or should
it be initialized to something else... 0? NS_MAXSIZE?
Status: NEW → ASSIGNED
Assignee: bryner → nobody
Status: ASSIGNED → NEW
QA Contact: chrispetersen → xptoolkit.xul
Target Milestone: mozilla1.0.1 → ---
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
While some of the code here no longer exists, the analysis from bryner still *might* be valid - in nsBoxFrame::Reflow() aDesiredSize is set after Layout(), so it seems possible there might still be a loopback that uses an uninitialized value.
nsHTMLReflowMetrics ctor zeroes all fields so I don't think this can occur anymore.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.