Switch from size_t to uint32_t to represent constraints better.

TBR=mtklein@google.com

Change-Id: Ib7400f2a3e2af0d8976998e5857c4d2c9cb6dbd0
Reviewed-on: https://skia-review.googlesource.com/7614
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Herb Derby <herb@google.com>
This commit is contained in:
Herb Derby 2017-02-17 10:45:47 -05:00 committed by Skia Commit-Bot
parent 35115eb3c7
commit 1bf3fc74b0
2 changed files with 44 additions and 41 deletions

View File

@ -42,10 +42,10 @@ char* SkArenaAlloc::NextBlock(char* footerEnd) {
SkArenaAlloc::SkArenaAlloc(char* block, size_t size, size_t extraSize) SkArenaAlloc::SkArenaAlloc(char* block, size_t size, size_t extraSize)
: fDtorCursor {block} : fDtorCursor {block}
, fCursor {block} , fCursor {block}
, fEnd {block + size} , fEnd {block + SkTo<uint32_t>(size)}
, fFirstBlock {block} , fFirstBlock {block}
, fFirstSize {size} , fFirstSize {SkTo<uint32_t>(size)}
, fExtraSize {extraSize} , fExtraSize {SkTo<uint32_t>(extraSize)}
{ {
if (size < sizeof(Footer)) { if (size < sizeof(Footer)) {
fEnd = fCursor = fDtorCursor = nullptr; fEnd = fCursor = fDtorCursor = nullptr;
@ -89,23 +89,23 @@ void SkArenaAlloc::installUint32Footer(FooterAction* action, uint32_t value, uin
this->installFooter(action, padding); this->installFooter(action, padding);
} }
void SkArenaAlloc::ensureSpace(size_t size, size_t alignment) { void SkArenaAlloc::ensureSpace(uint32_t size, uint32_t alignment) {
constexpr size_t headerSize = sizeof(Footer) + sizeof(ptrdiff_t); constexpr uint32_t headerSize = sizeof(Footer) + sizeof(ptrdiff_t);
// The chrome c++ library we use does not define std::max_align_t. // The chrome c++ library we use does not define std::max_align_t.
// This must be conservative to add the right amount of extra memory to handle the alignment // This must be conservative to add the right amount of extra memory to handle the alignment
// padding. // padding.
constexpr size_t alignof_max_align_t = 8; constexpr uint32_t alignof_max_align_t = 8;
auto objSizeAndOverhead = size + headerSize + sizeof(Footer); uint32_t objSizeAndOverhead = size + headerSize + sizeof(Footer);
if (alignment > alignof_max_align_t) { if (alignment > alignof_max_align_t) {
objSizeAndOverhead += alignment - 1; objSizeAndOverhead += alignment - 1;
} }
auto allocationSize = std::max(objSizeAndOverhead, fExtraSize); uint32_t allocationSize = std::max(objSizeAndOverhead, fExtraSize);
// Round up to a nice size. If > 32K align to 4K boundary else up to max_align_t. The > 32K // Round up to a nice size. If > 32K align to 4K boundary else up to max_align_t. The > 32K
// heuristic is from the JEMalloc behavior. // heuristic is from the JEMalloc behavior.
{ {
size_t mask = allocationSize > (1 << 15) ? (1 << 12) - 1 : 16 - 1; uint32_t mask = allocationSize > (1 << 15) ? (1 << 12) - 1 : 16 - 1;
allocationSize = (allocationSize + mask) & ~mask; allocationSize = (allocationSize + mask) & ~mask;
} }
@ -118,8 +118,9 @@ void SkArenaAlloc::ensureSpace(size_t size, size_t alignment) {
this->installPtrFooter(NextBlock, previousDtor, 0); this->installPtrFooter(NextBlock, previousDtor, 0);
} }
char* SkArenaAlloc::allocObject(size_t size, size_t alignment) { char* SkArenaAlloc::allocObject(uint32_t size, uint32_t alignment) {
size_t mask = alignment - 1; // Must be uint64 to mask 64-bit pointers properly.
uint64_t mask = alignment - 1;
char* objStart = (char*)((uintptr_t)(fCursor + mask) & ~mask); char* objStart = (char*)((uintptr_t)(fCursor + mask) & ~mask);
if ((ptrdiff_t)size > fEnd - objStart) { if ((ptrdiff_t)size > fEnd - objStart) {
this->ensureSpace(size, alignment); this->ensureSpace(size, alignment);
@ -128,17 +129,18 @@ char* SkArenaAlloc::allocObject(size_t size, size_t alignment) {
return objStart; return objStart;
} }
char* SkArenaAlloc::allocObjectWithFooter(size_t sizeIncludingFooter, size_t alignment) { char* SkArenaAlloc::allocObjectWithFooter(uint32_t sizeIncludingFooter, uint32_t alignment) {
size_t mask = alignment - 1; // Must be uint64 to mask 64-bit pointers properly.
uint64_t mask = alignment - 1;
restart: restart:
size_t skipOverhead = 0; uint32_t skipOverhead = 0;
bool needsSkipFooter = fCursor != fDtorCursor; bool needsSkipFooter = fCursor != fDtorCursor;
if (needsSkipFooter) { if (needsSkipFooter) {
skipOverhead = sizeof(Footer) + sizeof(uint32_t); skipOverhead = sizeof(Footer) + sizeof(uint32_t);
} }
char* objStart = (char*)((uintptr_t)(fCursor + skipOverhead + mask) & ~mask); char* objStart = (char*)((uintptr_t)(fCursor + skipOverhead + mask) & ~mask);
size_t totalSize = sizeIncludingFooter + skipOverhead; uint32_t totalSize = sizeIncludingFooter + skipOverhead;
if ((ptrdiff_t)totalSize > fEnd - objStart) { if ((ptrdiff_t)totalSize > fEnd - objStart) {
this->ensureSpace(totalSize, alignment); this->ensureSpace(totalSize, alignment);

View File

@ -69,19 +69,19 @@ public:
template <typename T, typename... Args> template <typename T, typename... Args>
T* make(Args&&... args) { T* make(Args&&... args) {
uint32_t size = SkTo<uint32_t>(sizeof(T));
SkASSERT(SkTFitsIn<uint32_t>(sizeof(T))); uint32_t alignment = SkTo<uint32_t>(alignof(T));
char* objStart; char* objStart;
if (skstd::is_trivially_destructible<T>::value) { if (skstd::is_trivially_destructible<T>::value) {
objStart = this->allocObject(sizeof(T), alignof(T)); objStart = this->allocObject(size, alignment);
fCursor = objStart + sizeof(T); fCursor = objStart + size;
} else { } else {
objStart = this->allocObjectWithFooter(sizeof(T) + sizeof(Footer), alignof(T)); objStart = this->allocObjectWithFooter(size + sizeof(Footer), alignment);
// Can never be UB because max value is alignof(T). // Can never be UB because max value is alignof(T).
uint32_t padding = SkTo<uint32_t>(objStart - fCursor); uint32_t padding = SkTo<uint32_t>(objStart - fCursor);
// Advance to end of object to install footer. // Advance to end of object to install footer.
fCursor = objStart + sizeof(T); fCursor = objStart + size;
FooterAction* releaser = [](char* objEnd) { FooterAction* releaser = [](char* objEnd) {
char* objStart = objEnd - (sizeof(T) + sizeof(Footer)); char* objStart = objEnd - (sizeof(T) + sizeof(Footer));
((T*)objStart)->~T(); ((T*)objStart)->~T();
@ -105,10 +105,11 @@ public:
template <typename T> template <typename T>
T* makeArrayDefault(size_t count) { T* makeArrayDefault(size_t count) {
T* array = (T*)this->commonArrayAlloc<T>(count); uint32_t safeCount = SkTo<uint32_t>(count);
T* array = (T*)this->commonArrayAlloc<T>(safeCount);
// If T is primitive then no initialization takes place. // If T is primitive then no initialization takes place.
for (size_t i = 0; i < count; i++) { for (size_t i = 0; i < safeCount; i++) {
new (&array[i]) T; new (&array[i]) T;
} }
return array; return array;
@ -116,11 +117,12 @@ public:
template <typename T> template <typename T>
T* makeArray(size_t count) { T* makeArray(size_t count) {
T* array = (T*)this->commonArrayAlloc<T>(count); uint32_t safeCount = SkTo<uint32_t>(count);
T* array = (T*)this->commonArrayAlloc<T>(safeCount);
// If T is primitive then the memory is initialized. For example, an array of chars will // If T is primitive then the memory is initialized. For example, an array of chars will
// be zeroed. // be zeroed.
for (size_t i = 0; i < count; i++) { for (size_t i = 0; i < safeCount; i++) {
new (&array[i]) T(); new (&array[i]) T();
} }
return array; return array;
@ -141,25 +143,24 @@ private:
void installUint32Footer(FooterAction* action, uint32_t value, uint32_t padding); void installUint32Footer(FooterAction* action, uint32_t value, uint32_t padding);
void installPtrFooter(FooterAction* action, char* ptr, uint32_t padding); void installPtrFooter(FooterAction* action, char* ptr, uint32_t padding);
void ensureSpace(size_t size, size_t alignment); void ensureSpace(uint32_t size, uint32_t alignment);
char* allocObject(size_t size, size_t alignment); char* allocObject(uint32_t size, uint32_t alignment);
char* allocObjectWithFooter(size_t sizeIncludingFooter, size_t alignment); char* allocObjectWithFooter(uint32_t sizeIncludingFooter, uint32_t alignment);
template <typename T> template <typename T>
char* commonArrayAlloc(size_t count) { char* commonArrayAlloc(uint32_t count) {
SkASSERT(SkTFitsIn<uint32_t>(count));
char* objStart; char* objStart;
size_t arraySize = count * sizeof(T); uint32_t arraySize = SkTo<uint32_t>(count * sizeof(T));
SkASSERT(SkTFitsIn<uint32_t>(arraySize)); uint32_t alignment = SkTo<uint32_t>(alignof(T));
if (skstd::is_trivially_destructible<T>::value) { if (skstd::is_trivially_destructible<T>::value) {
objStart = this->allocObject(arraySize, alignof(T)); objStart = this->allocObject(arraySize, alignment);
fCursor = objStart + arraySize; fCursor = objStart + arraySize;
} else { } else {
size_t totalSize = arraySize + sizeof(Footer) + sizeof(uint32_t); uint32_t totalSize = arraySize + sizeof(Footer) + sizeof(uint32_t);
objStart = this->allocObjectWithFooter(totalSize, alignof(T)); objStart = this->allocObjectWithFooter(totalSize, alignment);
// Can never be UB because max value is alignof(T). // Can never be UB because max value is alignof(T).
uint32_t padding = SkTo<uint32_t>(objStart - fCursor); uint32_t padding = SkTo<uint32_t>(objStart - fCursor);
@ -185,12 +186,12 @@ private:
return objStart; return objStart;
} }
char* fDtorCursor; char* fDtorCursor;
char* fCursor; char* fCursor;
char* fEnd; char* fEnd;
char* const fFirstBlock; char* const fFirstBlock;
const size_t fFirstSize; const uint32_t fFirstSize;
const size_t fExtraSize; const uint32_t fExtraSize;
}; };
#endif//SkFixedAlloc_DEFINED #endif//SkFixedAlloc_DEFINED