This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Do not take address of empty string front
- From: FranÃois Dumont <frs dot dumont at gmail dot com>
- To: "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: Sat, 20 Jun 2015 12:03:28 +0200
- Subject: Do not take address of empty string front
- Authentication-results: sourceware.org; auth=none
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) \