Fix off-by-one in Vector::insert.

2 files changed, 54 insertions, 2 deletions

diff --git a/mfbt/Vector.h b/mfbt/Vector.h
index fc43afcf1..07e370426 100644
--- a/mfbt/Vector.h
+++ b/mfbt/Vector.h
@@ -1232,10 +1232,10 @@ Vector<T, N, AP>::insert(T* aP, U&& aVal)
     }
   } else {
     T oldBack = Move(back());
-    if (!append(Move(oldBack))) { /* Dup the last element. */
+    if (!append(Move(oldBack))) {
       return nullptr;
     }
-    for (size_t i = oldLength; i > pos; --i) {
+    for (size_t i = oldLength - 1; i > pos; --i) {
       (*this)[i] = Move((*this)[i - 1]);
     }
     (*this)[pos] = Forward<U>(aVal);
diff --git a/mfbt/tests/TestVector.cpp b/mfbt/tests/TestVector.cpp
index d969bcbc2..e28b432d6 100644
--- a/mfbt/tests/TestVector.cpp
+++ b/mfbt/tests/TestVector.cpp
@@ -22,6 +22,7 @@ struct mozilla::detail::VectorTesting
   static void testReverse();
   static void testExtractRawBuffer();
   static void testExtractOrCopyRawBuffer();
+  static void testInsert();
 };
 
 void
@@ -141,6 +142,15 @@ struct S
     destructCount++;
   }
 
+  S& operator=(S&& rhs) {
+    j = rhs.j;
+    rhs.j = 0;
+    k = Move(rhs.k);
+    rhs.k.reset();
+    moveCount++;
+    return *this;
+  }
+
   S(const S&) = delete;
   S& operator=(const S&) = delete;
 };
@@ -346,6 +356,47 @@ mozilla::detail::VectorTesting::testExtractOrCopyRawBuffer()
   free(buf);
 }
 
+void
+mozilla::detail::VectorTesting::testInsert()
+{
+  S::resetCounts();
+
+  Vector<S, 8> vec;
+  MOZ_RELEASE_ASSERT(vec.reserve(8));
+  for (size_t i = 0; i < 7; i++) {
+    vec.infallibleEmplaceBack(i, i * i);
+  }
+
+  MOZ_RELEASE_ASSERT(vec.length() == 7);
+  MOZ_ASSERT(vec.reserved() == 8);
+  MOZ_RELEASE_ASSERT(S::constructCount == 7);
+  MOZ_RELEASE_ASSERT(S::moveCount == 0);
+  MOZ_RELEASE_ASSERT(S::destructCount == 0);
+
+  S s(42, 43);
+  MOZ_RELEASE_ASSERT(vec.insert(vec.begin() + 4, Move(s)));
+
+  for (size_t i = 0; i < vec.length(); i++) {
+    const S& s = vec[i];
+    MOZ_RELEASE_ASSERT(s.k);
+    if (i < 4) {
+      MOZ_RELEASE_ASSERT(s.j == i && *s.k == i * i);
+    } else if (i == 4) {
+      MOZ_RELEASE_ASSERT(s.j == 42 && *s.k == 43);
+    } else {
+      MOZ_RELEASE_ASSERT(s.j == i - 1 && *s.k == (i - 1) * (i - 1));
+    }
+  }
+
+  MOZ_RELEASE_ASSERT(vec.length() == 8);
+  MOZ_ASSERT(vec.reserved() == 8);
+  MOZ_RELEASE_ASSERT(S::constructCount == 8);
+  MOZ_RELEASE_ASSERT(S::moveCount == 1 /* move in insert() call */ +
+                                     1 /* move the back() element */ +
+                                     3 /* elements to shift */);
+  MOZ_RELEASE_ASSERT(S::destructCount == 1);
+}
+
 int
 main()
 {
@@ -355,4 +406,5 @@ main()
   VectorTesting::testReverse();
   VectorTesting::testExtractRawBuffer();
   VectorTesting::testExtractOrCopyRawBuffer();
+  VectorTesting::testInsert();
 }