With libstdc++ configured with --enable-libstdcxx-allocator=mt (on 4.0 branch or on HEAD for linux even without it, as mt is the default there), following testcase crashes: cat > O.c <<EOF #include <dlfcn.h> #include <pthread.h> void * tf (void *arg) { void *h = dlopen ("./libO.so", RTLD_LAZY); void (*fn) (void); if (!h) return 0; fn = dlsym (h, "foo"); fn (); dlclose (h); return 0; } int main (void) { pthread_t th; pthread_create (&th, NULL, tf, NULL); pthread_join (th, NULL); return 0; } EOF cat > libO.C <<EOF #include <string> extern "C" void foo (void) { std::string s; s += "hello"; } EOF g++ -g -O2 -shared -fpic -o libO.so libO.C gcc -g -O2 -o O O.c -ldl -lpthread The problem is that __gnu_cxx::__pool<true>::_M_initialize () calls pthread_key_create but doesn't ensure pthread_key_delete is called when libstdc++.so is unloaded. So when glibc attempts destroys a thread or program and calls the registered key cleanup routine (_S_destroy_thread_key), if libstdc++.so is not mapped at that moment any longer, either whatever other code happens to be mapped at that address is run, or the program crashes immediately. mt_allocator.cc should ensure that gthread_key_delete is called on the key after all users of the key have been destroyed.
Jakub, is this issue related to libstdc++/19265?
Yes, that's the same thing apparently. I'm pretty sure a reproducer can be written even for libstdc++ not configured to default to the mt allocator, by including <ext/mt_allocator.h> etc. or however you explicitely choose a specific allocator for some template instantiation. If init_priority attribute works, it could be perhaps used to make sure the destructor of some dummy object that calls gthread_key_delete in mt_allocator.cc will be run as last destructor in libstdc++.so.
> Yes, that's the same thing apparently. Thanks. And now we have also a compact testcase. > I'm pretty sure a reproducer can be written even for libstdc++ not configured > to default to the mt allocator, by including <ext/mt_allocator.h> etc. or > however you explicitely choose a specific allocator for some template > instantiation. Definitely. > If init_priority attribute works, it could be perhaps used to make sure > the destructor of some dummy object that calls gthread_key_delete in > mt_allocator.cc will be run as last destructor in libstdc++.so. Thanks for the hint. ... maybe Benjamin is willing to investigate this... ;)
First patch posted here <http://gcc.gnu.org/ml/gcc-patches/2005-07/msg00478.html>
That patch fixes the original testcase, but unfortunately does not fix the following one. cat > P.c <<EOF #include <dlfcn.h> #include <pthread.h> void * tf (void *arg) { void *h = dlopen ("./libP.so", RTLD_LAZY); void (*fn) (void); if (!h) return 0; fn = dlsym (h, "foo"); fn (); void *h2 = dlopen ("libstdc++.so.6", RTLD_LAZY); if (!h2) return 0; dlclose (h); return 0; } int main (void) { pthread_t th; pthread_create (&th, NULL, tf, NULL); pthread_join (th, NULL); return 0; } EOF cat > libP.C <<EOF #include <string> #include <ext/mt_allocator.h> template class __gnu_cxx::__mt_alloc<short, __gnu_cxx::__common_pool_policy<__gnu_cxx::__pool, true> >; typedef std::char_traits<char> my_traits_type; typedef __gnu_cxx::__mt_alloc<short, __gnu_cxx::__common_pool_policy<__gnu_cxx::__pool, true> > my_allocator_type; typedef std::basic_string<char, my_traits_type, my_allocator_type> my_string; extern "C" void foo (void) { my_string s; s += "hello"; } EOF g++ -g -O2 -shared -fpic -o libP.so libP.C gcc -g -O2 -o P P.c -ldl -lpthread
Another patch here: <http://gcc.gnu.org/ml/gcc-patches/2005-07/msg00993.html> To test whether _M_get_thread_id/_M_initialize/_M_destroy_thread_key works I was using following testcase under debugger: #include <pthread.h> #include <unistd.h> #include <memory> #include <ext/mt_allocator.h> using __gnu_cxx::__pool; using __gnu_cxx::__common_pool_policy; using __gnu_cxx::__per_type_pool_policy; typedef __common_pool_policy<__pool, true> cpolicy_type; typedef __per_type_pool_policy<char, __pool, true> tpolicy_type; typedef __per_type_pool_policy<unsigned char, __pool, true> upolicy_type; typedef __gnu_cxx::__mt_alloc<char, cpolicy_type> callocator_type; typedef __gnu_cxx::__mt_alloc<char, tpolicy_type> tallocator_type; typedef __gnu_cxx::__mt_alloc<char, upolicy_type> uallocator_type; typedef __gnu_cxx::__pool_base::_Tune tune_type; callocator_type ca; tallocator_type ta; uallocator_type ua; void *tf(void *arg) { callocator_type::pointer p1 = ca.allocate(128); callocator_type::pointer p2 = ca.allocate(5128); ca.deallocate(p1, 128); ca.deallocate(p2, 5128); int idx = (int) (long) arg; if (idx & 1) sleep (1); else sleep (1024); return NULL; } void *tf2(void *arg) { tallocator_type::pointer p1 = ta.allocate(128); tallocator_type::pointer p2 = ta.allocate(5128); ta.deallocate(p1, 128); ta.deallocate(p2, 5128); int idx = (int) (long) arg; if (idx & 1) sleep (1); else sleep (1024); return NULL; } void *tf3(void *arg) { uallocator_type::pointer p1 = ua.allocate(128); uallocator_type::pointer p2 = ua.allocate(5128); ua.deallocate(p1, 128); ua.deallocate(p2, 5128); int idx = (int) (long) arg; if (idx & 1) sleep (1); else sleep (1024); return NULL; } int main() { tune_type t_opt(16, 5120, 32, 5120, 20, 10, false); ca._M_set_options(t_opt); pthread_t th[48]; for (int i = 0; i < 16; ++i) pthread_create (&th[i], NULL, tf, (void *) (long) i); callocator_type::pointer p1 = ca.allocate(128); callocator_type::pointer p2 = ca.allocate(5128); ca.deallocate(p1, 128); ca.deallocate(p2, 5128); sleep (3); tune_type t_opt2(16, 5120, 32, 5120, 36, 10, false); ta._M_set_options(t_opt2); for (int i = 16; i < 32; ++i) pthread_create (&th[i], NULL, tf2, (void *) (long) i); tallocator_type::pointer p3 = ta.allocate(128); tallocator_type::pointer p4 = ta.allocate(5128); ta.deallocate(p3, 128); ta.deallocate(p4, 5128); sleep (3); tune_type t_opt3(16, 5120, 32, 5120, 52, 10, false); ua._M_set_options(t_opt3); for (int i = 32; i < 48; ++i) pthread_create (&th[i], NULL, tf3, (void *) (long) i); uallocator_type::pointer p5 = ua.allocate(128); uallocator_type::pointer p6 = ua.allocate(5128); ua.deallocate(p5, 128); ua.deallocate(p6, 5128); sleep (3); return 0; }
Jakub, sorry, having connectivity issues since yesterday. Can you put your most recent patch and commentary in this bug report? Thanks.
Created attachment 9614 [details] gcc41-libstdc++-pr22309.patch My latest patch. It is basically the same as in http://gcc.gnu.org/ml/gcc-patches/2005-07/msg00993.html, just attribute unused from arguments has been changed for nameless arguments. The commentary from http://gcc.gnu.org/ml/gcc-patches/2005-07/msg00993.html is still current. I'm using this patch for a month and half already in Fedora GCC and it cured all the problems people were reporting about this.
Jakub, any idea how to add a test case for this thing? Ugh. I cannot figure it out.
Created attachment 9677 [details] testsuite addition Well, it's not pretty, but it reliably fails as expected. This is probably only work on linux. The rest are XFAILED. Anyway. Jakub, your analysis is correct on this. I'm not quite sure init priority is the best way to solve this. I'd rather rely on __cxa_exit behavior and use static locals. The internal lock object is now static? Is that just for linkage clarity? Or should the rest of the _glibcxx_lock objects also be static?
Created attachment 9698 [details] patch so 7
Without init_priority, how can you be sure that other libstdc++.so destructors will not be run after __gnu_internal::freelist is destructed? If there are some destructors and they use the mt allocator (whether to allocate some temporary memory or more likely to free some memory), won't it cause big problems? As for the lock being static, mt-allocator.cc is the only user of that lock, so I don't see why it should be externally visible, even if just in libstdc++.a (in libstdc++.so symbol versioning will hide it). If there are other locks used solely in one .cc file, I think they should be made static too. Thanks for writing the testcase. Haven't tried to compile your latest patch yet, do the <ext/mt_allocator.h> changes preserve ABI compatibility?
Created attachment 9700 [details] patch for mainline/gcc-4_0-branch
Hey Jakub. Yeah, I think this can be back-ported. I put in my patch, which looks pretty good on x86/linux. We could proably do something more elaborate to not duplicate some of the symbols but I'm feeling lazy... this is good enough. Sorry about the confusion about the static freelist_mutex. Yes, this was what I was getting at by "linkage clarity" ... I think we are on the same page. Yes, this is a good idea, and I'll clean this up too as a follow-on patch. Umm... so, no, I don't get what you are getting at WRT destructors in libstdc++.so. I think you mean functions in testsuite_shared.so that are asm destrutors but that use __mt_alloc to do allocations or something. You mean, what then? Maybe we'd both be better off if you came up with an example... I think just static ordering in this file will work. Won't that just work? Man, I want that to work. If not, then the init priorty stuff can be re-added. The thing is, there are no config tests for init priority... ..... I thought support of that was kind of dodgy, non-ELF... if this feature is going to be used, there are other things with __ioinit that can also be done. Anyway. Of more immediate concern is that the testsuite dejagnu hacking is quite weak. For instance, The testsuite file should just be run on linux, so I think that is ok. But the shared object is unconditionally compiled, even for --disable-shared builds.... also, compiled with a complete hack for flags, likely to only work on linux... It is kind of funny, just to be re-creating an autoconf way of doing this, but doing it in dejagnu. Ugh. Some of this stuff would be easier if we could use Make to build instead of dejagnu....
Well, maybe we will need init priority. I'm still thinking about that part. Adding it is simple enough if we need it. However, I'd like to check this in just to see what happens with the rest of it. -benjamin
Subject: Bug 22309 CVSROOT: /cvs/gcc Module name: gcc Changes by: bkoz@gcc.gnu.org 2005-09-12 04:49:11 Modified files: libstdc++-v3 : ChangeLog libstdc++-v3/config: linker-map.gnu libstdc++-v3/include/ext: mt_allocator.h libstdc++-v3/src: mt_allocator.cc libstdc++-v3/testsuite/lib: dg-options.exp libstdc++.exp Added files: libstdc++-v3/testsuite: testsuite_shared.cc libstdc++-v3/testsuite/ext/mt_allocator: 22309_thread.cc Log message: 2005-09-11 Benjamin Kosnik <bkoz@redhat.com> PR libstdc++/19265 PR libstdc++/22309 * include/ext/mt_allocator.h (__gnu_cxx::__create_handler): Remove. (__pool<true>::_M_destroy_thread_key): Compatibility only. (__pool<true>::_M_initialize(__destroy): Same. (__pool<true>::_M_initialize): New. (__pool<true>::_M_initialize_once): Nothing fancy. (__pool<true>::_M_once): Remove. (__common_pool): New. (__common_pool_base): New. (__per_type_pool): New. (__per_type_pool_base): New. * src/mt_allocator.cc: Same. * config/linker-map.gnu (__pool<true>::_M_initialize()): Add. 2005-09-11 Jakub Jelinek <jakub@redhat.com> PR libstdc++/19265 PR libstdc++/22309 * src/mt_allocator.cc (__gnu_internal::freelist_mutex): Make static. (__gnu_internal::__freelist): New type. (__gnu_internal::freelist): New variable. (__gnu_internal::_M_destroy_thread_key): New function. (__gnu_cxx::__pool<true>::_M_destroy): Don't delete _M_thread_freelist_initial. (__gnu_cxx::__pool<true>::_M_initialize): Make argument nameless. Don't use _M_thread_freelist and _M_thread_freelist_initial __pool<true> fields, instead use __gnu_internal::freelist fields, call gthread_key_create just once. Use __gnu_internal::_M_destroy_thread_key as key destructor. (__gnu_cxx::__pool<true>::_M_get_thread_id): Store size_t id rather than _Thread_record* in the thread specific value. Don't use _M_thread_freelist __pool<true> field, instead use __gnu_internal::freelist fields. (__gnu_cxx::__pool<true>::_M_destroy_thread_key): Do nothing. 2005-09-11 Benjamin Kosnik <bkoz@redhat.com> Jakub Jelinek <jakub@redhat.com> PR libstdc++/19265 PR libstdc++/22309 * testsuite/testsuite_shared.cc: New. * testsuite/lib/dg-options.exp (dg-require-sharedlib): New. * testsuite/lib/libstdc++.exp (libstdc++_init): Look for shared library, and set v3-sharedlib based on this. (check_v3_target_sharedlib): New. (proc v3-build_support): Build shared objects. * testsuite/ext/mt_allocator/22309_thread.cc: New, use above. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&r1=1.3095&r2=1.3096 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/config/linker-map.gnu.diff?cvsroot=gcc&r1=1.84&r2=1.85 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/ext/mt_allocator.h.diff?cvsroot=gcc&r1=1.47&r2=1.48 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/src/mt_allocator.cc.diff?cvsroot=gcc&r1=1.12&r2=1.13 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/testsuite_shared.cc.diff?cvsroot=gcc&r1=NONE&r2=1.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/ext/mt_allocator/22309_thread.cc.diff?cvsroot=gcc&r1=NONE&r2=1.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/lib/dg-options.exp.diff?cvsroot=gcc&r1=1.4&r2=1.5 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/lib/libstdc++.exp.diff?cvsroot=gcc&r1=1.45&r2=1.46
*** Bug 19265 has been marked as a duplicate of this bug. ***
Mine.
Subject: Bug 22309 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-4_0-branch Changes by: bkoz@gcc.gnu.org 2005-09-20 05:24:50 Modified files: libstdc++-v3 : ChangeLog libstdc++-v3/config: linker-map.gnu libstdc++-v3/include/ext: mt_allocator.h libstdc++-v3/src: debug.cc locale_init.cc mt_allocator.cc pool_allocator.cc Log message: 2005-09-19 Benjamin Kosnik <bkoz@redhat.com> PR libstdc++/19265 PR libstdc++/22309 * include/ext/mt_allocator.h (__gnu_cxx::__create_handler): Remove. (__pool<true>::_M_destroy_thread_key): Compatibility only. (__pool<true>::_M_initialize(__destroy): Same. (__pool<true>::_M_initialize): New. (__pool<true>::_M_initialize_once): Nothing fancy. (__pool<true>::_M_once): Remove. (__common_pool): New. (__common_pool_base): New. (__per_type_pool): New. (__per_type_pool_base): New. * src/mt_allocator.cc: Same. * config/linker-map.gnu (__pool<true>::_M_initialize()): Add. 2005-09-19 Jakub Jelinek <jakub@redhat.com> PR libstdc++/19265 PR libstdc++/22309 * src/mt_allocator.cc (__gnu_internal::freelist_mutex): Make static. (__gnu_internal::__freelist): New type. (__gnu_internal::freelist): New variable. (__gnu_internal::_M_destroy_thread_key): New function. (__gnu_cxx::__pool<true>::_M_destroy): Don't delete _M_thread_freelist_initial. (__gnu_cxx::__pool<true>::_M_initialize): Make argument nameless. Don't use _M_thread_freelist and _M_thread_freelist_initial __pool<true> fields, instead use __gnu_internal::freelist fields, call gthread_key_create just once. Use __gnu_internal::_M_destroy_thread_key as key destructor. (__gnu_cxx::__pool<true>::_M_get_thread_id): Store size_t id rather than _Thread_record* in the thread specific value. Don't use _M_thread_freelist __pool<true> field, instead use __gnu_internal::freelist fields. (__gnu_cxx::__pool<true>::_M_destroy_thread_key): Do nothing. 2005-09-19 Benjamin Kosnik <bkoz@redhat.com> Jakub Jelinek <jakub@redhat.com> * src/debug.cc (iterator_base_mutex): Make static for internal linkage. * src/locale_init.cc (locale_mutex): Same. * src/mt_allocator.cc (freelist_mutex): Same. * src/pool_allocator.cc (palloc_init_mutex): Same. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.2917.2.83&r2=1.2917.2.84 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/config/linker-map.gnu.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.78.2.3&r2=1.78.2.4 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/ext/mt_allocator.h.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.45.8.1&r2=1.45.8.2 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/src/debug.cc.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.10&r2=1.10.18.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/src/locale_init.cc.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.15&r2=1.15.36.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/src/mt_allocator.cc.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.8&r2=1.8.22.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/src/pool_allocator.cc.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.2&r2=1.2.24.1
Hi, I already thought that is the same as with KDE's artsd but it still crashes for me (since GCC 4.0.0): #0 0xb7857b5e in __gnu_cxx::__pool<true>::_M_reclaim_block () from /usr/lib/libstdc++.so.6 #1 0xb7fc7c26 in __gnu_cxx::__mt_alloc<std::string, __gnu_cxx::__common_pool_policy<__gnu_cxx::__pool, true> >::deallocate (this=0xbffe8a20, __p=0x81015d0, __n=4) at mt_allocator.h:714 #2 0xb7fc8b2e in ~ObjectReference (this=0xbffe8a14) at stl_vector.h:123 #3 0xb7fbafd3 in Arts::SoundServerStartup_base::_fromString (objectref=@0x0) at ./soundserver/soundserver.cc:2545 #4 0x0806a832 in Arts::SoundServerStartup_impl::cleanReference ( this=0x8159838) at soundserver.h:1376 #5 0x0806b1f2 in Arts::SoundServerStartup_impl::lock (this=0x8159838) at /Devel/src/kde/arts/soundserver/soundserverstartup_impl.cc:78 I'm using Debian's GCC 4.0.1-9 (CVS 20050922) and compile with visibility support. Without visibility or with export GLIBCXX_FORCE_NEW=1 it doesn't crash. The specs are: Target: i486-linux-gnu Configured with: ../src/configure -v --enable-languages=c,c++,java,f95,objc,ada,treelang --prefix=/usr --enable-shared --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --program-suffix=-4.0 --enable-__cxa_atexit --enable-libstdcxx-allocator=mt --enable-clocale=gnu --enable-libstdcxx-debug --enable-java-gc=boehm --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.4.2-gcj-4.0-1.4.2.0/jre --enable-mpfr --disable-werror --enable-checking=release i486-linux-gnu Thread model: posix gcc version 4.0.2 (Debian 4.0.1-9) Cheers, André
André, Any chance you can detail how I can reproduce your failure? thanks, benjamin
Subject: Re: mt allocator doesn't pthread_key_delete its keys On Tuesday 27 September 2005 01:03, bkoz at gcc dot gnu dot org wrote: > ------- Additional Comments From bkoz at gcc dot gnu dot org > 2005-09-26 23:03 ------- > > André, > > Any chance you can detail how I can reproduce your failure? The only way I know is to compile artsd and run it from console, it crashes at once. I already tried to write a small test case but I totally failed. Let me know if a backtrace with more debug information could help.
If you are compiling with -fvisibility*, then the problem is PR libstdc++/22482, not this one.
Subject: Re: mt allocator doesn't pthread_key_delete its keys On Tuesday 27 September 2005 16:43, jakub at gcc dot gnu dot org wrote: > If you are compiling with -fvisibility*, > then the problem is PR libstdc++/22482, not this one. I compile with -fvisibility=hidden -fvisibility-inlines-hidden, but I get no linker errors. And as noted export GLIBCXX_FORCE_NEW=1 prevents the crash.
Fixed