Fix unpack parameter validation.

src/libANGLE/formatutils.cpp

@@ -684,23 +684,34 @@ gl::ErrorOrResult<GLuint> InternalFormat::computeRowPitch(GLenum formatType,
                                                           GLint alignment,
                                                           GLint rowLength) const
 {
-    ASSERT(alignment > 0 && isPow2(alignment));
-    GLuint rowBytes;
+    // Compressed images do not use pack/unpack parameters.
+    if (compressed)
+    {
+        ASSERT(rowLength == 0);
+        return computeCompressedImageSize(formatType, gl::Extents(width, 1, 1));
+    }
+
+    CheckedNumeric<GLuint> checkedRowBytes(0);
     if (rowLength > 0)
     {
-        ASSERT(!compressed);
-        CheckedNumeric<GLuint> checkedBytes(pixelBytes);
-        checkedBytes *= rowLength;
-        ANGLE_TRY_CHECKED_MATH(checkedBytes);
-        rowBytes = checkedBytes.ValueOrDie();
+        CheckedNumeric<GLuint> checkePixelBytes(pixelBytes);
+        checkedRowBytes = checkePixelBytes * rowLength;
     }
     else
     {
-        ANGLE_TRY_RESULT(computeBlockSize(formatType, gl::Extents(width, 1, 1)), rowBytes);
+        CheckedNumeric<GLuint> checkedWidth(width);
+        const auto &typeInfo = GetTypeInfo(formatType);
+        CheckedNumeric<GLuint> checkedComponents(typeInfo.specialInterpretation ? 1u
+                                                                                : componentCount);
+        CheckedNumeric<GLuint> checkedTypeBytes(typeInfo.bytes);
+        checkedRowBytes = checkedWidth * checkedComponents * checkedTypeBytes;
     }
-    auto checkedResult = rx::CheckedRoundUp(rowBytes, static_cast<GLuint>(alignment));
-    ANGLE_TRY_CHECKED_MATH(checkedResult);
-    return checkedResult.ValueOrDie();
+
+    ASSERT(alignment > 0 && isPow2(alignment));
+    CheckedNumeric<GLuint> checkedAlignment(alignment);
+    auto aligned = rx::roundUp(checkedRowBytes, checkedAlignment);
+    ANGLE_TRY_CHECKED_MATH(aligned);
+    return aligned.ValueOrDie();
 }
 
 gl::ErrorOrResult<GLuint> InternalFormat::computeDepthPitch(GLenum formatType,
@@ -710,15 +721,8 @@ gl::ErrorOrResult<GLuint> InternalFormat::computeDepthPitch(GLenum formatType,
                                                             GLint rowLength,
                                                             GLint imageHeight) const
 {
-    GLuint rows;
-    if (imageHeight > 0)
-    {
-        rows = imageHeight;
-    }
-    else
-    {
-        rows = height;
-    }
+    GLuint rows =
+        (imageHeight > 0 ? static_cast<GLuint>(imageHeight) : static_cast<GLuint>(height));
     GLuint rowPitch = 0;
     ANGLE_TRY_RESULT(computeRowPitch(formatType, width, alignment, rowLength), rowPitch);
 
@@ -728,28 +732,21 @@ gl::ErrorOrResult<GLuint> InternalFormat::computeDepthPitch(GLenum formatType,
     return depthPitch.ValueOrDie();
 }
 
-gl::ErrorOrResult<GLuint> InternalFormat::computeBlockSize(GLenum formatType,
-                                                           const gl::Extents &size) const
+gl::ErrorOrResult<GLuint> InternalFormat::computeCompressedImageSize(GLenum formatType,
+                                                                     const gl::Extents &size) const
 {
     CheckedNumeric<GLuint> checkedWidth(size.width);
     CheckedNumeric<GLuint> checkedHeight(size.height);
     CheckedNumeric<GLuint> checkedDepth(size.depth);
-
-    if (compressed)
-    {
-        auto numBlocksWide = (checkedWidth + compressedBlockWidth - 1u) / compressedBlockWidth;
-        auto numBlocksHigh = (checkedHeight + compressedBlockHeight - 1u) / compressedBlockHeight;
-        auto bytes         = numBlocksWide * numBlocksHigh * pixelBytes * checkedDepth;
-        ANGLE_TRY_CHECKED_MATH(bytes);
-        return bytes.ValueOrDie();
-    }
-
-    const Type &typeInfo = GetTypeInfo(formatType);
-    GLuint components    = typeInfo.specialInterpretation ? 1u : componentCount;
-
-    auto result = checkedWidth * checkedHeight * checkedDepth * components * typeInfo.bytes;
-    ANGLE_TRY_CHECKED_MATH(result);
-    return result.ValueOrDie();
+    CheckedNumeric<GLuint> checkedBlockWidth(compressedBlockWidth);
+    CheckedNumeric<GLuint> checkedBlockHeight(compressedBlockHeight);
+
+    ASSERT(compressed);
+    auto numBlocksWide = (checkedWidth + checkedBlockWidth - 1u) / checkedBlockWidth;
+    auto numBlocksHigh = (checkedHeight + checkedBlockHeight - 1u) / checkedBlockHeight;
+    auto bytes         = numBlocksWide * numBlocksHigh * pixelBytes * checkedDepth;
+    ANGLE_TRY_CHECKED_MATH(bytes);
+    return bytes.ValueOrDie();
 }
 
 GLuint InternalFormat::computeSkipPixels(GLint rowPitch,
@@ -761,26 +758,52 @@ GLuint InternalFormat::computeSkipPixels(GLint rowPitch,
     return skipImages * depthPitch + skipRows * rowPitch + skipPixels * pixelBytes;
 }
 
+gl::ErrorOrResult<GLuint> InternalFormat::computeUnpackSize(
+    GLenum formatType,
+    const gl::Extents &size,
+    const gl::PixelUnpackState &unpack) const
+{
+    // Compressed images do not use unpack parameters.
+    if (compressed)
+    {
+        return computeCompressedImageSize(formatType, size);
+    }
+
+    base::CheckedNumeric<GLuint> checkedGroups(unpack.rowLength > 0 ? unpack.rowLength
+                                                                    : size.width);
+    base::CheckedNumeric<GLuint> checkedRows(unpack.imageHeight > 0 ? unpack.imageHeight
+                                                                    : size.height);
+
+    // Compute the groups of all the layers in (0,depth-1)
+    auto layerGroups = checkedGroups * checkedRows * (size.depth - 1);
+
+    // Compute the groups in the last layer (for non-3D textures, the only one)
+    auto lastLayerGroups = checkedGroups * (size.height - 1) + size.width;
+
+    // The total size is the sum times the bytes per pixel.
+    auto totalSize = (layerGroups + lastLayerGroups) * pixelBytes;
+
+    ANGLE_TRY_CHECKED_MATH(totalSize);
+
+    return totalSize.ValueOrDie();
+}
+
 GLenum GetSizedInternalFormat(GLenum internalFormat, GLenum type)
 {
-    const InternalFormat& formatInfo = GetInternalFormatInfo(internalFormat);
+    const InternalFormat &formatInfo = GetInternalFormatInfo(internalFormat);
     if (formatInfo.pixelBytes > 0)
     {
         return internalFormat;
     }
-    else
+
+    static const FormatMap formatMap = BuildFormatMap();
+    auto iter                        = formatMap.find(FormatTypePair(internalFormat, type));
+    if (iter != formatMap.end())
     {
-        static const FormatMap formatMap = BuildFormatMap();
-        FormatMap::const_iterator iter = formatMap.find(FormatTypePair(internalFormat, type));
-        if (iter != formatMap.end())
-        {
-            return iter->second;
-        }
-        else
-        {
-            return GL_NONE;
-        }
+        return iter->second;
     }
+
+    return GL_NONE;
 }
 
 const FormatSet &GetAllSizedInternalFormats()

src/libANGLE/formatutils.h

@@ -44,12 +44,16 @@ struct InternalFormat
                                                 GLint alignment,
                                                 GLint rowLength,
                                                 GLint imageHeight) const;
-    gl::ErrorOrResult<GLuint> computeBlockSize(GLenum formatType, const gl::Extents &size) const;
+    gl::ErrorOrResult<GLuint> computeCompressedImageSize(GLenum formatType,
+                                                         const gl::Extents &size) const;
     GLuint computeSkipPixels(GLint rowPitch,
                              GLint depthPitch,
                              GLint skipImages,
                              GLint skipRows,
                              GLint skipPixels) const;
+    gl::ErrorOrResult<GLuint> computeUnpackSize(GLenum formatType,
+                                                const gl::Extents &size,
+                                                const gl::PixelUnpackState &unpack) const;
 
     GLuint redBits;
     GLuint greenBits;

src/libANGLE/renderer/d3d/d3d11/TextureStorage11.cpp

@@ -625,11 +625,7 @@ gl::Error TextureStorage11::setData(const gl::ImageIndex &index,
     ASSERT(!image->isDirty());
 
     ID3D11Resource *resource = nullptr;
-    gl::Error error = getResource(&resource);
-    if (error.isError())
-    {
-        return error;
-    }
+    ANGLE_TRY(getResource(&resource));
     ASSERT(resource);
 
     UINT destSubresource = getSubresourceIndex(index);
@@ -679,12 +675,7 @@ gl::Error TextureStorage11::setData(const gl::ImageIndex &index,
     d3d11::LoadImageFunctionInfo loadFunctionInfo = d3d11Format.loadFunctions.at(type);
     if (loadFunctionInfo.requiresConversion)
     {
-        error = mRenderer->getScratchMemoryBuffer(neededSize, &conversionBuffer);
-        if (error.isError())
-        {
-            return error;
-        }
-
+        ANGLE_TRY(mRenderer->getScratchMemoryBuffer(neededSize, &conversionBuffer));
         loadFunctionInfo.loadFunction(width, height, depth, pixelData + srcSkipBytes, srcRowPitch,
                                       srcDepthPitch, conversionBuffer->data(), bufferRowPitch,
                                       bufferDepthPitch);
@@ -720,7 +711,7 @@ gl::Error TextureStorage11::setData(const gl::ImageIndex &index,
                                             bufferRowPitch, bufferDepthPitch);
     }
 
-    return gl::Error(GL_NO_ERROR);
+    return gl::NoError();
 }
 
 TextureStorage11_2D::TextureStorage11_2D(Renderer11 *renderer, SwapChain11 *swapchain)

src/libANGLE/renderer/gl/TextureGL.cpp

@@ -390,9 +390,9 @@ gl::Error TextureGL::setStorage(GLenum target, size_t levels, GLenum internalFor
                     if (internalFormatInfo.compressed)
                     {
                         GLuint dataSize = 0;
-                        ANGLE_TRY_RESULT(
-                            internalFormatInfo.computeBlockSize(GL_UNSIGNED_BYTE, levelSize),
-                            dataSize);
+                        ANGLE_TRY_RESULT(internalFormatInfo.computeCompressedImageSize(
+                                             GL_UNSIGNED_BYTE, levelSize),
+                                         dataSize);
                         mFunctions->compressedTexImage2D(target, static_cast<GLint>(level),
                                                          texStorageFormat.internalFormat,
                                                          levelSize.width, levelSize.height, 0,
@@ -413,9 +413,9 @@ gl::Error TextureGL::setStorage(GLenum target, size_t levels, GLenum internalFor
                         if (internalFormatInfo.compressed)
                         {
                             GLuint dataSize = 0;
-                            ANGLE_TRY_RESULT(
-                                internalFormatInfo.computeBlockSize(GL_UNSIGNED_BYTE, levelSize),
-                                dataSize);
+                            ANGLE_TRY_RESULT(internalFormatInfo.computeCompressedImageSize(
+                                                 GL_UNSIGNED_BYTE, levelSize),
+                                             dataSize);
                             mFunctions->compressedTexImage2D(
                                 face, static_cast<GLint>(level), texStorageFormat.internalFormat,
                                 levelSize.width, levelSize.height, 0,
@@ -465,7 +465,8 @@ gl::Error TextureGL::setStorage(GLenum target, size_t levels, GLenum internalFor
                 {
                     GLuint dataSize = 0;
                     ANGLE_TRY_RESULT(
-                        internalFormatInfo.computeBlockSize(GL_UNSIGNED_BYTE, levelSize), dataSize);
+                        internalFormatInfo.computeCompressedImageSize(GL_UNSIGNED_BYTE, levelSize),
+                        dataSize);
                     mFunctions->compressedTexImage3D(target, i, texStorageFormat.internalFormat,
                                                      levelSize.width, levelSize.height,
                                                      levelSize.depth, 0,

src/libANGLE/validationES2.cpp

@@ -1831,7 +1831,7 @@ bool ValidateCompressedTexImage2D(Context *context,
 
     const InternalFormat &formatInfo = GetInternalFormatInfo(internalformat);
     auto blockSizeOrErr =
-        formatInfo.computeBlockSize(GL_UNSIGNED_BYTE, gl::Extents(width, height, 1));
+        formatInfo.computeCompressedImageSize(GL_UNSIGNED_BYTE, gl::Extents(width, height, 1));
     if (blockSizeOrErr.isError())
     {
         context->handleError(blockSizeOrErr.getError());
@@ -1879,7 +1879,7 @@ bool ValidateCompressedTexSubImage2D(Context *context,
 
     const InternalFormat &formatInfo = GetInternalFormatInfo(format);
     auto blockSizeOrErr =
-        formatInfo.computeBlockSize(GL_UNSIGNED_BYTE, gl::Extents(width, height, 1));
+        formatInfo.computeCompressedImageSize(GL_UNSIGNED_BYTE, gl::Extents(width, height, 1));
     if (blockSizeOrErr.isError())
     {
         context->handleError(blockSizeOrErr.getError());

src/libANGLE/validationES3.cpp

@@ -472,25 +472,12 @@ bool ValidateES3TexImageParametersBase(Context *context,
     {
         // ...the data would be unpacked from the buffer object such that the memory reads required
         // would exceed the data store size.
-        size_t widthSize = static_cast<size_t>(width);
-        size_t heightSize = static_cast<size_t>(height);
-        size_t depthSize = static_cast<size_t>(depth);
         GLenum sizedFormat = GetSizedInternalFormat(actualInternalFormat, type);
-
-        CheckedNumeric<size_t> checkedBytes(gl::GetInternalFormatInfo(sizedFormat).pixelBytes);
-        checkedBytes *= widthSize;
-        checkedBytes *= heightSize;
-        checkedBytes *= depthSize;
-
-        if (!checkedBytes.IsValid())
-        {
-            // Overflow past the end of the buffer
-            context->handleError(Error(GL_INVALID_OPERATION, "Integer overflow"));
-            return false;
-        }
-
         const gl::InternalFormat &formatInfo = gl::GetInternalFormatInfo(sizedFormat);
-        auto copyBytesOrErr = formatInfo.computeBlockSize(type, gl::Extents(width, height, depth));
+        const gl::Extents size(width, height, depth);
+        const auto &unpack = context->getState().getUnpackState();
+
+        auto copyBytesOrErr = formatInfo.computeUnpackSize(type, size, unpack);
         if (copyBytesOrErr.isError())
         {
             context->handleError(copyBytesOrErr.getError());
@@ -1549,6 +1536,12 @@ bool ValidateCompressedTexImage3D(Context *context,
         return false;
     }
 
+    if (!ValidTextureTarget(context, target))
+    {
+        context->handleError(Error(GL_INVALID_ENUM));
+        return false;
+    }
+
     // Validate image size
     if (!ValidImageSizeParameters(context, target, level, width, height, depth, false))
     {
@@ -1557,8 +1550,14 @@ bool ValidateCompressedTexImage3D(Context *context,
     }
 
     const InternalFormat &formatInfo = GetInternalFormatInfo(internalformat);
+    if (!formatInfo.compressed)
+    {
+        context->handleError(Error(GL_INVALID_ENUM, "Not a valid compressed texture format"));
+        return false;
+    }
+
     auto blockSizeOrErr =
-        formatInfo.computeBlockSize(GL_UNSIGNED_BYTE, gl::Extents(width, height, depth));
+        formatInfo.computeCompressedImageSize(GL_UNSIGNED_BYTE, gl::Extents(width, height, depth));
     if (blockSizeOrErr.isError())
     {
         context->handleError(Error(GL_INVALID_VALUE));
@@ -1907,7 +1906,7 @@ bool ValidateCompressedTexSubImage3D(Context *context,
 
     const InternalFormat &formatInfo = GetInternalFormatInfo(format);
     auto blockSizeOrErr =
-        formatInfo.computeBlockSize(GL_UNSIGNED_BYTE, gl::Extents(width, height, depth));
+        formatInfo.computeCompressedImageSize(GL_UNSIGNED_BYTE, gl::Extents(width, height, depth));
     if (blockSizeOrErr.isError())
     {
         context->handleError(blockSizeOrErr.getError());