From: "François Dumont" <frs.dumont@gmail.com>
To: "libstdc++@gcc.gnu.org" <libstdc++@gcc.gnu.org>,
gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Do not take address of empty string front
Date: Sat, 20 Jun 2015 10:47:00 -0000 [thread overview]
Message-ID: <55853A70.7030607@gmail.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]
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
[-- Attachment #2: debug_string.patch --]
[-- Type: text/x-patch, Size: 5201 bytes --]
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) \
next reply other threads:[~2015-06-20 10:03 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-20 10:47 François Dumont [this message]
2015-06-20 20:38 ` Jonathan Wakely
2015-06-22 15:11 ` Jonathan Wakely
2015-06-23 20:09 ` François Dumont
2015-06-25 18:27 ` Jonathan Wakely
2015-06-24 20:17 ` François Dumont
2015-07-02 22:54 ` Jonathan Wakely
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55853A70.7030607@gmail.com \
--to=frs.dumont@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=libstdc++@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).