This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: Container debug light mode
- From: FranÃois Dumont <frs dot dumont at gmail dot com>
- To: Jonathan Wakely <jwakely at redhat dot com>
- Cc: "libstdc++ at gcc dot gnu dot org" <libstdc++ at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 16 Jun 2016 21:28:48 +0200
- Subject: Re: Container debug light mode
- Authentication-results: sourceware.org; auth=none
- References: <575885E2 dot 1050000 at gmail dot com> <20160613102139 dot GB11538 at redhat dot com>
And here is the patch to only add light debug checks to vector and deque.
* include/debug/debug.h
(__glibcxx_requires_non_empty_range, __glibcxx_requires_nonempty)
(__glibcxx_requires_subscript): Move...
* include/debug/assertions.h: ...here and add __builtin_expect.
(_GLIBCXX_DEBUG_ONLY): Remove ; value.
* include/bits/stl_deque.h
(std::deque<>::operator[]): Add __glibcxx_requires_subscript check.
(std::deque<>::front()): Add __glibcxx_requires_nonempty check.
(std::deque<>::back()): Likewise.
(std::deque<>::pop_front()): Likewise.
(std::deque<>::pop_back()): Likewise.
(std::deque<>::swap(deque&)): Add allocator check.
* include/bits/stl_vector.h
(std::vector<>::operator[]): Add __glibcxx_requires_subscript check.
(std::vector<>::front()): Add __glibcxx_requires_nonempty check.
(std::vector<>::back()): Likewise.
(std::vector<>::pop_back()): Likewise.
(std::vector<>::swap(vector&)): Add allocator check.
Tested under Linux x86_64.
François
On 13/06/2016 12:21, Jonathan Wakely wrote:
On 08/06/16 22:53 +0200, François Dumont wrote:
Hi
Here is the patch I already proposed to introduce the debug light
mode for vector and deque containers.
It also simplify some internal calls.
This looks great, and I'd like to see it on trunk, but could you split
it into two patches please? The simplifications to use
__iterator_category and replace insert() with _M_insert_* are good but
are unrelated to the debug mode parts so if there are two separate
commits it's easier to backport one piece separately, or to identify
any regressions that might be introduced.
diff --git a/libstdc++-v3/include/bits/stl_deque.h b/libstdc++-v3/include/bits/stl_deque.h
index f63ae4c..66b8da6 100644
--- a/libstdc++-v3/include/bits/stl_deque.h
+++ b/libstdc++-v3/include/bits/stl_deque.h
@@ -63,6 +63,8 @@
#include <initializer_list>
#endif
+#include <debug/assertions.h>
+
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_CONTAINER
@@ -1365,7 +1367,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
operator[](size_type __n) _GLIBCXX_NOEXCEPT
- { return this->_M_impl._M_start[difference_type(__n)]; }
+ {
+ __glibcxx_requires_subscript(__n);
+ return this->_M_impl._M_start[difference_type(__n)];
+ }
/**
* @brief Subscript access to the data contained in the %deque.
@@ -1380,7 +1385,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
operator[](size_type __n) const _GLIBCXX_NOEXCEPT
- { return this->_M_impl._M_start[difference_type(__n)]; }
+ {
+ __glibcxx_requires_subscript(__n);
+ return this->_M_impl._M_start[difference_type(__n)];
+ }
protected:
/// Safety check used only from at().
@@ -1437,7 +1445,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
front() _GLIBCXX_NOEXCEPT
- { return *begin(); }
+ {
+ __glibcxx_requires_nonempty();
+ return *begin();
+ }
/**
* Returns a read-only (constant) reference to the data at the first
@@ -1445,7 +1456,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
front() const _GLIBCXX_NOEXCEPT
- { return *begin(); }
+ {
+ __glibcxx_requires_nonempty();
+ return *begin();
+ }
/**
* Returns a read/write reference to the data at the last element of the
@@ -1454,6 +1468,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
reference
back() _GLIBCXX_NOEXCEPT
{
+ __glibcxx_requires_nonempty();
iterator __tmp = end();
--__tmp;
return *__tmp;
@@ -1466,6 +1481,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
const_reference
back() const _GLIBCXX_NOEXCEPT
{
+ __glibcxx_requires_nonempty();
const_iterator __tmp = end();
--__tmp;
return *__tmp;
@@ -1549,6 +1565,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
pop_front() _GLIBCXX_NOEXCEPT
{
+ __glibcxx_requires_nonempty();
if (this->_M_impl._M_start._M_cur
!= this->_M_impl._M_start._M_last - 1)
{
@@ -1571,6 +1588,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
pop_back() _GLIBCXX_NOEXCEPT
{
+ __glibcxx_requires_nonempty();
if (this->_M_impl._M_finish._M_cur
!= this->_M_impl._M_finish._M_first)
{
@@ -1789,6 +1807,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
swap(deque& __x) _GLIBCXX_NOEXCEPT
{
+#if __cplusplus >= 201103L
+ __glibcxx_assert(_Alloc_traits::propagate_on_container_swap::value
+ || _M_get_Tp_allocator() == __x._M_get_Tp_allocator());
+#endif
_M_impl._M_swap_data(__x._M_impl);
_Alloc_traits::_S_on_swap(_M_get_Tp_allocator(),
__x._M_get_Tp_allocator());
diff --git a/libstdc++-v3/include/bits/stl_vector.h b/libstdc++-v3/include/bits/stl_vector.h
index 8badea3..e2c42cb 100644
--- a/libstdc++-v3/include/bits/stl_vector.h
+++ b/libstdc++-v3/include/bits/stl_vector.h
@@ -63,6 +63,8 @@
#include <initializer_list>
#endif
+#include <debug/assertions.h>
+
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_CONTAINER
@@ -784,7 +786,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
operator[](size_type __n) _GLIBCXX_NOEXCEPT
- { return *(this->_M_impl._M_start + __n); }
+ {
+ __glibcxx_requires_subscript(__n);
+ return *(this->_M_impl._M_start + __n);
+ }
/**
* @brief Subscript access to the data contained in the %vector.
@@ -799,7 +804,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
operator[](size_type __n) const _GLIBCXX_NOEXCEPT
- { return *(this->_M_impl._M_start + __n); }
+ {
+ __glibcxx_requires_subscript(__n);
+ return *(this->_M_impl._M_start + __n);
+ }
protected:
/// Safety check used only from at().
@@ -856,7 +864,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
front() _GLIBCXX_NOEXCEPT
- { return *begin(); }
+ {
+ __glibcxx_requires_nonempty();
+ return *begin();
+ }
/**
* Returns a read-only (constant) reference to the data at the first
@@ -864,7 +875,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
front() const _GLIBCXX_NOEXCEPT
- { return *begin(); }
+ {
+ __glibcxx_requires_nonempty();
+ return *begin();
+ }
/**
* Returns a read/write reference to the data at the last
@@ -872,7 +886,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
back() _GLIBCXX_NOEXCEPT
- { return *(end() - 1); }
+ {
+ __glibcxx_requires_nonempty();
+ return *(end() - 1);
+ }
/**
* Returns a read-only (constant) reference to the data at the
@@ -880,7 +897,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
back() const _GLIBCXX_NOEXCEPT
- { return *(end() - 1); }
+ {
+ __glibcxx_requires_nonempty();
+ return *(end() - 1);
+ }
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// DR 464. Suggestion for new member functions in standard containers.
@@ -955,6 +975,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
pop_back() _GLIBCXX_NOEXCEPT
{
+ __glibcxx_requires_nonempty();
--this->_M_impl._M_finish;
_Alloc_traits::destroy(this->_M_impl, this->_M_impl._M_finish);
}
@@ -1205,6 +1226,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
swap(vector& __x) _GLIBCXX_NOEXCEPT
{
+#if __cplusplus >= 201103L
+ __glibcxx_assert(_Alloc_traits::propagate_on_container_swap::value
+ || _M_get_Tp_allocator() == __x._M_get_Tp_allocator());
+#endif
this->_M_impl._M_swap_data(__x._M_impl);
_Alloc_traits::_S_on_swap(_M_get_Tp_allocator(),
__x._M_get_Tp_allocator());
diff --git a/libstdc++-v3/include/debug/assertions.h b/libstdc++-v3/include/debug/assertions.h
index 802645c..3708d12 100644
--- a/libstdc++-v3/include/debug/assertions.h
+++ b/libstdc++-v3/include/debug/assertions.h
@@ -33,20 +33,36 @@
# define _GLIBCXX_DEBUG_ASSERT(_Condition)
# define _GLIBCXX_DEBUG_PEDASSERT(_Condition)
-# define _GLIBCXX_DEBUG_ONLY(_Statement) ;
+# define _GLIBCXX_DEBUG_ONLY(_Statement)
-#else
-
-#define _GLIBCXX_DEBUG_ASSERT(_Condition) __glibcxx_assert(_Condition)
+#endif
-#ifdef _GLIBCXX_DEBUG_PEDANTIC
-# define _GLIBCXX_DEBUG_PEDASSERT(_Condition) _GLIBCXX_DEBUG_ASSERT(_Condition)
+#ifndef _GLIBCXX_ASSERTIONS
+# define __glibcxx_requires_non_empty_range(_First,_Last)
+# define __glibcxx_requires_nonempty()
+# define __glibcxx_requires_subscript(_N)
#else
-# define _GLIBCXX_DEBUG_PEDASSERT(_Condition)
+
+// Verify that [_First, _Last) forms a non-empty iterator range.
+# define __glibcxx_requires_non_empty_range(_First,_Last) \
+ __glibcxx_assert(__builtin_expect(_First != _Last, true))
+# define __glibcxx_requires_subscript(_N) \
+ __glibcxx_assert(__builtin_expect(_N < this->size(), true))
+// Verify that the container is nonempty
+# define __glibcxx_requires_nonempty() \
+ __glibcxx_assert(__builtin_expect(!this->empty(), true))
#endif
-# define _GLIBCXX_DEBUG_ONLY(_Statement) _Statement
+#ifdef _GLIBCXX_DEBUG
+# define _GLIBCXX_DEBUG_ASSERT(_Condition) __glibcxx_assert(_Condition)
+# ifdef _GLIBCXX_DEBUG_PEDANTIC
+# define _GLIBCXX_DEBUG_PEDASSERT(_Condition) _GLIBCXX_DEBUG_ASSERT(_Condition)
+# else
+# define _GLIBCXX_DEBUG_PEDASSERT(_Condition)
+# endif
+
+# define _GLIBCXX_DEBUG_ONLY(_Statement) _Statement
#endif
#endif // _GLIBCXX_DEBUG_ASSERTIONS
diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h
index 3d46b6d..79fe00d 100644
--- a/libstdc++-v3/include/debug/debug.h
+++ b/libstdc++-v3/include/debug/debug.h
@@ -74,24 +74,11 @@ namespace __gnu_debug
# define __glibcxx_requires_heap_pred(_First,_Last,_Pred)
# define __glibcxx_requires_string(_String)
# define __glibcxx_requires_string_len(_String,_Len)
-# define __glibcxx_requires_subscript(_N)
# define __glibcxx_requires_irreflexive(_First,_Last)
# define __glibcxx_requires_irreflexive2(_First,_Last)
# define __glibcxx_requires_irreflexive_pred(_First,_Last,_Pred)
# define __glibcxx_requires_irreflexive_pred2(_First,_Last,_Pred)
-#ifdef _GLIBCXX_ASSERTIONS
-// Verify that [_First, _Last) forms a non-empty iterator range.
-# define __glibcxx_requires_non_empty_range(_First,_Last) \
- __glibcxx_assert(_First != _Last)
-// Verify that the container is nonempty
-# define __glibcxx_requires_nonempty() \
- __glibcxx_assert(! this->empty())
-#else
-# define __glibcxx_requires_non_empty_range(_First,_Last)
-# define __glibcxx_requires_nonempty()
-#endif
-
#else
# include <debug/macros.h>
@@ -99,8 +86,6 @@ namespace __gnu_debug
# define __glibcxx_requires_cond(_Cond,_Msg) _GLIBCXX_DEBUG_VERIFY(_Cond,_Msg)
# define __glibcxx_requires_valid_range(_First,_Last) \
__glibcxx_check_valid_range(_First,_Last)
-# define __glibcxx_requires_non_empty_range(_First,_Last) \
- __glibcxx_check_non_empty_range(_First,_Last)
# define __glibcxx_requires_sorted(_First,_Last) \
__glibcxx_check_sorted(_First,_Last)
# define __glibcxx_requires_sorted_pred(_First,_Last,_Pred) \
@@ -121,11 +106,9 @@ namespace __gnu_debug
__glibcxx_check_heap(_First,_Last)
# define __glibcxx_requires_heap_pred(_First,_Last,_Pred) \
__glibcxx_check_heap_pred(_First,_Last,_Pred)
-# define __glibcxx_requires_nonempty() __glibcxx_check_nonempty()
# define __glibcxx_requires_string(_String) __glibcxx_check_string(_String)
# define __glibcxx_requires_string_len(_String,_Len) \
__glibcxx_check_string_len(_String,_Len)
-# define __glibcxx_requires_subscript(_N) __glibcxx_check_subscript(_N)
# define __glibcxx_requires_irreflexive(_First,_Last) \
__glibcxx_check_irreflexive(_First,_Last)
# define __glibcxx_requires_irreflexive2(_First,_Last) \
diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h
index 5ee33e8..f1a74eb 100644
--- a/libstdc++-v3/include/debug/helper_functions.h
+++ b/libstdc++-v3/include/debug/helper_functions.h
@@ -138,6 +138,7 @@ namespace __gnu_debug
return __dist.first >= 0;
}
+ // Can't tell so assume it is fine.
return true;
}