538479 - BCPaintBorderIterator::BCPaintBorderIterator(nsTableFrame*) doesn't initialise all fields

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Julian Seward [:jseward]]

Electrolysis branch, build as of 6 Jan 2010.

When looking at http://www.kde.org I get the complainage shown below
from Valgrind-Memcheck.

On examination of layout/tables/nsTableFrame.cpp it appears that
BCPaintBorderIterator::BCPaintBorderIterator(nsTableFrame* aTable) is
failing to initialise a whole bunch of fields, amongst them
mIsRepeatedHeader, mRowIndex, mRepeatedHeaderRowIndex.

Resulting complaint from Memcheck is shown below.  Bug is marked as
for x86_64-linux, but in fact I think it applies on all platforms.

Conditional jump or move depends on uninitialised value(s)
   at 0x5820DEE: BCPaintBorderIterator::AccumulateOrPaintVerticalSegment(nsIRenderingContext&) (nsTableFrame.cpp:6312)
   by 0x582C763: nsTableFrame::PaintBCBorders(nsIRenderingContext&, nsRect const&) (nsTableFrame.cpp:7385)
   by 0x582DC45: nsTableFrame::PaintTableBorderBackground(nsIRenderingContext&, nsRect const&, nsPoint, unsigned int) (nsTableFrame.cpp:1438)
   by 0x582DCFE: nsDisplayTableBorderBackground::Paint(nsDisplayListBuilder*, nsIRenderingContext*) (nsTableFrame.cpp:1209)
   by 0x56FF510: nsDisplayList::Paint(nsDisplayListBuilder*, nsIRenderingContext*) const (nsDisplayList.cpp:415)
   by 0x56FF530: nsDisplayWrapList::Paint(nsDisplayListBuilder*, nsIRenderingContext*) (nsDisplayList.cpp:999)
   by 0x56FF6AA: nsDisplayClip::Paint(nsDisplayListBuilder*, nsIRenderingContext*) (nsDisplayList.cpp:1195)
   by 0x56FF510: nsDisplayList::Paint(nsDisplayListBuilder*, nsIRenderingContext*) const (nsDisplayList.cpp:415)
   by 0x5711AC3: nsLayoutUtils::PaintFrame(nsIRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int) (nsLayoutUtils.cpp:1234)
   by 0x571C848: PresShell::Paint(nsIView*, nsIRenderingContext*, nsRegion const&) (nsPresShell.cpp:5771)
   by 0x5A17AD4: nsViewManager::RenderViews(nsView*, nsIRenderingContext&, nsRegion const&) (nsViewManager.cpp:515)
   by 0x5A17E3E: nsViewManager::Refresh(nsView*, nsIRenderingContext*, nsIRegion*, unsigned int) (nsViewManager.cpp:482)
 Uninitialised value was created by a stack allocation
   at 0x582C713: nsTableFrame::PaintBCBorders(nsIRenderingContext&, nsRect const&) (nsTableFrame.cpp:7371)

Comment 1

[Bernd]

Does the same happen for ordinary trunk?

Comment 2

[Bernd]

and if so do you see asserts if this is a debug build

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Electrolysis hasn't changed any of this code, so it should be equivalent to trunk.  The line numbers match up.

In any case, mRepeatedHeaderRowIndex is only initialized when mIsRepeatedHeader is true, but the code that's causing the warning:

  PRBool                IsAfterRepeatedHeader() { return !mIsRepeatedHeader && (mRowIndex == (mRepeatedHeaderRowIndex + 1));}

uses mRepeatedHeaderRowIndex when mIsRepeatedHeader is false.

Comment 4

[Bernd]

mine 

http://mxr.mozilla.org/mozilla/source/layout/tables/nsTableFrame.cpp#6426

Comment 5

[Julian Seward [:jseward]]

(In reply to comment #1)
> Does the same happen for ordinary trunk?

Yes, same problem on m-c, or even possibly worse; I see this 
same error with same line numbers, plus two more that look like
they are also layout related, that I did not see before.

Minor clarification: to repro, go to www.kde.org, wait till
the page has finished loading, then scroll up and down a couple
of times.  The problem is reported as soon as I start scrolling,
I think.

Comment 6

[Bernd]

Attachment: add the old initialization code

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 422071 [details] [diff] [review]
add the old initialization code

r=dbaron

Comment 8

[Bernd]

http://hg.mozilla.org/mozilla-central/rev/57e0aaffc860

Julian does that fix the issue? Or is there a reminder, please reopen if this the case.

Comment 10

[Jesse Ruderman]

in-testsuite+ per the dup.

Comment 11

[Julian Seward [:jseward]]

(In reply to comment #8)
> http://hg.mozilla.org/mozilla-central/rev/57e0aaffc860
> 
> Julian does that fix the issue? 

Yes, it appears to.

Bernd, are you using MacOS or Linux as your development platform?
If yes, it would be a good thing if you could check any such changes
using Valgrind-Memcheck before they get landed.

Comment 12

[Bernd]

I am using winXX since I started