Revert "make enum santizer fatal"
This reverts commit 166dbd3135e02d0e874591db02f9b1ad7fc961f5.
Reason for revert: illegal is not advanced, docs
Original change's description:
> 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>
TBR=mtklein@chromium.org,mtklein@google.com,brianosman@google.com
Change-Id: I691c08092340a6273e442c0f098b844f7d0363ba
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/168581
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
diff --git a/src/core/SkPathPriv.h b/src/core/SkPathPriv.h
index b6f6ff6..bcf85a1 100644
--- a/src/core/SkPathPriv.h
+++ b/src/core/SkPathPriv.h
@@ -18,7 +18,7 @@
static const int kPathRefGenIDBitCnt = 32;
#endif
- enum FirstDirection : int {
+ enum FirstDirection {
kCW_FirstDirection, // == SkPath::kCW_Direction
kCCW_FirstDirection, // == SkPath::kCCW_Direction
kUnknown_FirstDirection,
diff --git a/src/gpu/GrXferProcessor.cpp b/src/gpu/GrXferProcessor.cpp
index e40dfd5..b32f25f 100644
--- a/src/gpu/GrXferProcessor.cpp
+++ b/src/gpu/GrXferProcessor.cpp
@@ -99,9 +99,6 @@
return "hsl_color";
case kHSLLuminosity_GrBlendEquation:
return "hsl_luminosity";
- case kIllegal_GrBlendEquation:
- SkASSERT(false);
- return "<illegal>";
};
return "";
}
@@ -144,9 +141,6 @@
return "src2_alpha";
case kIS2A_GrBlendCoeff:
return "inv_src2_alpha";
- case kIllegal_GrBlendCoeff:
- SkASSERT(false);
- return "<illegal>";
}
return "";
}
diff --git a/src/gpu/effects/GrCustomXfermode.cpp b/src/gpu/effects/GrCustomXfermode.cpp
index 3d39c54..5c35260 100644
--- a/src/gpu/effects/GrCustomXfermode.cpp
+++ b/src/gpu/effects/GrCustomXfermode.cpp
@@ -46,10 +46,7 @@
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);
-
- // 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);
-
+ GR_STATIC_ASSERT(kGrBlendEquationCnt == (int)SkBlendMode::kLastMode + 1 + EQ_OFFSET);
return static_cast<GrBlendEquation>((int)mode + EQ_OFFSET);
#undef EQ_OFFSET
}
@@ -82,7 +79,7 @@
CustomXP(bool hasMixedSamples, SkBlendMode mode, GrProcessorAnalysisCoverage coverage)
: INHERITED(kCustomXP_ClassID, true, hasMixedSamples, coverage)
, fMode(mode)
- , fHWBlendEquation(kIllegal_GrBlendEquation) {
+ , fHWBlendEquation(static_cast<GrBlendEquation>(-1)) {
}
const char* name() const override { return "Custom Xfermode"; }
@@ -90,7 +87,7 @@
GrGLSLXferProcessor* createGLSLInstance() const override;
SkBlendMode mode() const { return fMode; }
- bool hasHWBlendEquation() const { return kIllegal_GrBlendEquation != fHWBlendEquation; }
+ bool hasHWBlendEquation() const { return -1 != static_cast<int>(fHWBlendEquation); }
GrBlendEquation hwBlendEquation() const {
SkASSERT(this->hasHWBlendEquation());
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 316d466..f9b6217 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -78,10 +78,7 @@
GR_GL_HSL_HUE,
GR_GL_HSL_SATURATION,
GR_GL_HSL_COLOR,
- GR_GL_HSL_LUMINOSITY,
-
- // Illegal... needs to map to something.
- GR_GL_FUNC_ADD,
+ GR_GL_HSL_LUMINOSITY
};
GR_STATIC_ASSERT(0 == kAdd_GrBlendEquation);
GR_STATIC_ASSERT(1 == kSubtract_GrBlendEquation);
@@ -124,9 +121,6 @@
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) {
@@ -151,9 +145,6 @@
false,
false,
false,
-
- // Illegal.
- false,
};
return gCoeffReferencesBlendConst[coeff];
GR_STATIC_ASSERT(kGrBlendCoeffCnt == SK_ARRAY_COUNT(gCoeffReferencesBlendConst));
diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h
index ded1ffb..270e28a 100644
--- a/src/gpu/gl/GrGLGpu.h
+++ b/src/gpu/gl/GrGLGpu.h
@@ -566,9 +566,9 @@
TriState fEnabled;
void invalidate() {
- fEquation = kIllegal_GrBlendEquation;
- fSrcCoeff = kIllegal_GrBlendCoeff;
- fDstCoeff = kIllegal_GrBlendCoeff;
+ fEquation = static_cast<GrBlendEquation>(-1);
+ fSrcCoeff = static_cast<GrBlendCoeff>(-1);
+ fDstCoeff = static_cast<GrBlendCoeff>(-1);
fConstColorValid = false;
fEnabled = kUnknown_TriState;
}
diff --git a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
index 35f0fba..788fd70 100644
--- a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
+++ b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
@@ -53,9 +53,8 @@
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 - 1);
+ kGrBlendEquationCnt - kFirstAdvancedGrBlendEquation);
}
uint8_t GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(GrSurfaceOrigin origin) {
diff --git a/src/gpu/mtl/GrMtlPipelineStateBuilder.mm b/src/gpu/mtl/GrMtlPipelineStateBuilder.mm
index 8e08073..eb65858 100644
--- a/src/gpu/mtl/GrMtlPipelineStateBuilder.mm
+++ b/src/gpu/mtl/GrMtlPipelineStateBuilder.mm
@@ -227,7 +227,6 @@
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);
@@ -264,7 +263,7 @@
GR_STATIC_ASSERT(1 == kSubtract_GrBlendEquation);
GR_STATIC_ASSERT(2 == kReverseSubtract_GrBlendEquation);
- SkASSERT((unsigned)equation < kGrBlendEquationCnt);
+ SkASSERT((unsigned)equation < kGrBlendCoeffCnt);
return gTable[equation];
}
diff --git a/src/gpu/vk/GrVkPipeline.cpp b/src/gpu/vk/GrVkPipeline.cpp
index 2675e7a..e0ede4c 100644
--- a/src/gpu/vk/GrVkPipeline.cpp
+++ b/src/gpu/vk/GrVkPipeline.cpp
@@ -320,7 +320,7 @@
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,10 +369,7 @@
VK_BLEND_OP_HSL_HUE_EXT,
VK_BLEND_OP_HSL_SATURATION_EXT,
VK_BLEND_OP_HSL_COLOR_EXT,
- VK_BLEND_OP_HSL_LUMINOSITY_EXT,
-
- // Illegal.
- VK_BLEND_OP_ADD,
+ VK_BLEND_OP_HSL_LUMINOSITY_EXT
};
GR_STATIC_ASSERT(0 == kAdd_GrBlendEquation);
GR_STATIC_ASSERT(1 == kSubtract_GrBlendEquation);
@@ -420,9 +417,6 @@
false,
false,
false,
-
- // Illegal
- false,
};
return gCoeffReferencesBlendConst[coeff];
GR_STATIC_ASSERT(kGrBlendCoeffCnt == SK_ARRAY_COUNT(gCoeffReferencesBlendConst));