[Bug libstdc++/17937] New: Critical ~__pool troubles

[Bug libstdc++/17937] New: Critical ~__pool troubles

[pcarlini at suse dot de]

Michael Matz distilled this simple testcase from KDE arts:

#include <string> 
#include <list> 
using namespace std; 
static list<string> modulePath; 
int main() 
{ 
  modulePath.push_back("hallo"); 
  return 0; 
}

The attached valgrind output clearly shows that ~__pool is still not ok,
unfortunately.

-- 
           Summary: Critical ~__pool troubles
           Product: gcc
           Version: 4.0.0
            Status: UNCONFIRMED
          Severity: critical
          Priority: P2
         Component: libstdc++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: pcarlini at suse dot de
                CC: gcc-bugs at gcc dot gnu dot org,matz at suse dot de
 GCC build triplet: x86-linux
  GCC host triplet: x86-linux
GCC target triplet: x86-linux


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[pcarlini at suse dot de]

------- Additional Comments From pcarlini at suse dot de  2004-10-11 17:26 -------
Created an attachment (id=7327)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=7327&action=view)
Valgrind (2.2, --tool=memcheck) output


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[pcarlini at suse dot de]

------- Additional Comments From pcarlini at suse dot de  2004-10-11 17:38 -------
Forgot to add: to reproduce, remember to provide --enable-__cxa_atexit at build
time!

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[bkoz at gcc dot gnu dot org]

------- Additional Comments From bkoz at gcc dot gnu dot org  2004-10-11 22:51 -------

It would be list.... ugh. Damn that allocator rebinding! I'm developing severe
issues with this particular container. Well, this kills off this QoI
improvement. I've reverted this on mainline, and fixed up the testcases that
test for this to be of the type that showed this, more advanced, problem. 

I'm going to keep the infrastructure in __mt_alloc to do the actual
deallocations, however. At least, for the time being.

Sorry about this madness.

-benjamin

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
   Last reconfirmed|0000-00-00 00:00:00         |2004-10-11 22:51:49
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[bkoz at gcc dot gnu dot org]

------- Additional Comments From bkoz at gcc dot gnu dot org  2004-10-11 22:52 -------
Mine.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |bkoz at gcc dot gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[pcarlini at suse dot de]

------- Additional Comments From pcarlini at suse dot de  2004-10-11 23:50 -------
> It would be list.... ugh. Damn that allocator rebinding! I'm developing severe
> issues with this particular container. Well, this kills off this QoI
> improvement. I've reverted this on mainline, and fixed up the testcases that
> test for this to be of the type that showed this, more advanced, problem. 

Many thanks.

> I'm going to keep the infrastructure in __mt_alloc to do the actual
> deallocations, however. At least, for the time being.

Ok, but as long as we don't use that, let's avoid taking the lock a second
time in _M_reserve_block, the new calls and so on... I propose to protect
with macros or #if 0 or whatelse those snippets too.

Thanks again.

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[bkoz at redhat dot com]

------- Additional Comments From bkoz at redhat dot com  2004-10-12 00:30 -------
Subject: Re:  Critical ~__pool troubles


>Ok, but as long as we don't use that, let's avoid taking the lock a second
>time in _M_reserve_block, the new calls and so on... I propose to protect
>with macros or #if 0 or whatelse those snippets too.

I'm all for eliminating this if indeed it turns out to be a significant overhead.

However, please give me a bit to finish this stuff up! I'll check in a
fix for this immediate problem, and then please give me a bit of time to
do a round of performance analysis. I'll ping you when I'm done, how about that?

best,
benjamin


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-10-12 01:10 -------
Subject: Bug 17937

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	bkoz@gcc.gnu.org	2004-10-12 01:10:39

Modified files:
	libstdc++-v3   : ChangeLog acconfig.h acinclude.m4 config.h.in 
	                 configure configure.ac 
	libstdc++-v3/config: linker-map.gnu 
	libstdc++-v3/docs/html/ext: mt_allocator.html 
	libstdc++-v3/include/bits: stl_deque.h 
	libstdc++-v3/include/ext: mt_allocator.h 
	libstdc++-v3/src: mt_allocator.cc 
	libstdc++-v3/testsuite/ext/mt_allocator: deallocate_global-2.cc 
	                                         deallocate_global-4.cc 
	                                         deallocate_global_thread-1.cc 
	                                         deallocate_global_thread-3.cc 
	                                         deallocate_local-2.cc 
	                                         deallocate_local-4.cc 
	                                         deallocate_local_thread-1.cc 
	                                         deallocate_local_thread-3.cc 
	                                         tune-3.cc tune-4.cc 

