[PATCH] D17053: [libcxx]: vector: Use < instead of != to improve failure mode

[Duncan P. N. Exon Smith via cfe-commits]

dexonsmith created this revision.
dexonsmith added reviewers: mclow.lists, EricWF.
dexonsmith added a subscriber: cfe-commits.

The following program is obviously broken:

    #include <vector>
    int main(int argc, const char * argv[]) {
        std::vector<int> v;
        v.push_back(0);
        v.pop_back();
        v.pop_back();
        return 0;
    }

The failure mode at -O0 is pretty painful though.  There's no crash;
instead std::vector<int>::~vector() hits an infinite loop in
`std::__vector_base<int>::destruct_at_end()`.  In the following snippet,
`__new_last` starts *smaller* than `__end_`:

    while (__new_last != __end_)
        __alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__end_));

Note that `__alloc_traits::destroy()` is a no-op because `int` is POD,
so we don't get any bad memory accesses here.  ASan does not catch this.

This patch changes the condition to:

    while (__new_last < __end_)
        __alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__end_));

making the infinite loop less likely.  Since `__new_last` and `__end_`
are raw pointers, `<` should be the same cost as `!=`.

Unfortunately, I'm not sure how to add a test for this change.

I'm also not confident this is the best approach; suggestions welcome.
Maybe someone can even argue that the infinite loop is *better*...?

http://reviews.llvm.org/D17053

Files:
  include/vector

Index: include/vector
===================================================================
--- include/vector
+++ include/vector
@@ -420,7 +420,7 @@
 void
 __vector_base<_Tp, _Allocator>::__destruct_at_end(pointer __new_last) _NOEXCEPT
 {
-    while (__new_last != __end_)
+    while (__new_last < __end_)
         __alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__end_));
 }
 



Index: include/vector
===================================================================
--- include/vector
+++ include/vector
@@ -420,7 +420,7 @@
 void
 __vector_base<_Tp, _Allocator>::__destruct_at_end(pointer __new_last) _NOEXCEPT
 {
-    while (__new_last != __end_)
+    while (__new_last < __end_)
         __alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__end_));
 }
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits