diff mbox

[google,gcc-4_7,gcc-4_8,integration] Add bounds checks to vector<bool>

Message ID CALoOobNTTBRvLKLg_arRYbpRTA+oiVo2KyGd+owE6RB=+CFUZA@mail.gmail.com
State New
Headers show

Commit Message

Paul Pluzhnikov May 23, 2013, 3:56 p.m. UTC 
+cc libstdc++@gcc.gnu.org

On Thu, May 23, 2013 at 8:51 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> Greetings,
>
> This patch adds (relatively) cheap bounds and dangling checks to
> vector<bool>, similar to the checks I added to vector<T> in r195373,
> r195356, etc.
>
> Ok for google branches (gcc-4_7, gcc-4_8, integration) ?
>
> Thanks,

Comments

Jonathan Wakely May 23, 2013, 4:14 p.m. UTC | #1 
On 23 May 2013 16:56, Paul Pluzhnikov wrote:
>>
>> This patch adds (relatively) cheap bounds and dangling checks to
>> vector<bool>, similar to the checks I added to vector<T> in r195373,
>> r195356, etc.

I was wondering the other day whether we should put these checks on
trunk and enable them automatically when !defined(__OPTIMIZE__)

Now that we have -Og you could use that to disable the checks without
sacrificing debuggability.
Paul Pluzhnikov May 23, 2013, 5:13 p.m. UTC | #2 
On Thu, May 23, 2013 at 9:14 AM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:

> I was wondering the other day whether we should put these checks on
> trunk and enable them automatically when !defined(__OPTIMIZE__)

FWIW, we keep this under a separate macro so we can turn it on or off
independent of other build options.

Our current code looks like this:

#if !defined(__google_stl_debug_vector)
# if !defined(NDEBUG)
#  define __google_stl_debug_vector 1
# endif
#endif


Keying off NDEBUG rather than __OPTIMIZE__ seems like a more
consistent approach -- if you want assert()s, then you probably also
want these checks.
Paul Pluzhnikov May 24, 2013, 6:35 p.m. UTC | #3 
Jonathan,

On Thu, May 23, 2013 at 10:13 AM, Paul Pluzhnikov
<ppluzhnikov@google.com> wrote:
> On Thu, May 23, 2013 at 9:14 AM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>
>> I was wondering the other day whether we should put these checks on
>> trunk and enable them automatically when !defined(__OPTIMIZE__)
>
> FWIW, we keep this under a separate macro so we can turn it on or off
> independent of other build options.
>
> Our current code looks like this:
>
> #if !defined(__google_stl_debug_vector)
> # if !defined(NDEBUG)
> #  define __google_stl_debug_vector 1
> # endif
> #endif
>
>
> Keying off NDEBUG rather than __OPTIMIZE__ seems like a more
> consistent approach -- if you want assert()s, then you probably also
> want these checks.

I'll be happy to try to push these changes into trunk, if you'd like me to.

The question is what macro(s) should guard the checks, and under what
conditions should they be turned on.

BTW, this is http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56109

Thanks,
Miles Bader June 5, 2013, 5:28 a.m. UTC | #4 
Paul Pluzhnikov <ppluzhnikov@google.com> writes:
> Keying off NDEBUG rather than __OPTIMIZE__ seems like a more
> consistent approach -- if you want assert()s, then you probably also
> want these checks.

That's a bad idea.  NDEBUG ("Be really slow unless the user has
positively defined this macro, whose use is rare enough that many
people will not even be aware of it") should be used less, not more.

The __OPTIMIZE__ is far superior, because it happens automatically
when the user specifies any optimization option.

-miles
diff mbox

Patch

Index: libstdc++-v3/include/bits/stl_bvector.h
===================================================================
--- libstdc++-v3/include/bits/stl_bvector.h	(revision 199261)
+++ libstdc++-v3/include/bits/stl_bvector.h	(working copy)
@@ -438,11 +438,31 @@ 
 #endif
 
       ~_Bvector_base()
-      { this->_M_deallocate(); }
+      {
+        this->_M_deallocate();
+#if __google_stl_debug_bvector
+        __builtin_memset(this, 0xcd, sizeof(*this));
+#endif
+      }
 
     protected:
       _Bvector_impl _M_impl;
 
