begin cleanup of malloc porting layer
1. Merge some of the allocators into sk_malloc_flags by redefining a flag to mean zero-init
2. Add more private helpers to simplify our call-sites (and handle some overflow mul checks)
3. The 2-param helpers rely on the saturating SkSafeMath::Mul to pass max_size_t as the request,
which should always fail.
Bug:508641
Change-Id: I322f1e6ed91113467e0fdb12c91c3dad33d890c8
Reviewed-on: https://skia-review.googlesource.com/90940
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Stephan Altmueller <stephana@google.com>
diff --git a/include/private/SkMalloc.h b/include/private/SkMalloc.h
index ba14d46..53c3d99 100644
--- a/include/private/SkMalloc.h
+++ b/include/private/SkMalloc.h
@@ -17,27 +17,53 @@
memory wrappers to be implemented by the porting layer (platform)
*/
-enum {
- SK_MALLOC_TEMP = 0x01, //!< hint to sk_malloc that the requested memory will be freed in the scope of the stack frame
- SK_MALLOC_THROW = 0x02 //!< instructs sk_malloc to not return normally if the memory cannot be allocated.
-};
-/** Return a block of memory (at least 4-byte aligned) of at least the
- specified size. If the requested memory cannot be returned, either
- return null (if SK_MALLOC_TEMP bit is clear) or throw an exception
- (if SK_MALLOC_TEMP bit is set). To free the memory, call sk_free().
-*/
-SK_API extern void* sk_malloc_flags(size_t size, unsigned flags);
-/** Same as sk_malloc(), but hard coded to pass SK_MALLOC_THROW as the flag
-*/
-SK_API extern void* sk_malloc_throw(size_t size);
-/** Same as standard realloc(), but this one never returns null on failure. It will throw
- an exception if it fails.
-*/
-SK_API extern void* sk_realloc_throw(void* buffer, size_t size);
-/** Free memory returned by sk_malloc(). It is safe to pass null.
-*/
+
+/** Free memory returned by sk_malloc(). It is safe to pass null. */
SK_API extern void sk_free(void*);
+/**
+ * Called internally if we run out of memory. The platform implementation must
+ * not return, but should either throw an exception or otherwise exit.
+ */
+SK_API extern void sk_out_of_memory(void);
+
+enum {
+#ifdef SK_SUPPORT_LEGACY_MALLOC_PORTING_LAYER
+ SK_MALLOC_TEMP = 1,
+#else
+ /**
+ * If this bit is set, the returned buffer must be zero-initialized. If this bit is not set
+ * the buffer can be uninitialized.
+ */
+ SK_MALLOC_ZERO_INITIALIZE = 1 << 0,
+#endif
+
+ /**
+ * If this bit is set, the implementation must throw/crash/quit if the request cannot
+ * be fulfilled. If this bit is not set, then it should return nullptr on failure.
+ */
+ SK_MALLOC_THROW = 1 << 1,
+};
+/**
+ * Return a block of memory (at least 4-byte aligned) of at least the specified size.
+ * If the requested memory cannot be returned, either return nullptr or throw/exit, depending
+ * on the SK_MALLOC_THROW bit. If the allocation succeeds, the memory will be zero-initialized
+ * if the SK_MALLOC_ZERO_INITIALIZE bit was set.
+ *
+ * To free the memory, call sk_free()
+ */
+SK_API extern void* sk_malloc_flags(size_t size, unsigned flags);
+
+/** Same as standard realloc(), but this one never returns null on failure. It will throw
+ * an exception if it fails.
+ */
+SK_API extern void* sk_realloc_throw(void* buffer, size_t size);
+
+#ifdef SK_SUPPORT_LEGACY_MALLOC_PORTING_LAYER
+
+/** Same as sk_malloc_flags(), but hard coded to pass SK_MALLOC_THROW as the flag */
+SK_API extern void* sk_malloc_throw(size_t size);
+
/** Much like calloc: returns a pointer to at least size zero bytes, or NULL on failure.
*/
SK_API extern void* sk_calloc(size_t size);
@@ -46,10 +72,36 @@
*/
SK_API extern void* sk_calloc_throw(size_t size);
-/** Called internally if we run out of memory. The platform implementation must
- not return, but should either throw an exception or otherwise exit.
-*/
-SK_API extern void sk_out_of_memory(void);
+#else
+static inline void* sk_malloc_throw(size_t size) {
+ return sk_malloc_flags(size, SK_MALLOC_THROW);
+}
+
+static inline void* sk_calloc_throw(size_t size) {
+ return sk_malloc_flags(size, SK_MALLOC_THROW | SK_MALLOC_ZERO_INITIALIZE);
+}
+#endif
+
+static inline void* sk_calloc_canfail(size_t size) {
+#ifdef SK_SUPPORT_LEGACY_MALLOC_PORTING_LAYER
+ return sk_calloc(size);
+#else
+ return sk_malloc_flags(size, SK_MALLOC_ZERO_INITIALIZE);
+#endif
+}
+
+// Performs a safe multiply count * elemSize, checking for overflow
+SK_API extern void* sk_calloc_throw(size_t count, size_t elemSize);
+SK_API extern void* sk_malloc_throw(size_t count, size_t elemSize);
+SK_API extern void* sk_realloc_throw(void* buffer, size_t count, size_t elemSize);
+
+/**
+ * These variants return nullptr on failure
+ */
+static inline void* sk_malloc_canfail(size_t size) {
+ return sk_malloc_flags(size, 0);
+}
+SK_API extern void* sk_malloc_canfail(size_t count, size_t elemSize);
// bzero is safer than memset, but we can't rely on it, so... sk_bzero()
static inline void sk_bzero(void* buffer, size_t size) {
diff --git a/include/private/SkTArray.h b/include/private/SkTArray.h
index 3f4cc42..68dab9a 100644
--- a/include/private/SkTArray.h
+++ b/include/private/SkTArray.h
@@ -445,7 +445,7 @@
fReserved = false;
} else {
fAllocCount = SkTMax(count, SkTMax(kMinHeapAllocCount, reserveCount));
- fMemArray = sk_malloc_throw(fAllocCount * sizeof(T));
+ fMemArray = sk_malloc_throw(fAllocCount, sizeof(T));
fOwnMemory = true;
fReserved = reserveCount > 0;
}
@@ -460,7 +460,7 @@
fReserved = false;
if (count > preallocCount) {
fAllocCount = SkTMax(count, kMinHeapAllocCount);
- fMemArray = sk_malloc_throw(fAllocCount * sizeof(T));
+ fMemArray = sk_malloc_throw(fAllocCount, sizeof(T));
fOwnMemory = true;
} else {
fAllocCount = preallocCount;
@@ -537,7 +537,7 @@
return;
}
fAllocCount = newAllocCount;
- void* newMemArray = sk_malloc_throw(fAllocCount * sizeof(T));
+ void* newMemArray = sk_malloc_throw(fAllocCount, sizeof(T));
this->move(newMemArray);
if (fOwnMemory) {
sk_free(fMemArray);
diff --git a/include/private/SkTemplates.h b/include/private/SkTemplates.h
index 8a605fb..3f70203 100644
--- a/include/private/SkTemplates.h
+++ b/include/private/SkTemplates.h
@@ -175,12 +175,7 @@
}
if (count > kCount) {
- const uint64_t size64 = sk_64_mul(count, sizeof(T));
- const size_t size = static_cast<size_t>(size64);
- if (size != size64) {
- sk_out_of_memory();
- }
- fArray = (T*) sk_malloc_throw(size);
+ fArray = (T*) sk_malloc_throw(count, sizeof(T));
} else if (count > 0) {
fArray = (T*) fStorage;
} else {
@@ -250,7 +245,7 @@
/** Allocates space for 'count' Ts. */
explicit SkAutoTMalloc(size_t count) {
- fPtr = count ? (T*)sk_malloc_flags(count * sizeof(T), SK_MALLOC_THROW) : nullptr;
+ fPtr = count ? (T*)sk_malloc_throw(count, sizeof(T)) : nullptr;
}
SkAutoTMalloc(SkAutoTMalloc<T>&& that) : fPtr(that.release()) {}
@@ -271,7 +266,7 @@
/** Resize the memory area pointed to by the current ptr without preserving contents. */
T* reset(size_t count = 0) {
sk_free(fPtr);
- fPtr = count ? (T*)sk_malloc_flags(count * sizeof(T), SK_MALLOC_THROW) : nullptr;
+ fPtr = count ? (T*)sk_malloc_throw(count, sizeof(T)) : nullptr;
return fPtr;
}
@@ -322,7 +317,7 @@
SkAutoSTMalloc(size_t count) {
if (count > kCount) {
- fPtr = (T*)sk_malloc_flags(count * sizeof(T), SK_MALLOC_THROW | SK_MALLOC_TEMP);
+ fPtr = (T*)sk_malloc_throw(count, sizeof(T));
} else if (count) {
fPtr = fTStorage;
} else {
@@ -342,7 +337,7 @@
sk_free(fPtr);
}
if (count > kCount) {
- fPtr = (T*)sk_malloc_throw(count * sizeof(T));
+ fPtr = (T*)sk_malloc_throw(count, sizeof(T));
} else if (count) {
fPtr = fTStorage;
} else {
@@ -373,14 +368,14 @@
void realloc(size_t count) {
if (count > kCount) {
if (fPtr == fTStorage) {
- fPtr = (T*)sk_malloc_throw(count * sizeof(T));
+ fPtr = (T*)sk_malloc_throw(count, sizeof(T));
memcpy(fPtr, fTStorage, kCount * sizeof(T));
} else {
- fPtr = (T*)sk_realloc_throw(fPtr, count * sizeof(T));
+ fPtr = (T*)sk_realloc_throw(fPtr, count, sizeof(T));
}
} else if (count) {
if (fPtr != fTStorage) {
- fPtr = (T*)sk_realloc_throw(fPtr, count * sizeof(T));
+ fPtr = (T*)sk_realloc_throw(fPtr, count, sizeof(T));
}
} else {
this->reset(0);
diff --git a/public.bzl b/public.bzl
index 1e28940..d856db2 100644
--- a/public.bzl
+++ b/public.bzl
@@ -586,6 +586,7 @@
"SK_JUMPER_DISABLE_8BIT",
# JPEG is in codec_limited
"SK_HAS_JPEG_LIBRARY",
+ "SK_SUPPORT_LEGACY_MALLOC_PORTING_LAYER",
] + skia_select(
os_conditions,
[
diff --git a/src/codec/SkBmpBaseCodec.cpp b/src/codec/SkBmpBaseCodec.cpp
index 1071ff3..c548514 100644
--- a/src/codec/SkBmpBaseCodec.cpp
+++ b/src/codec/SkBmpBaseCodec.cpp
@@ -13,5 +13,5 @@
std::unique_ptr<SkStream> stream,
uint16_t bitsPerPixel, SkCodec::SkScanlineOrder rowOrder)
: INHERITED(width, height, info, std::move(stream), bitsPerPixel, rowOrder)
- , fSrcBuffer(sk_malloc_flags(this->srcRowBytes(), 0))
+ , fSrcBuffer(sk_malloc_canfail(this->srcRowBytes()))
{}
diff --git a/src/codec/SkIcoCodec.cpp b/src/codec/SkIcoCodec.cpp
index 1b9321c..af91255 100644
--- a/src/codec/SkIcoCodec.cpp
+++ b/src/codec/SkIcoCodec.cpp
@@ -55,8 +55,7 @@
uint32_t offset;
uint32_t size;
};
- SkAutoFree dirEntryBuffer(sk_malloc_flags(sizeof(Entry) * numImages,
- SK_MALLOC_TEMP));
+ SkAutoFree dirEntryBuffer(sk_malloc_canfail(sizeof(Entry) * numImages));
if (!dirEntryBuffer) {
SkCodecPrintf("Error: OOM allocating ICO directory for %i images.\n",
numImages);
@@ -131,7 +130,7 @@
bytesRead = offset;
// Create a new stream for the embedded codec
- SkAutoFree buffer(sk_malloc_flags(size, 0));
+ SkAutoFree buffer(sk_malloc_canfail(size));
if (!buffer) {
SkCodecPrintf("Warning: OOM trying to create embedded stream.\n");
break;
diff --git a/src/core/SkAutoMalloc.h b/src/core/SkAutoMalloc.h
index 03cbf79..f5c8729 100644
--- a/src/core/SkAutoMalloc.h
+++ b/src/core/SkAutoMalloc.h
@@ -144,7 +144,7 @@
SkASSERT(fPtr != fStorage); // otherwise we lied when setting didChangeAlloc.
fPtr = fStorage;
} else {
- fPtr = sk_malloc_flags(size, SK_MALLOC_THROW | SK_MALLOC_TEMP);
+ fPtr = sk_malloc_throw(size);
}
fSize = size;
diff --git a/src/core/SkAutoPixmapStorage.cpp b/src/core/SkAutoPixmapStorage.cpp
index 47f41e7..7219423 100644
--- a/src/core/SkAutoPixmapStorage.cpp
+++ b/src/core/SkAutoPixmapStorage.cpp
@@ -40,7 +40,7 @@
if (SkImageInfo::ByteSizeOverflowed(size)) {
return false;
}
- void* pixels = sk_malloc_flags(size, 0);
+ void* pixels = sk_malloc_canfail(size);
if (nullptr == pixels) {
return false;
}
diff --git a/src/core/SkBitmapCache.cpp b/src/core/SkBitmapCache.cpp
index 210787e..cb88070 100644
--- a/src/core/SkBitmapCache.cpp
+++ b/src/core/SkBitmapCache.cpp
@@ -301,7 +301,7 @@
if (factory) {
dm.reset(factory(size));
} else {
- block = sk_malloc_flags(size, 0);
+ block = sk_malloc_canfail(size);
}
if (!dm && !block) {
return nullptr;
diff --git a/src/core/SkMallocPixelRef.cpp b/src/core/SkMallocPixelRef.cpp
index 6e402c9..2933e48 100644
--- a/src/core/SkMallocPixelRef.cpp
+++ b/src/core/SkMallocPixelRef.cpp
@@ -8,8 +8,27 @@
#include "SkMallocPixelRef.h"
#include "SkBitmap.h"
#include "SkReadBuffer.h"
+#include "SkSafeMath.h"
#include "SkWriteBuffer.h"
+void* sk_calloc_throw(size_t count, size_t elemSize) {
+ return sk_calloc_throw(SkSafeMath::Mul(count, elemSize));
+}
+
+void* sk_malloc_throw(size_t count, size_t elemSize) {
+ return sk_malloc_throw(SkSafeMath::Mul(count, elemSize));
+}
+
+void* sk_realloc_throw(void* buffer, size_t count, size_t elemSize) {
+ return sk_realloc_throw(buffer, SkSafeMath::Mul(count, elemSize));
+}
+
+void* sk_malloc_canfail(size_t count, size_t elemSize) {
+ return sk_malloc_canfail(SkSafeMath::Mul(count, elemSize));
+}
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
+
// assumes ptr was allocated via sk_malloc
static void sk_free_releaseproc(void* ptr, void*) {
sk_free(ptr);
@@ -63,15 +82,13 @@
sk_free_releaseproc, nullptr));
}
-sk_sp<SkPixelRef> SkMallocPixelRef::MakeAllocate(const SkImageInfo& info,
- size_t rowBytes) {
- auto sk_malloc_nothrow = [](size_t size) { return sk_malloc_flags(size, 0); };
- return MakeUsing(sk_malloc_nothrow, info, rowBytes);
+sk_sp<SkPixelRef> SkMallocPixelRef::MakeAllocate(const SkImageInfo& info, size_t rowBytes) {
+ return MakeUsing(sk_malloc_canfail, info, rowBytes);
}
sk_sp<SkPixelRef> SkMallocPixelRef::MakeZeroed(const SkImageInfo& info,
size_t rowBytes) {
- return MakeUsing(sk_calloc, info, rowBytes);
+ return MakeUsing(sk_calloc_canfail, info, rowBytes);
}
static void sk_data_releaseproc(void*, void* dataPtr) {
@@ -93,8 +110,8 @@
}
sk_sp<SkPixelRef> SkMallocPixelRef::MakeWithData(const SkImageInfo& info,
- size_t rowBytes,
- sk_sp<SkData> data) {
+ size_t rowBytes,
+ sk_sp<SkData> data) {
SkASSERT(data != nullptr);
if (!is_valid(info)) {
return nullptr;
diff --git a/src/core/SkRegion_path.cpp b/src/core/SkRegion_path.cpp
index fec7825..e6af059 100644
--- a/src/core/SkRegion_path.cpp
+++ b/src/core/SkRegion_path.cpp
@@ -144,12 +144,7 @@
}
fStorageCount = sk_64_asS32(count);
- int64_t size = sk_64_mul(fStorageCount, sizeof(SkRegion::RunType));
- if (size < 0 || !sk_64_isS32(size)) {
- return false;
- }
-
- fStorage = (SkRegion::RunType*)sk_malloc_flags(sk_64_asS32(size), 0);
+ fStorage = (SkRegion::RunType*)sk_malloc_canfail(fStorageCount, sizeof(SkRegion::RunType));
if (nullptr == fStorage) {
return false;
}
diff --git a/src/gpu/GrBuffer.cpp b/src/gpu/GrBuffer.cpp
index c92b296..cd7862a 100644
--- a/src/gpu/GrBuffer.cpp
+++ b/src/gpu/GrBuffer.cpp
@@ -16,7 +16,7 @@
if (gpu->caps()->mustClearUploadedBufferData()) {
cpuData = sk_calloc_throw(sizeInBytes);
} else {
- cpuData = sk_malloc_flags(sizeInBytes, SK_MALLOC_THROW);
+ cpuData = sk_malloc_throw(sizeInBytes);
}
if (data) {
memcpy(cpuData, data, sizeInBytes);
diff --git a/src/lazy/SkDiscardableMemoryPool.cpp b/src/lazy/SkDiscardableMemoryPool.cpp
index 6e5582a..394e325 100644
--- a/src/lazy/SkDiscardableMemoryPool.cpp
+++ b/src/lazy/SkDiscardableMemoryPool.cpp
@@ -164,7 +164,7 @@
}
std::unique_ptr<SkDiscardableMemory> DiscardableMemoryPool::make(size_t bytes) {
- SkAutoFree addr(sk_malloc_flags(bytes, 0));
+ SkAutoFree addr(sk_malloc_canfail(bytes));
if (nullptr == addr) {
return nullptr;
}
diff --git a/src/ports/SkMemory_malloc.cpp b/src/ports/SkMemory_malloc.cpp
index 05c69d5..aa1cfac 100644
--- a/src/ports/SkMemory_malloc.cpp
+++ b/src/ports/SkMemory_malloc.cpp
@@ -54,10 +54,6 @@
#endif
}
-void* sk_malloc_throw(size_t size) {
- return sk_malloc_flags(size, SK_MALLOC_THROW);
-}
-
void* sk_realloc_throw(void* addr, size_t size) {
return throw_on_failure(size, realloc(addr, size));
}
@@ -69,7 +65,16 @@
}
void* sk_malloc_flags(size_t size, unsigned flags) {
- void* p = malloc(size);
+ void* p;
+#ifdef SK_SUPPORT_LEGACY_MALLOC_PORTING_LAYER
+ p = malloc(size);
+#else
+ if (flags & SK_MALLOC_ZERO_INITIALIZE) {
+ p = calloc(size, 1);
+ } else {
+ p = malloc(size);
+ }
+#endif
if (flags & SK_MALLOC_THROW) {
return throw_on_failure(size, p);
} else {
@@ -77,10 +82,15 @@
}
}
+#ifdef SK_SUPPORT_LEGACY_MALLOC_PORTING_LAYER
+void* sk_malloc_throw(size_t size) {
+ return sk_malloc_flags(size, SK_MALLOC_THROW);
+}
void* sk_calloc(size_t size) {
return calloc(size, 1);
}
-
void* sk_calloc_throw(size_t size) {
return throw_on_failure(size, sk_calloc(size));
}
+#endif
+
diff --git a/src/ports/SkMemory_mozalloc.cpp b/src/ports/SkMemory_mozalloc.cpp
index bf5971b..e13b946 100644
--- a/src/ports/SkMemory_mozalloc.cpp
+++ b/src/ports/SkMemory_mozalloc.cpp
@@ -22,20 +22,26 @@
mozalloc_handle_oom(0);
}
-void* sk_malloc_throw(size_t size) {
- return sk_malloc_flags(size, SK_MALLOC_THROW);
+void sk_free(void* p) {
+ free(p);
}
void* sk_realloc_throw(void* addr, size_t size) {
return moz_xrealloc(addr, size);
}
-void sk_free(void* p) {
- free(p);
+void* sk_malloc_flags(size_t size, unsigned flags) {
+#ifndef SK_SUPPORT_LEGACY_MALLOC_PORTING_LAYER
+ if (flags & SK_MALLOC_ZERO_INITIALIZE) {
+ return (flags & SK_MALLOC_THROW) ? moz_xcalloc(size, 1) : calloc(size, 1);
+ }
+#endif
+ return (flags & SK_MALLOC_THROW) ? moz_xmalloc(size) : malloc(size);
}
-void* sk_malloc_flags(size_t size, unsigned flags) {
- return (flags & SK_MALLOC_THROW) ? moz_xmalloc(size) : malloc(size);
+#ifdef SK_SUPPORT_LEGACY_MALLOC_PORTING_LAYER
+void* sk_malloc_throw(size_t size) {
+ return sk_malloc_flags(size, SK_MALLOC_THROW);
}
void* sk_calloc(size_t size) {
@@ -45,3 +51,5 @@
void* sk_calloc_throw(size_t size) {
return moz_xcalloc(size, 1);
}
+#endif
+