Hi 2 experimental tests are failing in debug mode because __do_str_codecvt is sometimes taking address of string front() and back() even if empty. It wasn't use so not a big issue but it still seems better to avoid. I propose to rather use string begin() to get buffer address. I kept operator& as it is what was used, do you think we should use addressof ?
At the same time I improve string debug mode cause with front() implemented as operator[](0) it is ok even if string is empty while back() implemented as operator[](size()-1) is not which is quite inconsistent. So I added a number of !empty() checks in several methods to detect more easily all those invalid usages. * include/bits/basic_string.h (basic_string<>::front()): Add !empty debug check. (basic_string<>::back()): Likewise. (basic_string<>::pop_back()): Likewise. * include/bits/locale_env.h (__do_std_codecvt): Do not take address of string::front() when string is empty. * include/debug/macros.h (__glibcxx_check_string, __glibcxx_check_string_len): Implement using _GLIBCXX_DEBUG_PEDASSERT. Tested under Linux x86_64 debug mode. Ok to commit ? François
diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index 093f502..923fb83 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -39,6 +39,7 @@ #include <ext/atomicity.h> #include <ext/alloc_traits.h> #include <debug/debug.h> + #if __cplusplus >= 201103L #include <initializer_list> #endif @@ -903,7 +904,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 */ reference front() noexcept - { return operator[](0); } + { + _GLIBCXX_DEBUG_ASSERT(!empty()); + return operator[](0); + } /** * Returns a read-only (constant) reference to the data at the first @@ -911,7 +915,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 */ const_reference front() const noexcept - { return operator[](0); } + { + _GLIBCXX_DEBUG_ASSERT(!empty()); + return operator[](0); + } /** * Returns a read/write reference to the data at the last @@ -919,7 +926,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 */ reference back() noexcept - { return operator[](this->size() - 1); } + { + _GLIBCXX_DEBUG_ASSERT(!empty()); + return operator[](this->size() - 1); + } /** * Returns a read-only (constant) reference to the data at the @@ -927,7 +937,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 */ const_reference back() const noexcept - { return operator[](this->size() - 1); } + { + _GLIBCXX_DEBUG_ASSERT(!empty()); + return operator[](this->size() - 1); + } #endif // Modifiers: @@ -1506,7 +1519,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 */ void pop_back() noexcept - { _M_erase(size()-1, 1); } + { + _GLIBCXX_DEBUG_ASSERT(!empty()); + _M_erase(size() - 1, 1); + } #endif // C++11 /** @@ -3308,7 +3324,10 @@ _GLIBCXX_END_NAMESPACE_CXX11 */ reference front() - { return operator[](0); } + { + _GLIBCXX_DEBUG_ASSERT(!empty()); + return operator[](0); + } /** * Returns a read-only (constant) reference to the data at the first @@ -3316,7 +3335,10 @@ _GLIBCXX_END_NAMESPACE_CXX11 */ const_reference front() const _GLIBCXX_NOEXCEPT - { return operator[](0); } + { + _GLIBCXX_DEBUG_ASSERT(!empty()); + return operator[](0); + } /** * Returns a read/write reference to the data at the last @@ -3324,7 +3346,10 @@ _GLIBCXX_END_NAMESPACE_CXX11 */ reference back() - { return operator[](this->size() - 1); } + { + _GLIBCXX_DEBUG_ASSERT(!empty()); + return operator[](this->size() - 1); + } /** * Returns a read-only (constant) reference to the data at the @@ -3332,7 +3357,10 @@ _GLIBCXX_END_NAMESPACE_CXX11 */ const_reference back() const _GLIBCXX_NOEXCEPT - { return operator[](this->size() - 1); } + { + _GLIBCXX_DEBUG_ASSERT(!empty()); + return operator[](this->size() - 1); + } #endif // Modifiers: @@ -3819,7 +3847,10 @@ _GLIBCXX_END_NAMESPACE_CXX11 */ void pop_back() // FIXME C++11: should be noexcept. - { erase(size()-1, 1); } + { + _GLIBCXX_DEBUG_ASSERT(!empty()); + erase(size() - 1, 1); + } #endif // C++11 /** diff --git a/libstdc++-v3/include/bits/locale_conv.h b/libstdc++-v3/include/bits/locale_conv.h index 61b535c..8def30d 100644 --- a/libstdc++-v3/include/bits/locale_conv.h +++ b/libstdc++-v3/include/bits/locale_conv.h @@ -66,10 +66,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION do { __outstr.resize(__outstr.size() + (__last - __next) * __maxlen); - auto __outnext = &__outstr.front() + __outchars; - auto const __outlast = &__outstr.back() + 1; + auto __outnext = &(*__outstr.begin()) + __outchars; + auto const __outlast = &(*__outstr.begin()) + __outstr.size(); __result = (__cvt.*__fn)(__state, __next, __last, __next, - __outnext, __outlast, __outnext); + __outnext, __outlast, __outnext); __outchars = __outnext - &__outstr.front(); } while (__result == codecvt_base::partial && __next != __last diff --git a/libstdc++-v3/include/debug/macros.h b/libstdc++-v3/include/debug/macros.h index 5136c4b..39cb0d6 100644 --- a/libstdc++-v3/include/debug/macros.h +++ b/libstdc++-v3/include/debug/macros.h @@ -358,14 +358,9 @@ _GLIBCXX_DEBUG_VERIFY(_This.get_allocator() == _Other.get_allocator(), \ _M_message(__gnu_debug::__msg_equal_allocs) \ ._M_sequence(_This, "this")) -#ifdef _GLIBCXX_DEBUG_PEDANTIC -# define __glibcxx_check_string(_String) _GLIBCXX_DEBUG_ASSERT(_String != 0) -# define __glibcxx_check_string_len(_String,_Len) \ - _GLIBCXX_DEBUG_ASSERT(_String != 0 || _Len == 0) -#else -# define __glibcxx_check_string(_String) -# define __glibcxx_check_string_len(_String,_Len) -#endif +#define __glibcxx_check_string(_String) _GLIBCXX_DEBUG_PEDASSERT(_String != 0) +#define __glibcxx_check_string_len(_String,_Len) \ + _GLIBCXX_DEBUG_PEDASSERT(_String != 0 || _Len == 0) // Verify that a predicate is irreflexive #define __glibcxx_check_irreflexive(_First,_Last) \