1597764 - Conditional jump or move depends on uninitialised value(s) in [@ SkStroke::strokePath]

Status: RESOLVED FIXED

(Core :: SVG, defect, P3)

Description

[Tyson Smith [:tsmith]]

This is triggered during launch with a clean profile.

Found with Valgrind on m-c:
BuildID=20191119105922
SourceStamp=b5c5ba07d3dbd0d07b66fa42a103f4df2c27d3a2

Conditional jump or move depends on uninitialised value(s)
   at 0x13EEA995: SkStroke::strokePath(SkPath const&, SkPath*) const (gfx/skia/skia/src/core/SkStroke.cpp:1395)
   by 0x13EEB18B: SkStrokeRec::applyToPath(SkPath*, SkPath const&) const (gfx/skia/skia/src/core/SkStrokeRec.cpp:121)
   by 0x13EB25B5: SkPaint::getFillPath(SkPath const&, SkPath*, SkRect const*, float) const (gfx/skia/skia/src/core/SkPaint.cpp:459)
   by 0x13E8B9F9: SkDraw::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool, bool, SkBlitter*) const (gfx/skia/skia/src/core/SkDraw.cpp:957)
   by 0x13E4F32F: SkBitmapDevice::drawPath(SkPath const&, SkPaint const&, bool) (gfx/skia/skia/src/core/SkDraw.h:56)
   by 0x13E5DD6D: SkCanvas::onDrawPath(SkPath const&, SkPaint const&) (gfx/skia/skia/src/core/SkCanvas.cpp:2365)
   by 0x13E5C184: SkCanvas::drawPath(SkPath const&, SkPaint const&) (gfx/skia/skia/src/core/SkCanvas.cpp:1878)
   by 0x11769B20: mozilla::gfx::DrawTargetSkia::Stroke(mozilla::gfx::Path const*, mozilla::gfx::Pattern const&, mozilla::gfx::StrokeOptions const&, mozilla::gfx::DrawOptions const&) (gfx/2d/DrawTargetSkia.cpp:784)
   by 0x13613F23: mozilla::SVGGeometryFrame::Render(gfxContext*, unsigned int, mozilla::gfx::BaseMatrix<double> const&, mozilla::image::imgDrawingParams&) (layout/svg/SVGGeometryFrame.cpp:757)
   by 0x1361380E: mozilla::SVGGeometryFrame::PaintSVG(gfxContext&, mozilla::gfx::BaseMatrix<double> const&, mozilla::image::imgDrawingParams&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const*) (layout/svg/SVGGeometryFrame.cpp:264)
   by 0x136130F6: nsDisplaySVGGeometry::Paint(nsDisplayListBuilder*, gfxContext*) (layout/svg/SVGGeometryFrame.cpp:120)
   by 0x136EE32C: mozilla::FrameLayerBuilder::PaintItems(std::vector<mozilla::AssignedDisplayItem, std::allocator<mozilla::AssignedDisplayItem> >&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, gfxContext*, nsDisplayListBuilder*, nsPresContext*, mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits> const&, float, float) (layout/painting/FrameLayerBuilder.cpp:7125)
 Uninitialised value was created by a stack allocation
   at 0x13EEA900: SkStroke::strokePath(SkPath const&, SkPath*) const (gfx/skia/skia/src/core/SkStroke.cpp:1379)

Julian, is this a false positive?

From https://searchfox.org/mozilla-central/source/gfx/skia/skia/src/core/SkStroke.cpp#1392

SkRect rect;
bool isClosed;
SkPath::Direction dir;
if (src.isRect(&rect, &isClosed, &dir) && isClosed) {  // <=== error is on this line

I believe in the past there were situations where the args in the if were evaluated out of order and that tripped up Valgrind.

Comment 1

[Julian Seward [:jseward]]

Yeah, this is almost certainly a false positive, of the form you mention.

I have been working on a fix for this. It's mostly done, and appears to work
for Firefox on x86_64. I plan to merge the fix to the Valgrind trunk
in the next couple of weeks. And at that point perhaps we should
update the Valgrind used in automation. You can read more about
the fix here:

https://internals.rust-lang.org/t/valgrind-memcheck-false-positive-fixes-for-rust/11166

Comment 2

[Tyson Smith [:tsmith]]

I am no longer able reproducible with the latest version of Valgrind.

I tested with m-c 20200102-c7082b580eeb and Valgrind commit 2a7d3ae7681.