[Bug c++/29179] New: bugs in mt_allocator

[Bug c++/29179] New: bugs in mt_allocator

[random at adriver dot ru]

This example will demonstrate 2 problems in mt_allocator.cc:

----------------
typedef __gnu_cxx::__mt_alloc<char> allocator_type;
typedef __gnu_cxx::__pool_base::_Tune tune_type;

allocator_type mt_char;
tune_type t(8, 40000, 8, (50000 - 4 * sizeof(void*)), 4096, 10, false);
mt_char._M_set_options(t);
allocator_type::pointer pc = mt_char.allocate(40000);
----------------

First bug in __pool<..>::_M_initialize():
        Binmap_type __bin_max = _M_options._M_min_bin;  // not correct.
        size_t __bin_max = _M_options._M_min_bin; // correct.


Second bug in __pool<..>::_M_reserve_block():

while (--__block_count > 0) // not correct because __block_count may be equal
0(and size_t is unsigned)
{
__c += __bin_size;
__block->_M_next = reinterpret_cast<_Block_record*>(__c);
__block = __block->_M_next;
}

  while (__block_count > 0) // correct
      {
        __c += __bin_size;
        __block->_M_next = reinterpret_cast<_Block_record*>(__c);
        __block = __block->_M_next;
       --__block_count;
      }


-- 
           Summary: bugs in mt_allocator
           Product: gcc
           Version: 4.1.1
            Status: UNCONFIRMED
          Severity: critical
          Priority: P3
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: random at adriver dot ru


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

[Bug libstdc++/29179] bugs in mt_allocator

[pcarlini at suse dot de]

------- Comment #1 from pcarlini at suse dot de  2006-09-22 11:19 -------
The first "bug" simply doesn't exist given the comment at the beginning of
__pool_base. The second one is at most a documentation issue: _M_chunk_size
shall be always much bigger than _M_max_bytes, thus __block_count always > 0.


-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c++                         |libstdc++
           Keywords|                            |documentation


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

[Bug libstdc++/29179] bugs in mt_allocator

[random at adriver dot ru]

