turn on alignment sanitizer

This sanitizer checks for overaligned reads and writes,
or put another way, use of underaligned pointers.

This usually happens when you cast, e.g. char* to int*
without checking that the char* is 4-byte aligned.  Each
of the changes under src/ fixes something just like that.

The unusual setup for tools/xsan.blacklist is there to
force a rebuild whenever tools/xsan.blacklist changes.
I spent a good few minutes debugging rebuilds not happening
this morning, perhaps from some strange ccache interaction.

Align SkTextBlobs as void* (today they're just 4-byte) so the
SkTextBlob::RunRecords we put after them in SkTextBlobBuilder
buffers are properly aligned (for the SkTypeface* inside).

There's no obvious error in void SkRRect::computeType(),
but one bot seems to have seen some sort of issue with

    SK_AT_SCOPE_EXIT(SkASSERT(this->isValid()));

I can't reproduce it locally, so I'm just going to unroll it.

Change-Id: I904d94f65f695e1b626b684c32216a4930b72b0c
Reviewed-on: https://skia-review.googlesource.com/146104
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
This commit is contained in:
Mike Klein 2018-08-08 10:23:17 -04:00 committed by Skia Commit-Bot
parent f1b1464d81
commit 475c5e93fe
7 changed files with 57 additions and 22 deletions

View File

@ -231,7 +231,7 @@ config("default") {
fyi_sanitizers = fyi_sanitize
if (sanitize == "ASAN" || sanitize == "UBSAN") {
# ASAN implicitly runs all UBSAN checks also.
sanitizers = "bool,float-cast-overflow,integer-divide-by-zero,nonnull-attribute,null,pointer-overflow,return,returns-nonnull-attribute,shift,signed-integer-overflow,unreachable,vla-bound"
sanitizers = "alignment,bool,float-cast-overflow,integer-divide-by-zero,nonnull-attribute,null,pointer-overflow,return,returns-nonnull-attribute,shift,signed-integer-overflow,unreachable,vla-bound"
if (fyi_sanitize == "" && !is_android) {
fyi_sanitizers = "float-divide-by-zero"
@ -266,9 +266,13 @@ config("default") {
sanitizers = "memory"
}
_blacklist = rebase_path("../tools/xsan.blacklist")
cflags += [
"-fsanitize=$sanitizers,$fyi_sanitizers",
"-fno-sanitize-recover=$sanitizers",
"-fsanitize-blacklist=$_blacklist",
"-include$_blacklist",
]
if (!is_win) {
cflags += [ "-fno-omit-frame-pointer" ]

View File

@ -21,7 +21,7 @@ struct SkDeserialProcs;
SkTextBlob combines multiple text runs into an immutable, ref-counted structure.
*/
class SK_API SkTextBlob final : public SkNVRefCnt<SkTextBlob> {
class SK_API alignas(void*) SkTextBlob final : public SkNVRefCnt<SkTextBlob> {
public:
/**
* Returns a conservative blob bounding box.

View File

@ -6,7 +6,6 @@
*/
#include "SkRRectPriv.h"
#include "SkScopeExit.h"
#include "SkBuffer.h"
#include "SkMalloc.h"
#include "SkMatrix.h"
@ -323,14 +322,13 @@ static bool radii_are_nine_patch(const SkVector radii[4]) {
// There is a simplified version of this method in setRectXY
void SkRRect::computeType() {
SK_AT_SCOPE_EXIT(SkASSERT(this->isValid()));
if (fRect.isEmpty()) {
SkASSERT(fRect.isSorted());
for (size_t i = 0; i < SK_ARRAY_COUNT(fRadii); ++i) {
SkASSERT((fRadii[i] == SkVector{0, 0}));
}
fType = kEmpty_Type;
SkASSERT(this->isValid());
return;
}
@ -350,6 +348,7 @@ void SkRRect::computeType() {
if (allCornersSquare) {
fType = kRect_Type;
SkASSERT(this->isValid());
return;
}
@ -360,6 +359,7 @@ void SkRRect::computeType() {
} else {
fType = kSimple_Type;
}
SkASSERT(this->isValid());
return;
}
@ -368,6 +368,7 @@ void SkRRect::computeType() {
} else {
fType = kComplex_Type;
}
SkASSERT(this->isValid());
}
static bool matrix_only_scale_and_translate(const SkMatrix& matrix) {

View File

@ -113,7 +113,9 @@ public:
}
void writePtr(void* value) {
*(void**)this->reserve(sizeof(value)) = value;
// this->reserve() only returns 4-byte aligned pointers,
// so this may be an under-aligned write if we were to do this like the others.
memcpy(this->reserve(sizeof(value)), &value, sizeof(value));
}
void writeScalar(SkScalar value) {

View File

@ -65,7 +65,8 @@ inline Sk4px Sk4px::Wide::div255() const {
}
inline Sk4px Sk4px::Load4Alphas(const SkAlpha a[4]) {
uint32_t as = *(const uint32_t*)a;
uint32_t as;
memcpy(&as, a, 4);
__m128i splat = _mm_set_epi8(3,3,3,3, 2,2,2,2, 1,1,1,1, 0,0,0,0);
return Sk16b(_mm_shuffle_epi8(_mm_cvtsi32_si128(as), splat));
}
@ -80,7 +81,8 @@ inline Sk4px Sk4px::Wide::div255() const {
}
inline Sk4px Sk4px::Load4Alphas(const SkAlpha a[4]) {
__m128i as = _mm_cvtsi32_si128(*(const uint32_t*)a); // ____ ____ ____ 3210
__m128i as;
memcpy(&as, a, 4); // ____ ____ ____ 3210
as = _mm_unpacklo_epi8 (as, as); // ____ ____ 3322 1100
as = _mm_unpacklo_epi16(as, as); // 3333 2222 1111 0000
return Sk16b(as);
@ -88,8 +90,11 @@ inline Sk4px Sk4px::Wide::div255() const {
#endif
inline Sk4px Sk4px::Load2Alphas(const SkAlpha a[2]) {
uint32_t as = *(const uint16_t*)a; // Aa -> Aa00
return Load4Alphas((const SkAlpha*)&as);
uint16_t alphas;
memcpy(&alphas, a, 2);
uint32_t alphas_and_two_zeros = alphas; // Aa -> Aa00
return Load4Alphas((const SkAlpha*)&alphas_and_two_zeros);
}
inline Sk4px Sk4px::zeroColors() const {

View File

@ -13,15 +13,22 @@
#include "SkTemplates.h"
#include "SkUtils.h"
static SkUnichar SkUTF16BE_NextUnichar(const uint16_t** srcPtr) {
static SkUnichar next_unichar_UTF16BE(const char** srcPtr) {
SkASSERT(srcPtr && *srcPtr);
const uint16_t* src = *srcPtr;
SkUnichar c = SkEndian_SwapBE16(*src++);
const char* src = *srcPtr;
uint16_t lo;
memcpy(&lo, src, 2);
src += 2;
SkUnichar c = SkEndian_SwapBE16(lo);
SkASSERT(!SkUTF16_IsLowSurrogate(c));
if (SkUTF16_IsHighSurrogate(c)) {
unsigned c2 = SkEndian_SwapBE16(*src++);
uint16_t hi;
memcpy(&hi, src, 2);
src += 2;
unsigned c2 = SkEndian_SwapBE16(hi);
SkASSERT(SkUTF16_IsLowSurrogate(c2));
c = (c << 10) + c2 + (0x10000 - (0xD800 << 10) - 0xDC00);
@ -30,14 +37,15 @@ static SkUnichar SkUTF16BE_NextUnichar(const uint16_t** srcPtr) {
return c;
}
static void SkStringFromUTF16BE(const uint16_t* utf16be, size_t length, SkString& utf8) {
static void SkString_from_UTF16BE(const char* utf16be, size_t length, SkString& utf8) {
// Note that utf16be may not be 2-byte aligned.
SkASSERT(utf16be != nullptr);
utf8.reset();
size_t numberOf16BitValues = length / 2;
const uint16_t* end = utf16be + numberOf16BitValues;
const char* end = utf16be + numberOf16BitValues*2;
while (utf16be < end) {
utf8.appendUnichar(SkUTF16BE_NextUnichar(&utf16be));
utf8.appendUnichar(next_unichar_UTF16BE(&utf16be));
}
}
@ -475,7 +483,7 @@ bool SkOTTableName::Iterator::next(SkOTTableName::Iterator::Record& record) {
}
case SkOTTableName::Record::PlatformID::Unicode:
case SkOTTableName::Record::PlatformID::ISO:
SkStringFromUTF16BE((const uint16_t*)nameString, nameLength, record.name);
SkString_from_UTF16BE(nameString, nameLength, record.name);
break;
case SkOTTableName::Record::PlatformID::Macintosh:
@ -513,8 +521,8 @@ bool SkOTTableName::Iterator::next(SkOTTableName::Iterator::Record& record) {
uint16_t offset = SkEndian_SwapBE16(languageTagRecord[languageTagRecordIndex].offset);
uint16_t length = SkEndian_SwapBE16(languageTagRecord[languageTagRecordIndex].length);
const uint16_t* string = SkTAddOffset<const uint16_t>(stringTable, offset);
SkStringFromUTF16BE(string, length, record.language);
const char* string = SkTAddOffset<const char>(stringTable, offset);
SkString_from_UTF16BE(string, length, record.language);
return true;
}
}

15
tools/xsan.blacklist Normal file
View File

@ -0,0 +1,15 @@
#if 0
# This file must be a no-op C #include header, and a valid *SAN blacklist file.
# Luckily, anything starting with # is a comment to *SAN blacklist files,
# and anything inside #if 0 is ignored by C. Yippee!
#
# If you want to type '*', type '.*' instead. Don't make C comments!
# libpng and zlib both dereference under-aligned pointers.
# TODO: it'd be nice to tag these as [alignment] only but our Mac toolchain can't yet.
# [alignment]
src:.*third_party/externals/libpng/intel/filter_sse2_intrinsics.c
src:.*third_party/externals/zlib/deflate.c
#endif