From patchwork Mon Jan 26 23:42:45 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 433073 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 8A7B7140145 for ; Tue, 27 Jan 2015 10:42:58 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; q=dns; s= default; b=ytDW8kEV2FfCcXK2EVHkfKxyNs5nD8IrdMtmnm1ZJ9WJvcBuSR3zT KD9ta9Uqa0RutTjmVkpNDRmBr5xkWRPv9lz01SgK3GOyF/SCK/YFGPJOJRV8gRsc lVWHTZNy7u0cn9DxDhIPama86YLUMdhItErA/HLWfSHk0SXfKYmiD0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; s= default; bh=i2IkVtlCC91On0/GYVAko8wRq9w=; b=FyOi5ISYwcNcqoSH5Gk5 eiFASQrBYqQP4u2z5+uGyeDs1Ckw0Oe5uwZFirXLCBYDwGCzaqJSXFvYA2L493Sg /vt6sR0e9zDBgEXk8ckABCQXYSTSP5AjL7Ikz4ZN15RXKPKMk+7X8l7Yx0msZsqo Qkp2Jq+7Rr246ecr1HfH05Y= Received: (qmail 11868 invoked by alias); 26 Jan 2015 23:42:50 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 11844 invoked by uid 89); 26 Jan 2015 23:42:48 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL, SPF_HELO_PASS, T_RP_MATCHES_RCVD 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, 26 Jan 2015 23:42:47 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0QNgkCi011584 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Mon, 26 Jan 2015 18:42:46 -0500 Received: from localhost (ovpn-116-136.ams2.redhat.com [10.36.116.136]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t0QNgj5o022665; Mon, 26 Jan 2015 18:42:45 -0500 Date: Mon, 26 Jan 2015 23:42:45 +0000 From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [patch] libstdc++/64368 Fix invalid frees in numpunct_shim with clocale=generic Message-ID: <20150126234244.GY3360@redhat.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) This should fix most of the FAILs seen on targets that use --enable-clocale=generic When I added numpunct_shim I think I got confused/scared by the clocale=gnu version of numpunct::_M_initialize_numpunct() and decided not to touch the _M_grouping member of the cache. Looking at it again now, _M_grouping will only ever point to "" when called by the base class of a numpunct_shim object, and my comment about numpunct using _M_grouping for its own purposes seems to be wrong, so numpunct_shim can just use that member of the cache in the same way it uses the others. Tested x86_64-linux, with the gnu and generic clocale models, and on i686-linux with clocale=gnu only. commit 06b0c37a34789f14726f8280cae2e70d8942b864 Author: Jonathan Wakely Date: Mon Jan 26 19:57:08 2015 +0000 PR libstdc++/64368 * config/locale/gnu/numeric_members.cc (numpunct::~numpunct(), numpunct::~numpunct()): Do not set _M_data->_M_grouping. * src/c++11/cxx11-shim_facets.cc (numpunct_shim): Remove _M_grouping and use cache's _M_grouping field. (__numpunct_fill_cache): Likewise. (__moneypunct_fill_cache): Improve comments. diff --git a/libstdc++-v3/config/locale/gnu/numeric_members.cc b/libstdc++-v3/config/locale/gnu/numeric_members.cc index 919a5c6..aab01f9c 100644 --- a/libstdc++-v3/config/locale/gnu/numeric_members.cc +++ b/libstdc++-v3/config/locale/gnu/numeric_members.cc @@ -117,7 +117,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { if (_M_data->_M_grouping_size) delete [] _M_data->_M_grouping; - _M_data->_M_grouping = 0; delete _M_data; } @@ -210,7 +209,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { if (_M_data->_M_grouping_size) delete [] _M_data->_M_grouping; - _M_data->_M_grouping = 0; delete _M_data; } #endif diff --git a/libstdc++-v3/src/c++11/cxx11-shim_facets.cc b/libstdc++-v3/src/c++11/cxx11-shim_facets.cc index 407b7b9..82bdf6f 100644 --- a/libstdc++-v3/src/c++11/cxx11-shim_facets.cc +++ b/libstdc++-v3/src/c++11/cxx11-shim_facets.cc @@ -170,8 +170,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template void - __numpunct_fill_cache(other_abi, const facet*, __numpunct_cache*, - const char*&, size_t&); + __numpunct_fill_cache(other_abi, const facet*, __numpunct_cache*); template int @@ -235,24 +234,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION numpunct_shim(const facet* f, __cache_type* c = new __cache_type) : std::numpunct<_CharT>(c), __shim(f), _M_cache(c) { - __numpunct_fill_cache(other_abi{}, f, c, _M_grouping, - _M_grouping_size); + __numpunct_fill_cache(other_abi{}, f, c); } - ~numpunct_shim() { delete[] _M_grouping; } - - virtual string - do_grouping() const - { return string(_M_grouping, _M_grouping_size); } + ~numpunct_shim() + { + // Stop GNU locale's ~numpunct() from freeing the cached string. + _M_cache->_M_grouping_size = 0; + } - // No need to override other virtual functions, the base definitions + // No need to override any virtual functions, the base definitions // will return the cached data. __cache_type* _M_cache; - // numpunct uses __numpunct_cache::_M_grouping for its own purposes - // so we can't store that in the cache - const char* _M_grouping; - size_t _M_grouping_size; }; template @@ -348,7 +342,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION ~moneypunct_shim() { - // stop GNU locale's ~moneypunct() from freeing these strings + // Stop GNU locale's ~moneypunct() from freeing the cached strings. _M_cache->_M_grouping_size = 0; _M_cache->_M_curr_symbol_size = 0; _M_cache->_M_positive_sign_size = 0; @@ -497,37 +491,36 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Now define and instantiate the functions that will be called by the // shim facets defined when this file is recompiled for the other ABI. + // Cache the values returned by the numpunct facet f. + // Sets c->_M_allocated so that the __numpunct_cache destructor will + // delete[] the strings allocated by this function. template void - __numpunct_fill_cache(current_abi, const facet* f, __numpunct_cache* c, - const char*& grouping, size_t& grouping_size) + __numpunct_fill_cache(current_abi, const facet* f, __numpunct_cache* c) { auto* m = static_cast*>(f); c->_M_decimal_point = m->decimal_point(); c->_M_thousands_sep = m->thousands_sep(); + c->_M_grouping = nullptr; c->_M_truename = nullptr; c->_M_falsename = nullptr; // set _M_allocated so that if any allocation fails the previously - // allocated strings will be deleted in ~__numpunct_c() + // allocated strings will be deleted in ~__numpunct_cache() c->_M_allocated = true; + c->_M_grouping_size = __copy(c->_M_grouping, m->grouping()); c->_M_truename_size = __copy(c->_M_truename, m->truename()); c->_M_falsename_size = __copy(c->_M_falsename, m->falsename()); - // Set grouping last as it is only deleted by ~numpunct_shim() which - // won't run if this function throws an exception. - grouping_size = __copy(grouping, m->grouping()); } template void - __numpunct_fill_cache(current_abi, const facet*, __numpunct_cache*, - const char*&, size_t&); + __numpunct_fill_cache(current_abi, const facet*, __numpunct_cache*); #ifdef _GLIBCXX_USE_WCHAR_T template void - __numpunct_fill_cache(current_abi, const facet*, __numpunct_cache*, - const char*&, size_t&); + __numpunct_fill_cache(current_abi, const facet*, __numpunct_cache*); #endif template @@ -567,6 +560,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION const wchar_t*, const wchar_t*); #endif + // Cache the values returned by the moneypunct facet, f. + // Sets c->_M_allocated so that the __moneypunct_cache destructor will + // delete[] the strings allocated by this function. template void __moneypunct_fill_cache(current_abi, const facet* f, @@ -582,8 +578,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION c->_M_curr_symbol = nullptr; c->_M_positive_sign = nullptr; c->_M_negative_sign = nullptr; - // set _M_allocated so that if any allocation fails the previously - // allocated strings will be deleted in ~__moneypunct_c() + // Set _M_allocated so that if any allocation fails the previously + // allocated strings will be deleted in ~__moneypunct_cache(). c->_M_allocated = true; c->_M_grouping_size = __copy(c->_M_grouping, m->grouping());