commit 833b057519dcd6c62ac3f16cff93b17311c6a4c4 Author: Benjamin Bouvier benj@benj.me Date: Fri Mar 31 11:01:35 2017 +0200

Bug 1352073 - Fix off-by-one in Vector::insert. r=luke, a=lizzard

MozReview-Commit-ID: HY0DYSAbi6M --- mfbt/Vector.h | 4 ++-- mfbt/tests/TestVector.cpp | 52 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/mfbt/Vector.h b/mfbt/Vector.h index fc43afcf163e..07e370426492 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 d969bcbc2ceb..e28b432d6919 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(); }