diff options
| author |  caryclark <caryclark@google.com> | 2016-09-23 09:32:26 -0700 |
|---|---|---|
| committer |  Commit bot <commit-bot@chromium.org> | 2016-09-23 09:32:27 -0700 |
| commit | cc09372730301be78b9d26c1198db1584622cdd9 (patch) | |
| tree | 99be1f7a5c3cfefbb75b47f03ddf5d377a5c4fd8 | |
| parent | 94663795410c214db5232162076b8aea7e5f62d2 (diff) | |
fix msan bug in pathops
Msan and Valgrind found an uninitialized memory mistake in
pathops. This also fixes similar bugs where not all parts
of the geometry were covered in the loop iteration.
R=borenet@google.com
NOTREECHECKS=true
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2366893003
Review-Url: https://codereview.chromium.org/2366893003
| -rw-r--r-- | src/pathops/SkOpEdgeBuilder.cpp | 10 | ||||
| -rw-r--r-- | src/pathops/SkPathOpsCurve.cpp | 4 | ||||
| -rw-r--r-- | tests/PathOpsOpTest.cpp | 6 |
3 files changed, 9 insertions, 11 deletions
diff --git a/src/pathops/SkOpEdgeBuilder.cpp b/src/pathops/SkOpEdgeBuilder.cpp index d67ed44ddc..c304357575 100644 --- a/src/pathops/SkOpEdgeBuilder.cpp +++ b/src/pathops/SkOpEdgeBuilder.cpp @@ -31,7 +31,7 @@ static bool can_add_curve(SkPath::Verb verb, SkPoint* curve) { if (SkPath::kMove_Verb == verb) { return false; } - for (int index = 0; index < SkPathOpsVerbToPoints(verb); ++index) { + for (int index = 0; index <= SkPathOpsVerbToPoints(verb); ++index) { force_small_to_zero(&curve[index]); } return SkPath::kLine_Verb != verb || !SkDPoint::ApproximatelyEqual(curve[0], curve[1]); @@ -209,8 +209,8 @@ bool SkOpEdgeBuilder::walk() { SkPoint cStorage[2][2]; SkPath::Verb v1 = SkReduceOrder::Quad(&pair[0], cStorage[0]); SkPath::Verb v2 = SkReduceOrder::Quad(&pair[2], cStorage[1]); - SkPoint* curve1 = v1 == SkPath::kQuad_Verb ? &pair[0] : cStorage[0]; - SkPoint* curve2 = v2 == SkPath::kQuad_Verb ? &pair[2] : cStorage[1]; + SkPoint* curve1 = v1 != SkPath::kLine_Verb ? &pair[0] : cStorage[0]; + SkPoint* curve2 = v2 != SkPath::kLine_Verb ? &pair[2] : cStorage[1]; if (can_add_curve(v1, curve1) && can_add_curve(v2, curve2)) { fCurrentContour->addCurve(v1, curve1); fCurrentContour->addCurve(v2, curve2); @@ -235,8 +235,8 @@ bool SkOpEdgeBuilder::walk() { SkPoint cStorage[2][3]; SkPath::Verb v1 = SkReduceOrder::Conic(pair[0], cStorage[0]); SkPath::Verb v2 = SkReduceOrder::Conic(pair[1], cStorage[1]); - SkPoint* curve1 = v1 == SkPath::kConic_Verb ? pair[0].fPts : cStorage[0]; - SkPoint* curve2 = v2 == SkPath::kConic_Verb ? pair[1].fPts : cStorage[1]; + SkPoint* curve1 = v1 != SkPath::kLine_Verb ? pair[0].fPts : cStorage[0]; + SkPoint* curve2 = v2 != SkPath::kLine_Verb ? pair[1].fPts : cStorage[1]; if (can_add_curve(v1, curve1) && can_add_curve(v2, curve2)) { fCurrentContour->addCurve(v1, curve1, pair[0].fW); fCurrentContour->addCurve(v2, curve2, pair[1].fW); diff --git a/src/pathops/SkPathOpsCurve.cpp b/src/pathops/SkPathOpsCurve.cpp index e96c4e89dd..503c140aa6 100644 --- a/src/pathops/SkPathOpsCurve.cpp +++ b/src/pathops/SkPathOpsCurve.cpp @@ -54,7 +54,7 @@ double SkDCurve::nearPoint(SkPath::Verb verb, const SkDPoint& xy, const SkDPoint void SkDCurve::offset(SkPath::Verb verb, const SkDVector& off) { int count = SkPathOpsVerbToPoints(verb); - for (int index = 0; index < count; ++index) { + for (int index = 0; index <= count; ++index) { fCubic.fPts[index] += off; } } @@ -101,7 +101,7 @@ void SkDCurveSweep::setCurveHullSweep(SkPath::Verb verb) { // OPTIMIZE: I do the following float check a lot -- probably need a // central place for this val-is-small-compared-to-curve check double maxVal = 0; - for (int index = 0; index < SkPathOpsVerbToPoints(verb); ++index) { + for (int index = 0; index <= SkPathOpsVerbToPoints(verb); ++index) { maxVal = SkTMax(maxVal, SkTMax(SkTAbs(fCurve[index].fX), SkTAbs(fCurve[index].fY))); } diff --git a/tests/PathOpsOpTest.cpp b/tests/PathOpsOpTest.cpp index 9bb3a0fbb6..92ef105ce8 100644 --- a/tests/PathOpsOpTest.cpp +++ b/tests/PathOpsOpTest.cpp @@ -7990,12 +7990,10 @@ DEF_TEST(PathOpsFailOp, reporter) { } static struct TestDesc repTests[] = { - TEST(loops44i), - TEST(loops45i), - TEST(loops46i), + TEST(fuzz763_5a), }; DEF_TEST(PathOpsRepOp, reporter) { - for (int index = 0; index < 2; ++index) + for (int index = 0; index < 1; ++index) RunTestSet(reporter, repTests, SK_ARRAY_COUNT(repTests), nullptr, nullptr, nullptr, false); } |