aboutsummaryrefslogtreecommitdiffhomepage
path: root/src
diff options
context:
space:
mode:
authorGravatar Greg Daniel <egdaniel@google.com>2017-09-26 20:07:58 +0000
committerGravatar Skia Commit-Bot <skia-commit-bot@chromium.org>2017-09-26 20:08:07 +0000
commitf46633f8af5296341e33ec4cdb69c71dfa997396 (patch)
treed6d59760f023f6855d921253d684716ae93bdc28 /src
parent58e23039bd4a8aee2c67fa6c9e5105b925d2e561 (diff)
Revert "guard old apis for querying byte-size of a bitmap/imageinfo/pixmap"
This reverts commit 98a6216b18b57c2f7a0d58f542c60503686aed69. Reason for revert: breaking the chrome roll. Looks like they may be writing data to create an image across all the row bytes and thus writing to unalloced data on the last row. Link to example failing bot: https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/539960 Original change's description: > guard old apis for querying byte-size of a bitmap/imageinfo/pixmap > > Previously we had size_t and uint64_t variations. > > The new (simpler) API always.. > - returns size_t, or 0 if the calculation overflowed > - returns the trimmed size (does not include rowBytes padding for the last row) > > Bug: skia: > Change-Id: I05173e877918327c7b207d2f7f1ab0db36892e2e > Reviewed-on: https://skia-review.googlesource.com/50980 > Commit-Queue: Mike Reed <reed@google.com> > Reviewed-by: Florin Malita <fmalita@chromium.org> > Reviewed-by: Leon Scroggins <scroggo@google.com> TBR=mtklein@google.com,herb@google.com,scroggo@google.com,fmalita@chromium.org,reed@google.com Change-Id: I726f6ab1b36b14979ba6f37105e0a469b3f0dbc0 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia: Reviewed-on: https://skia-review.googlesource.com/51262 Reviewed-by: Greg Daniel <egdaniel@google.com> Commit-Queue: Greg Daniel <egdaniel@google.com>
Diffstat (limited to 'src')
-rw-r--r--src/android/SkBitmapRegionCodec.cpp2
-rw-r--r--src/codec/SkIcoCodec.cpp2
-rw-r--r--src/codec/SkSampler.cpp4
-rw-r--r--src/codec/SkWebpCodec.cpp2
-rw-r--r--src/core/SkAutoPixmapStorage.cpp4
-rw-r--r--src/core/SkBitmap.cpp2
-rw-r--r--src/core/SkBitmapCache.cpp4
-rw-r--r--src/core/SkImageInfo.cpp12
-rw-r--r--src/core/SkMallocPixelRef.cpp46
-rw-r--r--src/core/SkScalerContext.cpp2
-rw-r--r--src/core/SkSpecialImage.cpp2
-rw-r--r--src/gpu/ops/GrSmallPathRenderer.cpp4
-rw-r--r--src/image/SkImage.cpp5
-rw-r--r--src/image/SkImage_Gpu.cpp17
-rw-r--r--src/image/SkImage_Raster.cpp4
-rw-r--r--src/image/SkSurface_Raster.cpp2
-rw-r--r--src/utils/mac/SkCreateCGImageRef.cpp2
17 files changed, 56 insertions, 60 deletions
diff --git a/src/android/SkBitmapRegionCodec.cpp b/src/android/SkBitmapRegionCodec.cpp
index 493e4b5672..f77c70264f 100644
--- a/src/android/SkBitmapRegionCodec.cpp
+++ b/src/android/SkBitmapRegionCodec.cpp
@@ -96,7 +96,7 @@ bool SkBitmapRegionCodec::decodeRegion(SkBitmap* bitmap, SkBRDAllocator* allocat
if (SubsetType::kPartiallyInside_SubsetType == type &&
SkCodec::kNo_ZeroInitialized == zeroInit) {
void* pixels = bitmap->getPixels();
- size_t bytes = outInfo.computeByteSize(bitmap->rowBytes());
+ size_t bytes = outInfo.getSafeSize(bitmap->rowBytes());
memset(pixels, 0, bytes);
}
diff --git a/src/codec/SkIcoCodec.cpp b/src/codec/SkIcoCodec.cpp
index da6d0c360c..74affe6709 100644
--- a/src/codec/SkIcoCodec.cpp
+++ b/src/codec/SkIcoCodec.cpp
@@ -173,7 +173,7 @@ std::unique_ptr<SkCodec> SkIcoCodec::MakeFromStream(std::unique_ptr<SkStream> st
int maxIndex = 0;
for (int i = 0; i < codecs->count(); i++) {
SkImageInfo info = codecs->operator[](i)->getInfo();
- size_t size = info.computeMinByteSize();
+ size_t size = info.getSafeSize(info.minRowBytes());
if (size > maxSize) {
maxSize = size;
diff --git a/src/codec/SkSampler.cpp b/src/codec/SkSampler.cpp
index d18410be3b..c7d9a3ac23 100644
--- a/src/codec/SkSampler.cpp
+++ b/src/codec/SkSampler.cpp
@@ -14,8 +14,8 @@ void SkSampler::Fill(const SkImageInfo& info, void* dst, size_t rowBytes,
uint64_t colorOrIndex, SkCodec::ZeroInitialized zeroInit) {
SkASSERT(dst != nullptr);
- // Calculate bytes to fill.
- const size_t bytesToFill = info.computeByteSize(rowBytes);
+ // Calculate bytes to fill. We use getSafeSize since the last row may not be padded.
+ const size_t bytesToFill = info.getSafeSize(rowBytes);
const int width = info.width();
const int numRows = info.height();
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index 864fa96a1d..f7082de42a 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -542,7 +542,7 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
config.output.u.RGBA.rgba = reinterpret_cast<uint8_t*>(webpDst.getAddr(dstX, dstY));
config.output.u.RGBA.stride = static_cast<int>(webpDst.rowBytes());
- config.output.u.RGBA.size = webpDst.computeByteSize();
+ config.output.u.RGBA.size = webpDst.getSafeSize();
SkAutoTCallVProc<WebPIDecoder, WebPIDelete> idec(WebPIDecode(nullptr, 0, &config));
if (!idec) {
diff --git a/src/core/SkAutoPixmapStorage.cpp b/src/core/SkAutoPixmapStorage.cpp
index df0c0fa878..be13e7197f 100644
--- a/src/core/SkAutoPixmapStorage.cpp
+++ b/src/core/SkAutoPixmapStorage.cpp
@@ -29,7 +29,7 @@ size_t SkAutoPixmapStorage::AllocSize(const SkImageInfo& info, size_t* rowBytes)
if (rowBytes) {
*rowBytes = rb;
}
- return info.computeByteSize(rb);
+ return info.getSafeSize(rb);
}
bool SkAutoPixmapStorage::tryAlloc(const SkImageInfo& info) {
@@ -58,7 +58,7 @@ const SkData* SkAutoPixmapStorage::detachPixelsAsData() {
return nullptr;
}
- auto data = SkData::MakeFromMalloc(fStorage, this->computeByteSize());
+ auto data = SkData::MakeFromMalloc(fStorage, this->getSafeSize());
fStorage = nullptr;
this->INHERITED::reset();
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 4a82d54990..c3bc4a9d63 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -675,7 +675,7 @@ bool SkBitmap::ReadRawPixels(SkReadBuffer* buffer, SkBitmap* bitmap) {
}
// write_raw_pixels() always writes snug buffers with rowBytes == minRowBytes().
- size_t bytes = info.computeMinByteSize();
+ size_t bytes = info.getSafeSize(info.minRowBytes());
if (!buffer->validate(bytes != 0)) {
return false;
}
diff --git a/src/core/SkBitmapCache.cpp b/src/core/SkBitmapCache.cpp
index 8d3991a9d5..b8767644a2 100644
--- a/src/core/SkBitmapCache.cpp
+++ b/src/core/SkBitmapCache.cpp
@@ -160,7 +160,7 @@ public:
const Key& getKey() const override { return fKey; }
size_t bytesUsed() const override {
- return sizeof(fKey) + fInfo.computeByteSize(fRowBytes);
+ return sizeof(fKey) + fInfo.getSafeSize(fRowBytes);
}
bool canBePurged() override {
SkAutoMutexAcquire ama(fMutex);
@@ -289,7 +289,7 @@ SkBitmapCache::RecPtr SkBitmapCache::Alloc(const SkBitmapCacheDesc& desc, const
}
const size_t rb = info.minRowBytes();
- size_t size = info.computeByteSize(rb);
+ size_t size = info.getSafeSize(rb);
if (0 == size) {
return nullptr;
}
diff --git a/src/core/SkImageInfo.cpp b/src/core/SkImageInfo.cpp
index 353397e99d..1baf0b77de 100644
--- a/src/core/SkImageInfo.cpp
+++ b/src/core/SkImageInfo.cpp
@@ -70,18 +70,6 @@ static SkColorType stored_to_live(unsigned stored) {
///////////////////////////////////////////////////////////////////////////////////////////////////
-#include "SkSafeMath.h"
-
-size_t SkImageInfo::computeByteSize(size_t rowBytes) const {
- if (0 == fHeight) {
- return 0;
- }
- SkSafeMath safe;
- size_t bytes = safe.add(safe.mul(fHeight - 1, rowBytes),
- safe.mul(fWidth, this->bytesPerPixel()));
- return safe ? bytes : 0;
-}
-
static bool alpha_type_is_valid(SkAlphaType alphaType) {
return (alphaType >= kUnknown_SkAlphaType) && (alphaType <= kLastEnum_SkAlphaType);
}
diff --git a/src/core/SkMallocPixelRef.cpp b/src/core/SkMallocPixelRef.cpp
index 565a757ec2..6928f38ee2 100644
--- a/src/core/SkMallocPixelRef.cpp
+++ b/src/core/SkMallocPixelRef.cpp
@@ -35,28 +35,37 @@ sk_sp<SkPixelRef> SkMallocPixelRef::MakeDirect(const SkImageInfo& info,
}
-sk_sp<SkPixelRef> SkMallocPixelRef::MakeUsing(void*(*allocProc)(size_t),
- const SkImageInfo& info,
- size_t rowBytes) {
- if (rowBytes == 0) {
- rowBytes = info.minRowBytes();
+ sk_sp<SkPixelRef> SkMallocPixelRef::MakeUsing(void*(*alloc)(size_t),
+ const SkImageInfo& info,
+ size_t requestedRowBytes) {
+ if (!is_valid(info)) {
+ return nullptr;
}
- if (!is_valid(info) || !info.validRowBytes(rowBytes)) {
- return nullptr;
+ // only want to permit 31bits of rowBytes
+ int64_t minRB = (int64_t)info.minRowBytes64();
+ if (minRB < 0 || !sk_64_isS32(minRB)) {
+ return nullptr; // allocation will be too large
+ }
+ if (requestedRowBytes > 0 && (int32_t)requestedRowBytes < minRB) {
+ return nullptr; // cannot meet requested rowbytes
}
- size_t size = 0;
- // if the info is empty, or rowBytes is 0 (which can be valid), then we don't need to compute
- // a size.
- if (!info.isEmpty() && rowBytes > 0) {
- size = info.computeByteSize(rowBytes);
- if (size == 0) {
- return nullptr; // overflow
- }
+ int32_t rowBytes;
+ if (requestedRowBytes) {
+ rowBytes = SkToS32(requestedRowBytes);
+ } else {
+ rowBytes = minRB;
}
- void* addr = allocProc(size);
+ int64_t bigSize = (int64_t)info.height() * rowBytes;
+ if (!sk_64_isS32(bigSize)) {
+ return nullptr;
+ }
+
+ size_t size = sk_64_asS32(bigSize);
+ SkASSERT(size >= info.getSafeSize(rowBytes));
+ void* addr = alloc(size);
if (nullptr == addr) {
return nullptr;
}
@@ -98,11 +107,10 @@ sk_sp<SkPixelRef> SkMallocPixelRef::MakeWithData(const SkImageInfo& info,
size_t rowBytes,
sk_sp<SkData> data) {
SkASSERT(data != nullptr);
- if (!is_valid(info) || !info.validRowBytes(rowBytes)) {
+ if (!is_valid(info)) {
return nullptr;
}
- size_t sizeNeeded = info.computeByteSize(rowBytes);
- if (sizeNeeded == 0 || sizeNeeded > data->size()) {
+ if ((rowBytes < info.minRowBytes()) || (data->size() < info.getSafeSize(rowBytes))) {
return nullptr;
}
// must get this address before we call release
diff --git a/src/core/SkScalerContext.cpp b/src/core/SkScalerContext.cpp
index 460907f164..98036cf8a0 100644
--- a/src/core/SkScalerContext.cpp
+++ b/src/core/SkScalerContext.cpp
@@ -400,7 +400,7 @@ static void generateMask(const SkMask& mask, const SkPath& path,
} else {
dst.reset(info, mask.fImage, dstRB);
}
- sk_bzero(dst.writable_addr(), dst.computeByteSize());
+ sk_bzero(dst.writable_addr(), dst.getSafeSize());
SkDraw draw;
draw.fDst = dst;
diff --git a/src/core/SkSpecialImage.cpp b/src/core/SkSpecialImage.cpp
index db4fadcce2..db40329204 100644
--- a/src/core/SkSpecialImage.cpp
+++ b/src/core/SkSpecialImage.cpp
@@ -219,7 +219,7 @@ public:
SkAlphaType alphaType() const override { return fBitmap.alphaType(); }
- size_t getSize() const override { return fBitmap.computeByteSize(); }
+ size_t getSize() const override { return fBitmap.getSize(); }
void onDraw(SkCanvas* canvas, SkScalar x, SkScalar y, const SkPaint* paint) const override {
SkRect dst = SkRect::MakeXYWH(x, y,
diff --git a/src/gpu/ops/GrSmallPathRenderer.cpp b/src/gpu/ops/GrSmallPathRenderer.cpp
index 222f6626a7..fd868a1dfe 100644
--- a/src/gpu/ops/GrSmallPathRenderer.cpp
+++ b/src/gpu/ops/GrSmallPathRenderer.cpp
@@ -455,7 +455,7 @@ private:
devPathBounds.height()))) {
return false;
}
- sk_bzero(dst.writable_addr(), dst.computeByteSize());
+ sk_bzero(dst.writable_addr(), dst.getSafeSize());
// rasterize path
SkPaint paint;
@@ -562,7 +562,7 @@ private:
devPathBounds.height()))) {
return false;
}
- sk_bzero(dst.writable_addr(), dst.computeByteSize());
+ sk_bzero(dst.writable_addr(), dst.getSafeSize());
// rasterize path
SkPaint paint;
diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp
index 705efaec30..3ac8ba80ca 100644
--- a/src/image/SkImage.cpp
+++ b/src/image/SkImage.cpp
@@ -422,10 +422,7 @@ sk_sp<SkImage> SkImageMakeRasterCopyAndAssignColorSpace(const SkImage* src,
}
size_t rowBytes = info.minRowBytes();
- size_t size = info.computeByteSize(rowBytes);
- if (size == 0) {
- return nullptr;
- }
+ size_t size = info.getSafeSize(rowBytes);
auto data = SkData::MakeUninitialized(size);
if (!data) {
return nullptr;
diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index 1a79d59356..d3a1865039 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -518,7 +518,7 @@ sk_sp<SkImage> SkImage::makeNonTextureImage() const {
}
SkImageInfo info = as_IB(this)->onImageInfo();
size_t rowBytes = info.minRowBytes();
- size_t size = info.computeByteSize(rowBytes);
+ size_t size = info.getSafeSize(rowBytes);
auto data = SkData::MakeUninitialized(size);
if (!data) {
return nullptr;
@@ -667,7 +667,7 @@ size_t SkImage::getDeferredTextureImageData(const GrContextThreadSafeProxy& prox
size_t pixelSize = 0;
if (!isScaled && this->peekPixels(&pixmap) && pixmap.info().colorType() == dstColorType) {
info = pixmap.info();
- pixelSize = SkAlign8(pixmap.computeByteSize());
+ pixelSize = SkAlign8(pixmap.getSafeSize());
if (!dstColorSpace) {
pixmap.setColorSpace(nullptr);
info = info.makeColorSpace(nullptr);
@@ -773,7 +773,7 @@ size_t SkImage::getDeferredTextureImageData(const GrContextThreadSafeProxy& prox
void* pixels = pixelsAsCharPtr;
memcpy(reinterpret_cast<void*>(SkAlign8(reinterpret_cast<uintptr_t>(pixelsAsCharPtr))),
- pixmap.addr(), pixmap.computeByteSize());
+ pixmap.addr(), pixmap.getSafeSize());
// If the context has sRGB support, and we're intending to render to a surface with an attached
// color space, and the image has an sRGB-like color space attached, then use our gamma (sRGB)
@@ -824,7 +824,7 @@ size_t SkImage::getDeferredTextureImageData(const GrContextThreadSafeProxy& prox
}
// Fill in the mipmap levels if they exist
- char* mipLevelPtr = pixelsAsCharPtr + SkAlign8(pixmap.computeByteSize());
+ char* mipLevelPtr = pixelsAsCharPtr + SkAlign8(pixmap.getSafeSize());
if (useMipMaps) {
static_assert(std::is_standard_layout<MipMapLevelData>::value,
@@ -845,10 +845,13 @@ size_t SkImage::getDeferredTextureImageData(const GrContextThreadSafeProxy& prox
// Make sure the mipmap data starts before the end of the buffer
SkASSERT(mipLevelPtr < bufferAsCharPtr + pixelOffset + pixelSize);
// Make sure the mipmap data ends before the end of the buffer
- SkASSERT(mipLevelPtr + mipLevel.fPixmap.computeByteSize() <=
+ SkASSERT(mipLevelPtr + mipLevel.fPixmap.getSafeSize() <=
bufferAsCharPtr + pixelOffset + pixelSize);
- memcpy(mipLevelPtr, mipLevel.fPixmap.addr(), mipLevel.fPixmap.computeByteSize());
+ // getSafeSize includes rowbyte padding except for the last row,
+ // right?
+
+ memcpy(mipLevelPtr, mipLevel.fPixmap.addr(), mipLevel.fPixmap.getSafeSize());
memcpy(bufferAsCharPtr + offsetof(DeferredTextureImage, fMipMapLevelData) +
sizeof(MipMapLevelData) * (generatedMipLevelIndex + 1) +
@@ -858,7 +861,7 @@ size_t SkImage::getDeferredTextureImageData(const GrContextThreadSafeProxy& prox
sizeof(MipMapLevelData) * (generatedMipLevelIndex + 1) +
offsetof(MipMapLevelData, fRowBytes), &rowBytes, sizeof(rowBytes));
- mipLevelPtr += SkAlign8(mipLevel.fPixmap.computeByteSize());
+ mipLevelPtr += SkAlign8(mipLevel.fPixmap.getSafeSize());
}
}
return size;
diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp
index 5258f64b3e..bade1b6235 100644
--- a/src/image/SkImage_Raster.cpp
+++ b/src/image/SkImage_Raster.cpp
@@ -57,11 +57,11 @@ public:
if (kUnknown_SkColorType == info.colorType()) {
return false;
}
- if (!info.validRowBytes(rowBytes)) {
+ if (rowBytes < info.minRowBytes()) {
return false;
}
- size_t size = info.computeByteSize(rowBytes);
+ size_t size = info.getSafeSize(rowBytes);
if (0 == size) {
return false;
}
diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp
index e9d14fea7d..38694876c3 100644
--- a/src/image/SkSurface_Raster.cpp
+++ b/src/image/SkSurface_Raster.cpp
@@ -162,7 +162,7 @@ void SkSurface_Raster::onCopyOnWrite(ContentChangeMode mode) {
fBitmap.allocPixels();
SkASSERT(prev.info() == fBitmap.info());
SkASSERT(prev.rowBytes() == fBitmap.rowBytes());
- memcpy(fBitmap.getPixels(), prev.getPixels(), fBitmap.computeByteSize());
+ memcpy(fBitmap.getPixels(), prev.getPixels(), fBitmap.getSafeSize());
}
SkASSERT(fBitmap.rowBytes() == fRowBytes); // be sure we always use the same value
diff --git a/src/utils/mac/SkCreateCGImageRef.cpp b/src/utils/mac/SkCreateCGImageRef.cpp
index 1514ba6e8a..fd55d91707 100644
--- a/src/utils/mac/SkCreateCGImageRef.cpp
+++ b/src/utils/mac/SkCreateCGImageRef.cpp
@@ -132,7 +132,7 @@ CGImageRef SkCreateCGImageRefWithColorspace(const SkBitmap& bm,
const int w = bitmap->width();
const int h = bitmap->height();
- const size_t s = bitmap->computeByteSize();
+ const size_t s = bitmap->getSize();
// our provider "owns" the bitmap*, and will take care of deleting it
CGDataProviderRef dataRef = CGDataProviderCreateWithData(bitmap, bitmap->getPixels(), s,