commit cc09372730301be78b9d26c1198db1584622cdd9
author caryclark <caryclark@google.com> Fri Sep 23 09:32:26 2016 -0700
committer Commit bot <commit-bot@chromium.org> Fri Sep 23 09:32:27 2016 -0700
tree 99be1f7a5c3cfefbb75b47f03ddf5d377a5c4fd8
parent 94663795410c214db5232162076b8aea7e5f62d2

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

src/pathops/SkOpEdgeBuilder.cpp

diff --git a/src/pathops/SkOpEdgeBuilder.cpp b/src/pathops/SkOpEdgeBuilder.cpp
index d67ed44..c304357 100644
--- a/src/pathops/SkOpEdgeBuilder.cpp
+++ b/src/pathops/SkOpEdgeBuilder.cpp
@@ -31,7 +31,7 @@
     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 @@
                         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 @@
                         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);

src/pathops/SkPathOpsCurve.cpp

diff --git a/src/pathops/SkPathOpsCurve.cpp b/src/pathops/SkPathOpsCurve.cpp
index e96c4e8..503c140 100644
--- a/src/pathops/SkPathOpsCurve.cpp
+++ b/src/pathops/SkPathOpsCurve.cpp
@@ -54,7 +54,7 @@
 
 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 @@
     // 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)));
     }

tests/PathOpsOpTest.cpp

diff --git a/tests/PathOpsOpTest.cpp b/tests/PathOpsOpTest.cpp
index 9bb3a0f..92ef105 100644
--- a/tests/PathOpsOpTest.cpp
+++ b/tests/PathOpsOpTest.cpp
@@ -7990,12 +7990,10 @@
 }
 
 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);
 }