This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
[patch] libstdc++/61086 - fix ubsan errors in std::vector
- From: Jonathan Wakely <jwakely at redhat dot com>
- To: libstdc++ at gcc dot gnu dot org, gcc-patches at gcc dot gnu dot org
- Date: Wed, 7 May 2014 13:07:44 +0100
- Subject: [patch] libstdc++/61086 - fix ubsan errors in std::vector
- Authentication-results: sourceware.org; auth=none
The testcase in the PR calls __position._M_const_cast() to get a
mutable iterator and that dereferences the pointer as suggested in
http://gcc.gnu.org/ml/libstdc++/2013-05/msg00031.html
That's invalid because the pointer is not dereferenceable (in this
case it's null but is past-the-end at all times).
I played around with changing the __normal_iterator so we would do
__postition._M_const_cast(begin()) then decided we don't need it at
all and can just as easily obtain a mutable iterator using:
auto __pos = begin() + (__position - cbegin());
I plan to commit the attached patch to trunk and 4.9 soon. I've tested
it on x86_64-linux but not added a testcase because we don't test with
-fsanitize (though we should do) and it only shows up with Clang
anyway.
commit 566623def309c70387e41da2346ff89aa7619b13
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Wed May 7 12:17:41 2014 +0100
PR libstdc++/61086
* include/bits/stl_iterator.h (__normal_iterator::_M_const_cast):
Remove.
* include/bits/stl_vector.h (vector::insert, vector::erase): Use
arithmetic to obtain a mutable iterator from const_iterator.
* include/bits/vector.tcc (vector::insert): Likewise.
* include/debug/vector (vector::erase): Likewise.
* testsuite/23_containers/vector/requirements/dr438/assign_neg.cc:
Adjust dg-error line number.
* testsuite/23_containers/vector/requirements/dr438/
constructor_1_neg.cc: Likewise.
* testsuite/23_containers/vector/requirements/dr438/
constructor_2_neg.cc: Likewise.
* testsuite/23_containers/vector/requirements/dr438/insert_neg.cc:
Likewise.
diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index 16f992c..f4522a4 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -736,21 +736,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Container>::__type>& __i) _GLIBCXX_NOEXCEPT
: _M_current(__i.base()) { }
-#if __cplusplus >= 201103L
- __normal_iterator<typename _Container::pointer, _Container>
- _M_const_cast() const noexcept
- {
- using _PTraits = std::pointer_traits<typename _Container::pointer>;
- return __normal_iterator<typename _Container::pointer, _Container>
- (_PTraits::pointer_to(const_cast<typename _PTraits::element_type&>
- (*_M_current)));
- }
-#else
- __normal_iterator
- _M_const_cast() const
- { return *this; }
-#endif
-
// Forward iterator requirements
reference
operator*() const _GLIBCXX_NOEXCEPT
diff --git a/libstdc++-v3/include/bits/stl_vector.h b/libstdc++-v3/include/bits/stl_vector.h
index 3d3a2cf..0a56c65 100644
--- a/libstdc++-v3/include/bits/stl_vector.h
+++ b/libstdc++-v3/include/bits/stl_vector.h
@@ -1051,7 +1051,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
insert(const_iterator __position, size_type __n, const value_type& __x)
{
difference_type __offset = __position - cbegin();
- _M_fill_insert(__position._M_const_cast(), __n, __x);
+ _M_fill_insert(begin() + __offset, __n, __x);
return begin() + __offset;
}
#else
@@ -1096,7 +1096,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_InputIterator __last)
{
difference_type __offset = __position - cbegin();
- _M_insert_dispatch(__position._M_const_cast(),
+ _M_insert_dispatch(begin() + __offset,
__first, __last, __false_type());
return begin() + __offset;
}
@@ -1144,10 +1144,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
iterator
#if __cplusplus >= 201103L
erase(const_iterator __position)
+ { return _M_erase(begin() + (__position - cbegin())); }
#else
erase(iterator __position)
+ { return _M_erase(__position); }
#endif
- { return _M_erase(__position._M_const_cast()); }
/**
* @brief Remove a range of elements.
@@ -1170,10 +1171,15 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
iterator
#if __cplusplus >= 201103L
erase(const_iterator __first, const_iterator __last)
+ {
+ const auto __beg = begin();
+ const auto __cbeg = cbegin();
+ return _M_erase(__beg + (__first - __cbeg), __beg + (__last - __cbeg));
+ }
#else
erase(iterator __first, iterator __last)
+ { return _M_erase(__first, __last); }
#endif
- { return _M_erase(__first._M_const_cast(), __last._M_const_cast()); }
/**
* @brief Swaps data with another %vector.
diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc
index 299e614..5c3dfae 100644
--- a/libstdc++-v3/include/bits/vector.tcc
+++ b/libstdc++-v3/include/bits/vector.tcc
@@ -121,14 +121,17 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
else
{
#if __cplusplus >= 201103L
+ const auto __pos = begin() + (__position - cbegin());
if (this->_M_impl._M_finish != this->_M_impl._M_end_of_storage)
{
_Tp __x_copy = __x;
- _M_insert_aux(__position._M_const_cast(), std::move(__x_copy));
+ _M_insert_aux(__pos, std::move(__x_copy));
}
else
+ _M_insert_aux(__pos, __x);
+#else
+ _M_insert_aux(__position, __x);
#endif
- _M_insert_aux(__position._M_const_cast(), __x);
}
return iterator(this->_M_impl._M_start + __n);
}
@@ -307,7 +310,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
++this->_M_impl._M_finish;
}
else
- _M_insert_aux(__position._M_const_cast(),
+ _M_insert_aux(begin() + (__position - cbegin()),
std::forward<_Args>(__args)...);
return iterator(this->_M_impl._M_start + __n);
}
diff --git a/libstdc++-v3/include/debug/vector b/libstdc++-v3/include/debug/vector
index 427d19f..f92a77f 100644
--- a/libstdc++-v3/include/debug/vector
+++ b/libstdc++-v3/include/debug/vector
@@ -647,7 +647,7 @@ namespace __debug
}
else
#if __cplusplus >= 201103L
- return iterator(__first.base()._M_const_cast(), this);
+ return begin() + (__first.base() - cbegin().base());
#else
return __first;
#endif
diff --git a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc
index 191fbc7..655c7e8 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1320 }
+// { dg-error "no matching" "" { target *-*-* } 1326 }
#include <vector>
diff --git a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc
index 8818a88..d938aa2 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1246 }
+// { dg-error "no matching" "" { target *-*-* } 1252 }
#include <vector>
diff --git a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc
index 09499bc..f888285 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1246 }
+// { dg-error "no matching" "" { target *-*-* } 1252 }
#include <vector>
#include <utility>
diff --git a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc
index 674e3b5..316249b 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1361 }
+// { dg-error "no matching" "" { target *-*-* } 1367 }
#include <vector>