------- Comment #2 from random at adriver dot ru  2006-09-22 13:32 -------
(In reply to comment #1)
> The first "bug" simply doesn't exist given the comment at the beginning of
> __pool_base

In the beginning of __pool_base we see:

  // Using short int as type for the binmap implies we are never
  // caching blocks larger than 65535 with this allocator.

So, it says that I can cache blocks of up to 65535 bytes, while in reality
limit is 32768.

Code below will generate sigfault:
// 
int main()
{
   typedef __gnu_cxx::__mt_alloc<char> allocator_type;
   typedef __gnu_cxx::__pool_base::_Tune tune_type;
   //3.4: typedef __gnu_cxx::__mt_alloc<char>::_Tune tune_type;

   allocator_type mt_char;
   tune_type t(8, 50000, 8, (200000 - 4 * sizeof(void*)), 4096, 10, false);
   mt_char._M_set_options(t);
   allocator_type::pointer pc = mt_char.allocate(40000);
   return 0;
}


_Binmap_type* __bp = _M_binmap;
_Binmap_type __bin_max = _M_options._M_min_bin; // not correct since you cast
size_t into u_short 

//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

_Binmap_type __bint = 0;

for (_Binmap_type __ct = 0; __ct <= _M_options._M_max_bytes; ++__ct)
 {
   if (__ct > __bin_max)
     {
       __bin_max <<= 1;
       ++__bint;
     }
   printf("__ct %d __bint %d __bin_max %d\n", __ct, __bint, __bin_max);
   *__bp++ = __bint;
 }
__ct 32757 __bint 12 __bin_max 32768
__ct 32758 __bint 12 __bin_max 32768
__ct 32759 __bint 12 __bin_max 32768
__ct 32760 __bint 12 __bin_max 32768
__ct 32761 __bint 12 __bin_max 32768
__ct 32762 __bint 12 __bin_max 32768
__ct 32763 __bint 12 __bin_max 32768
__ct 32764 __bint 12 __bin_max 32768
__ct 32765 __bint 12 __bin_max 32768
__ct 32766 __bint 12 __bin_max 32768
__ct 32767 __bint 12 __bin_max 32768
__ct 32768 __bint 12 __bin_max 32768
__ct 32769 __bint 13 __bin_max 0    // incorrect values start here
__ct 32770 __bint 14 __bin_max 0
__ct 32771 __bint 15 __bin_max 0
__ct 32772 __bint 16 __bin_max 0
__ct 32773 __bint 17 __bin_max 0
__ct 32774 __bint 18 __bin_max 0
__ct 32775 __bint 19 __bin_max 0
__ct 32776 __bint 20 __bin_max 0
__ct 32777 __bint 21 __bin_max 0
__ct 32778 __bint 22 __bin_max 0
__ct 32779 __bint 22 __bin_max 0

so we have incorrect binmap array.

> The second one is at most a documentation issue: _M_chunk_size
> shall be always much bigger than _M_max_bytes, thus __block_count always > 0.

would it not be easier to do a post increment and not have a problem with
people never reading documentation? especially considering that it's so easy to
fix?


-- 


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

[Bug libstdc++/29179] bugs in mt_allocator

[pcarlini at suse dot de]

------- Comment #3 from pcarlini at suse dot de  2006-09-22 13:42 -------
(In reply to comment #2)
> would it not be easier to do a post increment and not have a problem with
> people never reading documentation? especially considering that it's so easy to
> fix?

No, for the simple reason that the allocator does not work is __block_count
turns out to be zero. The problem with your PR is that you are doing sort of
syntactical analysis of the code without understandind the functioning of the
allocator. 


-- 


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

[Bug libstdc++/29179] bugs in mt_allocator

[random at adriver dot ru]

------- Comment #4 from random at adriver dot ru  2006-09-22 14:40 -------
(In reply to comment #3)
> No, for the simple reason that the allocator does not work is __block_count
> turns out to be zero. The problem with your PR is that you are doing sort of
> syntactical analysis of the code without understandind the functioning of the
> allocator. 

ok, perhaps this is not really a bug, however segfault is not very user
friendly. could ASSERT solve it?

about the first one, I take it you agree that it is a real bug?


-- 


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

[Bug libstdc++/29179] bugs in mt_allocator

[pcarlini at suse dot de]

------- Comment #5 from pcarlini at suse dot de  2006-09-22 14:48 -------
(In reply to comment #4)
> ok, perhaps this is not really a bug, however segfault is not very user
> friendly. could ASSERT solve it?

No, we don't have asserts anywhere, for various reasons. Really, the
documentation must be asjusted and then the user *must* carefully follow it.
This piece of code is only for espert usage, not for novices, this should be
clear.

> about the first one, I take it you agree that it is a real bug?

Yes, the documentation must be slightly adjusted to 32767. 


-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2006-09-22 14:48:24
               date|                            |


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

[Bug libstdc++/29179] bugs in mt_allocator

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|critical                    |normal


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

[Bug libstdc++/29179] bugs in mt_allocator

[paolo at gcc dot gnu dot org]

------- Comment #6 from paolo at gcc dot gnu dot org  2006-09-25 10:05 -------
Subject: Bug 29179

Author: paolo
Date: Mon Sep 25 10:05:27 2006
New Revision: 117193

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=117193
Log:
2006-09-25  Paolo Carlini  <pcarlini@suse.de>

        PR libstdc++/29179
        * include/ext/mt_allocator.h (__pool_base): Adjust/extend
        documentation in comments.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/ext/mt_allocator.h


-- 


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

[Bug libstdc++/29179] bugs in mt_allocator

[paolo at gcc dot gnu dot org]

------- Comment #7 from paolo at gcc dot gnu dot org  2006-09-25 10:05 -------
Subject: Bug 29179

Author: paolo
Date: Mon Sep 25 10:05:43 2006
New Revision: 117194

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=117194
Log:
2006-09-25  Paolo Carlini  <pcarlini@suse.de>

        PR libstdc++/29179
        * include/ext/mt_allocator.h (__pool_base): Adjust/extend
        documentation in comments.

Modified:
    branches/gcc-4_1-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_1-branch/libstdc++-v3/include/ext/mt_allocator.h


-- 


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

[Bug libstdc++/29179] bugs in mt_allocator

[pcarlini at suse dot de]

------- Comment #8 from pcarlini at suse dot de  2006-09-25 10:07 -------
Fixed for 4.1.2.


-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.1.2


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