Merge #7888: prevector: fix 2 bugs in currently unreached code paths

laanwj · laanwj · commit ec870e13991f · 2016-04-18T12:42:14.000+02:00
a7af72a prevector::swap: fix (unreached) data corruption (Kaz Wesley) 4ed41a2 test prevector::swap (Kaz Wesley) 1e2c29f prevector: destroy elements only via erase() (Kaz Wesley)
diff --git a/src/prevector.h b/src/prevector.h
@@ -298,9 +298,8 @@ class prevector {
     }
 
     void resize(size_type new_size) {
-        while (size() > new_size) {
-            item_ptr(size() - 1)->~T();
-            _size--;
+        if (size() > new_size) {
+            erase(item_ptr(new_size), end());
         }
         if (new_size > capacity()) {
             change_capacity(new_size);
@@ -368,10 +367,7 @@ class prevector {
     }
 
     iterator erase(iterator pos) {
-        (*pos).~T();
-        memmove(&(*pos), &(*pos) + 1, ((char*)&(*end())) - ((char*)(1 + &(*pos))));
-        _size--;
-        return pos;
+        return erase(pos, pos + 1);
     }
 
     iterator erase(iterator first, iterator last) {
@@ -396,7 +392,7 @@ class prevector {
     }
 
     void pop_back() {
-        _size--;
+        erase(end() - 1, end());
     }
 
     T& front() {
@@ -416,12 +412,7 @@ class prevector {
     }
 
     void swap(prevector<N, T, Size, Diff>& other) {
-        if (_size & other._size & 1) {
-            std::swap(_union.capacity, other._union.capacity);
-            std::swap(_union.indirect, other._union.indirect);
-        } else {
-            std::swap(_union, other._union);
-        }
+        std::swap(_union, other._union);
         std::swap(_size, other._size);
     }
 
diff --git a/src/test/prevector_tests.cpp b/src/test/prevector_tests.cpp
@@ -19,9 +19,11 @@ template<unsigned int N, typename T>
 class prevector_tester {
     typedef std::vector<T> realtype;
     realtype real_vector;
+    realtype real_vector_alt;
 
     typedef prevector<N, T> pretype;
     pretype pre_vector;
+    pretype pre_vector_alt;
 
     typedef typename pretype::size_type Size;
 
@@ -149,6 +151,12 @@ class prevector_tester {
         pre_vector.shrink_to_fit();
         test();
     }
+
+    void swap() {
+        real_vector.swap(real_vector_alt);
+        pre_vector.swap(pre_vector_alt);
+        test();
+    }
 };
 
 BOOST_AUTO_TEST_CASE(PrevectorTestInt)
@@ -204,12 +212,15 @@ BOOST_AUTO_TEST_CASE(PrevectorTestInt)
             if (test.size() > 0) {
                 test.update(insecure_rand() % test.size(), insecure_rand());
             }
-            if (((r >> 11) & 1024) == 11) {
+            if (((r >> 11) % 1024) == 11) {
                 test.clear();
             }
-            if (((r >> 21) & 512) == 12) {
+            if (((r >> 21) % 512) == 12) {
                 test.assign(insecure_rand() % 32, insecure_rand());
             }
+            if (((r >> 15) % 64) == 3) {
+                test.swap();
+            }
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -298,9 +298,8 @@ class prevector {`
`298`	`298`	`}`
`299`	`299`
`300`	`300`	`void resize(size_type new_size) {`
`301`		`- while (size() > new_size) {`
`302`		`- item_ptr(size() - 1)->~T();`
`303`		`- _size--;`
	`301`	`+ if (size() > new_size) {`
	`302`	`+ erase(item_ptr(new_size), end());`
`304`	`303`	`}`
`305`	`304`	`if (new_size > capacity()) {`
`306`	`305`	`change_capacity(new_size);`
`@@ -368,10 +367,7 @@ class prevector {`
`368`	`367`	`}`
`369`	`368`
`370`	`369`	`iterator erase(iterator pos) {`
`371`		`- (*pos).~T();`
`372`		`- memmove(&(pos), &(pos) + 1, ((char)&(end())) - ((char)(1 + &(pos))));`
`373`		`- _size--;`
`374`		`- return pos;`
	`370`	`+ return erase(pos, pos + 1);`
`375`	`371`	`}`
`376`	`372`
`377`	`373`	`iterator erase(iterator first, iterator last) {`
`@@ -396,7 +392,7 @@ class prevector {`
`396`	`392`	`}`
`397`	`393`
`398`	`394`	`void pop_back() {`
`399`		`- _size--;`
	`395`	`+ erase(end() - 1, end());`
`400`	`396`	`}`
`401`	`397`
`402`	`398`	`T& front() {`
`@@ -416,12 +412,7 @@ class prevector {`
`416`	`412`	`}`
`417`	`413`
`418`	`414`	`void swap(prevector<N, T, Size, Diff>& other) {`
`419`		`- if (_size & other._size & 1) {`
`420`		`- std::swap(_union.capacity, other._union.capacity);`
`421`		`- std::swap(_union.indirect, other._union.indirect);`
`422`		`- } else {`
`423`		`- std::swap(_union, other._union);`
`424`		`- }`
	`415`	`+ std::swap(_union, other._union);`
`425`	`416`	`std::swap(_size, other._size);`
`426`	`417`	`}`
`427`	`418`
Original file line number	Diff line number	Diff line change
`@@ -19,9 +19,11 @@ template<unsigned int N, typename T>`
`19`	`19`	`class prevector_tester {`
`20`	`20`	`typedef std::vector<T> realtype;`
`21`	`21`	`realtype real_vector;`
	`22`	`+ realtype real_vector_alt;`
`22`	`23`
`23`	`24`	`typedef prevector<N, T> pretype;`
`24`	`25`	`pretype pre_vector;`
	`26`	`+ pretype pre_vector_alt;`
`25`	`27`
`26`	`28`	`typedef typename pretype::size_type Size;`
`27`	`29`
`@@ -149,6 +151,12 @@ class prevector_tester {`
`149`	`151`	`pre_vector.shrink_to_fit();`
`150`	`152`	`test();`
`151`	`153`	`}`
	`154`	`+`
	`155`	`+ void swap() {`
	`156`	`+ real_vector.swap(real_vector_alt);`
	`157`	`+ pre_vector.swap(pre_vector_alt);`
	`158`	`+ test();`
	`159`	`+ }`
`152`	`160`	`};`
`153`	`161`
`154`	`162`	`BOOST_AUTO_TEST_CASE(PrevectorTestInt)`
`@@ -204,12 +212,15 @@ BOOST_AUTO_TEST_CASE(PrevectorTestInt)`
`204`	`212`	`if (test.size() > 0) {`
`205`	`213`	`test.update(insecure_rand() % test.size(), insecure_rand());`
`206`	`214`	`}`
`207`		`- if (((r >> 11) & 1024) == 11) {`
	`215`	`+ if (((r >> 11) % 1024) == 11) {`
`208`	`216`	`test.clear();`
`209`	`217`	`}`
`210`		`- if (((r >> 21) & 512) == 12) {`
	`218`	`+ if (((r >> 21) % 512) == 12) {`
`211`	`219`	`test.assign(insecure_rand() % 32, insecure_rand());`
`212`	`220`	`}`
	`221`	`+ if (((r >> 15) % 64) == 3) {`
	`222`	`+ test.swap();`
	`223`	`+ }`
`213`	`224`	`}`
`214`	`225`	`}`
`215`	`226`	`}`