ubsan shift fixes

Use an inline function that does a normal shift. When built for the sanitizer, add casts so that the shift is unsigned.

Also make a few fixes to do unsigned shifts or avoid the shift altogether; and add an argument spec to some macros.

R=reed@google.com,mtklein@google.com
BUG=skia:4633

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

19 files changed, 46 insertions, 38 deletions

diff --git a/include/core/SkFixed.h b/include/core/SkFixed.h
index fefa718d0f..3140b7a35d 100644
--- a/include/core/SkFixed.h
+++ b/include/core/SkFixed.h
@@ -88,7 +88,7 @@ typedef int32_t             SkFixed;
 #else
     // TODO(reed): this clamp shouldn't be needed.  Use SkToS32().
     #define SkFixedDiv(numer, denom) \
-        SkTPin<int32_t>(((int64_t)numer << 16) / denom, SK_MinS32, SK_MaxS32)
+        SkTPin<int32_t>((int32_t)(SkLeftShift((int64_t)numer, 16) / denom), SK_MinS32, SK_MaxS32)
 #endif
 
 //////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -145,9 +145,9 @@ inline SkFixed SkFixedMul_longlong(SkFixed a, SkFixed b) {
 
 typedef int64_t SkFixed3232;   // 32.32
 
-#define SkIntToFixed3232(x)       ((SkFixed3232)(x) << 32)
+#define SkIntToFixed3232(x)       (SkLeftShift((SkFixed3232)(x), 32))
 #define SkFixed3232ToInt(x)       ((int)((x) >> 32))
-#define SkFixedToFixed3232(x)     ((SkFixed3232)(x) << 16)
+#define SkFixedToFixed3232(x)     (SkLeftShift((SkFixed3232)(x), 16))
 #define SkFixed3232ToFixed(x)     ((SkFixed)((x) >> 16))
 #define SkFloatToFixed3232(x)     ((SkFixed3232)((x) * (65536.0f * 65536.0f)))
 
diff --git a/include/core/SkTypes.h b/include/core/SkTypes.h
index e4569f21b1..261fcaea42 100644
--- a/include/core/SkTypes.h
+++ b/include/core/SkTypes.h
@@ -306,6 +306,14 @@ static inline bool SkIsU16(long x) {
     return (uint16_t)x == x;
 }
 
+static inline int32_t SkLeftShift(int32_t value, int32_t shift) {
+    return (int32_t) ((uint32_t) value << shift);
+}
+
+static inline int64_t SkLeftShift(int64_t value, int32_t shift) {
+    return (int64_t) ((uint64_t) value << shift);
+}
+
 //////////////////////////////////////////////////////////////////////////////
 
 /** Returns the number of entries in an array (not a pointer) */
