commit 0bc4d60fe0a1ceae6dde0103c3fdf78ee1673ffa
author Herb Derby <herb@google.com> Tue Aug 15 16:21:44 2017 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Tue Aug 15 20:40:27 2017 +0000
tree 7968da25f0e1a8aaeec3e727dd13166ee9965629
parent 3ad19e59571f1a2952209b1e25ff54f0da52acf4

Fix bogus math in object allocation.

When a size_t is convert from a very large number to ptrdiff_t, it
becomes negative causing the existing block to be used instead of
allocating a new block.

BUG=chromium:744109

Change-Id: I0bf98e3fb924851c162f6eca43d29a3f40dc9eaa
Reviewed-on: https://skia-review.googlesource.com/34541
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>

src/core/SkArenaAlloc.h

diff --git a/src/core/SkArenaAlloc.h b/src/core/SkArenaAlloc.h
index 414e8c1..9e97161 100644
--- a/src/core/SkArenaAlloc.h
+++ b/src/core/SkArenaAlloc.h
@@ -156,12 +156,16 @@
 
     char* allocObject(uint32_t size, uint32_t alignment) {
         uintptr_t mask = alignment - 1;
-        char* objStart = (char*)((uintptr_t)(fCursor + mask) & ~mask);
-        if ((ptrdiff_t)size > fEnd - objStart) {
-            this->ensureSpace(size, alignment);
-            objStart = (char*)((uintptr_t)(fCursor + mask) & ~mask);
+        uintptr_t alignedOffset = (~reinterpret_cast<uintptr_t>(fCursor) + 1) & mask;
+        uintptr_t totalSize = size + alignedOffset;
+        if (totalSize < size) {
+            SK_ABORT("The total size of allocation overflowed uintptr_t.");
         }
-        return objStart;
+        if (totalSize > static_cast<uintptr_t>(fEnd - fCursor)) {
+            this->ensureSpace(size, alignment);
+            alignedOffset = (~reinterpret_cast<uintptr_t>(fCursor) + 1) & mask;
+        }
+        return fCursor + alignedOffset;
     }
 
     char* allocObjectWithFooter(uint32_t sizeIncludingFooter, uint32_t alignment);

tests/ArenaAllocTest.cpp

diff --git a/tests/ArenaAllocTest.cpp b/tests/ArenaAllocTest.cpp
index 137e60e..b565095 100644
--- a/tests/ArenaAllocTest.cpp
+++ b/tests/ArenaAllocTest.cpp
@@ -181,3 +181,12 @@
     REPORTER_ASSERT(r, created == 1);
     REPORTER_ASSERT(r, destroyed == 1);
 }
+
+// Chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=744109
+// Must be compiled with target_cpu = "x86" to fail with segfault.
+DEF_TEST(ArenaAllocReallyBigAlloc, r) {
+    SkSTArenaAlloc<64> arena;
+    using T = struct {int f[1<<28];};
+    auto a = arena.makeArrayDefault<T>(3);
+    a[2].f[(1<<28) - 1] = 0;
+}