+#if __google_stl_debug_bvector
+      bool _M_is_valid() const
+      {
+	return (this->_M_impl._M_start._M_p == 0
+		&& this->_M_impl._M_finish._M_p == 0
+		&& this->_M_impl._M_end_of_storage == 0)
+	  || (this->_M_impl._M_start._M_p <= this->_M_impl._M_finish._M_p
+	      && this->_M_impl._M_finish._M_p <= this->_M_impl._M_end_of_storage
+	      && (this->_M_impl._M_start._M_p < this->_M_impl._M_end_of_storage
+                  || (this->_M_impl._M_start._M_p == this->_M_impl._M_end_of_storage
+                      && this->_M_impl._M_start._M_offset == 0
+                      && this->_M_impl._M_finish._M_offset == 0)));
+      }
+#endif
+
       _Bit_type*
       _M_allocate(size_t __n)
       { return _M_impl.allocate(_S_nword(__n)); }
@@ -571,6 +591,10 @@ 
     vector&
     operator=(const vector& __x)
     {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("op=() on corrupt (dangling?) vector");
+#endif
       if (&__x == this)
 	return *this;
       if (__x.size() > capacity())
@@ -587,6 +611,10 @@ 
     vector&
     operator=(vector&& __x)
     {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("op=() on corrupt (dangling?) vector");
+#endif
       // NB: DR 1204.
       // NB: DR 675.
       this->clear();
@@ -608,12 +636,22 @@ 
     // or not the type is an integer.
     void
     assign(size_type __n, const bool& __x)
-    { _M_fill_assign(__n, __x); }
+    {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("assign on corrupt (dangling?) vector");
+#endif
+      _M_fill_assign(__n, __x);
+    }
 
     template<typename _InputIterator>
       void
       assign(_InputIterator __first, _InputIterator __last)
       {
+#if __google_stl_debug_bvector
+	if (!this->_M_is_valid())
+	  __throw_logic_error("assign() on corrupt (dangling?) vector");
+#endif
 	typedef typename std::__is_integer<_InputIterator>::__type _Integral;
 	_M_assign_dispatch(__first, __last, _Integral());
       }
@@ -626,19 +664,43 @@ 
 
     iterator
     begin() _GLIBCXX_NOEXCEPT
-    { return this->_M_impl._M_start; }
+    {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("begin() on corrupt (dangling?) vector");
+#endif
+      return this->_M_impl._M_start;
+    }
 
     const_iterator
     begin() const _GLIBCXX_NOEXCEPT
-    { return this->_M_impl._M_start; }
+    {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("begin() on corrupt (dangling?) vector");
+#endif
+      return this->_M_impl._M_start;
+    }
 
     iterator
     end() _GLIBCXX_NOEXCEPT
-    { return this->_M_impl._M_finish; }
+    {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("end() on corrupt (dangling?) vector");
+#endif
+      return this->_M_impl._M_finish;
+    }
 
     const_iterator
     end() const _GLIBCXX_NOEXCEPT
-    { return this->_M_impl._M_finish; }
+    {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("end() on corrupt (dangling?) vector");
+#endif
+      return this->_M_impl._M_finish;
+    }
 
     reverse_iterator
     rbegin() _GLIBCXX_NOEXCEPT
@@ -659,11 +721,23 @@ 
 #ifdef __GXX_EXPERIMENTAL_CXX0X__
     const_iterator
     cbegin() const noexcept
-    { return this->_M_impl._M_start; }
+    {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("cbegin() on corrupt (dangling?) vector");
+#endif
+      return this->_M_impl._M_start;
+    }
 
     const_iterator
     cend() const noexcept
-    { return this->_M_impl._M_finish; }
+    {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("cend() on corrupt (dangling?) vector");
+#endif
+      return this->_M_impl._M_finish;
+    }
 
     const_reverse_iterator
     crbegin() const noexcept
@@ -681,6 +755,10 @@ 
     size_type
     max_size() const _GLIBCXX_NOEXCEPT
     {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("max_size() on corrupt (dangling?) vector");
+#endif
       const size_type __isize =
 	__gnu_cxx::__numeric_traits<difference_type>::__max
 	- int(_S_word_bit) + 1;
@@ -701,6 +779,9 @@ 
     reference
     operator[](size_type __n)
     {
+#if __google_stl_debug_bvector
+      _M_range_check(__n);
+#endif
       return *iterator(this->_M_impl._M_start._M_p
 		       + __n / int(_S_word_bit), __n % int(_S_word_bit));
     }
@@ -708,6 +789,9 @@ 
     const_reference
     operator[](size_type __n) const
     {
+#if __google_stl_debug_bvector
+      _M_range_check(__n);
+#endif
       return *const_iterator(this->_M_impl._M_start._M_p
 			     + __n / int(_S_word_bit), __n % int(_S_word_bit));
     }
@@ -740,19 +824,39 @@ 
 
     reference
     front()
-    { return *begin(); }
+    {
+#if __google_stl_debug_bvector
+      _M_range_check(0);
+#endif
+      return *begin();
+    }
 
     const_reference
     front() const
-    { return *begin(); }
+    {
+#if __google_stl_debug_bvector
+      _M_range_check(0);
+#endif
+      return *begin();
+    }
 
     reference
     back()
-    { return *(end() - 1); }
+    {
+#if __google_stl_debug_bvector
+      _M_range_check(0);
+#endif
+      return *(end() - 1);
+    }
 
     const_reference
     back() const
-    { return *(end() - 1); }
+    {
+#if __google_stl_debug_bvector
+      _M_range_check(0);
+#endif
+      return *(end() - 1);
+    }
 
     // _GLIBCXX_RESOLVE_LIB_DEFECTS
     // DR 464. Suggestion for new member functions in standard containers.
@@ -765,6 +869,10 @@ 
     void
     push_back(bool __x)
     {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("push_back() on corrupt (dangling?) vector");
+#endif
       if (this->_M_impl._M_finish._M_p != this->_M_impl._M_end_of_storage)
         *this->_M_impl._M_finish++ = __x;
       else
@@ -774,6 +882,10 @@ 
     void
     swap(vector& __x)
     {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid() || !__x._M_is_valid())
+	__throw_logic_error("swap() on corrupt (dangling?) vector");
+#endif
       std::swap(this->_M_impl._M_start, __x._M_impl._M_start);
       std::swap(this->_M_impl._M_finish, __x._M_impl._M_finish);
       std::swap(this->_M_impl._M_end_of_storage, 
@@ -811,26 +923,50 @@ 
       insert(iterator __position,
 	     _InputIterator __first, _InputIterator __last)
       {
+#if __google_stl_debug_bvector
+	if (!this->_M_is_valid())
+	  __throw_logic_error("insert() on corrupt (dangling?) vector");
+#endif
 	typedef typename std::__is_integer<_InputIterator>::__type _Integral;
 	_M_insert_dispatch(__position, __first, __last, _Integral());
       }
 
     void
     insert(iterator __position, size_type __n, const bool& __x)
-    { _M_fill_insert(__position, __n, __x); }
+    {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("insert() on corrupt (dangling?) vector");
+#endif
+      _M_fill_insert(__position, __n, __x);
+    }
 
 #ifdef __GXX_EXPERIMENTAL_CXX0X__
     void insert(iterator __p, initializer_list<bool> __l)
-    { this->insert(__p, __l.begin(), __l.end()); }
+    {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("insert() on corrupt (dangling?) vector");
 #endif
+      this->insert(__p, __l.begin(), __l.end());
+    }
+#endif
 
     void
     pop_back()
-    { --this->_M_impl._M_finish; }
+    {
+#if __google_stl_debug_bvector
+      _M_range_check(0);
+#endif
+      --this->_M_impl._M_finish;
+    }
 
     iterator
     erase(iterator __position)
     {
+#if __google_stl_debug_bvector
+      _M_range_check(__position - begin());
+#endif
       if (__position + 1 != end())
         std::copy(__position + 1, end(), __position);
       --this->_M_impl._M_finish;
@@ -840,6 +976,10 @@ 
     iterator
     erase(iterator __first, iterator __last)
     {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("erase() on corrupt (dangling?) vector");
+#endif
       if (__first != __last)
 	_M_erase_at_end(std::copy(__last, end(), __first));
       return __first;
@@ -857,12 +997,22 @@ 
 #ifdef __GXX_EXPERIMENTAL_CXX0X__
     void
     shrink_to_fit()
-    { _M_shrink_to_fit(); }
+    {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("shrink_to_fit() on corrupt (dangling?) vector");
 #endif
+      _M_shrink_to_fit();
+    }
+#endif
 
     void
     flip() _GLIBCXX_NOEXCEPT
     {
+#if __google_stl_debug_bvector
+      if (!this->_M_is_valid())
+	__throw_logic_error("flip() on corrupt (dangling?) vector");
+#endif
       for (_Bit_type * __p = this->_M_impl._M_start._M_p;
 	   __p != this->_M_impl._M_end_of_storage; ++__p)
         *__p = ~*__p;