From 58ad13c05daf619f77f2d7ad943c62a2c905bc33 Mon Sep 17 00:00:00 2001
From: Sameer Agarwal <sameeragarwal@google.com>
Date: Tue, 5 Jun 2012 17:46:53 -0700
Subject: [PATCH] BlockRandomAccessSparseMatrix::IntPairToLong suffers from
 integer overflow.

Even though the return value of this function is a long int, the
computation happens with three ints, which causes an overflow before
the upgrade happens.

The fix is to upgrade the constant used int his computation to be a
long int, which causes the computation to be done in longs instead of
ints.

A test has been added to verify that the fix works.

Change-Id: Ibb0aef877125bb37ca28754cb07b8e1627fd1d5a
---
 .../ceres/block_random_access_sparse_matrix.h |  6 ++--
 .../block_random_access_sparse_matrix_test.cc | 28 +++++++++++++++++++
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/internal/ceres/block_random_access_sparse_matrix.h b/internal/ceres/block_random_access_sparse_matrix.h
index 27e30a753..48a00437c 100644
--- a/internal/ceres/block_random_access_sparse_matrix.h
+++ b/internal/ceres/block_random_access_sparse_matrix.h
@@ -38,6 +38,7 @@
 #include "ceres/block_random_access_matrix.h"
 #include "ceres/collections_port.h"
 #include "ceres/triplet_sparse_matrix.h"
+#include "ceres/integral_types.h"
 #include "ceres/internal/macros.h"
 #include "ceres/internal/port.h"
 #include "ceres/internal/scoped_ptr.h"
@@ -84,11 +85,11 @@ class BlockRandomAccessSparseMatrix : public BlockRandomAccessMatrix {
   TripletSparseMatrix* mutable_matrix() { return tsm_.get(); }
 
  private:
-  long int IntPairToLong(int a, int b) {
+  int64 IntPairToLong(int a, int b) {
     return a * kMaxRowBlocks + b;
   }
 
-  const int kMaxRowBlocks;
+  const int64 kMaxRowBlocks;
   // row/column block sizes.
   const vector<int> blocks_;
 
@@ -100,6 +101,7 @@ class BlockRandomAccessSparseMatrix : public BlockRandomAccessMatrix {
   // The underlying matrix object which actually stores the cells.
   scoped_ptr<TripletSparseMatrix> tsm_;
 
+  friend class BlockRandomAccessSparseMatrixTest;
   CERES_DISALLOW_COPY_AND_ASSIGN(BlockRandomAccessSparseMatrix);
 };
 
diff --git a/internal/ceres/block_random_access_sparse_matrix_test.cc b/internal/ceres/block_random_access_sparse_matrix_test.cc
index 01a8c6674..e4e676925 100644
--- a/internal/ceres/block_random_access_sparse_matrix_test.cc
+++ b/internal/ceres/block_random_access_sparse_matrix_test.cc
@@ -28,6 +28,7 @@
 //
 // Author: sameeragarwal@google.com (Sameer Agarwal)
 
+#include <limits>
 #include <vector>
 #include <glog/logging.h>
 #include "gtest/gtest.h"
@@ -117,5 +118,32 @@ TEST(BlockRandomAccessSparseMatrix, GetCell) {
   EXPECT_NEAR(dense.norm(), sqrt(9 + 16 * 16 + 36 * 20 + 9 * 15), kTolerance);
 }
 
+// IntPairToLong is private, thus this fixture is needed to access and
+// test it.
+class BlockRandomAccessSparseMatrixTest : public ::testing::Test {
+ public:
+  virtual void SetUp() {
+    vector<int> blocks;
+    blocks.push_back(1);
+    set< pair<int, int> > block_pairs;
+    block_pairs.insert(make_pair(0, 0));
+    m_.reset(new BlockRandomAccessSparseMatrix(blocks, block_pairs));
+  }
+
+  void CheckIntPair(int a, int b) {
+    int64 value = m_->IntPairToLong(a, b);
+    EXPECT_GT(value, 0) << "Overflow a = " << a << " b = " << b;
+    EXPECT_GT(value, a) << "Overflow a = " << a << " b = " << b;
+    EXPECT_GT(value, b) << "Overflow a = " << a << " b = " << b;
+  }
+
+ private:
+  scoped_ptr<BlockRandomAccessSparseMatrix> m_;
+};
+
+TEST_F(BlockRandomAccessSparseMatrixTest, IntPairToLongOverflow) {
+  CheckIntPair(numeric_limits<int>::max(), numeric_limits<int>::max());
+}
+
 }  // namespace internal
 }  // namespace ceres