Log message:
	2004-10-11  Benjamin Kosnik  <bkoz@redhat.com>
	
	* include/bits/stl_deque.h: Correct for over-long lines.
	
	2004-10-11  Benjamin Kosnik  <bkoz@redhat.com>
	
	PR libstdc++/17937
	* include/ext/mt_allocator.h (__pool::_M_destroy): New.
	* src/mt_allocator.cc (__pool::~__pool): Change definitions to
	_M_destroy.
	* acconfig.h: Remove _GLIBCXX_USE___CXA_ATEXIT.
	* acinclude.m4 (GLIBCXX_ENABLE_CXA_ATEXIT): Remove.
	* configure.ac: Remove call to GLIBCXX_ENABLE_CXA_EXIT.
	* configure: Regenerate.
	* config/linker-map.gnu: Tweak exports.
	* docs/html/ext/mt_allocator.html: Update docs.
	* testsuite/ext/mt_allocator/deallocate_global-2.cc: Fix.
	* testsuite/ext/mt_allocator/deallocate_global-4.cc: Fix.
	* testsuite/ext/mt_allocator/deallocate_global_thread-1.cc: Fix.
	* testsuite/ext/mt_allocator/deallocate_global_thread-3.cc: Fix.
	* testsuite/ext/mt_allocator/deallocate_local-2.cc: Fix.
	* testsuite/ext/mt_allocator/deallocate_local-4.cc: Fix.
	* testsuite/ext/mt_allocator/deallocate_local_thread-3.cc: Fix.
	* testsuite/ext/mt_allocator/deallocate_local_thread-1.cc: Fix.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&r1=1.2699&r2=1.2700
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/acconfig.h.diff?cvsroot=gcc&r1=1.44&r2=1.45
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/acinclude.m4.diff?cvsroot=gcc&r1=1.300&r2=1.301
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/config.h.in.diff?cvsroot=gcc&r1=1.85&r2=1.86
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/configure.diff?cvsroot=gcc&r1=1.412&r2=1.413
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/configure.ac.diff?cvsroot=gcc&r1=1.29&r2=1.30
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/config/linker-map.gnu.diff?cvsroot=gcc&r1=1.72&r2=1.73
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/docs/html/ext/mt_allocator.html.diff?cvsroot=gcc&r1=1.6&r2=1.7
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/stl_deque.h.diff?cvsroot=gcc&r1=1.53&r2=1.54
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/ext/mt_allocator.h.diff?cvsroot=gcc&r1=1.36&r2=1.37
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/src/mt_allocator.cc.diff?cvsroot=gcc&r1=1.4&r2=1.5
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/ext/mt_allocator/deallocate_global-2.cc.diff?cvsroot=gcc&r1=1.2&r2=1.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/ext/mt_allocator/deallocate_global-4.cc.diff?cvsroot=gcc&r1=1.2&r2=1.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/ext/mt_allocator/deallocate_global_thread-1.cc.diff?cvsroot=gcc&r1=1.2&r2=1.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/ext/mt_allocator/deallocate_global_thread-3.cc.diff?cvsroot=gcc&r1=1.2&r2=1.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/ext/mt_allocator/deallocate_local-2.cc.diff?cvsroot=gcc&r1=1.2&r2=1.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/ext/mt_allocator/deallocate_local-4.cc.diff?cvsroot=gcc&r1=1.2&r2=1.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/ext/mt_allocator/deallocate_local_thread-1.cc.diff?cvsroot=gcc&r1=1.2&r2=1.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/ext/mt_allocator/deallocate_local_thread-3.cc.diff?cvsroot=gcc&r1=1.2&r2=1.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/ext/mt_allocator/tune-3.cc.diff?cvsroot=gcc&r1=1.3&r2=1.4
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/ext/mt_allocator/tune-4.cc.diff?cvsroot=gcc&r1=1.3&r2=1.4



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[pcarlini at suse dot de]

------- Additional Comments From pcarlini at suse dot de  2004-10-12 08:27 -------
> However, please give me a bit to finish this stuff up! I'll check in a
> fix for this immediate problem, and then please give me a bit of time to
> do a round of performance analysis. I'll ping you when I'm done, how about
> that?

Ok, agreed! Something worth looking into is

  performance/21_strings/string_append.cc

in the string case (vs char): for some reason, with mt_allocator I'm seeing a
*noticeable* performance regression wrt pool_allocator, something like 40-50%
longer times. Anyway, we'll keep in touch about this, agreed...

P.S. By the way, I like the _M_destroy idea!

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[pcarlini at suse dot de]

------- Additional Comments From pcarlini at suse dot de  2004-10-12 08:53 -------
Sorry, actually I meant around 25%, rather troublesome, anyway... Frankly,
I don't know where the slowdown comes from, we should profile the testcase for
both the allocators. These are some numbers on my P4-2400:
mt_allocator
------------
Execution time of 10000000 string::append(const string&) calls: 0.5 sec.

