Bug 40712 - locale(const locale&, const char*, locale::category) can create broken locale
Summary: locale(const locale&, const char*, locale::category) can create broken locale
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: unknown
: P3 normal
Target Milestone: 4.5.0
Assignee: Paolo Carlini
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-10 12:56 UTC by Andrey Tsyvarev
Modified: 2009-08-03 09:24 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2009-07-10 21:14:43


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Tsyvarev 2009-07-10 12:56:33 UTC
This code causes SIGFAULT on Ubuntu 8.10:

#include <locale>
using namespace std;

int main()
{
    locale loc(locale("C"), "en_US", locale::monetary);
    use_facet<moneypunct<char> >(loc).grouping();
    return 0;
}

Tested both with native gcc and one builded from svn.
According to gdb, sigfault is caused by strlen while converting c-string to c++-string when returns from moneypunct<char>::do_grouping().

andrew@andrew-desktop:~/work/test$ gcc --version
gcc (Ubuntu 4.3.2-1ubuntu12) 4.3.2
Copyright (C) 2008 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

andrew@andrew-desktop:~/work/test$ /home/andrew/gcc/bin/gcc --version
gcc (GCC) 4.5.0 20090709 (experimental)
Copyright (C) 2009 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

andrew@andrew-desktop:~/work/test$ g++ test.cpp && ./a.out
Segmentation fault
Comment 1 Paolo Carlini 2009-07-10 21:14:43 UTC
By the way, sEgfault, not sIgfault.
Comment 2 Paolo Carlini 2009-07-11 19:43:19 UTC
I think this constructor never ever worked correctly. The only solution I can see at the moment is consistently dynamically allocating _M_data->_M_grouping, and copying the characters of __nl_langinfo_l(__MON_GROUPING, __cloc) into it as part of _M_initialize_moneypunct. The same for the other C strings, for numpunct too, of course. Isn't such a big issue, after all, but I'm rather surprised that we didn't notice the issue much earlier: destroying the __cloc at the end of locale::_Impl::_Impl(const char*, size_t) after having referred to the various __nl_langinfo_l(..., __cloc) in _M_initialize_moneypunct without actually copying the data should unavoidably cause problems...
Comment 3 Andrey Tsyvarev 2009-07-13 11:55:35 UTC
(In reply to comment #2)
> I think this constructor never ever worked correctly. The only solution I can
> see at the moment is consistently dynamically allocating _M_data->_M_grouping,
> and copying the characters of __nl_langinfo_l(__MON_GROUPING, __cloc) into it
> as part of _M_initialize_moneypunct.
Reasonable solution. Actually, I thougth that _M_initialize_moneypunct had already implemented in such way.

As for "we didn't notice the issue much earlier" - it is strange, but in many other cases locale, created with this constructor, behaves correctly(at least, does not cause program to abort and remains internal properties). And this case crashes program not on every system.

Comment 4 paolo@gcc.gnu.org 2009-07-18 22:58:22 UTC
Subject: Bug 40712

Author: paolo
Date: Sat Jul 18 22:58:10 2009
New Revision: 149782

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149782
Log:
2009-07-18  Paolo Carlini  <paolo.carlini@oracle.com>

	PR libstdc++/40712
	* config/locale/gnu/numeric_members.cc (numpunct<>::
	_M_initialize_numpunct): Dynamically allocate _M_data->_M_grouping
	and copy the langinfo data into it.
	(numpunct<>::~numpunct): Free the allocated memory.
	* config/locale/gnu/monetary_members.cc (moneypunct<>::
	_M_initialize_moneypunct): Dynamically allocate _M_data->_M_grouping,
	_M_data->_M_positive_sign, _M_data->_M_negative_sign,
	_M_data->_M_curr_symbol.
	(moneypunct<>::~moneypunct): Free the allocated memory.
	* testsuite/22_locale/moneypunct/40712.cc: New.

	* include/bits/locale_facets.tcc (__numpunct_cache<>::_M_cache):
	Do not leak memory if new throws.
	* include/bits/locale_facets_nonio.tcc
	(__moneypunct_cache<>::_M_cache): Likewise.

Added:
    trunk/libstdc++-v3/testsuite/22_locale/moneypunct/40712.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/config/locale/gnu/monetary_members.cc
    trunk/libstdc++-v3/config/locale/gnu/numeric_members.cc
    trunk/libstdc++-v3/include/bits/locale_facets.tcc
    trunk/libstdc++-v3/include/bits/locale_facets_nonio.tcc

Comment 5 Paolo Carlini 2009-08-03 09:24:49 UTC
Unless there are many requests, let's not fix this for gcc-4_4-branch, isn't a regression and nobody noticed for many years...