Closed Bug 795631 Opened 7 years ago Closed 7 years ago

Valgrind on tbpl detects "Conditional jump or move depends on uninitialised value(s)" with mozilla::FrameLayerBuilder::ClippedDisplayItem::~ClippedDisplayItem on the stack

Categories

(Core :: Layout, defect, major)

All
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla18
Tracking Status
firefox15 --- unaffected
firefox16 --- unaffected
firefox17 --- unaffected
firefox18 --- fixed
firefox-esr10 --- unaffected

People

(Reporter: gkw, Assigned: mattwoodrow)

References

(Blocks 1 open bug)

Details

(Keywords: regression, valgrind, Whiteboard: [qa-])

Attachments

(2 files)

Valgrind detects "Conditional jump or move depends on uninitialised value(s)" with mozilla::FrameLayerBuilder::ClippedDisplayItem::~ClippedDisplayItem on the stack on m-c changeset c09a0c022b2e, see attached snippet which comes from:

https://tbpl.mozilla.org/php/getParsedLog.php?id=15660507&tree=Firefox&full=1

Bug 539356 may likely be the cause, because it did not occur on m-c changeset 895f66c4eada at:

https://tbpl.mozilla.org/php/getParsedLog.php?id=15623197&tree=Firefox&full=1

s-s until otherwise shown, some conditional jump errors can be bad.
I also see this, on every startup of Firefox, on x86_64-linux.  It
wasn't happening a week ago, so maybe as Gary says, due to recent
landings of bug 539356.

Here it is with the origin of uninitialised value(s) shown.

Conditional jump or move depends on uninitialised value(s)
   at 0x6299EA9: mozilla::FrameLayerBuilder::ClippedDisplayItem::~ClippedDisplayItem() (layout/base/FrameLayerBuilder.cpp:2436)
   by 0x629A563: nsTHashtable<mozilla::FrameLayerBuilder::ThebesLayerItemsEntry>::s_ClearEntry(PLDHashTable*, PLDHashEntryHdr*) (ff-opt/layout/base/../../dist/include/nsTArray.h:360)
   by 0x6F5CED9: PL_DHashTableFinish (ff-opt/xpcom/build/pldhash.cpp:361)
   by 0x629965F: mozilla::FrameLayerBuilder::~FrameLayerBuilder() (ff-opt/layout/base/../../dist/include/nsTHashtable.h:385)
   by 0x62D888B: nsDisplayList::PaintForFrame(nsDisplayListBuilder*, nsRenderingContext*, nsIFrame*, unsigned int) const (ff-opt/layout/base/../../dist/include/nsAutoPtr.h:71)
   by 0x62D8C12: nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int) const (layout/base/nsDisplayList.cpp:966)
   by 0x62F8919: nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int) (layout/base/nsLayoutUtils.cpp:1743)
   by 0x63118C4: PresShell::Paint(nsIView*, nsRegion const&, nsIPresShell::PaintType, bool) (layout/base/nsPresShell.cpp:5323)
   by 0x66FE679: nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) (view/src/nsViewManager.cpp:436)
   by 0x66FE7AD: nsViewManager::ProcessPendingUpdates() (view/src/nsViewManager.cpp:1210)
   by 0x6319170: nsRefreshDriver::Notify(nsITimer*) (layout/base/nsRefreshDriver.cpp:431)
   by 0x6F96A9D: nsTimerImpl::Fire() (xpcom/threads/nsTimerImpl.cpp:476)

 Uninitialised value was created by a heap allocation
   at 0x402ADDC: malloc (/home/sewardj/Vg38BRANCH/branch38/coregrind/m_replacemalloc/vg_replace_malloc.c:270)
   by 0x403EFEB: moz_xmalloc (memory/mozalloc/mozalloc.cpp:57)
   by 0x629BB2A: mozilla::FrameLayerBuilder::AddThebesDisplayItem(mozilla::layers::ThebesLayer*, nsDisplayItem*, mozilla::FrameLayerBuilder::Clip const&, nsIFrame*, mozilla::LayerState, nsPoint const&) (ff-opt/layout/base/../../dist/include/mozilla/mozalloc.h:200)
   by 0x629F773: mozilla::(anonymous namespace)::ContainerState::ProcessDisplayItems(nsDisplayList const&, mozilla::FrameLayerBuilder::Clip&, unsigned int) (layout/base/FrameLayerBuilder.cpp:2202)
   by 0x629F445: mozilla::(anonymous namespace)::ContainerState::ProcessDisplayItems(nsDisplayList const&, mozilla::FrameLayerBuilder::Clip&, unsigned int) (layout/base/FrameLayerBuilder.cpp:2063)
   by 0x62A215E: mozilla::FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsIFrame*, nsDisplayItem*, nsDisplayList const&, mozilla::FrameLayerBuilder::ContainerParameters const&, gfx3DMatrix const*) (layout/base/FrameLayerBuilder.cpp:2868)
   by 0x62D8246: nsDisplayList::PaintForFrame(nsDisplayListBuilder*, nsRenderingContext*, nsIFrame*, unsigned int) const (layout/base/nsDisplayList.cpp:1044)
   by 0x62D8C12: nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int) const (layout/base/nsDisplayList.cpp:966)
   by 0x62F8919: nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int) (layout/base/nsLayoutUtils.cpp:1743)
   by 0x63118C4: PresShell::Paint(nsIView*, nsRegion const&, nsIPresShell::PaintType, bool) (layout/base/nsPresShell.cpp:5323)
   by 0x66FE679: nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) (view/src/nsViewManager.cpp:436)
   by 0x66FE7AD: nsViewManager::ProcessPendingUpdates() (view/src/nsViewManager.cpp:1210)
This was in the original patch (part 2) of bug 539356 - Must have accidentally lost it during rebasing.

I don't think this is really a security bug, calling EndTransaction() every time should be harmless.
Attachment #666264 - Flags: review?(roc)
Opening up as per comment 2.
Group: core-security
Keywords: checkin-needed
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee: nobody → matt.woodrow
Target Milestone: --- → mozilla18
Is there any way a QA can test this?
Whiteboard: [qa?]
I don't think so, without setting up a valgrind install. I don't think we need explicit QA here.
Thanks for the reply
Whiteboard: [qa?] → [qa-]
You need to log in before you can comment on or make changes to this bug.