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
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);
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)));
}
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);
}