Fix UB in SkDivBits

This used to:
  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.

Instead it now:
  Only call SkDivBits if the old behavior is required.
  Usually, just do the divide with /.

BUG=skia:3562

Committed: https://skia.googlesource.com/skia/+/988adddd48322bfa3e3cb0c017cfce71fbbf1123

Committed: https://skia.googlesource.com/skia/+/6c7b104b4c08ae2332a6ce3c8c906da4e8c51e5f

TBR=reed@google.com
No API change.

Review URL: https://codereview.chromium.org/1455163004
This commit is contained in:
mtklein 2015-11-20 13:58:18 -08:00 committed by Commit bot
parent 6c221b4068
commit e47f1a7c74
4 changed files with 10 additions and 10 deletions

View File

@ -439,7 +439,7 @@ cc_test(
"--nogpu", "--nogpu",
"--verbose", "--verbose",
# TODO(mtklein): maybe investigate why these fail? # TODO(mtklein): maybe investigate why these fail?
"--match ~FontMgr ~Scalar ~Canvas ~Codec_stripes ~Codec_Dimensions ~Codec ~Stream ~skps ~Math ~RecordDraw_TextBounds", "--match ~FontMgr ~Scalar ~Canvas ~Codec_stripes ~Codec_Dimensions ~Codec ~Stream ~skps ~RecordDraw_TextBounds",
"--resourcePath %s/resources" % BASE_DIR, "--resourcePath %s/resources" % BASE_DIR,
"--images %s/resources" % BASE_DIR, "--images %s/resources" % BASE_DIR,
], ],

View File

@ -82,7 +82,14 @@ typedef int32_t SkFixed;
#define SkFixedAbs(x) SkAbs32(x) #define SkFixedAbs(x) SkAbs32(x)
#define SkFixedAve(a, b) (((a) + (b)) >> 1) #define SkFixedAve(a, b) (((a) + (b)) >> 1)
#define SkFixedDiv(numer, denom) SkDivBits(numer, denom, 16) // Blink layout tests are baselined to Clang optimizing through undefined behavior in SkDivBits.
#if defined(SK_SUPPORT_LEGACY_DIVBITS_UB)
#define SkFixedDiv(numer, denom) SkDivBits(numer, denom, 16)
#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)
#endif
////////////////////////////////////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////////////////////////////////////
// Now look for ASM overrides for our portable versions (should consider putting this in its own file) // Now look for ASM overrides for our portable versions (should consider putting this in its own file)

View File

@ -1221,9 +1221,7 @@
"_", "_",
"image", "image",
"_", "_",
"interlaced3.png", "interlaced3.png"
"--match",
"~Math"
], ],
"Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Valgrind": [ "Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Valgrind": [
"--matrix", "--matrix",

View File

@ -169,11 +169,6 @@ def get_args(bot):
if 'Valgrind' in bot: # skia:3021 if 'Valgrind' in bot: # skia:3021
match.append('~Threaded') match.append('~Threaded')
# skia:3562
if ('TSAN' in bot or
'Test-Mac10.8-Clang-MacMini4.1-CPU-SSE4-x86_64-Release' in bot):
match.append('~Math')
if 'GalaxyS3' in bot: # skia:1699 if 'GalaxyS3' in bot: # skia:1699
match.append('~WritePixels') match.append('~WritePixels')