make enum santizer fatal

This enum sanitizer checks that all the values of the enum we use fall
within the range of the enumerated values.

The main thing this helps point out is that the size of enum types in
C++ need only be large enough to hold the largest declared value; larger
values are undefined.  In practice, most enums are implemented as ints
for compatibility with C, so while this hasn't pointed out anything
egregiously broken, the sanitizer has found a couple possibly dangerous
situations in our codebase.

For most types using values outside the enum range, we can just
explicitly size them to int.  This makes their de facto size de jure.

But we need to actually make GrBlendEquation and GrBlendCoeff not store
values outside their enumerated range.  They're packed into bitfields
that really can't represent those (negative) values.  So for these I've
added new kIllegal values to the enums, forcing us to deal with our
once-silent illegal values a bit more explicitly.

Change-Id: Ib617694cf1aaa83ae99289e9e760f49cb6393a2f
Reviewed-on: https://skia-review.googlesource.com/c/168484
Reviewed-by: Brian Osman <brianosman@google.com>

gn/BUILD.gn

@@ -237,7 +237,7 @@ config("default") {
 
       # A whitelist of checks we can't yet pass.
       if (fyi_sanitize == "" && !is_android) {
-        fyi_sanitizers = "enum,float-divide-by-zero"
+        fyi_sanitizers = "float-divide-by-zero"
       }
 
       if (is_android) {

include/core/SkBlurTypes.h

@@ -10,7 +10,7 @@
 
 #include "SkTypes.h"
 
-enum SkBlurStyle {
+enum SkBlurStyle : int {
     kNormal_SkBlurStyle,  //!< fuzzy inside and outside
     kSolid_SkBlurStyle,   //!< solid inside, fuzzy outside
     kOuter_SkBlurStyle,   //!< nothing inside, fuzzy outside

include/core/SkPath.h

@@ -63,7 +63,7 @@ public:
         kCW_Direction travel clockwise; the same added with kCCW_Direction
         travel counterclockwise.
     */
-    enum Direction {
+    enum Direction : int {
         kCW_Direction,  //!< contour travels clockwise
         kCCW_Direction, //!< contour travels counterclockwise
     };

include/gpu/GrBlend.h

@@ -37,8 +37,10 @@ enum GrBlendEquation {
     kHSLColor_GrBlendEquation,
     kHSLLuminosity_GrBlendEquation,
 
+    kIllegal_GrBlendEquation,
+
     kFirstAdvancedGrBlendEquation = kScreen_GrBlendEquation,
-    kLast_GrBlendEquation = kHSLLuminosity_GrBlendEquation
+    kLast_GrBlendEquation = kIllegal_GrBlendEquation,
 };
 
 static const int kGrBlendEquationCnt = kLast_GrBlendEquation + 1;
@@ -67,7 +69,9 @@ enum GrBlendCoeff {
     kS2A_GrBlendCoeff,
     kIS2A_GrBlendCoeff,
 
-    kLast_GrBlendCoeff = kIS2A_GrBlendCoeff
+    kIllegal_GrBlendCoeff,
+
+    kLast_GrBlendCoeff = kIllegal_GrBlendCoeff,
 };
 
 static const int kGrBlendCoeffCnt = kLast_GrBlendCoeff + 1;

include/gpu/GrTypes.h

@@ -227,7 +227,7 @@ enum class GrMipMapped : bool {
  * GPU SkImage and SkSurfaces can be stored such that (0, 0) in texture space may correspond to
  * either the top-left or bottom-left content pixel.
  */
-enum GrSurfaceOrigin {
+enum GrSurfaceOrigin : int {
     kTopLeft_GrSurfaceOrigin,
     kBottomLeft_GrSurfaceOrigin,
 };

include/private/GrTypesPriv.h

@@ -384,7 +384,7 @@ GR_MAKE_BITFIELD_OPS(GrShaderFlags);
  * vary the internal precision based on the qualifiers. These currently only apply to float types (
  * including float vectors and matrices).
  */
-enum GrSLPrecision {
+enum GrSLPrecision : int {
     kLow_GrSLPrecision,
     kMedium_GrSLPrecision,
     kHigh_GrSLPrecision,

src/core/SkPathPriv.h

@@ -18,7 +18,7 @@ public:
     static const int kPathRefGenIDBitCnt = 32;
 #endif
 
-    enum FirstDirection {
+    enum FirstDirection : int {
         kCW_FirstDirection,         // == SkPath::kCW_Direction
         kCCW_FirstDirection,        // == SkPath::kCCW_Direction
         kUnknown_FirstDirection,

src/gpu/GrXferProcessor.cpp

@@ -99,6 +99,9 @@ static const char* equation_string(GrBlendEquation eq) {
             return "hsl_color";
         case kHSLLuminosity_GrBlendEquation:
             return "hsl_luminosity";
+        case kIllegal_GrBlendEquation:
+            SkASSERT(false);
+            return "<illegal>";
     };
     return "";
 }
@@ -141,6 +144,9 @@ static const char* coeff_string(GrBlendCoeff coeff) {
             return "src2_alpha";
         case kIS2A_GrBlendCoeff:
             return "inv_src2_alpha";
+        case kIllegal_GrBlendCoeff:
+            SkASSERT(false);
+            return "<illegal>";
     }
     return "";
 }

src/gpu/effects/GrCustomXfermode.cpp

@@ -46,7 +46,10 @@ static constexpr GrBlendEquation hw_blend_equation(SkBlendMode mode) {
     GR_STATIC_ASSERT(kHSLSaturation_GrBlendEquation == (int)SkBlendMode::kSaturation + EQ_OFFSET);
     GR_STATIC_ASSERT(kHSLColor_GrBlendEquation == (int)SkBlendMode::kColor + EQ_OFFSET);
     GR_STATIC_ASSERT(kHSLLuminosity_GrBlendEquation == (int)SkBlendMode::kLuminosity + EQ_OFFSET);
-    GR_STATIC_ASSERT(kGrBlendEquationCnt == (int)SkBlendMode::kLastMode + 1 + EQ_OFFSET);
+
+    // There's an illegal GrBlendEquation that corresponds to no SkBlendMode, hence the extra +1.
+    GR_STATIC_ASSERT(kGrBlendEquationCnt == (int)SkBlendMode::kLastMode + 1 + 1 + EQ_OFFSET);
+
     return static_cast<GrBlendEquation>((int)mode + EQ_OFFSET);
 #undef EQ_OFFSET
 }
@@ -79,7 +82,7 @@ public:
     CustomXP(bool hasMixedSamples, SkBlendMode mode, GrProcessorAnalysisCoverage coverage)
             : INHERITED(kCustomXP_ClassID, true, hasMixedSamples, coverage)
             , fMode(mode)
-            , fHWBlendEquation(static_cast<GrBlendEquation>(-1)) {
+            , fHWBlendEquation(kIllegal_GrBlendEquation) {
     }
 
     const char* name() const override { return "Custom Xfermode"; }
@@ -87,7 +90,7 @@ public:
     GrGLSLXferProcessor* createGLSLInstance() const override;
 
     SkBlendMode mode() const { return fMode; }
-    bool hasHWBlendEquation() const { return -1 != static_cast<int>(fHWBlendEquation); }
+    bool hasHWBlendEquation() const { return kIllegal_GrBlendEquation != fHWBlendEquation; }
 
     GrBlendEquation hwBlendEquation() const {
         SkASSERT(this->hasHWBlendEquation());

src/gpu/gl/GrGLGpu.cpp

@@ -78,7 +78,10 @@ static const GrGLenum gXfermodeEquation2Blend[] = {
     GR_GL_HSL_HUE,
     GR_GL_HSL_SATURATION,
     GR_GL_HSL_COLOR,
-    GR_GL_HSL_LUMINOSITY
+    GR_GL_HSL_LUMINOSITY,
+
+    // Illegal... needs to map to something.
+    GR_GL_FUNC_ADD,
 };
 GR_STATIC_ASSERT(0 == kAdd_GrBlendEquation);
 GR_STATIC_ASSERT(1 == kSubtract_GrBlendEquation);
@@ -121,6 +124,9 @@ static const GrGLenum gXfermodeCoeff2Blend[] = {
     GR_GL_ONE_MINUS_SRC1_COLOR,
     GR_GL_SRC1_ALPHA,
     GR_GL_ONE_MINUS_SRC1_ALPHA,
+
+    // Illegal... needs to map to something.
+    GR_GL_ZERO,
 };
 
 bool GrGLGpu::BlendCoeffReferencesConstant(GrBlendCoeff coeff) {
@@ -145,6 +151,9 @@ bool GrGLGpu::BlendCoeffReferencesConstant(GrBlendCoeff coeff) {
         false,
         false,
         false,
+
+        // Illegal.
+        false,
     };
     return gCoeffReferencesBlendConst[coeff];
     GR_STATIC_ASSERT(kGrBlendCoeffCnt == SK_ARRAY_COUNT(gCoeffReferencesBlendConst));

src/gpu/gl/GrGLGpu.h

@@ -566,9 +566,9 @@ private:
         TriState        fEnabled;
 
         void invalidate() {
-            fEquation = static_cast<GrBlendEquation>(-1);
+            fEquation = kIllegal_GrBlendEquation;
-            fSrcCoeff = static_cast<GrBlendCoeff>(-1);
+            fSrcCoeff = kIllegal_GrBlendCoeff;
-            fDstCoeff = static_cast<GrBlendCoeff>(-1);
+            fDstCoeff = kIllegal_GrBlendCoeff;
             fConstColorValid = false;
             fEnabled = kUnknown_TriState;
         }

src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp

@@ -53,8 +53,9 @@ static const char* specific_layout_qualifier_name(GrBlendEquation equation) {
     GR_STATIC_ASSERT(12 == kHSLSaturation_GrBlendEquation - kFirstAdvancedGrBlendEquation);
     GR_STATIC_ASSERT(13 == kHSLColor_GrBlendEquation - kFirstAdvancedGrBlendEquation);
     GR_STATIC_ASSERT(14 == kHSLLuminosity_GrBlendEquation - kFirstAdvancedGrBlendEquation);
+    // There's an illegal GrBlendEquation at the end there, hence the -1.
     GR_STATIC_ASSERT(SK_ARRAY_COUNT(kLayoutQualifierNames) ==
-                     kGrBlendEquationCnt - kFirstAdvancedGrBlendEquation);
+                     kGrBlendEquationCnt - kFirstAdvancedGrBlendEquation - 1);
 }
 
 uint8_t GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(GrSurfaceOrigin origin) {

src/gpu/mtl/GrMtlPipelineStateBuilder.mm

@@ -227,6 +227,7 @@ static MTLBlendFactor blend_coeff_to_mtl_blend(GrBlendCoeff coeff) {
         MTLBlendFactorOneMinusSource1Color,      // kIS2C_GrBlendCoeff
         MTLBlendFactorSource1Alpha,              // kS2A_GrBlendCoeff
         MTLBlendFactorOneMinusSource1Alpha,      // kIS2A_GrBlendCoeff
+        MTLBlendFactorZero,                      // kIllegal_GrBlendCoeff
     };
     GR_STATIC_ASSERT(SK_ARRAY_COUNT(gTable) == kGrBlendCoeffCnt);
     GR_STATIC_ASSERT(0 == kZero_GrBlendCoeff);
@@ -263,7 +264,7 @@ static MTLBlendOperation blend_equation_to_mtl_blend_op(GrBlendEquation equation
     GR_STATIC_ASSERT(1 == kSubtract_GrBlendEquation);
     GR_STATIC_ASSERT(2 == kReverseSubtract_GrBlendEquation);
 
-    SkASSERT((unsigned)equation < kGrBlendCoeffCnt);
+    SkASSERT((unsigned)equation < kGrBlendEquationCnt);
     return gTable[equation];
 }
 

src/gpu/vk/GrVkPipeline.cpp

@@ -320,7 +320,7 @@ static VkBlendFactor blend_coeff_to_vk_blend(GrBlendCoeff coeff) {
         VK_BLEND_FACTOR_ONE_MINUS_SRC1_COLOR,      // kIS2C_GrBlendCoeff
         VK_BLEND_FACTOR_SRC1_ALPHA,                // kS2A_GrBlendCoeff
         VK_BLEND_FACTOR_ONE_MINUS_SRC1_ALPHA,      // kIS2A_GrBlendCoeff
-
+        VK_BLEND_FACTOR_ZERO,                      // kIllegal_GrBlendCoeff
     };
     GR_STATIC_ASSERT(SK_ARRAY_COUNT(gTable) == kGrBlendCoeffCnt);
     GR_STATIC_ASSERT(0 == kZero_GrBlendCoeff);
@@ -369,7 +369,10 @@ static VkBlendOp blend_equation_to_vk_blend_op(GrBlendEquation equation) {
         VK_BLEND_OP_HSL_HUE_EXT,
         VK_BLEND_OP_HSL_SATURATION_EXT,
         VK_BLEND_OP_HSL_COLOR_EXT,
-        VK_BLEND_OP_HSL_LUMINOSITY_EXT
+        VK_BLEND_OP_HSL_LUMINOSITY_EXT,
+
+        // Illegal.
+        VK_BLEND_OP_ADD,
     };
     GR_STATIC_ASSERT(0 == kAdd_GrBlendEquation);
     GR_STATIC_ASSERT(1 == kSubtract_GrBlendEquation);
@@ -417,6 +420,9 @@ static bool blend_coeff_refs_constant(GrBlendCoeff coeff) {
         false,
         false,
         false,
+
+        // Illegal
+        false,
     };
     return gCoeffReferencesBlendConst[coeff];
     GR_STATIC_ASSERT(kGrBlendCoeffCnt == SK_ARRAY_COUNT(gCoeffReferencesBlendConst));