I think we also need this to make __numpunct_cache and __moneypunct_cache exception-safe. If we set _M_allocated=true at the start of the _M_cache functions and then an allocation throws we will delete[] the memory allocated in _M_cache, but then the cache's destructor will see _M_allocated==true and will try to delete[] them again:
template<typename _CharT> __numpunct_cache<_CharT>::~__numpunct_cache() { if (_M_allocated) { delete [] _M_grouping; delete [] _M_truename; delete [] _M_falsename; } } Delaying setting the bool and the pointers themselves until after all allocations avoids that. The _M_cache functions were also calling virtual functions twice when once is enough. Finally, __timepunct::_M_cache() is declared but never defined, so I want to remove that. Tested x86_64-linux and powerpc64-linux. Does anyone see anything wrong with my reasoning above before I commit this?
diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index d9b11b5..72a4020 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -2,6 +2,14 @@ * config/abi/pre/gnu.ver: Fix ios_base::failure exports. + * include/bits/locale_facets.h (numpunct::_M_cache): Avoid calling + virtual functions twice. Only update _M_allocated after all + allocations have succeeded. + * include/bits/locale_facets_nonio.tcc (moneypunct::_M_cache): + Likewise. + * include/bits/locale_facets_nonio.h (__timepunct::_M_cache): Remove + unused declaration. + 2014-11-29 Jonathan Wakely <jwak...@redhat.com> * include/bits/locale_facets/nonio.h (__timepunct): Remove unused diff --git a/libstdc++-v3/include/bits/locale_facets.tcc b/libstdc++-v3/include/bits/locale_facets.tcc index 88adc0d..200e099 100644 --- a/libstdc++-v3/include/bits/locale_facets.tcc +++ b/libstdc++-v3/include/bits/locale_facets.tcc @@ -77,8 +77,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION void __numpunct_cache<_CharT>::_M_cache(const locale& __loc) { - _M_allocated = true; - const numpunct<_CharT>& __np = use_facet<numpunct<_CharT> >(__loc); char* __grouping = 0; @@ -86,24 +84,24 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _CharT* __falsename = 0; __try { - _M_grouping_size = __np.grouping().size(); + const string& __g = __np.grouping(); + _M_grouping_size = __g.size(); __grouping = new char[_M_grouping_size]; - __np.grouping().copy(__grouping, _M_grouping_size); - _M_grouping = __grouping; + __g.copy(__grouping, _M_grouping_size); _M_use_grouping = (_M_grouping_size - && static_cast<signed char>(_M_grouping[0]) > 0 - && (_M_grouping[0] + && static_cast<signed char>(__grouping[0]) > 0 + && (__grouping[0] != __gnu_cxx::__numeric_traits<char>::__max)); - _M_truename_size = __np.truename().size(); + const basic_string<_CharT>& __tn = __np.truename(); + _M_truename_size = __tn.size(); __truename = new _CharT[_M_truename_size]; - __np.truename().copy(__truename, _M_truename_size); - _M_truename = __truename; + __tn.copy(__truename, _M_truename_size); - _M_falsename_size = __np.falsename().size(); + const basic_string<_CharT>& __fn = __np.falsename(); + _M_falsename_size = __fn.size(); __falsename = new _CharT[_M_falsename_size]; - __np.falsename().copy(__falsename, _M_falsename_size); - _M_falsename = __falsename; + __fn.copy(__falsename, _M_falsename_size); _M_decimal_point = __np.decimal_point(); _M_thousands_sep = __np.thousands_sep(); @@ -115,6 +113,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __ct.widen(__num_base::_S_atoms_in, __num_base::_S_atoms_in + __num_base::_S_iend, _M_atoms_in); + + _M_grouping = __grouping; + _M_truename = __truename; + _M_falsename = __falsename; + _M_allocated = true; } __catch(...) { diff --git a/libstdc++-v3/include/bits/locale_facets_nonio.h b/libstdc++-v3/include/bits/locale_facets_nonio.h index 5c1eeb7..1b0aff9 100644 --- a/libstdc++-v3/include/bits/locale_facets_nonio.h +++ b/libstdc++-v3/include/bits/locale_facets_nonio.h @@ -138,9 +138,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION ~__timepunct_cache(); - void - _M_cache(const locale& __loc); - private: __timepunct_cache& operator=(const __timepunct_cache&); diff --git a/libstdc++-v3/include/bits/locale_facets_nonio.tcc b/libstdc++-v3/include/bits/locale_facets_nonio.tcc index c9f8dac..42c3504 100644 --- a/libstdc++-v3/include/bits/locale_facets_nonio.tcc +++ b/libstdc++-v3/include/bits/locale_facets_nonio.tcc @@ -68,8 +68,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION void __moneypunct_cache<_CharT, _Intl>::_M_cache(const locale& __loc) { - _M_allocated = true; - const moneypunct<_CharT, _Intl>& __mp = use_facet<moneypunct<_CharT, _Intl> >(__loc); @@ -83,29 +81,29 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _CharT* __negative_sign = 0; __try { - _M_grouping_size = __mp.grouping().size(); + const string& __g = __mp.grouping(); + _M_grouping_size = __g.size(); __grouping = new char[_M_grouping_size]; - __mp.grouping().copy(__grouping, _M_grouping_size); - _M_grouping = __grouping; + __g.copy(__grouping, _M_grouping_size); _M_use_grouping = (_M_grouping_size - && static_cast<signed char>(_M_grouping[0]) > 0 - && (_M_grouping[0] + && static_cast<signed char>(__grouping[0]) > 0 + && (__grouping[0] != __gnu_cxx::__numeric_traits<char>::__max)); - _M_curr_symbol_size = __mp.curr_symbol().size(); + const basic_string<_CharT>& __cs = __mp.curr_symbol(); + _M_curr_symbol_size = __cs.size(); __curr_symbol = new _CharT[_M_curr_symbol_size]; - __mp.curr_symbol().copy(__curr_symbol, _M_curr_symbol_size); - _M_curr_symbol = __curr_symbol; + __cs.copy(__curr_symbol, _M_curr_symbol_size); - _M_positive_sign_size = __mp.positive_sign().size(); + const basic_string<_CharT>& __ps = __mp.positive_sign(); + _M_positive_sign_size = __ps.size(); __positive_sign = new _CharT[_M_positive_sign_size]; - __mp.positive_sign().copy(__positive_sign, _M_positive_sign_size); - _M_positive_sign = __positive_sign; + __ps.copy(__positive_sign, _M_positive_sign_size); - _M_negative_sign_size = __mp.negative_sign().size(); + const basic_string<_CharT>& __ns = __mp.negative_sign(); + _M_negative_sign_size = __ns.size(); __negative_sign = new _CharT[_M_negative_sign_size]; - __mp.negative_sign().copy(__negative_sign, _M_negative_sign_size); - _M_negative_sign = __negative_sign; + __ns.copy(__negative_sign, _M_negative_sign_size); _M_pos_format = __mp.pos_format(); _M_neg_format = __mp.neg_format(); @@ -113,6 +111,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION const ctype<_CharT>& __ct = use_facet<ctype<_CharT> >(__loc); __ct.widen(money_base::_S_atoms, money_base::_S_atoms + money_base::_S_end, _M_atoms); + + _M_grouping = __grouping; + _M_curr_symbol = __curr_symbol; + _M_positive_sign = __positive_sign; + _M_negative_sign = __negative_sign; + _M_allocated = true; } __catch(...) {