diff mbox

Do not take address of empty string front

Message ID 20150702225451.GI18520@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely July 2, 2015, 10:54 p.m. UTC 
On 22/06/15 16:10 +0100, Jonathan Wakely wrote:
>On 20/06/15 12:59 +0100, Jonathan Wakely wrote:
>>On 20/06/15 12:03 +0200, François Dumont wrote:
>>>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.
>>
>>But derefencing begin() is still undefined for an empty string.
>>Shouldn't that fail for debug mode too? Why change one form of
>>undefined behaviour that we diagnose to another form that we don't
>>diagnose?
>>
>>It would be better if that function didn't do any work when the input
>>range is empty:
>>
>>--- a/libstdc++-v3/include/bits/locale_conv.h
>>+++ b/libstdc++-v3/include/bits/locale_conv.h
>>@@ -58,6 +58,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>                   _OutStr& __outstr, const _Codecvt& __cvt, _State& __state,
>>                   size_t& __count, _Fn __fn)
>>   {
>>+      if (__first == __last)
>>+       {
>>+         __outstr.clear();
>>+         return true;
>>+       }
>>+
>>     size_t __outchars = 0;
>>     auto __next = __first;
>>     const auto __maxlen = __cvt.max_length() + 1;
>
>This makes that change, and also moves wstring_convert into the
>ABI-tagged __cxx11 namespace, and fixes a copy&paste error in the
>exception thrown from wbuffer_convert.

This is the equivalent patch for the branch.

Tested powerpc64le-linux, committed to gcc-5-branch.
diff mbox

Patch

commit ab6011c5ffbd16f7f3f509f6e9fec6dc9f7daf36
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jul 1 15:41:33 2015 +0100

    	* include/bits/locale_conv.h (wstring_convert): Use __cxx11 inline
    	namespace in new ABI.
    	(wstring_convert::_M_conv): Handle empty range.

diff --git a/libstdc++-v3/include/bits/locale_conv.h b/libstdc++-v3/include/bits/locale_conv.h
index 9be2866..de49dd5 100644
--- a/libstdc++-v3/include/bits/locale_conv.h
+++ b/libstdc++-v3/include/bits/locale_conv.h
@@ -51,6 +51,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
    * @{
    */
 
+_GLIBCXX_BEGIN_NAMESPACE_CXX11
   /// String conversions
   template<typename _Codecvt, typename _Elem = wchar_t,
 	   typename _Wide_alloc = allocator<_Elem>,
@@ -192,10 +193,17 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	_M_conv(const _InChar* __first, const _InChar* __last,
 		const _OutStr* __err, _MemFn __memfn)
 	{
+	  auto __outstr = __err ? _OutStr(__err->get_allocator()) : _OutStr();
+
+	  if (__first == __last)
+	    {
+	      _M_count = 0;
+	      return __outstr;
+	    }
+
 	  if (!_M_with_cvtstate)
 	    _M_state = state_type();
 
-	  auto __outstr = __err ? _OutStr(__err->get_allocator()) : _OutStr();
 	  size_t __outchars = 0;
 	  auto __next = __first;
 	  const auto __maxlen = _M_cvt->max_length() + 1;
@@ -239,6 +247,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       bool			_M_with_cvtstate = false;
       bool			_M_with_strings = false;
     };
+_GLIBCXX_END_NAMESPACE_CXX11
 
   /// Buffer conversions
   template<typename _Codecvt, typename _Elem = wchar_t,
@@ -264,7 +273,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       : _M_buf(__bytebuf), _M_cvt(__pcvt), _M_state(__state)
       {
 	if (!_M_cvt)
-	  __throw_logic_error("wstring_convert");
+	  __throw_logic_error("wbuffer_convert");
 
 	_M_always_noconv = _M_cvt->always_noconv();