Merge 155478 - Fix SafeAdd and SafeMultiply

BUG=145648,145544


Review URL: https://chromiumcodereview.appspot.com/10916165

TBR=gman@chromium.org
Review URL: https://codereview.chromium.org/10928241

git-svn-id: svn://svn.chromium.org/chrome/branches/1229/src@157200 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 83 insertions, 31 deletions

diff --git a/gpu/command_buffer/common/gles2_cmd_utils.h b/gpu/command_buffer/common/gles2_cmd_utils.h
index 8b3d8a7..68ff41b 100644
--- a/gpu/command_buffer/common/gles2_cmd_utils.h
+++ b/gpu/command_buffer/common/gles2_cmd_utils.h
@@ -8,6 +8,7 @@
 #ifndef GPU_COMMAND_BUFFER_COMMON_GLES2_CMD_UTILS_H_
 #define GPU_COMMAND_BUFFER_COMMON_GLES2_CMD_UTILS_H_
 
+#include <limits>
 #include <string>
 #include <vector>
 
@@ -19,13 +20,15 @@ namespace gles2 {
 
 // Does a multiply and checks for overflow.  If the multiply did not overflow
 // returns true.
-template <typename T>
-inline bool SafeMultiply(T a, T b, T* dst) {
+
+// Multiplies 2 32 bit unsigned numbers checking for overflow.
+// If there was no overflow returns true.
+inline bool SafeMultiplyUint32(uint32 a, uint32 b, uint32* dst) {
   if (b == 0) {
     *dst = 0;
     return true;
   }
-  T v = a * b;
+  uint32 v = a * b;
   if (v / b != a) {
     *dst = 0;
     return false;
@@ -34,14 +37,8 @@ inline bool SafeMultiply(T a, T b, T* dst) {
   return true;
 }
 
-// A wrapper for SafeMultiply to remove the need to cast.
-inline bool SafeMultiplyUint32(uint32 a, uint32 b, uint32* dst) {
-  return SafeMultiply(a, b, dst);
-}
-
 // Does an add checking for overflow.  If there was no overflow returns true.
-template <typename T>
-inline bool SafeAdd(T a, T b, T* dst) {
+inline bool SafeAddUint32(uint32 a, uint32 b, uint32* dst) {
   if (a + b < a) {
     *dst = 0;
     return false;
@@ -50,9 +47,13 @@ inline bool SafeAdd(T a, T b, T* dst) {
   return true;
 }
 
-// A wrapper for SafeAdd to remove the need to cast.
-inline bool SafeAddUint32(uint32 a, uint32 b, uint32* dst) {
-  return SafeAdd(a, b, dst);
+// Does an add checking for overflow.  If there was no overflow returns true.
+inline bool SafeAddInt32(int32 a, int32 b, int32* dst) {
+  int64 sum64 = static_cast<int64>(a) + b;
+  int32 sum32 = static_cast<int32>(sum64);
+  bool safe = sum64 == static_cast<int64>(sum32);
+  *dst = safe ? sum32 : 0;
+  return safe;
 }
 
 // Utilties for GLES2 support.
diff --git a/gpu/command_buffer/common/gles2_cmd_utils_unittest.cc b/gpu/command_buffer/common/gles2_cmd_utils_unittest.cc
index 0bf11f0..0e157db 100644
--- a/gpu/command_buffer/common/gles2_cmd_utils_unittest.cc
+++ b/gpu/command_buffer/common/gles2_cmd_utils_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "gpu/command_buffer/common/gles2_cmd_utils.h"
 
+#include <limits>
 #include <GLES2/gl2.h>
 #include <GLES2/gl2ext.h>
 
@@ -17,6 +18,58 @@ class GLES2UtilTest : public testing:: Test {
   GLES2Util util_;
 };
 
+TEST_F(GLES2UtilTest, SafeMultiplyUint32) {
+  uint32 result = 0;
+  EXPECT_TRUE(SafeMultiplyUint32(2u, 3u, &result));
+  EXPECT_EQ(6u, result);
+  EXPECT_FALSE(SafeMultiplyUint32(0x80000000u, 2u, &result));
+  EXPECT_EQ(0u, result);
+  EXPECT_TRUE(SafeMultiplyUint32(0x2u, 0x7FFFFFFFu, &result));
+  EXPECT_EQ(0xFFFFFFFEu, result);
+  EXPECT_FALSE(SafeMultiplyUint32(2u, 0x80000000u, &result));
+  EXPECT_EQ(0u, result);
+}
+
+TEST_F(GLES2UtilTest, SafeAddUint32) {
+  uint32 result = 0;
+  EXPECT_TRUE(SafeAddUint32(2u, 3u, &result));
+  EXPECT_EQ(5u, result);
+  EXPECT_FALSE(SafeAddUint32(0x80000000u, 0x80000000u, &result));
+  EXPECT_EQ(0u, result);
+  EXPECT_TRUE(SafeAddUint32(0xFFFFFFFEu, 0x1u, &result));
+  EXPECT_EQ(0xFFFFFFFFu, result);
+  EXPECT_FALSE(SafeAddUint32(0xFFFFFFFEu, 0x2u, &result));
+  EXPECT_EQ(0u, result);
+  EXPECT_TRUE(SafeAddUint32(0x1u, 0xFFFFFFFEu, &result));
+  EXPECT_EQ(0xFFFFFFFFu, result);
+  EXPECT_FALSE(SafeAddUint32(0x2u, 0xFFFFFFFEu, &result));
+  EXPECT_EQ(0u, result);
+}
+
+TEST_F(GLES2UtilTest, SafeAddInt32) {
+  int32 result = 0;
+  const int32 kMax = std::numeric_limits<int32>::max();
+  const int32 kMin = std::numeric_limits<int32>::min();
+  EXPECT_TRUE(SafeAddInt32(2, 3, &result));
+  EXPECT_EQ(5, result);
+  EXPECT_FALSE(SafeAddInt32(kMax, 1, &result));
+  EXPECT_EQ(0, result);
+  EXPECT_TRUE(SafeAddInt32(kMin + 1, -1, &result));
+  EXPECT_EQ(kMin, result);
+  EXPECT_FALSE(SafeAddInt32(kMin, -1, &result));
+  EXPECT_EQ(0, result);
+  EXPECT_TRUE(SafeAddInt32(kMax - 1, 1, &result));
+  EXPECT_EQ(kMax, result);
+  EXPECT_FALSE(SafeAddInt32(1, kMax, &result));
+  EXPECT_EQ(0, result);
+  EXPECT_TRUE(SafeAddInt32(-1, kMin + 1, &result));
+  EXPECT_EQ(kMin, result);
+  EXPECT_FALSE(SafeAddInt32(-1, kMin, &result));
+  EXPECT_EQ(0, result);
+  EXPECT_TRUE(SafeAddInt32(1, kMax - 1, &result));
+  EXPECT_EQ(kMax, result);
+}
+
 TEST_F(GLES2UtilTest, GLGetNumValuesReturned) {
   EXPECT_EQ(0, util_.GLGetNumValuesReturned(GL_COMPRESSED_TEXTURE_FORMATS));
   EXPECT_EQ(0, util_.GLGetNumValuesReturned(GL_SHADER_BINARY_FORMATS));
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 7b18eba4..7dba42b 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -5359,11 +5359,10 @@ bool GLES2DecoderImpl::SimulateAttrib0(
   typedef VertexAttribManager::VertexAttribInfo::Vec4 Vec4;
 
   GLuint num_vertices = max_vertex_accessed + 1;
-  GLuint size_needed = 0;
+  uint32 size_needed = 0;
 
   if (num_vertices == 0 ||
-      !SafeMultiply(num_vertices, static_cast<GLuint>(sizeof(Vec4)),
-                    &size_needed) ||
+      !SafeMultiplyUint32(num_vertices, sizeof(Vec4), &size_needed) ||
       size_needed > 0x7FFFFFFFU) {
     SetGLError(GL_OUT_OF_MEMORY, function_name, "Simulating attrib 0");
     return false;
@@ -5473,10 +5472,9 @@ bool GLES2DecoderImpl::SimulateFixedAttribs(
     if (attrib_info &&
         info->CanAccess(max_accessed) &&
         info->type() == GL_FIXED) {
-      GLuint elements_used = 0;
-      if (!SafeMultiply(num_vertices,
-                        static_cast<GLuint>(info->size()), &elements_used) ||
-          !SafeAdd(elements_needed, elements_used, &elements_needed)) {
+      uint32 elements_used = 0;
+      if (!SafeMultiplyUint32(num_vertices, info->size(), &elements_used) ||
+          !SafeAddUint32(elements_needed, elements_used, &elements_needed)) {
         SetGLError(
             GL_OUT_OF_MEMORY, function_name, "simulating GL_FIXED attribs");
         return false;
@@ -5484,9 +5482,9 @@ bool GLES2DecoderImpl::SimulateFixedAttribs(
     }
   }
 
-  const GLuint kSizeOfFloat = sizeof(float);  // NOLINT
-  GLuint size_needed = 0;
-  if (!SafeMultiply(elements_needed, kSizeOfFloat, &size_needed) ||
+  const uint32 kSizeOfFloat = sizeof(float);  // NOLINT
+  uint32 size_needed = 0;
+  if (!SafeMultiplyUint32(elements_needed, kSizeOfFloat, &size_needed) ||
       size_needed > 0x7FFFFFFFU) {
     SetGLError(GL_OUT_OF_MEMORY, function_name, "simulating GL_FIXED attribs");
     return false;
@@ -6406,9 +6404,9 @@ error::Error GLES2DecoderImpl::HandleReadPixels(
   // Get the size of the current fbo or backbuffer.
   gfx::Size max_size = GetBoundReadFrameBufferSize();
 
-  GLint max_x;
-  GLint max_y;
-  if (!SafeAdd(x, width, &max_x) || !SafeAdd(y, height, &max_y)) {
+  int32 max_x;
+  int32 max_y;
+  if (!SafeAddInt32(x, width, &max_x) || !SafeAddInt32(y, height, &max_y)) {
     SetGLError(GL_INVALID_VALUE, "glReadPixels", "dimensions out of range");
     return error::kNoError;
   }
@@ -8459,7 +8457,7 @@ error::Error GLES2DecoderImpl::HandleGetMultipleIntegervCHROMIUM(
     }
     // Num will never be more than 4.
     DCHECK_LE(num, 4u);
-    if (!SafeAdd(num_results, num, &num_results)) {
+    if (!SafeAddUint32(num_results, num, &num_results)) {
       return error::kOutOfBounds;
     }
   }
diff --git a/gpu/command_buffer/service/texture_manager.cc b/gpu/command_buffer/service/texture_manager.cc
index 5c8c189..11a4de0 100644
--- a/gpu/command_buffer/service/texture_manager.cc
+++ b/gpu/command_buffer/service/texture_manager.cc
@@ -331,10 +331,10 @@ bool TextureManager::TextureInfo::ValidForTexture(
   if (level >= 0 && face_index < level_infos_.size() &&
       static_cast<size_t>(level) < level_infos_[face_index].size()) {
     const LevelInfo& info = level_infos_[GLTargetToFaceIndex(face)][level];
-    GLint right;
-    GLint top;
-    return SafeAdd(xoffset, width, &right) &&
-           SafeAdd(yoffset, height, &top) &&
+    int32 right;
+    int32 top;
+    return SafeAddInt32(xoffset, width, &right) &&
+           SafeAddInt32(yoffset, height, &top) &&
            xoffset >= 0 &&
            yoffset >= 0 &&
            right <= info.width &&