Skip to content

Commit 26c24ed

Browse files
fmalitaSkia Commit-Bot
authored andcommitted
SkArenaAlloc aligned under-allocation
We cannot rely on natural allocation alignment (8) after footer installation. Always compute and apply explicit alignment. Bug: chromium:1124776 Change-Id: I5ff880409df83284b1f9668a771c2a750f72c148 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315646 Commit-Queue: Florin Malita <fmalita@chromium.org> Commit-Queue: Florin Malita <fmalita@google.com> Reviewed-by: Herb Derby <herb@google.com>
1 parent bbbc28e commit 26c24ed

File tree

3 files changed

+16
-10
lines changed

3 files changed

+16
-10
lines changed

src/core/SkArenaAlloc.cpp

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,19 +74,14 @@ char* SkArenaAlloc::NextBlock(char* footerEnd) {
7474

7575
void SkArenaAlloc::ensureSpace(uint32_t size, uint32_t alignment) {
7676
constexpr uint32_t headerSize = sizeof(Footer) + sizeof(ptrdiff_t);
77-
// The chrome c++ library we use does not define std::max_align_t.
78-
// This must be conservative to add the right amount of extra memory to handle the alignment
79-
// padding.
80-
constexpr uint32_t alignof_max_align_t = 8;
8177
constexpr uint32_t maxSize = std::numeric_limits<uint32_t>::max();
8278
constexpr uint32_t overhead = headerSize + sizeof(Footer);
8379
AssertRelease(size <= maxSize - overhead);
8480
uint32_t objSizeAndOverhead = size + overhead;
85-
if (alignment > alignof_max_align_t) {
86-
uint32_t alignmentOverhead = alignment - 1;
87-
AssertRelease(objSizeAndOverhead <= maxSize - alignmentOverhead);
88-
objSizeAndOverhead += alignmentOverhead;
89-
}
81+
82+
const uint32_t alignmentOverhead = alignment - 1;
83+
AssertRelease(objSizeAndOverhead <= maxSize - alignmentOverhead);
84+
objSizeAndOverhead += alignmentOverhead;
9085

9186
uint32_t minAllocationSize = fNextHeapAlloc;
9287

src/core/SkArenaAlloc.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#ifndef SkArenaAlloc_DEFINED
99
#define SkArenaAlloc_DEFINED
1010

11+
#include "include/core/SkTypes.h"
1112
#include "include/private/SkTFitsIn.h"
1213

1314
#include <array>
@@ -170,7 +171,13 @@ class SkArenaAlloc {
170171
this->ensureSpace(size, alignment);
171172
alignedOffset = (~reinterpret_cast<uintptr_t>(fCursor) + 1) & mask;
172173
}
173-
return fCursor + alignedOffset;
174+
175+
char* object = fCursor + alignedOffset;
176+
177+
SkASSERT((reinterpret_cast<uintptr_t>(object) & (alignment - 1)) == 0);
178+
SkASSERT(object + size <= fEnd);
179+
180+
return object;
174181
}
175182

176183
char* allocObjectWithFooter(uint32_t sizeIncludingFooter, uint32_t alignment);

tests/ArenaAllocTest.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,4 +180,8 @@ DEF_TEST(ArenaAlloc, r) {
180180
REPORTER_ASSERT(r, created == 128);
181181
REPORTER_ASSERT(r, destroyed == 128);
182182

183+
{
184+
SkArenaAlloc arena(4096);
185+
arena.makeBytesAlignedTo(4081, 8);
186+
}
183187
}

0 commit comments

Comments
 (0)