Add support for MSVC run-time checks (and control flow guard)

This enables four different options in the compiler, described
below. I also added enough masks to satisfy RTCc when running
all GMs in both 8888 and gl configs.

---

/RTCc - Detects when a value is assigned to a smaller data
type and results in data loss. This happens even when casting.
Masking is required to suppress this.

/RTCs - Various stack-related checks, including uninitialized
data (by initializing locals to a non-zero value), array bounds
checking, and stack pointer corruption that can occur with a
calling convention mismatch.

/RTCu - Reports when a variable is used without having been
initialized. Mostly redundant with compile-time checks.

/guard:cf - This is more of a security option, that computes
all possible targets for indirect calls at compile time, and
verifies that those are the only targets reached at compile
time. Also generates similar logic around switch statements
that turn into jump tables.

Bug: skia:
Change-Id: I7b527af8fd67dec0b6556f38bcd0efc3fd505856
Reviewed-on: https://skia-review.googlesource.com/c/188625
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This commit is contained in:
Brian Osman 2019-02-04 10:01:53 -05:00 committed by Skia Commit-Bot
parent f5f0cd8063
commit 50ea3c06b8
25 changed files with 129 additions and 62 deletions

View File

@ -221,7 +221,7 @@ config("default") {
ldflags += [ "-Wl,-w" ]
}
if (sanitize != "") {
if (sanitize != "" && sanitize != "MSVC") {
# You can either pass the sanitizers directly, e.g. "address,undefined",
# or pass one of the couple common aliases used by the bots.
sanitizers = sanitize
@ -428,7 +428,10 @@ config("debug_symbols") {
} else if (is_win) {
cflags = [ "/Z7" ]
if (is_clang) {
cflags += [ "-mllvm", "-emit-codeview-ghash-section" ]
cflags += [
"-mllvm",
"-emit-codeview-ghash-section",
]
ldflags = [ "/DEBUG:GHASH" ]
} else {
ldflags = [ "/DEBUG:FASTLINK" ]
@ -483,6 +486,15 @@ config("NDEBUG") {
defines = [ "NDEBUG" ]
}
config("msvc_rtc") {
defines = [ "_ALLOW_RTCc_IN_STL" ]
cflags = [
"/RTCcsu",
"/guard:cf",
]
ldflags = [ "/guard:cf" ]
}
config("executable") {
if (is_android) {
ldflags = [

View File

@ -238,6 +238,9 @@ default_configs += [
"//gn:warnings_except_public_headers",
"//gn:extra_flags",
]
if (sanitize == "MSVC") {
default_configs += [ "//gn:msvc_rtc" ]
}
set_defaults("executable") {
configs = [ "//gn:executable" ] + default_configs

View File

@ -9,8 +9,23 @@
#define SkTFitsIn_DEFINED
#include <limits>
#include <stdint.h>
#include <type_traits>
/**
* std::underlying_type is only defined for enums. For integral types, we just want the type.
*/
template <typename T, class Enable = void>
struct sk_strip_enum {
typedef T type;
};
template <typename T>
struct sk_strip_enum<T, typename std::enable_if<std::is_enum<T>::value>::type> {
typedef typename std::underlying_type<T>::type type;
};
/**
* In C++ an unsigned to signed cast where the source value cannot be represented in the destination
* type results in an implementation defined destination value. Unlike C, C++ does not allow a trap.
@ -65,10 +80,20 @@ typename std::enable_if<(std::is_integral<S>::value || std::is_enum<S>::value) &
// E.g. (uint8_t)(int8_t) uint8_t(255) == 255, but the int8_t == -1.
(std::is_signed<D>::value && std::is_unsigned<S>::value && sizeof(D) <= sizeof(S)) ?
src <= (S)std::numeric_limits<D>::max() :
src <= (S)std::numeric_limits<typename sk_strip_enum<D>::type>::max() :
// else
(S)(D)src == src;
#if !defined(SK_DEBUG) && !defined(__MSVC_RUNTIME_CHECKS )
// Correct (simple) version. This trips up MSVC's /RTCc run-time checking.
(S)(D)src == src;
#else
// More complex version that's safe with /RTCc. Used in all debug builds, for coverage.
(std::is_signed<S>::value) ?
(intmax_t)src >= (intmax_t)std::numeric_limits<typename sk_strip_enum<D>::type>::min() &&
(intmax_t)src <= (intmax_t)std::numeric_limits<typename sk_strip_enum<D>::type>::max() :
// std::is_unsigned<S> ?
(uintmax_t)src <= (uintmax_t)std::numeric_limits<typename sk_strip_enum<D>::type>::max();
#endif
}
#endif

View File

@ -114,10 +114,10 @@ bool SkBitmap::setInfo(const SkImageInfo& info, size_t rowBytes) {
// require that rowBytes fit in 31bits
int64_t mrb = info.minRowBytes64();
if ((int32_t)mrb != mrb) {
if (!SkTFitsIn<int32_t>(mrb)) {
return reset_return_false(this);
}
if ((int64_t)rowBytes != (int32_t)rowBytes) {
if (!SkTFitsIn<int32_t>(rowBytes)) {
return reset_return_false(this);
}

View File

@ -293,7 +293,7 @@ static inline void bits_to_runs(SkBlitter* blitter, int x, int y,
// maskBitCount is the number of 1's to place in the mask. It must be in the range between 1 and 8.
static uint8_t generate_right_mask(int maskBitCount) {
return static_cast<uint8_t>(0xFF00U >> maskBitCount);
return static_cast<uint8_t>((0xFF00U >> maskBitCount) & 0xFF);
}
void SkBlitter::blitMask(const SkMask& mask, const SkIRect& clip) {

View File

@ -28,7 +28,7 @@
e.g. 0x1234 -> 0x3412
*/
static inline uint16_t SkEndianSwap16(uint16_t value) {
return static_cast<uint16_t>((value >> 8) | (value << 8));
return static_cast<uint16_t>((value >> 8) | ((value & 0xFF) << 8));
}
template<uint16_t N> struct SkTEndianSwap16 {

View File

@ -68,7 +68,7 @@ inline SkFixed SkFDot6ToFixed(SkFDot6 x) {
inline SkFixed SkFDot6Div(SkFDot6 a, SkFDot6 b) {
SkASSERT(b != 0);
if (a == (int16_t)a) {
if (SkTFitsIn<int16_t>(a)) {
return SkLeftShift(a, 16) / b;
} else {
return SkFixedDiv(a, b);

View File

@ -42,7 +42,7 @@ struct ColorTypeFilter_565 {
return (x & ~SK_G16_MASK_IN_PLACE) | ((x & SK_G16_MASK_IN_PLACE) << 16);
}
static uint16_t Compact(uint32_t x) {
return (x & ~SK_G16_MASK_IN_PLACE) | ((x >> 16) & SK_G16_MASK_IN_PLACE);
return ((x & ~SK_G16_MASK_IN_PLACE) & 0xFFFF) | ((x >> 16) & SK_G16_MASK_IN_PLACE);
}
};

View File

@ -45,7 +45,7 @@ SkCanvas::SaveLayerFlags SkCanvasPriv::LegacySaveFlagsToSaveLayerFlags(uint32_t
DrawType SkPicturePlayback::ReadOpAndSize(SkReadBuffer* reader, uint32_t* size) {
uint32_t temp = reader->readInt();
uint32_t op;
if (((uint8_t)temp) == temp) {
if ((temp & 0xFF) == temp) {
// old skp file - no size information
op = temp;
*size = 0;

View File

@ -52,7 +52,7 @@ void SkResourceCache::Key::init(void* nameSpace, uint64_t sharedID, size_t dataS
"namespace_field_must_be_last");
fCount32 = SkToS32(kLocal32s + (dataSize >> 2));
fSharedID_lo = (uint32_t)sharedID;
fSharedID_lo = (uint32_t)(sharedID & 0xFFFFFFFF);
fSharedID_hi = (uint32_t)(sharedID >> 32);
fNamespace = nameSpace;
// skip unhashed fields when computing the hash

View File

@ -636,7 +636,7 @@ static inline void computeAlphaBelowLine(
alphas[R-1] = SkFixedMul(last, lastH) >> 9; // triangle alpha
SkFixed alpha16 = lastH + (dY >> 1); // rectangle plus triangle
for (int i = R - 2; i > 0; i--) {
alphas[i] = alpha16 >> 8;
alphas[i] = (alpha16 >> 8) & 0xFF;
alpha16 += dY;
}
alphas[0] = fullAlpha - partialTriangleToAlpha(first, dY);

View File

@ -112,7 +112,7 @@ public:
fy += SK_Fixed1/2;
int y = fy >> 16;
uint8_t a = (uint8_t)(fy >> 8);
uint8_t a = (uint8_t)((fy >> 8) & 0xFF);
// lower line
unsigned ma = SmallDot6Scale(a, mod64);
@ -136,7 +136,7 @@ public:
fy += SK_Fixed1/2;
int y = fy >> 16;
uint8_t a = (uint8_t)(fy >> 8);
uint8_t a = (uint8_t)((fy >> 8) & 0xFF);
// lower line
if (a) {
@ -159,7 +159,7 @@ public:
fy += SK_Fixed1/2;
int lower_y = fy >> 16;
uint8_t a = (uint8_t)(fy >> 8);
uint8_t a = (uint8_t)((fy >> 8) & 0xFF);
unsigned a0 = SmallDot6Scale(255 - a, mod64);
unsigned a1 = SmallDot6Scale(a, mod64);
this->getBlitter()->blitAntiV2(x, lower_y - 1, a0, a1);
@ -174,7 +174,7 @@ public:
SkBlitter* blitter = this->getBlitter();
do {
int lower_y = fy >> 16;
uint8_t a = (uint8_t)(fy >> 8);
uint8_t a = (uint8_t)((fy >> 8) & 0xFF);
blitter->blitAntiV2(x, lower_y - 1, 255 - a, a);
fy += dy;
} while (++x < stopx);
@ -190,7 +190,7 @@ public:
fx += SK_Fixed1/2;
int x = fx >> 16;
int a = (uint8_t)(fx >> 8);
int a = (uint8_t)((fx >> 8) & 0xFF);
unsigned ma = SmallDot6Scale(a, mod64);
if (ma) {
@ -210,7 +210,7 @@ public:
fx += SK_Fixed1/2;
int x = fx >> 16;
int a = (uint8_t)(fx >> 8);
int a = (uint8_t)((fx >> 8) & 0xFF);
if (a) {
this->getBlitter()->blitV(x, y, stopy - y, a);
@ -230,7 +230,7 @@ public:
fx += SK_Fixed1/2;
int x = fx >> 16;
uint8_t a = (uint8_t)(fx >> 8);
uint8_t a = (uint8_t)((fx >> 8) & 0xFF);
this->getBlitter()->blitAntiH2(x - 1, y,
SmallDot6Scale(255 - a, mod64), SmallDot6Scale(a, mod64));
@ -242,7 +242,7 @@ public:
fx += SK_Fixed1/2;
do {
int x = fx >> 16;
uint8_t a = (uint8_t)(fx >> 8);
uint8_t a = (uint8_t)((fx >> 8) & 0xFF);
this->getBlitter()->blitAntiH2(x - 1, y, 255 - a, a);
fx += dx;
} while (++y < stopy);

View File

@ -22,10 +22,10 @@ namespace SK_OPTS_NS {
static void RGBA_to_rgbA_portable(uint32_t* dst, const uint32_t* src, int count) {
for (int i = 0; i < count; i++) {
uint8_t a = src[i] >> 24,
b = src[i] >> 16,
g = src[i] >> 8,
r = src[i] >> 0;
uint8_t a = (src[i] >> 24) & 0xFF,
b = (src[i] >> 16) & 0xFF,
g = (src[i] >> 8) & 0xFF,
r = (src[i] >> 0) & 0xFF;
b = (b*a+127)/255;
g = (g*a+127)/255;
r = (r*a+127)/255;
@ -38,10 +38,10 @@ static void RGBA_to_rgbA_portable(uint32_t* dst, const uint32_t* src, int count)
static void RGBA_to_bgrA_portable(uint32_t* dst, const uint32_t* src, int count) {
for (int i = 0; i < count; i++) {
uint8_t a = src[i] >> 24,
b = src[i] >> 16,
g = src[i] >> 8,
r = src[i] >> 0;
uint8_t a = (src[i] >> 24) & 0xFF,
b = (src[i] >> 16) & 0xFF,
g = (src[i] >> 8) & 0xFF,
r = (src[i] >> 0) & 0xFF;
b = (b*a+127)/255;
g = (g*a+127)/255;
r = (r*a+127)/255;
@ -54,10 +54,10 @@ static void RGBA_to_bgrA_portable(uint32_t* dst, const uint32_t* src, int count)
static void RGBA_to_BGRA_portable(uint32_t* dst, const uint32_t* src, int count) {
for (int i = 0; i < count; i++) {
uint8_t a = src[i] >> 24,
b = src[i] >> 16,
g = src[i] >> 8,
r = src[i] >> 0;
uint8_t a = (src[i] >> 24) & 0xFF,
b = (src[i] >> 16) & 0xFF,
g = (src[i] >> 8) & 0xFF,
r = (src[i] >> 0) & 0xFF;
dst[i] = (uint32_t)a << 24
| (uint32_t)r << 16
| (uint32_t)g << 8
@ -127,10 +127,10 @@ static void grayA_to_rgbA_portable(uint32_t dst[], const uint8_t* src, int count
static void inverted_CMYK_to_RGB1_portable(uint32_t* dst, const uint32_t* src, int count) {
for (int i = 0; i < count; i++) {
uint8_t k = src[i] >> 24,
y = src[i] >> 16,
m = src[i] >> 8,
c = src[i] >> 0;
uint8_t k = (src[i] >> 24) & 0xFF,
y = (src[i] >> 16) & 0xFF,
m = (src[i] >> 8) & 0xFF,
c = (src[i] >> 0) & 0xFF;
// See comments in SkSwizzler.cpp for details on the conversion formula.
uint8_t b = (y*k+127)/255,
g = (m*k+127)/255,
@ -144,10 +144,10 @@ static void inverted_CMYK_to_RGB1_portable(uint32_t* dst, const uint32_t* src, i
static void inverted_CMYK_to_BGR1_portable(uint32_t* dst, const uint32_t* src, int count) {
for (int i = 0; i < count; i++) {
uint8_t k = src[i] >> 24,
y = src[i] >> 16,
m = src[i] >> 8,
c = src[i] >> 0;
uint8_t k = (src[i] >> 24) & 0xFF,
y = (src[i] >> 16) & 0xFF,
m = (src[i] >> 8) & 0xFF,
c = (src[i] >> 0) & 0xFF;
uint8_t b = (y*k+127)/255,
g = (m*k+127)/255,
r = (c*k+127)/255;

View File

@ -83,9 +83,9 @@ bool SkEncodeImageWithWIC(SkWStream* stream, const SkPixmap& pixmap,
uint8_t* dstRow = SkTAddOffset<uint8_t>(pixelStorage.get(), y * rowBytes);
for (int x = 0; x < bitmap.width(); x++) {
uint32_t bgra = *bitmap.getAddr32(x, y);
dstRow[0] = (uint8_t) (bgra >> 0);
dstRow[1] = (uint8_t) (bgra >> 8);
dstRow[2] = (uint8_t) (bgra >> 16);
dstRow[0] = (uint8_t) ((bgra >> 0) & 0xFF);
dstRow[1] = (uint8_t) ((bgra >> 8) & 0xFF);
dstRow[2] = (uint8_t) ((bgra >> 16) & 0xFF);
dstRow += 3;
}
}

View File

@ -79,10 +79,10 @@ handlePad:
int one = (uint8_t) (bytes[0] << 2);
two = bytes[1];
one |= two >> 4;
two = (uint8_t) (two << 4);
two = (uint8_t) ((two << 4) & 0xFF);
three = bytes[2];
two |= three >> 2;
three = (uint8_t) (three << 6);
three = (uint8_t) ((three << 6) & 0xFF);
three |= bytes[3];
SkASSERT(one < 256 && two < 256 && three < 256);
*dst = (unsigned char) one;

View File

@ -13,6 +13,12 @@
#define TEST(S, s, D, expected) REPORTER_ASSERT(reporter, (SkTFitsIn<D>((S)(s)) == (expected)))
enum TestEnum_t : uint8_t {
kFoo,
kBar,
kBaz,
};
DEF_TEST(FitsIn, reporter) {
TEST(uint16_t, 257, int8_t, false);
@ -32,6 +38,9 @@ DEF_TEST(FitsIn, reporter) {
TEST(int32_t, -127, uint8_t, false);
TEST(int32_t, -128, uint8_t, false);
TEST(uint8_t, 2, TestEnum_t, true);
TEST(TestEnum_t, kBar, uint8_t, true);
TEST(int32_t, 1000, int8_t, false);
TEST(int32_t, 1000, uint8_t, false);

View File

@ -19,10 +19,10 @@ class A {
public:
A() {}
virtual void setValues(int v) {
fChar = static_cast<char>(v);
fChar = static_cast<char>(v & 0xFF);
}
virtual bool checkValues(int v) {
return fChar == static_cast<char>(v);
return fChar == static_cast<char>(v & 0xFF);
}
virtual ~A() {}

View File

@ -266,8 +266,9 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadOnlyTexture, reporter, context_info) {
static constexpr int kSize = 100;
SkAutoPixmapStorage pixels;
pixels.alloc(SkImageInfo::Make(kSize, kSize, kRGBA_8888_SkColorType, kPremul_SkAlphaType));
fillPixels(&pixels,
[](int x, int y) { return (0xFFU << 24) | (x << 16) | (y << 8) | uint8_t(x * y); });
fillPixels(&pixels, [](int x, int y) {
return (0xFFU << 24) | (x << 16) | (y << 8) | uint8_t((x * y) & 0xFF);
});
GrContext* context = context_info.grContext();
GrProxyProvider* proxyProvider = context->contextPriv().proxyProvider();

View File

@ -196,7 +196,9 @@ public:
}
private:
static int ValueAt(uint64_t i) { return static_cast<int>(123456789 + 987654321 * i); }
static int ValueAt(uint64_t i) {
return static_cast<int>((123456789 + 987654321 * i) & 0xFFFFFFFF);
}
int fLength;
};

View File

@ -522,6 +522,7 @@ DEF_TEST(TestEndian, reporter) {
template <typename T>
static void test_divmod(skiatest::Reporter* r) {
#if !defined(__MSVC_RUNTIME_CHECKS)
const struct {
T numer;
T denom;
@ -557,6 +558,7 @@ static void test_divmod(skiatest::Reporter* r) {
REPORTER_ASSERT(r, numer/denom == div);
REPORTER_ASSERT(r, numer%denom == mod);
}
#endif
}
DEF_TEST(divmod_u8, r) {

View File

@ -55,10 +55,10 @@ class PathOpsThreadedRunnable {
public:
PathOpsThreadedRunnable(void (*testFun)(PathOpsThreadState*), int a, int b, int c, int d,
PathOpsThreadedTestRunner* runner) {
fState.fA = a;
fState.fB = b;
fState.fC = c;
fState.fD = d;
fState.fA = (a & 0xFF);
fState.fB = (b & 0xFF);
fState.fC = (c & 0xFF);
fState.fD = (d & 0xFF);
fState.fReporter = runner->fReporter;
fTestFun = testFun;
}

View File

@ -237,7 +237,10 @@ static GrColor input_texel_color(int i, int j, SkScalar delta) {
// Delta must be less than 0.5 to prevent over/underflow issues with the input color
SkASSERT(delta <= 0.5);
SkColor color = SkColorSetARGB((uint8_t)i, (uint8_t)j, (uint8_t)(i + j), (uint8_t)(2 * j - i));
SkColor color = SkColorSetARGB((uint8_t)(i & 0xFF),
(uint8_t)(j & 0xFF),
(uint8_t)((i + j) & 0xFF),
(uint8_t)((2 * j - i) & 0xFF));
SkColor4f color4f = SkColor4f::FromColor(color);
for (int i = 0; i < 4; i++) {
if (color4f[i] > 0.5) {
@ -481,6 +484,10 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ProcessorOptimizationValidationTest, repor
// written.
timesToInvokeFactory *= FPFactory::Count() / 2;
}
#if defined(__MSVC_RUNTIME_CHECKS)
// This test is infuriatingly slow with MSVC runtime checks enabled
timesToInvokeFactory = 1;
#endif
for (int j = 0; j < timesToInvokeFactory; ++j) {
fp = FPFactory::MakeIdx(i, &testData);
if (!fp->instantiate(resourceProvider)) {

View File

@ -12,9 +12,10 @@
DEF_TEST(SkBase64, reporter) {
char all[256];
for (int index = 0; index < 256; ++index) {
for (int index = 0; index < 255; ++index) {
all[index] = (signed char) (index + 1);
}
all[255] = 0;
for (int offset = 0; offset < 6; ++offset) {
size_t length = 256 - offset;

View File

@ -480,10 +480,10 @@ void etc_encode_block_helper(const etc1_byte* pIn, etc1_uint32 inMask,
}
static void writeBigEndian(etc1_byte* pOut, etc1_uint32 d) {
pOut[0] = (etc1_byte)(d >> 24);
pOut[1] = (etc1_byte)(d >> 16);
pOut[2] = (etc1_byte)(d >> 8);
pOut[3] = (etc1_byte) d;
pOut[0] = (etc1_byte) (d >> 24);
pOut[1] = (etc1_byte)((d >> 16) & 0xFF);
pOut[2] = (etc1_byte)((d >> 8) & 0xFF);
pOut[3] = (etc1_byte)((d >> 0) & 0xFF);
}
// Input is a 4 x 4 square of 3-byte pixels in form R, G, B

View File

@ -55,6 +55,11 @@ set_defaults("third_party") {
# Official builds don't have warnings to begin with.
configs -= [ "//gn:warnings" ]
}
# Don't want to to deal with this (especially /RTCc)
if (sanitize == "MSVC") {
configs -= [ "//gn:msvc_rtc" ]
}
if (is_debug) {
configs += [ "//gn:optimize" ]
}