Watch out for SkFixed overflow in SkMipMap.cpp.
authormtklein <mtklein@chromium.org>
Fri, 21 Nov 2014 19:38:53 +0000 (11:38 -0800)
committerCommit bot <commit-bot@chromium.org>
Fri, 21 Nov 2014 19:38:53 +0000 (11:38 -0800)
Tested with -fsanitize=signed-integer-overflow.
This new assert used to trigger in MipMap unit test.

Don't appear to be any GM diffs.

BUG=skia:

Review URL: https://codereview.chromium.org/729373004

include/core/SkTypes.h
src/core/SkMipMap.cpp

index 0e9e2303493f40fa03e386d5a189cd50be4e6e51..a38be84d8fbc38325ae217fc265a2ccc5dfb0b23 100644 (file)
@@ -352,6 +352,7 @@ template <typename T> inline void SkTSwap(T& a, T& b) {
 }
 
 static inline int32_t SkAbs32(int32_t value) {
+    SkASSERT(value != SK_NaN32);  // The most negative int32_t can't be negated.
     if (value < 0) {
         value = -value;
     }
index fdfb660ccc66a2aa06deca8a052350b30ead8aaa..83164b15267e0169754ddfd5d7848093ba078e6b 100644 (file)
@@ -228,7 +228,11 @@ SkMipMap* SkMipMap::Build(const SkBitmap& src, SkDiscardableFactoryProc fact) {
 //static int gCounter;
 
 static SkFixed compute_level(SkScalar scale) {
-    SkFixed s = SkAbs32(SkScalarToFixed(SkScalarInvert(scale)));
+    SkScalar inv = SkScalarAbs(SkScalarInvert(scale));
+    if (inv > 32767) {  // Watch out for SkFixed overflow.
+        inv = 32767;
+    }
+    SkFixed s = SkScalarToFixed(inv);
 
     if (s < SK_Fixed1) {
         return 0;