This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[patch] libstdc++/61086 - fix ubsan errors in std::vector


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>
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]