diff --git a/src/core/SkBitmapProcState_matrixProcs.cpp b/src/core/SkBitmapProcState_matrixProcs.cpp
index ee61ee8556..1c4b7b6bcf 100644
--- a/src/core/SkBitmapProcState_matrixProcs.cpp
+++ b/src/core/SkBitmapProcState_matrixProcs.cpp
@@ -178,7 +178,7 @@ static inline U16CPU fixed_repeat(SkFixed x) {
 #endif
 
 static inline U16CPU fixed_mirror(SkFixed x) {
-    SkFixed s = x << 15 >> 31;
+    SkFixed s = SkLeftShift(x, 15) >> 31;
     // s is FFFFFFFF if we're on an odd interval, or 0 if an even interval
     return (x ^ s) & 0xFFFF;
 }
diff --git a/src/core/SkEdge.cpp b/src/core/SkEdge.cpp
index f91f5f8782..c64896f2e0 100644
--- a/src/core/SkEdge.cpp
+++ b/src/core/SkEdge.cpp
@@ -26,7 +26,7 @@
 static inline SkFixed SkFDot6ToFixedDiv2(SkFDot6 value) {
     // we want to return SkFDot6ToFixed(value >> 1), but we don't want to throw
     // away data in value, so just perform a modify up-shift
-    return value << (16 - 6 - 1);
+    return SkLeftShift(value, 16 - 6 - 1);
 }
 
 /////////////////////////////////////////////////////////////////////////
@@ -214,8 +214,8 @@ int SkQuadraticEdge::setQuadratic(const SkPoint pts[3], int shift)
 
     // compute number of steps needed (1 << shift)
     {
-        SkFDot6 dx = ((x1 << 1) - x0 - x2) >> 2;
-        SkFDot6 dy = ((y1 << 1) - y0 - y2) >> 2;
+        SkFDot6 dx = (SkLeftShift(x1, 1) - x0 - x2) >> 2;
+        SkFDot6 dy = (SkLeftShift(y1, 1) - y0 - y2) >> 2;
         shift = diff_to_shift(dx, dy);
         SkASSERT(shift >= 0);
     }
@@ -312,8 +312,8 @@ int SkQuadraticEdge::updateQuadratic()
 /////////////////////////////////////////////////////////////////////////
 
 static inline int SkFDot6UpShift(SkFDot6 x, int upShift) {
-    SkASSERT((x << upShift >> upShift) == x);
-    return x << upShift;
+    SkASSERT((SkLeftShift(x, upShift) >> upShift) == x);
+    return SkLeftShift(x, upShift);
 }
 
 /*  f(1/3) = (8a + 12b + 6c + d) / 27
@@ -403,7 +403,7 @@ int SkCubicEdge::setCubic(const SkPoint pts[4], int shift) {
     }
 
     fWinding    = SkToS8(winding);
-    fCurveCount = SkToS8(-1 << shift);
+    fCurveCount = SkToS8(SkLeftShift(-1, shift));
     fCurveShift = SkToU8(shift);
     fCubicDShift = SkToU8(downShift);
 
diff --git a/src/core/SkEdge.h b/src/core/SkEdge.h
index db6f43085d..c3adbf85cb 100644
--- a/src/core/SkEdge.h
+++ b/src/core/SkEdge.h
@@ -15,7 +15,7 @@
 #include "SkMath.h"
 
 // This correctly favors the lower-pixel when y0 is on a 1/2 pixel boundary
-#define SkEdge_Compute_DY(top, y0)  ((top << 6) + 32 - (y0))
+#define SkEdge_Compute_DY(top, y0)  (SkLeftShift(top, 6) + 32 - (y0))
 
 struct SkEdge {
     enum Type {
diff --git a/src/core/SkFDot6.h b/src/core/SkFDot6.h
index 3da753da41..b536729829 100644
--- a/src/core/SkFDot6.h
+++ b/src/core/SkFDot6.h
@@ -56,9 +56,9 @@ inline SkFDot6 SkScalarRoundToFDot6(SkScalar x, int shift = 0)
 #define SkFixedToFDot6(x)   ((x) >> 10)
 
 inline SkFixed SkFDot6ToFixed(SkFDot6 x) {
-    SkASSERT((x << 10 >> 10) == x);
+    SkASSERT((SkLeftShift(x, 10) >> 10) == x);
 
-    return x << 10;
+    return SkLeftShift(x, 10);
 }
 
 #define SkScalarToFDot6(x)  (SkFDot6)((x) * 64)
@@ -68,7 +68,7 @@ inline SkFixed SkFDot6Div(SkFDot6 a, SkFDot6 b) {
     SkASSERT(b != 0);
 
     if (a == (int16_t)a) {
-        return (a << 16) / b;
+        return SkLeftShift(a, 16) / b;
     } else {
         return SkFixedDiv(a, b);
     }
diff --git a/src/core/SkFloatBits.cpp b/src/core/SkFloatBits.cpp
index 919fd0610f..ea705513ac 100644
--- a/src/core/SkFloatBits.cpp
+++ b/src/core/SkFloatBits.cpp
@@ -65,7 +65,7 @@ int32_t SkFloatBits_toIntCast(int32_t packed) {
 // same as (int)floor(float)
 int32_t SkFloatBits_toIntFloor(int32_t packed) {
     // curse you negative 0
-    if ((packed << 1) == 0) {
+    if (SkLeftShift(packed, 1) == 0) {
         return 0;
     }
 
@@ -104,7 +104,7 @@ int32_t SkFloatBits_toIntFloor(int32_t packed) {
 // same as (int)floor(float + 0.5)
 int32_t SkFloatBits_toIntRound(int32_t packed) {
     // curse you negative 0
-    if ((packed << 1) == 0) {
+    if (SkLeftShift(packed, 1) == 0) {
         return 0;
     }
 
@@ -134,7 +134,7 @@ int32_t SkFloatBits_toIntRound(int32_t packed) {
 // same as (int)ceil(float)
 int32_t SkFloatBits_toIntCeil(int32_t packed) {
     // curse you negative 0
-    if ((packed << 1) == 0) {
+    if (SkLeftShift(packed, 1) == 0) {
         return 0;
     }
 
@@ -200,6 +200,6 @@ float SkIntToFloatCast(int32_t value) {
     SkASSERT(shift >= 0 && shift <= 255);
 
     SkFloatIntUnion data;
-    data.fSignBitInt = (sign << 31) | (shift << 23) | (value & ~MATISSA_MAGIC_BIG);
+    data.fSignBitInt = SkLeftShift(sign, 31) | SkLeftShift(shift, 23) | (value & ~MATISSA_MAGIC_BIG);
     return data.fFloat;
 }
diff --git a/src/core/SkScan_AntiPath.cpp b/src/core/SkScan_AntiPath.cpp
index 6ea6b8b07e..1c222b9c80 100644
--- a/src/core/SkScan_AntiPath.cpp
+++ b/src/core/SkScan_AntiPath.cpp
@@ -545,7 +545,7 @@ void MaskSuperBlitter::blitH(int x, int y, int width) {
     }
 #endif
 
-    x -= (fMask.fBounds.fLeft << SHIFT);
+    x -= SkLeftShift(fMask.fBounds.fLeft, SHIFT);
 
     // hack, until I figure out why my cubics (I think) go beyond the bounds
     if (x < 0) {
@@ -592,7 +592,7 @@ static bool fitsInsideLimit(const SkRect& r, SkScalar max) {
 
 static int overflows_short_shift(int value, int shift) {
     const int s = 16 + shift;
-    return (value << s >> s) - value;
+    return (SkLeftShift(value, s) >> s) - value;
 }
 
 /**
diff --git a/src/core/SkScan_Antihair.cpp b/src/core/SkScan_Antihair.cpp
index b14e12bab8..8ee0ba5f98 100644
--- a/src/core/SkScan_Antihair.cpp
+++ b/src/core/SkScan_Antihair.cpp
@@ -250,9 +250,9 @@ public:
 };
 
 static inline SkFixed fastfixdiv(SkFDot6 a, SkFDot6 b) {
-    SkASSERT((a << 16 >> 16) == a);
+    SkASSERT((SkLeftShift(a, 16) >> 16) == a);
     SkASSERT(b != 0);
-    return (a << 16) / b;
+    return SkLeftShift(a, 16) / b;
 }
 
 #define SkBITCOUNT(x)   (sizeof(x) << 3)
diff --git a/src/core/SkScan_Path.cpp b/src/core/SkScan_Path.cpp
index fc79fc8537..30a7c57659 100644
--- a/src/core/SkScan_Path.cpp
+++ b/src/core/SkScan_Path.cpp
@@ -450,8 +450,8 @@ void sk_fill_path(const SkPath& path, const SkIRect* clipRect, SkBlitter* blitte
 
     // now edge is the head of the sorted linklist
 
-    start_y <<= shiftEdgesUp;
-    stop_y <<= shiftEdgesUp;
+    start_y = SkLeftShift(start_y, shiftEdgesUp);
+    stop_y = SkLeftShift(stop_y, shiftEdgesUp);
     if (clipRect && start_y < clipRect->fTop) {
         start_y = clipRect->fTop;
     }
diff --git a/src/core/SkUtils.cpp b/src/core/SkUtils.cpp
index b3f698b4e7..f706cb9f25 100644
--- a/src/core/SkUtils.cpp
+++ b/src/core/SkUtils.cpp
@@ -74,11 +74,11 @@ SkUnichar SkUTF8_ToUnichar(const char utf8[]) {
 
     if (hic < 0) {
         uint32_t mask = (uint32_t)~0x3F;
-        hic <<= 1;
+        hic = SkLeftShift(hic, 1);
         do {
             c = (c << 6) | (*++p & 0x3F);
             mask <<= 5;
-        } while ((hic <<= 1) < 0);
+        } while ((hic = SkLeftShift(hic, 1)) < 0);
         c &= ~mask;
     }
     return c;
@@ -95,11 +95,11 @@ SkUnichar SkUTF8_NextUnichar(const char** ptr) {
 
     if (hic < 0) {
         uint32_t mask = (uint32_t)~0x3F;
-        hic <<= 1;
+        hic = SkLeftShift(hic, 1);
         do {
             c = (c << 6) | (*++p & 0x3F);
             mask <<= 5;
-        } while ((hic <<= 1) < 0);
+        } while ((hic = SkLeftShift(hic, 1)) < 0);
         c &= ~mask;
     }
     *ptr = (char*)p + 1;
diff --git a/src/effects/gradients/SkClampRange.h b/src/effects/gradients/SkClampRange.h
index 945f9a7ff1..d3d2d08c86 100644
--- a/src/effects/gradients/SkClampRange.h
+++ b/src/effects/gradients/SkClampRange.h
@@ -12,8 +12,8 @@
 #include "SkScalar.h"
 
 #define SkGradFixed             SkFixed3232
-#define SkScalarToGradFixed     SkScalarToFixed3232
-#define SkFixedToGradFixed      SkFixedToFixed3232
+#define SkScalarToGradFixed(x)  SkScalarToFixed3232(x)
+#define SkFixedToGradFixed(x)   SkFixedToFixed3232(x)
 #define SkGradFixedToFixed(x)   (SkFixed)((x) >> 16)
 #define kFracMax_SkGradFixed    0xFFFFFFFFLL
 
diff --git a/src/effects/gradients/SkGradientShaderPriv.h b/src/effects/gradients/SkGradientShaderPriv.h
index 203f791ceb..f8a968d1a2 100644
--- a/src/effects/gradients/SkGradientShaderPriv.h
+++ b/src/effects/gradients/SkGradientShaderPriv.h
@@ -58,7 +58,7 @@ static inline SkFixed repeat_tileproc(SkFixed x) {
 #endif
 
 static inline SkFixed mirror_tileproc(SkFixed x) {
-    int s = x << 15 >> 31;
+    int s = SkLeftShift(x, 15) >> 31;
     return (x ^ s) & 0xFFFF;
 }
 
diff --git a/src/images/SkImageDecoder_libico.cpp b/src/images/SkImageDecoder_libico.cpp
index e8b4bc5c31..cb21b6906e 100644
--- a/src/images/SkImageDecoder_libico.cpp
+++ b/src/images/SkImageDecoder_libico.cpp
@@ -32,8 +32,8 @@ private:
 //read in Intel order, and return an integer
 
 #define readByte(buffer,begin) buffer[begin]
-#define read2Bytes(buffer,begin) buffer[begin]+(buffer[begin+1]<<8)
-#define read4Bytes(buffer,begin) buffer[begin]+(buffer[begin+1]<<8)+(buffer[begin+2]<<16)+(buffer[begin+3]<<24)
+#define read2Bytes(buffer,begin) buffer[begin]+SkLeftShift(buffer[begin+1],8)
+#define read4Bytes(buffer,begin) buffer[begin]+SkLeftShift(buffer[begin+1],8)+SkLeftShift(buffer[begin+2],16)+SkLeftShift(buffer[begin+3],24)
 
 /////////////////////////////////////////////////////////////////////////////////////////
 
diff --git a/src/pathops/SkOpAngle.cpp b/src/pathops/SkOpAngle.cpp
index 655df45b15..c2eb0c9326 100644
--- a/src/pathops/SkOpAngle.cpp
+++ b/src/pathops/SkOpAngle.cpp
@@ -1016,7 +1016,7 @@ deferTilLater:
     if (!crossesZero) {
         fSectorMask = (unsigned) -1 >> (31 - end + start) << start;
     } else {
-        fSectorMask = (unsigned) -1 >> (31 - start) | (-1 << end);
+        fSectorMask = (unsigned) -1 >> (31 - start) | ((unsigned) -1 << end);
     }
 }
 
diff --git a/src/ports/SkFontMgr_android_parser.h b/src/ports/SkFontMgr_android_parser.h
index aca1573952..b841bc6d18 100644
--- a/src/ports/SkFontMgr_android_parser.h
+++ b/src/ports/SkFontMgr_android_parser.h
@@ -211,7 +211,7 @@ template <int N, typename T> static bool parse_fixed(const char* s, T* value) {
         n = -n;
         frac = -frac;
     }
-    *value = (n << N) + frac;
+    *value = SkLeftShift(n, N) + frac;
     return true;
 }
 
diff --git a/src/utils/SkDashPath.cpp b/src/utils/SkDashPath.cpp
index e0cbe9732d..4e34b87eac 100644
--- a/src/utils/SkDashPath.cpp
+++ b/src/utils/SkDashPath.cpp
@@ -10,7 +10,7 @@
 #include "SkStrokeRec.h"
 
 static inline int is_even(int x) {
-    return (~x) << 31;
+    return !(x & 1);
 }
 
 static SkScalar find_first_interval(const SkScalar intervals[], SkScalar phase,
diff --git a/tests/MathTest.cpp b/tests/MathTest.cpp
index 3c2bc64407..24e46f3097 100644
--- a/tests/MathTest.cpp
+++ b/tests/MathTest.cpp
@@ -561,7 +561,7 @@ DEF_TEST(Math, reporter) {
         SkFixed numer = rand.nextS();
         SkFixed denom = rand.nextS();
         SkFixed result = SkFixedDiv(numer, denom);
-        int64_t check = ((int64_t)numer << 16) / denom;
+        int64_t check = SkLeftShift((int64_t)numer, 16) / denom;
 
         (void)SkCLZ(numer);
         (void)SkCLZ(denom);
diff --git a/tests/RandomTest.cpp b/tests/RandomTest.cpp
index 8d93d87df6..448f07340f 100644
--- a/tests/RandomTest.cpp
+++ b/tests/RandomTest.cpp
@@ -119,7 +119,7 @@ static double test_single_gorilla(skiatest::Reporter* reporter, int shift) {
 
     // now make some strings and track them
     for (int i = 0; i < kN; ++i) {
-        value <<= 1;
+        value = SkLeftShift(value, 1);
         unsigned int rnd = rand.nextU();
         value |= ((rnd >> shift) & 0x1);