diff options
author | mtklein <mtklein@google.com> | 2015-11-18 14:01:07 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-11-18 14:01:07 -0800 |
commit | c8e8fc6e98e9272293fcf24d53428ddd6129b011 (patch) | |
tree | 760504b3ab3c6c1bb7c9d348c649b01d28a6efae /src/core/SkMath.cpp | |
parent | a445cbccefb018b8163aae0a0c0a19c850a39be5 (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.cpp | 40 |
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 |