pool_allocator
--------------
Execution time of 10000000 string::append(const string&) calls: 0.39 sec.

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[bkoz at gcc dot gnu dot org]

------- Additional Comments From bkoz at gcc dot gnu dot org  2004-10-12 15:50 -------

Yep.... I'll add this to my other tests (insert-thread) to profile today, thanks
for the pointer.

Can I close this out? I believe this is fixed, no?

-benjamin

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[pcarlini at suse dot de]

------- Additional Comments From pcarlini at suse dot de  2004-10-12 15:54 -------
Yes, can only be ok, now! ;)

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[bkoz at gcc dot gnu dot org]

------- Additional Comments From bkoz at gcc dot gnu dot org  2004-10-13 02:56 -------
Fixed.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[matz at suse dot de]

------- Additional Comments From matz at suse dot de  2004-10-13 18:28 -------
Still doesn't fully work.  The initial testcase now works, but aRts still 
segfaults.  This time I need this testcase: 
------------------------------------- 
#include <string> 
std::string idl_filename; 
extern std::string idl_filename; 
int main() 
{ 
        idl_filename = "bla"; 
        return 0; 
} 
------------------------------------- 
 
This has to be compiled with -pthread, then it segfaults for me.  Note that 
the "extern" line is important, including it's place (after the definition). 
 
% g++ -g -pthread mcopidl.cc 
% ./a.out 
Segmentation fault 
% valgrind -q --num-callers=42 --tool=memcheck ./a.out 
==7097== Invalid read of size 4 
==7097==    at 0x1B94C2CB: __gnu_cxx::__pool<true>::_M_reclaim_block(char*, 
unsigned) (in /usr/lib/libstdc++.so.6.0.3) 
==7097==    by 0x1B94CD13: __gnu_cxx::__mt_alloc<char, 
__gnu_cxx::__common_pool_policy<true> >::deallocate(char*, unsigned) 
(in /usr/lib/libstdc++.so.6.0.3) 
==7097==    by 0x1B9945A2: std::string::_Rep::_M_destroy(std::allocator<char> 
const&) (in /usr/lib/libstdc++.so.6.0.3) 
==7097==    by 0x1B995A1C: std::string::~string() 
(in /usr/lib/libstdc++.so.6.0.3) 
==7097==    by 0x804866A: __tcf_1 (mcopidl.cc:3) 
==7097==    by 0x1BA697EF: exit (in /lib/tls/libc.so.6) 
==7097==    by 0x1BA54AE9: __libc_start_main (in /lib/tls/libc.so.6) 
==7097==    by 0x8048580: ??? (start.S:119) 
==7097==  Address 0x8AA04420 is not stack'd, malloc'd or (recently) free'd 
Segmentation fault 
 
Note that it works either without the "extern" line or that one before the 
definition, or when compiling without -pthread (but KDE is compiled with 
that). 
 
It seems I can't reopen this bug report, so if someone else could ... 

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-10-13 18:33 -------
I think that is a bug in the front-end:
<L2>:;
  if (__priority == 65535) goto <L4>; else goto <L6>;

<L4>:;
  if (__initialize_p == 0) goto <L5>; else goto <L6>;

<L5>:;
  __comp_dtor (&idl_filename);

<L6>:;
  if (__priority == 65535) goto <L8>; else goto <L10>;

<L8>:;
  if (__initialize_p == 0) goto <L9>; else goto <L10>;

<L9>:;
  __comp_dtor (&idl_filename);

Please file it seperately.

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[matz at suse dot de]

------- Additional Comments From matz at suse dot de  2004-10-13 18:37 -------
Ah, the problem is, that for some reasons GCC emits _two_ dtor functions 
for 'idl_filename' (__tcf_2 and __tcf_1), but only if the extern decl comes 
after the definition.  Okay, this is an error in itself, but it seems the 
other allocators deal with this breakage more graceful.  OTOH it's a real 
double free, so it's not really something which has to be "fixed" in the 
allocator itself. 

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-10-13 18:39 -------
I filed the c++ front-end bug under PR 17976 with a simple example.

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[matz at suse dot de]

------- Additional Comments From matz at suse dot de  2004-10-13 18:41 -------
Oh, wire-crossing.  I filed this now as PR17977. 

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[matz at suse dot de]

------- Additional Comments From matz at suse dot de  2004-10-13 18:42 -------
Argh.  Don't ask people to do stuff, if you are doing it yourself ;-) 

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937

[Bug libstdc++/17937] Critical ~__pool troubles

[pinskia at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.0.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17937