From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21713 invoked by alias); 22 Jun 2015 15:10:14 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 21616 invoked by uid 89); 22 Jun 2015 15:10:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.7 required=5.0 tests=AWL,BAYES_50,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 22 Jun 2015 15:10:04 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id E3DAA2CD80A; Mon, 22 Jun 2015 15:10:01 +0000 (UTC) Received: from localhost (ovpn-116-102.ams2.redhat.com [10.36.116.102]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t5MFA1CD023796; Mon, 22 Jun 2015 11:10:01 -0400 Date: Mon, 22 Jun 2015 15:11:00 -0000 From: Jonathan Wakely To: =?iso-8859-1?Q?Fran=E7ois?= Dumont Cc: "libstdc++@gcc.gnu.org" , gcc-patches Subject: Re: Do not take address of empty string front Message-ID: <20150622151000.GI2856@redhat.com> References: <55853A70.7030607@gmail.com> <20150620115928.GG2856@redhat.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="YIleam+9adpUeYf+" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20150620115928.GG2856@redhat.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2015-06/txt/msg01437.txt.bz2 --YIleam+9adpUeYf+ Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit Content-length: 1505 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. Tested powerpc64le-linux, committed to trunk. François, your changes to add extra checks in std::string are still useful separately. --YIleam+9adpUeYf+ Content-Type: text/x-patch; charset=us-ascii Content-Disposition: attachment; filename="patch.txt" Content-length: 1758 commit 4ab3f0a76f7e18074c91c4644cbfdf23084e93ba Author: Jonathan Wakely Date: Mon Jun 22 13:47:24 2015 +0100 * include/bits/locale_conv.h (__do_str_codecvt): Handle empty range. (wstring_convert): Move into __cxx11 namespace. (wbuffer_convert(streambuf*, _Codecvt*, state_type)): Fix exception message. diff --git a/libstdc++-v3/include/bits/locale_conv.h b/libstdc++-v3/include/bits/locale_conv.h index 61b535c..fd99499 100644 --- 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; @@ -150,6 +156,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return __str_codecvt_out(__first, __last, __outstr, __cvt, __state, __n); } +_GLIBCXX_BEGIN_NAMESPACE_CXX11 + /// String conversions template, @@ -301,6 +309,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION bool _M_with_strings = false; }; +_GLIBCXX_END_NAMESPACE_CXX11 + /// Buffer conversions template> @@ -325,7 +335,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(); --YIleam+9adpUeYf+--