Add validation of RRects to SkValidatingReadBuffer
This comes from the Skia fuzzer where it is inverting the RRect's rect which causes trouble down the line.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2591
Change-Id: I5c34105a47369492d2df99d39a4e29116060ad37
Reviewed-on: https://skia-review.googlesource.com/2591
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
diff --git a/include/core/SkRRect.h b/include/core/SkRRect.h
index 64db2f3..3b691aa 100644
--- a/include/core/SkRRect.h
+++ b/include/core/SkRRect.h
@@ -16,7 +16,6 @@
// Path forward:
// core work
-// add validate method (all radii positive, all radii sums < rect size, etc.)
// add contains(SkRect&) - for clip stack
// add contains(SkRRect&) - for clip stack
// add heart rect computation (max rect inside RR)
@@ -90,7 +89,7 @@
* Returns the RR's sub type.
*/
Type getType() const {
- SkDEBUGCODE(this->validate();)
+ SkASSERT(this->isValid());
return static_cast<Type>(fType);
}
@@ -124,7 +123,7 @@
memset(fRadii, 0, sizeof(fRadii));
fType = kEmpty_Type;
- SkDEBUGCODE(this->validate();)
+ SkASSERT(this->isValid());
}
/**
@@ -142,7 +141,7 @@
memset(fRadii, 0, sizeof(fRadii));
fType = kRect_Type;
- SkDEBUGCODE(this->validate();)
+ SkASSERT(this->isValid());
}
static SkRRect MakeEmpty() {
@@ -190,7 +189,7 @@
}
fType = kOval_Type;
- SkDEBUGCODE(this->validate();)
+ SkASSERT(this->isValid());
}
/**
@@ -288,7 +287,7 @@
*/
bool contains(const SkRect& rect) const;
- SkDEBUGCODE(void validate() const;)
+ bool isValid() const;
enum {
kSizeInMemory = 12 * sizeof(SkScalar)
diff --git a/src/core/SkRRect.cpp b/src/core/SkRRect.cpp
index 2195dcf..1188989 100644
--- a/src/core/SkRRect.cpp
+++ b/src/core/SkRRect.cpp
@@ -46,7 +46,7 @@
// TODO: assert that all the x&y radii are already W/2 & H/2
}
- SkDEBUGCODE(this->validate();)
+ SkASSERT(this->isValid());
}
void SkRRect::setNinePatch(const SkRect& rect, SkScalar leftRad, SkScalar topRad,
@@ -108,7 +108,7 @@
fRadii[kLowerRight_Corner].set(rightRad, bottomRad);
fRadii[kLowerLeft_Corner].set(leftRad, bottomRad);
- SkDEBUGCODE(this->validate();)
+ SkASSERT(this->isValid());
}
// These parameters intentionally double. Apropos crbug.com/463920, if one of the
@@ -193,7 +193,7 @@
// At this point we're either oval, simple, or complex (not empty or rect).
this->computeType();
- SkDEBUGCODE(this->validate();)
+ SkASSERT(this->isValid());
}
// This method determines if a point known to be inside the RRect's bounds is
@@ -290,7 +290,7 @@
void SkRRect::computeType() {
struct Validator {
Validator(const SkRRect* r) : fR(r) {}
- ~Validator() { SkDEBUGCODE(fR->validate();) }
+ ~Validator() { SkASSERT(fR->isValid()); }
const SkRRect* fR;
} autoValidate(this);
@@ -386,7 +386,7 @@
dst->fRadii[i].fX = SkScalarHalf(newRect.width());
dst->fRadii[i].fY = SkScalarHalf(newRect.height());
}
- SkDEBUGCODE(dst->validate();)
+ SkASSERT(dst->isValid());
return true;
}
@@ -503,18 +503,14 @@
///////////////////////////////////////////////////////////////////////////////
-#ifdef SK_DEBUG
/**
* We need all combinations of predicates to be true to have a "safe" radius value.
*/
-static void validate_radius_check_predicates(SkScalar rad, SkScalar min, SkScalar max) {
- SkASSERT(min <= max);
- SkASSERT(rad <= max - min);
- SkASSERT(min + rad <= max);
- SkASSERT(max - rad >= min);
+static bool are_radius_check_predicates_valid(SkScalar rad, SkScalar min, SkScalar max) {
+ return (min <= max) && (rad <= max - min) && (min + rad <= max) && (max - rad >= min);
}
-void SkRRect::validate() const {
+bool SkRRect::isValid() const {
bool allRadiiZero = (0 == fRadii[0].fX && 0 == fRadii[0].fY);
bool allCornersSquare = (0 == fRadii[0].fX || 0 == fRadii[0].fY);
bool allRadiiSame = true;
@@ -536,43 +532,54 @@
switch (fType) {
case kEmpty_Type:
- SkASSERT(fRect.isEmpty());
- SkASSERT(allRadiiZero && allRadiiSame && allCornersSquare);
+ if (!fRect.isEmpty() || !allRadiiZero || !allRadiiSame || !allCornersSquare) {
+ return false;
+ }
break;
case kRect_Type:
- SkASSERT(!fRect.isEmpty());
- SkASSERT(allRadiiZero && allRadiiSame && allCornersSquare);
+ if (fRect.isEmpty() || !allRadiiZero || !allRadiiSame || !allCornersSquare) {
+ return false;
+ }
break;
case kOval_Type:
- SkASSERT(!fRect.isEmpty());
- SkASSERT(!allRadiiZero && allRadiiSame && !allCornersSquare);
+ if (fRect.isEmpty() || allRadiiZero || !allRadiiSame || allCornersSquare) {
+ return false;
+ }
for (int i = 0; i < 4; ++i) {
- SkASSERT(SkScalarNearlyEqual(fRadii[i].fX, SkScalarHalf(fRect.width())));
- SkASSERT(SkScalarNearlyEqual(fRadii[i].fY, SkScalarHalf(fRect.height())));
+ if (!SkScalarNearlyEqual(fRadii[i].fX, SkScalarHalf(fRect.width())) ||
+ !SkScalarNearlyEqual(fRadii[i].fY, SkScalarHalf(fRect.height()))) {
+ return false;
+ }
}
break;
case kSimple_Type:
- SkASSERT(!fRect.isEmpty());
- SkASSERT(!allRadiiZero && allRadiiSame && !allCornersSquare);
+ if (fRect.isEmpty() || allRadiiZero || !allRadiiSame || allCornersSquare) {
+ return false;
+ }
break;
case kNinePatch_Type:
- SkASSERT(!fRect.isEmpty());
- SkASSERT(!allRadiiZero && !allRadiiSame && !allCornersSquare);
- SkASSERT(patchesOfNine);
+ if (fRect.isEmpty() || allRadiiZero || allRadiiSame || allCornersSquare ||
+ !patchesOfNine) {
+ return false;
+ }
break;
case kComplex_Type:
- SkASSERT(!fRect.isEmpty());
- SkASSERT(!allRadiiZero && !allRadiiSame && !allCornersSquare);
- SkASSERT(!patchesOfNine);
+ if (fRect.isEmpty() || allRadiiZero || allRadiiSame || allCornersSquare ||
+ patchesOfNine) {
+ return false;
+ }
break;
}
for (int i = 0; i < 4; ++i) {
- validate_radius_check_predicates(fRadii[i].fX, fRect.fLeft, fRect.fRight);
- validate_radius_check_predicates(fRadii[i].fY, fRect.fTop, fRect.fBottom);
+ if (!are_radius_check_predicates_valid(fRadii[i].fX, fRect.fLeft, fRect.fRight) ||
+ !are_radius_check_predicates_valid(fRadii[i].fY, fRect.fTop, fRect.fBottom)) {
+ return false;
+ }
}
+
+ return true;
}
-#endif // SK_DEBUG
///////////////////////////////////////////////////////////////////////////////
diff --git a/src/core/SkValidatingReadBuffer.cpp b/src/core/SkValidatingReadBuffer.cpp
index 326d0cf..1566af9 100644
--- a/src/core/SkValidatingReadBuffer.cpp
+++ b/src/core/SkValidatingReadBuffer.cpp
@@ -144,6 +144,11 @@
const void* ptr = this->skip(sizeof(SkRRect));
if (!fError) {
memcpy(rrect, ptr, sizeof(SkRRect));
+ this->validate(rrect->isValid());
+ }
+
+ if (fError) {
+ rrect->setEmpty();
}
}