Diff
Modified: branches/safari-602-branch/Source/WebCore/ChangeLog (208162 => 208163)
--- branches/safari-602-branch/Source/WebCore/ChangeLog 2016-10-31 18:43:25 UTC (rev 208162)
+++ branches/safari-602-branch/Source/WebCore/ChangeLog 2016-10-31 18:43:30 UTC (rev 208163)
@@ -1,5 +1,60 @@
2016-10-31 Matthew Hanson <[email protected]>
+ Merge r206635 and r206637. rdar://problem/28718754
+
+ 2016-10-28 Said Abou-Hallawa <[email protected]>
+
+ Change the MemoryCache and CachedResource adjustSize functions to take a long argument
+ https://bugs.webkit.org/show_bug.cgi?id=162708
+
+ Reviewed by Brent Fulgham.
+
+ Because the MemoryCache stores the size of the cached memory in unsigned,
+ two problems my happen when reporting a change in the size of the memory:
+
+ 1. Signed integer overflow -- which can happen because MemoryCache::adjustSize()
+ takes a signed integer argument. If the allocated or the freed memory size is
+ larger than the maximum of a signed integer, an overflow will happen.
+ For the image caching code, this can be seen where the unsigned decodedSize
+ is casted to an integer before passing it to ImageObserver::decodedSizeChanged().
+
+ 2. Unsigned integer overflow -- which can happen if the new allocated memory
+ size plus the currentSize exceeds the maximum of unsigned.
+ This can be seen in MemoryCache::adjustSize() where we add delta to m_liveSize
+ or m_deadSize without checking whether this addition will overflow or not. We
+ do not assert for overflow although we assert for underflow.
+
+ The fix for these two problems can be the following:
+
+ 1. Make all the adjustSize functions all the way till MemoryCache::adjustSize()
+ take a signed long integer argument.
+
+ 2. Do not create a NativeImagePtr for an ImageFrame if its frameBytes plus the
+ ImageFrameCache::decodedSize() will exceed the maximum of an unsigned integer.
+
+ * loader/cache/CachedImage.cpp:
+ (WebCore::CachedImage::decodedSizeChanged): Change the argument to be long. No overflow will happen when casting the argument from unsigned to long.
+ * loader/cache/CachedImage.h:
+ * loader/cache/CachedResource.cpp:
+ (WebCore::CachedResource::setDecodedSize): Use long integer casting when calling MemoryCache::adjustSize().
+ (WebCore::CachedResource::setEncodedSize): Ditto.
+ * loader/cache/MemoryCache.cpp:
+ (WebCore::MemoryCache::MemoryCache): Add as static assert to ensure sizeof(long long) can hold any unsigned or its negation.
+ (WebCore::MemoryCache::revalidationSucceeded): Use long integer casting when calling MemoryCache::adjustSize().
+ (WebCore::MemoryCache::remove): Ditto.
+ (WebCore::MemoryCache::adjustSize): Change the function argument to long integer. No overflow will happen when casting the argument from unsigned to long.
+ * loader/cache/MemoryCache.h:
+ * platform/graphics/BitmapImage.cpp:
+ (WebCore::BitmapImage::destroyMetadataAndNotify): Use long long casting when calling ImageObserver::decodedSizeChanged().
+ (WebCore::BitmapImage::cacheFrame): Do not create the NativeImage if adding its frameByes to the MemoryCache will cause numerical overflow.
+ (WebCore::BitmapImage::didDecodeProperties): Use long long casting.
+ (WebCore::BitmapImage::frameImageAtIndex): Use long long casting when calling ImageObserver::decodedSizeChanged().
+ * platform/graphics/ImageObserver.h:
+ * platform/graphics/cg/PDFDocumentImage.cpp:
+ (WebCore::PDFDocumentImage::decodedSizeChanged): Use long long casting when calling ImageObserver::decodedSizeChanged().
+
+2016-10-31 Matthew Hanson <[email protected]>
+
Merge r206802. rdar://problem/28409525
2016-10-28 Said Abou-Hallawa <[email protected]>
Modified: branches/safari-602-branch/Source/WebCore/loader/cache/CachedImage.cpp (208162 => 208163)
--- branches/safari-602-branch/Source/WebCore/loader/cache/CachedImage.cpp 2016-10-31 18:43:25 UTC (rev 208162)
+++ branches/safari-602-branch/Source/WebCore/loader/cache/CachedImage.cpp 2016-10-31 18:43:30 UTC (rev 208163)
@@ -463,11 +463,12 @@
m_image->destroyDecodedData();
}
-void CachedImage::decodedSizeChanged(const Image* image, int delta)
+void CachedImage::decodedSizeChanged(const Image* image, long long delta)
{
if (!image || image != m_image)
return;
-
+
+ ASSERT(delta >= 0 || decodedSize() + delta >= 0);
setDecodedSize(decodedSize() + delta);
}
Modified: branches/safari-602-branch/Source/WebCore/loader/cache/CachedImage.h (208162 => 208163)
--- branches/safari-602-branch/Source/WebCore/loader/cache/CachedImage.h 2016-10-31 18:43:25 UTC (rev 208162)
+++ branches/safari-602-branch/Source/WebCore/loader/cache/CachedImage.h 2016-10-31 18:43:30 UTC (rev 208163)
@@ -117,7 +117,7 @@
bool stillNeedsLoad() const override { return !errorOccurred() && status() == Unknown && !isLoading(); }
// ImageObserver
- void decodedSizeChanged(const Image*, int delta) override;
+ void decodedSizeChanged(const Image*, long long delta) override;
void didDraw(const Image*) override;
void animationAdvanced(const Image*) override;
Modified: branches/safari-602-branch/Source/WebCore/loader/cache/CachedResource.cpp (208162 => 208163)
--- branches/safari-602-branch/Source/WebCore/loader/cache/CachedResource.cpp 2016-10-31 18:43:25 UTC (rev 208162)
+++ branches/safari-602-branch/Source/WebCore/loader/cache/CachedResource.cpp 2016-10-31 18:43:30 UTC (rev 208163)
@@ -570,15 +570,15 @@
if (size == m_decodedSize)
return;
- int delta = size - m_decodedSize;
+ long long delta = static_cast<long long>(size) - m_decodedSize;
// The object must be moved to a different queue, since its size has been changed.
// Remove before updating m_decodedSize, so we find the resource in the correct LRU list.
if (allowsCaching() && inCache())
MemoryCache::singleton().removeFromLRUList(*this);
-
+
m_decodedSize = size;
-
+
if (allowsCaching() && inCache()) {
auto& memoryCache = MemoryCache::singleton();
// Now insert into the new LRU list.
@@ -607,7 +607,7 @@
if (size == m_encodedSize)
return;
- int delta = size - m_encodedSize;
+ long long delta = static_cast<long long>(size) - m_encodedSize;
// The object must be moved to a different queue, since its size has been changed.
// Remove before updating m_encodedSize, so we find the resource in the correct LRU list.
Modified: branches/safari-602-branch/Source/WebCore/loader/cache/MemoryCache.cpp (208162 => 208163)
--- branches/safari-602-branch/Source/WebCore/loader/cache/MemoryCache.cpp 2016-10-31 18:43:25 UTC (rev 208162)
+++ branches/safari-602-branch/Source/WebCore/loader/cache/MemoryCache.cpp 2016-10-31 18:43:30 UTC (rev 208163)
@@ -71,6 +71,7 @@
, m_deadSize(0)
, m_pruneTimer(*this, &MemoryCache::prune)
{
+ static_assert(sizeof(long long) > sizeof(unsigned), "Numerical overflow can happen when adjusting the size of the cached memory.");
}
auto MemoryCache::sessionResourceMap(SessionID sessionID) const -> CachedResourceMap*
@@ -148,7 +149,7 @@
resource.setInCache(true);
resource.updateResponseAfterRevalidation(response);
insertInLRUList(resource);
- int delta = resource.size();
+ long long delta = resource.size();
if (resource.decodedSize() && resource.hasClients())
insertInLiveDecodedResourcesList(resource);
if (delta)
@@ -448,7 +449,7 @@
// Remove from the appropriate LRU list.
removeFromLRUList(resource);
removeFromLiveDecodedResourcesList(resource);
- adjustSize(resource.hasClients(), -static_cast<int>(resource.size()));
+ adjustSize(resource.hasClients(), -static_cast<long long>(resource.size()));
} else
ASSERT(resources->get(key) != &resource);
}
@@ -641,13 +642,13 @@
m_deadSize += resource.size();
}
-void MemoryCache::adjustSize(bool live, int delta)
+void MemoryCache::adjustSize(bool live, long long delta)
{
if (live) {
- ASSERT(delta >= 0 || ((int)m_liveSize + delta >= 0));
+ ASSERT(delta >= 0 || (static_cast<long long>(m_liveSize) + delta >= 0));
m_liveSize += delta;
} else {
- ASSERT(delta >= 0 || ((int)m_deadSize + delta >= 0));
+ ASSERT(delta >= 0 || (static_cast<long long>(m_deadSize) + delta >= 0));
m_deadSize += delta;
}
}
Modified: branches/safari-602-branch/Source/WebCore/loader/cache/MemoryCache.h (208162 => 208163)
--- branches/safari-602-branch/Source/WebCore/loader/cache/MemoryCache.h 2016-10-31 18:43:25 UTC (rev 208162)
+++ branches/safari-602-branch/Source/WebCore/loader/cache/MemoryCache.h 2016-10-31 18:43:30 UTC (rev 208163)
@@ -133,7 +133,7 @@
void removeFromLRUList(CachedResource&);
// Called to adjust the cache totals when a resource changes size.
- void adjustSize(bool live, int delta);
+ void adjustSize(bool live, long long delta);
// Track decoded resources that are in the cache and referenced by a Web page.
void insertInLiveDecodedResourcesList(CachedResource&);
Modified: branches/safari-602-branch/Source/WebCore/platform/graphics/BitmapImage.cpp (208162 => 208163)
--- branches/safari-602-branch/Source/WebCore/platform/graphics/BitmapImage.cpp 2016-10-31 18:43:25 UTC (rev 208162)
+++ branches/safari-602-branch/Source/WebCore/platform/graphics/BitmapImage.cpp 2016-10-31 18:43:30 UTC (rev 208163)
@@ -174,7 +174,7 @@
}
if (frameBytesCleared && imageObserver())
- imageObserver()->decodedSizeChanged(this, -safeCast<int>(frameBytesCleared));
+ imageObserver()->decodedSizeChanged(this, -safeCast<long long>(frameBytesCleared));
}
void BitmapImage::cacheFrame(size_t index, SubsamplingLevel subsamplingLevel, ImageFrameCaching frameCaching)
@@ -186,8 +186,13 @@
m_frames.grow(numFrames);
if (frameCaching == CacheMetadataAndFrame) {
- m_frames[index].m_image = m_source.createFrameImageAtIndex(index, subsamplingLevel);
- m_frames[index].m_subsamplingLevel = subsamplingLevel;
+ size_t frameBytes = size().area() * sizeof(RGBA32);
+
+ // Do not create the NativeImage if adding its frameByes to the MemoryCache will cause numerical overflow.
+ if (WTF::isInBounds<unsigned>(frameBytes + decodedSize())) {
+ m_frames[index].m_image = m_source.createFrameImageAtIndex(index, subsamplingLevel);
+ m_frames[index].m_subsamplingLevel = subsamplingLevel;
+ }
}
m_frames[index].m_orientation = m_source.orientationAtIndex(index);
@@ -203,11 +208,11 @@
LOG(Images, "BitmapImage %p cacheFrame %lu (%s%u bytes, complete %d)", this, index, frameCaching == CacheMetadataOnly ? "metadata only, " : "", m_frames[index].m_frameBytes, m_frames[index].m_isComplete);
if (m_frames[index].m_image) {
- int deltaBytes = safeCast<int>(m_frames[index].m_frameBytes);
- m_decodedSize += deltaBytes;
+ unsigned decodedSize = m_frames[index].m_frameBytes;
+ m_decodedSize += decodedSize;
// The fully-decoded frame will subsume the partially decoded data used
// to determine image properties.
- deltaBytes -= m_decodedPropertiesSize;
+ long long deltaBytes = static_cast<long long>(decodedSize) - m_decodedPropertiesSize;
m_decodedPropertiesSize = 0;
if (imageObserver())
imageObserver()->decodedSizeChanged(this, deltaBytes);
@@ -219,17 +224,17 @@
if (m_decodedSize)
return;
- size_t updatedSize = m_source.bytesDecodedToDetermineProperties();
- if (m_decodedPropertiesSize == updatedSize)
+ size_t decodedPropertiesSize = m_source.bytesDecodedToDetermineProperties();
+ if (m_decodedPropertiesSize == decodedPropertiesSize)
return;
- int deltaBytes = updatedSize - m_decodedPropertiesSize;
+ long long deltaBytes = static_cast<long long>(decodedPropertiesSize) - m_decodedPropertiesSize;
#if !ASSERT_DISABLED
- bool overflow = updatedSize > m_decodedPropertiesSize && deltaBytes < 0;
- bool underflow = updatedSize < m_decodedPropertiesSize && deltaBytes > 0;
+ bool overflow = decodedPropertiesSize > m_decodedPropertiesSize && deltaBytes < 0;
+ bool underflow = decodedPropertiesSize < m_decodedPropertiesSize && deltaBytes > 0;
ASSERT(!overflow && !underflow);
#endif
- m_decodedPropertiesSize = updatedSize;
+ m_decodedPropertiesSize = decodedPropertiesSize;
if (imageObserver())
imageObserver()->decodedSizeChanged(this, deltaBytes);
}
@@ -390,12 +395,12 @@
// re-decode with a lower level.
if (index < m_frames.size() && m_frames[index].m_image && subsamplingLevel < m_frames[index].m_subsamplingLevel) {
// If the image is already cached, but at too small a size, re-decode a larger version.
- int sizeChange = -m_frames[index].m_frameBytes;
+ unsigned decodedSize = m_frames[index].m_frameBytes;
m_frames[index].clear(true);
invalidatePlatformData();
- m_decodedSize += sizeChange;
+ m_decodedSize -= decodedSize;
if (imageObserver())
- imageObserver()->decodedSizeChanged(this, sizeChange);
+ imageObserver()->decodedSizeChanged(this, -static_cast<long long>(decodedSize));
}
// If we haven't fetched a frame yet, do so.
Modified: branches/safari-602-branch/Source/WebCore/platform/graphics/ImageObserver.h (208162 => 208163)
--- branches/safari-602-branch/Source/WebCore/platform/graphics/ImageObserver.h 2016-10-31 18:43:25 UTC (rev 208162)
+++ branches/safari-602-branch/Source/WebCore/platform/graphics/ImageObserver.h 2016-10-31 18:43:30 UTC (rev 208163)
@@ -37,7 +37,7 @@
protected:
virtual ~ImageObserver() {}
public:
- virtual void decodedSizeChanged(const Image*, int delta) = 0;
+ virtual void decodedSizeChanged(const Image*, long long delta) = 0;
virtual void didDraw(const Image*) = 0;
virtual void animationAdvanced(const Image*) = 0;
Modified: branches/safari-602-branch/Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp (208162 => 208163)
--- branches/safari-602-branch/Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp 2016-10-31 18:43:25 UTC (rev 208162)
+++ branches/safari-602-branch/Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp 2016-10-31 18:43:30 UTC (rev 208163)
@@ -182,7 +182,7 @@
return;
if (imageObserver())
- imageObserver()->decodedSizeChanged(this, -safeCast<int>(m_cachedBytes) + newCachedBytes);
+ imageObserver()->decodedSizeChanged(this, -safeCast<long long>(m_cachedBytes) + newCachedBytes);
ASSERT(s_allDecodedDataSize >= m_cachedBytes);
// Update with the difference in two steps to avoid unsigned underflow subtraction.
