aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/core/SkMath.cpp
diff options
context:
space:
mode:
authorGravatar mtklein <mtklein@google.com>2015-11-18 14:01:07 -0800
committerGravatar Commit bot <commit-bot@chromium.org>2015-11-18 14:01:07 -0800
commitc8e8fc6e98e9272293fcf24d53428ddd6129b011 (patch)
tree760504b3ab3c6c1bb7c9d348c649b01d28a6efae /src/core/SkMath.cpp
parenta445cbccefb018b8163aae0a0c0a19c850a39be5 (diff)
Revert of Fix UB in SkDivBits (patchset #2 id:10002 of https://codereview.chromium.org/1455163004/ )
Reason for revert: Need to reland with #define guards for tiny layout test changes. (Yikes!) Original issue's description: > Fix UB in SkDivBits > > DIVBITS_ITER was shifting bits up into the sign bit, which is a no-no. > This turns numer into a uint32_t to make those defined, and adds a few notes. > > x >= 0 is always true for unsigned x, so we needed a few small logic refactors. > > BUG=skia:3562 > > Committed: https://skia.googlesource.com/skia/+/988adddd48322bfa3e3cb0c017cfce71fbbf1123 TBR=caryclark@google.com,mtklein@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=skia:3562 Review URL: https://codereview.chromium.org/1457863002
Diffstat (limited to 'src/core/SkMath.cpp')
-rw-r--r--src/core/SkMath.cpp40
1 files changed, 15 insertions, 25 deletions
diff --git a/src/core/SkMath.cpp b/src/core/SkMath.cpp
index c5d468dd83..af93d7ecb2 100644
--- a/src/core/SkMath.cpp
+++ b/src/core/SkMath.cpp
@@ -45,32 +45,21 @@ int SkCLZ_portable(uint32_t x) {
///////////////////////////////////////////////////////////////////////////////
-
-#define DIVBITS_ITER(k) \
- case k: \
- if (numer*2 >= denom) { \
- numer = numer*2 - denom; \
- result |= 1 << (k-1); \
- } else { \
- numer *= 2; \
- }
-
-// NOTE: if you're thinking of editing this method, consider replacing it with
-// a simple shift and divide. This legacy method predated reliable hardware division.
-int32_t SkDivBits(int32_t n, int32_t d, int shift_bias) {
- SkASSERT(d != 0);
- if (n == 0) {
+#define DIVBITS_ITER(n) \
+ case n: \
+ if ((numer = (numer << 1) - denom) >= 0) \
+ result |= 1 << (n - 1); else numer += denom
+
+int32_t SkDivBits(int32_t numer, int32_t denom, int shift_bias) {
+ SkASSERT(denom != 0);
+ if (numer == 0) {
return 0;
}
- // Make numer and denom positive, and sign hold the resulting sign
- // We'll be left-shifting numer, so it's important it's a uint32_t.
- // We put denom in a uint32_t just to keep things simple.
- int32_t sign = SkExtractSign(n ^ d);
- uint32_t numer = SkAbs32(n);
- uint32_t denom = SkAbs32(d);
-
- // It's probably a bug to use n or d below here.
+ // make numer and denom positive, and sign hold the resulting sign
+ int32_t sign = SkExtractSign(numer ^ denom);
+ numer = SkAbs32(numer);
+ denom = SkAbs32(denom);
int nbits = SkCLZ(numer) - 1;
int dbits = SkCLZ(denom) - 1;
@@ -89,9 +78,10 @@ int32_t SkDivBits(int32_t n, int32_t d, int shift_bias) {
SkFixed result = 0;
// do the first one
- if (numer >= denom) {
- numer -= denom;
+ if ((numer -= denom) >= 0) {
result = 1;
+ } else {
+ numer += denom;
}
// Now fall into our switch statement if there are more bits to compute