It appears that a basic_string created with a default constructor can't live in shared memory because basic_string uses a static variable to hold a single empty string (stored in _S_empty_rep_storage) The memory associated with the empty string representation is local to the process that created the string. Therefore one process can not access empty strings created by a different process. I would guess that basic_string should use it's allocator to acquire the memory for the empty string representation.
HUH? Why would do such a thing in the first place, C++ is not designed to go across shared memory at all.
(In reply to comment #1) > HUH? Why would do such a thing in the first place, C++ is not designed to go across shared memory at > all. It's fairly common to use allocators to put STL collections in shared memory. Here are some references that mention exactly that: http://gcc.gnu.org/ml/libstdc++/2003-01/msg00099.html http://www.spcspringboard.com/Catalog/effectivestl.htm http://www.codeproject.com/cpp/allocator.asp For the most part, I've found that libstdc++ handles doing this very well.
What about giving an example and maybe trying 3.3.4 or 3.4.1 since 3.2.x is a non updated version?
The claim is valid. Let's let the libstdc++ maintainers comment, as I'm sure there are standards and/or ABI implications in storing a single empty string representation. Andrew, please try to be less dismissive for bug reports where you don't understand what's going on. C++'s allocator scheme is created _exactly_ for the case the submitter wants to use it for. W.
Investigating...
An example, or at least the custom allocator for shared memory, would be useful. -benjamin
Created attachment 6793 [details] example of problem Tested on: gcc 3.4.1 on Mac OS X and gcc 3.2.3 on RedHat Enterprice Linux 3 AS (update 1) The makefile builds 'test-bigger', 'test', and cleanShm. cleanShm deletes the shared memory segment created by the other programs. 'test' and 'test-bigger' are created from the same source files. The only different between 'test-bigger' and 'test' is that 'test-bigger' declares a static array of integers before any includes in order to force basic_string's _S_empty_rep_storage to move to a different location. Execute 'test-bigger' before 'test' to observe the problem.
Not knowing much about the code in question, but here is where I see the problem: in basic_string.h, we have a representation of the empty string: struct _Rep : _Rep_base { /*...*/ // The following storage is init'd to 0 by the linker, resulting // (carefully) in an empty string with one reference. static size_type _S_empty_rep_storage[]; This member variable is indeed zero initialized in the linker, see basic_string.tcc: // Linker sets _S_empty_rep_storage to all 0s (one reference, empty string) // at static init time (before static ctors are run). template<typename _CharT, typename _Traits, typename _Alloc> typename basic_string<_CharT, _Traits, _Alloc>::size_type basic_string<_CharT, _Traits, _Alloc>::_Rep::_S_empty_rep_storage[ (sizeof(_Rep_base) + sizeof(_CharT) + sizeof(size_type) - 1) / sizeof(size_type)]; Note that allocation of this memory is done by the linker in the address space of each program that uses strings, not by an allocator class. Thus, if two programs use shared memory to communicate with each other, their _S_empty_rep_storage objects may lie at different addresses. Whether that is a problem I don't know, but it seems unsafe indeed. W.
It works for me with both Apple's 3.3 and the FSF mainline on powerpc-apple-darwin so ...
It crashes on me. Below is what I see in gdb for the 'test' executable. Note that for the empty string created in 'test-bigger' (e.g. header->string3) _M_p is <Address 0x40642c out of bounds> For the empty string declared in 'test', _M_p is 0x642c "" The non-empty strings populated in 'test-bigger' are all in 'test's address space. Calling mmap(addr=0x6400000, size=65536, mprod=3, mflags=1, fd=6, offset=0); mmap succeeded! Got the requested address. First string: [hello world] Second string: [foo bar] Breakpoint 3, main (argc=1, argv=0xbffffb44) at test.C:53 53 std::cout << "Third string: [" << *header->string3 << "]" << std::endl; (gdb) p emptyString $4 = { static npos = <optimized out>, _M_dataplus = { <ShmAllocator<char>> = {<No data fields>}, members of basic_string<char,std::char_traits<char>,ShmAllocator<char> >::_Alloc_hider: _M_p = 0x642c "" } } (gdb) p header->string3 $5 = (ShmString *) 0x640002c (gdb) p *header->string3 $6 = { static npos = <optimized out>, _M_dataplus = { <ShmAllocator<char>> = {<No data fields>}, members of basic_string<char,std::char_traits<char>,ShmAllocator<char> >::_Alloc_hider: _M_p = 0x40642c <Address 0x40642c out of bounds> } } (gdb) p *header->string2 $7 = { static npos = <optimized out>, _M_dataplus = { <ShmAllocator<char>> = {<No data fields>}, members of basic_string<char,std::char_traits<char>,ShmAllocator<char> >::_Alloc_hider: _M_p = 0x6400064 "foo bar" } } (gdb) n Program received signal EXC_BAD_ACCESS, Could not access memory. 0x0000361c in std::basic_string<char, std::char_traits<char>, ShmAllocator<char> >::size() const (this=0x640002c) at /Users/horns/gcc-3.4.1/lib/gcc/powerpc-apple-darwin7.4.0/3.4.1/../../../../ include/c++/3.4.1/bits/basic_string.h:532 532 size() const { return _M_rep()->_M_length; }
I can reproduce this on x86/linux (FC2) with both gcc-3.3.3 and gcc-3.4.2. I wonder how to fix this gracefully. A shared memory allocator of some kind should probably be added as an extension, regardless. -benjamin
Created attachment 6798 [details] same test, for vector this switches containers to vector, which works without failure.
Created attachment 6799 [details] preliminary removal of _S_empty_rep_storage This is a hacked patch, just to have an allocator-allocated memory for the empty string. Using this version of string, the testcase that used to fail will pass for me. This is probably not the right way to do this, so I'll wait for Paolo to look at this when he gets back. -benjamin
Thanks Benjamin, I'm looking into it now.
I am sympathetic to this usage, and would like to accommodate it. At the same time, it would be tragic if such accomodation demolished the performance of the regular string. A lot rides on that fixed address -- particularly in 3.4 and later, which actually checks for it specifically, and avoids incrementing or decrementing the refcount at that address. Anything that makes _S_empty_rep() slower slows down the default string constructor, and a lot of user code. Note too that it is very common to use strings during programmed "static" initialization, so it has to be initialized by the linker, and not at some random point during that process. (There is no control over the order in which such initializations run, in general, although some targets' linkers support special hacks.) Now that nothing modifies the empty-string rep object, it would almost be possible to use NULL as the address. The fly in the ointment is the recent extension (grrr!) requiring that s.operator[](s.length()) actually yield a zero. (There's no requirement for c_str's result to match anything else (it might yield "(charT*)this"!), and data() could yield zero + N safely because it can't be dereferenced.) Of course any sort of conditional check in operator[] could be a performance disaster, and length() and many other operations would need a conditional check too. Still, it might well be a win, overall. What else might we do? We certainly ought to be able to partial-specialize on the default allocator to continue using the purely static initialization. But what about user allocators, including shared-memory allocators? Using shared memory doesn't, by itself, mean you really want string operations to be slow. More importantly, the shared memory region might not be ready to use at program startup time, or when the library is being loaded, so any kind of programmed static initialization (init order problems aside) is out. That leaves something akin to Benjamin's approach -- although inlining all that probably would be a mistake. It seems to me the comparison to zero need not involve a memory barrier -- once it's nonzero, it will never change again. It appears that each process that maps the shared storage would leak its own empty string into it. A memory region shared among lots of transient processes would fill up quickly. Copying some other process's empty string will involve a real atomic increment, because the address won't match its own -- which also means the originating process really can't safely clean up. In principle, we could document that we allocate some distinguished type, so that if the user controls the allocator (or can partial-specialize for that case) and give all processes the same address, then only one instance leaks. (Of course that address would have to be stored in the shared-memory region, too, but that would be the allocator's business.) A more robust alternative might be to export a block of zeroes from libsupc++ specifically for this purpose; it could be shared among all string types (or at least among all basic_string<char,T,A> and basic_string<wchar_t,T,A>). Too, it should speed up regular string operations when libstdc++ is dynamic-linked, because it would avoid one stage of relocation (I think). I assume here that libsupc++'s static storage would always appear at a fixed address. Maybe I'm wrong. (Maybe there's already a block of zeroes there that we can share!) That's all I can think of at this hour, but there ought to be other practical alternatives, too. Let's not be hasty.
Paolo, can you reassign this to me? Thanks! -benjamin
Created attachment 7090 [details] testsuite additions, ext additions round 1 Current thinking on how to add this as an extension, plus how to add the testcases. This is currently failing on gcc-3.3.x as expected. There are additional fails for 3.4.x. I'm working on it...
Created attachment 7159 [details] patch, testsuite, ext This patch set removes _S_empty_rep completely, uses NULL for new strings and a local static for the empty string. This does seem to solve the problem at hand. I have two regressions, looking into it. I think the current interface to shared_allocator could be improved. However, I'm willing to let that slide for the moment. I've not benchmarked this. I suspect the difference will be slight, but then again, I'm expecting Nathan to come up with a benchmark....... :) -benjamin
Created attachment 7180 [details] mainline patch
Created attachment 7181 [details] patch, testsuite, ext not for mainline
Created attachment 7189 [details] patch, testsuite, ext
Subject: Bug 16612 CVSROOT: /cvs/gcc Module name: gcc Changes by: paolo@gcc.gnu.org 2004-09-28 08:58:36 Modified files: libstdc++-v3 : ChangeLog README acconfig.h acinclude.m4 config.h.in configure configure.ac libstdc++-v3/docs/html: configopts.html libstdc++-v3/include/bits: basic_string.h basic_string.tcc Added files: libstdc++-v3/testsuite/21_strings/basic_string/element_access/char: empty.cc libstdc++-v3/testsuite/21_strings/basic_string/element_access/wchar_t: empty.cc libstdc++-v3/testsuite/21_strings/basic_string/operations/char: 1.cc libstdc++-v3/testsuite/21_strings/basic_string/operations/wchar_t: 1.cc Log message: 2004-09-28 Paolo Carlini <pcarlini@suse.de> PR libstdc++/16612 * include/bits/basic_string.h (_M_dispose, _M_refcopy, basic_string()): When _GLIBCXX_FULLY_DYNAMIC_STRING is defined, don't deal with _S_empty_rep. * include/bits/basic_string.tcc (_S_construct, _M_destroy, _M_leak_hard, _M_mutate): Likewise. * acinclude.m4 (GLIBCXX_ENABLE_FULLY_DYNAMIC_STRING): New. * acconfig.h: Add corresponding undef. * configure.ac: Use GLIBCXX_ENABLE_FULLY_DYNAMIC_STRING. * docs/html/configopts.html: Document --enable-fully-dynamic-string. * configure: Regenerate. * config.h.in: Likewise. 2004-09-28 Benjamin Kosnik <bkoz@redhat.com> Paolo Carlini <pcarlini@suse.de> * testsuite/21_strings/basic_string/operations/char/1.cc: New. * testsuite/21_strings/basic_string/operations/wchar_t/1.cc: New. * testsuite/21_strings/basic_string/element_access/char/empty.cc: New. * testsuite/21_strings/basic_string/element_access/wchar_t/empty.cc: New. 2004-09-28 Paolo Carlini <pcarlini@suse.de> * README: Remove obsolete entry about include/c_shadow. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&r1=1.2672&r2=1.2673 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/README.diff?cvsroot=gcc&r1=1.16&r2=1.17 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/acconfig.h.diff?cvsroot=gcc&r1=1.42&r2=1.43 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/acinclude.m4.diff?cvsroot=gcc&r1=1.296&r2=1.297 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/config.h.in.diff?cvsroot=gcc&r1=1.83&r2=1.84 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/configure.diff?cvsroot=gcc&r1=1.406&r2=1.407 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/configure.ac.diff?cvsroot=gcc&r1=1.25&r2=1.26 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/docs/html/configopts.html.diff?cvsroot=gcc&r1=1.37&r2=1.38 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/basic_string.h.diff?cvsroot=gcc&r1=1.62&r2=1.63 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/basic_string.tcc.diff?cvsroot=gcc&r1=1.71&r2=1.72 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/21_strings/basic_string/element_access/char/empty.cc.diff?cvsroot=gcc&r1=NONE&r2=1.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/21_strings/basic_string/element_access/wchar_t/empty.cc.diff?cvsroot=gcc&r1=NONE&r2=1.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/21_strings/basic_string/operations/char/1.cc.diff?cvsroot=gcc&r1=NONE&r2=1.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/21_strings/basic_string/operations/wchar_t/1.cc.diff?cvsroot=gcc&r1=NONE&r2=1.1
I give this back to Paolo, since he's got the patch. -benjamin
Subject: Bug 16612 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_4-branch Changes by: paolo@gcc.gnu.org 2004-10-28 21:52:42 Modified files: libstdc++-v3 : ChangeLog configure configure.ac acinclude.m4 aclocal.m4 acconfig.h README config.h.in libstdc++-v3/include/bits: basic_string.h basic_string.tcc libstdc++-v3/docs/html: configopts.html Log message: 2004-10-28 Paolo Carlini <pcarlini@suse.de> * include/bits/basic_string.tcc (_M_mutate): Do not reallocate unnecessarily when _M_rep() == &_S_empty_rep() and __new_size == capacity() (== 0): is ok to just leave everything unchanged. 2004-10-28 Paolo Carlini <pcarlini@suse.de> PR libstdc++/16612 * include/bits/basic_string.h (_M_dispose, _M_refcopy, basic_string()): When _GLIBCXX_FULLY_DYNAMIC_STRING is defined, don't deal with _S_empty_rep. * include/bits/basic_string.tcc (_S_construct, _M_destroy, _M_leak_hard, _M_mutate): Likewise. * acinclude.m4 (GLIBCXX_ENABLE_FULLY_DYNAMIC_STRING): New. * acconfig.h: Add corresponding undef. * configure.ac: Use GLIBCXX_ENABLE_FULLY_DYNAMIC_STRING. * docs/html/configopts.html: Document --enable-fully-dynamic-string. * aclocal.m4: Regenerate. * configure: Likewise. * config.h.in: Likewise. 2004-10-28 Paolo Carlini <pcarlini@suse.de> * README: Remove obsolete entry about include/c_shadow. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.2224.2.191&r2=1.2224.2.192 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/configure.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.373.4.20&r2=1.373.4.21 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/configure.ac.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.14.4.5&r2=1.14.4.6 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/acinclude.m4.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.280.4.6&r2=1.280.4.7 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/aclocal.m4.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.296.4.6&r2=1.296.4.7 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/acconfig.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.39.4.2&r2=1.39.4.3 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/README.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.16&r2=1.16.10.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/config.h.in.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.79.4.3&r2=1.79.4.4 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/basic_string.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.41.4.5&r2=1.41.4.6 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/basic_string.tcc.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.46.4.7&r2=1.46.4.8 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/docs/html/configopts.html.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.33.4.3&r2=1.33.4.4
Fixed for 3.4.3.
Created attachment 7447 [details] ext/shared_allocator, docs, testsuite, config All done, waiting on assignmnet. tested x86/linux tested x86/linux cross arm-elf tested ppc/darwin
Subject: Bug 16612 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_2-rhl8-branch Changes by: jakub@gcc.gnu.org 2005-01-05 09:49:05 Modified files: libstdc++-v3 : ChangeLog libstdc++-v3/include/bits: basic_string.h basic_string.tcc Log message: 2004-10-28 Paolo Carlini <pcarlini@suse.de> PR libstdc++/16612 * include/bits/basic_string.h (basic_string()): When _GLIBCXX_FULLY_DYNAMIC_STRING is defined, don't deal with _S_empty_rep. * include/bits/basic_string.tcc (_S_construct): Likewise. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_2-rhl8-branch&r1=1.1057.2.159.2.10.2.29&r2=1.1057.2.159.2.10.2.30 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/basic_string.h.diff?cvsroot=gcc&only_with_tag=gcc-3_2-rhl8-branch&r1=1.19.2.5&r2=1.19.2.5.8.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/basic_string.tcc.diff?cvsroot=gcc&only_with_tag=gcc-3_2-rhl8-branch&r1=1.20.2.5.4.1&r2=1.20.2.5.4.2
Reopening...
... as "enhancement".
It crashes on Cygwin too. I wonder if --enable-fully-dynamic-string should be the default when building on platforms where it is known to fail without this option.
Created attachment 12801 [details] basic_string test code with visibility option Something similar happens when I use GCC option -fvisibility=hidden. I made a DSO which uses basic_string<>. And the main function created a basic_string instance with default constructor. It called a function of the DSO with the basic_string reference in main function. Inside the DSO, the function assigned new string to the reference. Then it crashes. I made a simple test code. For test follow this step: tar xzf stringtest.tar.gz cd stringtest make ./test then it will be abort with an error.
Comment on attachment 12801 [details] basic_string test code with visibility option Something similar happens when I use GCC option -fvisibility=hidden. I made a DSO which uses basic_string<>. And the main function created a basic_string instance with default constructor. It called a function of the DSO with the basic_string reference in main function. Inside the DSO, the function assigned new string to the reference. Then it crashes. I made a simple test code. For test follow this step: tar xzf stringtest.tar.gz cd stringtest make ./test then it will be abort with an error. I tested it on Debian unstable, GCC 4.0.3.
FYI: -fvisibility is broken on 4.0.x. If you want to use this option, I suggest you use 4.2.x. Both the compiler and library can handle it, or should be able to handle it. -benjamin
For what it's worth, it appears as if I was just bitten by this issue in a slightly different context. I was passing a std::string across a dylib boundary which worked on Apple's gcc 4.2.1 but crashed on Windows (MSVC). Figuring that with MSVC the allocator would use a different heap in each dylib I wrote a custom allocator, and now it worked on MSVC but crashed on gcc on Darwin. I worked out that on Darwin, the custom allocator attempted to release the empty string created in the other dylib. Defining _GLIBCXX_FULLY_DYNAMIC_STRING solved the issue. Anyway, I haven't yet dug deep enough to be certain that this is the same issue, but it very much appears as if this is not only a problem for shared memory but also when two dynamic libraries both link against libstdc++ and exchange std::strings. The one thing that baffles me, though, is why this wasn't a problem before I introduced the custom allocator. Does std::allocator have any safeguards against this?
(In reply to comment #34) > For what it's worth, it appears as if I was just bitten by this issue in a > slightly different context. You haven't given enough detail to know if it's the same issue or just user error. > I was passing a std::string across a dylib boundary which worked on Apple's gcc > 4.2.1 but crashed on Windows (MSVC). Figuring that with MSVC the allocator > would use a different heap in each dylib I wrote a custom allocator, and now it > worked on MSVC but crashed on gcc on Darwin. > > I worked out that on Darwin, the custom allocator attempted to release the > empty string created in the other dylib. Defining > _GLIBCXX_FULLY_DYNAMIC_STRING solved the issue. That doesn't make sense. Nothing should release the empty string unless string is fully dynamic already, so defining it shouldn't make any difference. Further, simply defining _GLIBCXX_FULLY_DYNAMIC_STRING when compiling your program is incorrect and not supported. You need to build GCC with --enable-fully-dynamic-string, which is how Apple's GCC and system libraries are built, so building GCC on Darwin should usually use that option. > Anyway, I haven't yet dug deep enough to be certain that this is the same > issue, but it very much appears as if this is not only a problem for shared > memory but also when two dynamic libraries both link against libstdc++ and > exchange std::strings. That should (and does) work fine on GNU/Linux and similar platforms, as long as both libs were built with the same setting for fully dynamic strings (either both using them of neither using them.) Enabling fully dynamic strings changes the library ABI, so you can't mix code with different settings. > The one thing that baffles me, though, is why this wasn't a problem before I > introduced the custom allocator. Does std::allocator have any safeguards > against this? No, but all instances of std::allocator are equivalent (i.e. it's stateless) and (in libstdc++'s default configuration) just call new/delete, so there's nothing std::allocator really needs to do. My guess is you're mixing incompatible libraries, but if not then please open a new bugzilla report instead of adding to this one, thanks.
Is there anything left to "fix" here? Fully dynamic string should help the COW string cases. The new __cxx11::basic_string fully supports C++11 allocators and so should work with allocators using "fancy pointers".