1051527 - BackgroundClipState::mHasRounded corners is used uninitialised

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Julian Seward [:jseward]]

For content/base/test/csp/test_CSP_bug663567.html
Valgrind complaints as shown below.

What happens is:

layout/base/nsCSSRendering.cpp:
function nsCSSRendering::PaintBackgroundWithSC 

  BackgroundClipState clipState;  // all fields uninitialised
  if (aBGClipRect) {              // TAKEN
    clipState.mBGClipArea = *aBGClipRect;
    clipState.mCustomClip = true;
    SetupDirtyRects(clipState.mBGClipArea, aDirtyRect, appUnitsPerPixel,
                    &clipState.mDirtyRect, &clipState.mDirtyRectGfx);
    // clipState.mHasRoundedCorners is still uninitialised
  } else {
    GetBackgroundClip(bg->BottomLayer(),
                      aForFrame, aBorder, aBorderArea,
                      aDirtyRect, (aFlags & PAINTBG_WILL_PAINT_BORDER), appUnitsPerPixel,
                      &clipState);
  }

2740
    DrawBackgroundColor(clipState, ctx, appUnitsPerPixel);

which leads to line
  if (!aClipState.mHasRoundedCorners || aClipState.mCustomClip) {

So clipState.mHasRoundedCorners is used uninitialised.

Comment 1

[Julian Seward [:jseward]]

Conditional jump or move depends on uninitialised value(s)
   at 0x6EDD2D5: DrawBackgroundColor(nsCSSRendering::BackgroundClipState&, gfxContext*, int) (layout/base/nsCSSRendering.cpp:1846)
   by 0x6EF7F52: nsCSSRendering::PaintBackgroundWithSC(nsPresContext*, nsRenderingContext&, nsIFrame*, nsRect const&, nsRect const&, nsStyleContext*, nsStyleBorder const&, unsigned int, nsRect*, int) (layout/base/nsCSSRendering.cpp:2740)
   by 0x6FC60C4: TableBackgroundPainter::PaintCell(nsTableCellFrame*, bool) (layout/tables/nsTablePainter.cpp:637)
   by 0x6FC7ED5: TableBackgroundPainter::PaintRow(nsTableRowFrame*, bool) (layout/tables/nsTablePainter.cpp:570)
   by 0x6FCBCA7: TableBackgroundPainter::PaintRowGroup(nsTableRowGroupFrame*, bool) (layout/tables/nsTablePainter.cpp:510)
   by 0x6FCFCCB: TableBackgroundPainter::PaintTable(nsTableFrame*, nsMargin const&, bool) (layout/tables/nsTablePainter.cpp:426)
   by 0x6FDAE53: nsTableFrame::PaintTableBorderBackground(nsRenderingContext&, nsRect const&, nsPoint, unsigned int) (layout/tables/nsTableFrame.cpp:1367)
   by 0x6FDAF96: nsDisplayTableBorderBackground::Paint(nsDisplayListBuilder*, nsRenderingContext*) (layout/tables/nsTableFrame.cpp:1183)
   by 0x6EA7767: mozilla::FrameLayerBuilder::PaintItems(nsTArray<mozilla::FrameLayerBuilder::ClippedDisplayItem>&, nsIntRect const&, gfxContext*, nsRenderingContext*, nsDisplayListBuilder*, nsPresContext*, nsIntPoint const&, float, float, int) (layout/base/FrameLayerBuilder.cpp:4195)
   by 0x6EA7EC6: mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*) (layout/base/FrameLayerBuilder.cpp:4357)
   by 0x6052A43: mozilla::layers::BasicThebesLayer::PaintBuffer(gfxContext*, nsIntRegion const&, nsIntRegion const&, nsIntRegion const&, bool, mozilla::layers::DrawRegionClip, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*) (gfx/layers/basic/BasicThebesLayer.h:117)
   by 0x604D415: mozilla::layers::BasicThebesLayer::Validate(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) (gfx/layers/basic/BasicThebesLayer.cpp:188)
   by 0x604C894: mozilla::layers::BasicContainerLayer::Validate(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) (gfx/layers/basic/BasicContainerLayer.cpp:128)
   by 0x604C894: mozilla::layers::BasicContainerLayer::Validate(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) (gfx/layers/basic/BasicContainerLayer.cpp:128)
   by 0x604E84B: mozilla::layers::BasicLayerManager::EndTransactionInternal(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) (gfx/layers/basic/BasicLayerManager.cpp:501)
   by 0x604EC82: mozilla::layers::BasicLayerManager::EndTransaction(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) (gfx/layers/basic/BasicLayerManager.cpp:462)

 Uninitialised value was created by a stack allocation
   at 0x6EF79B0: nsCSSRendering::PaintBackgroundWithSC(nsPresContext*, nsRenderingContext&, nsIFrame*, nsRect const&, nsRect const&, nsStyleContext*, nsStyleBorder const&, unsigned int, nsRect*, int) (layout/base/nsCSSRendering.cpp:2612)

Comment 2

[Julian Seward [:jseward]]

Attachment: A possible fix

Comment 3

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

bug 921341 is somewhat related.

Comment 4

[Julian Seward [:jseward]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2ed9339b41e9

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2ed9339b41e9