[Bug libstdc++/47354] New: bitmap_allocator free_list::_M_get never locks mutex

[Bug libstdc++/47354] New: bitmap_allocator free_list::_M_get never locks mutex

[greed at pobox dot com]

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

           Summary: bitmap_allocator free_list::_M_get never locks mutex
           Product: gcc
           Version: 4.5.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: greed@pobox.com


Created attachment 23023
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=23023
Add a mutex lock step in bitmap_allocator.cc

On AIX, I received a test failure in
ext/bitmap_allocator/variadic_construct.cc.

terminate called after throwing an instance of
'__gnu_cxx::__concurrence_unlock_
error'
  what():  __gnu_cxx::__concurrence_unlock_error
FAIL: ext/bitmap_allocator/variadic_construct.cc execution test

I was able to trace this back to src/bitmap_allocator.c free_list::_M_get,
which unlocks its mutex, and that unlock causes the abort.  It seems, however,
that no code path ever locks the mutex in the first place.  (The other use of
the mutex use the __scoped_lock type, which does lock.)

Linux allows you to unlock a mutex that was never locked.  AIX, at least 5.3
TL4, gives you EINVAL when you attempt that.  But, it seems to me that the
mutex should be locked.  To this end, I propose the (very simple) patch
attached, which simply adds __bfl_mutex.lock() inside the #ifdef block.

I have inspected the 'trunk' versions of src/bitmap_allocator.cc and
include/ext/bitmap_allocator.h and believe that the problem remains in the
trunk, not just 4.5.2.

[Bug libstdc++/47354] bitmap_allocator free_list::_M_get never locks mutex

[redi at gcc dot gnu.org]

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

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-01-18 23:46:01 UTC ---
I wouldn't be surprised if that code has bit-rotted, I'll check it out now ...

[Bug libstdc++/47354] bitmap_allocator free_list::_M_get never locks mutex

[paolo.carlini at oracle dot com]

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

--- Comment #2 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-01-19 00:27:45 UTC ---
Thanks for checking Jon. If we can't fix the issue very quickly, I vote for
simply removing that specific allocator, it never worked very well and the code
itself is rather ugly, IMHO.

[Bug libstdc++/47354] bitmap_allocator free_list::_M_get never locks mutex

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #23023|0                           |1
           is patch|                            |
  Attachment #23023|application/octet-stream    |text/plain
          mime type|                            |

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-01-19 00:37:39 UTC ---
Comment on attachment 23023
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=23023
Add a mutex lock step in bitmap_allocator.cc

the patch looks correct

[Bug libstdc++/47354] bitmap_allocator free_list::_M_get never locks mutex

[paolo.carlini at oracle dot com]

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

--- Comment #4 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-01-19 00:59:41 UTC ---
Great.

[Bug libstdc++/47354] bitmap_allocator free_list::_M_get never locks mutex

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2011.01.19 01:25:11
         AssignedTo|unassigned at gcc dot       |redi at gcc dot gnu.org
                   |gnu.org                     |
     Ever Confirmed|0                           |1

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-01-19 01:25:11 UTC ---
Revision 116942 removed the lock - I'm testing the fix now

[Bug libstdc++/47354] bitmap_allocator free_list::_M_get never locks mutex

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-01-19 01:31:12 UTC ---
Confirmed by modifying __gnu_cxx::__mutex to use
PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP to initialize the mutex:

$ lib0x ./var
terminate called after throwing an instance of
'__gnu_cxx::__concurrence_unlock_error'
  what():  __gnu_cxx::__concurrence_unlock_error
Aborted (core dumped)

[Bug libstdc++/47354] [4.3/4.4/4.5/4.6 Regression] bitmap_allocator free_list::_M_get never locks mutex

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.1.2
            Summary|bitmap_allocator            |[4.3/4.4/4.5/4.6
                   |free_list::_M_get never     |Regression]
                   |locks mutex                 |bitmap_allocator
                   |                            |free_list::_M_get never
                   |                            |locks mutex

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-01-19 01:48:54 UTC ---
this is a regression in all active branches

rather than modifying the source to use an error-checking mutex, valgrind's drd
tool can be used to show it was ok in 4.1 and is wrong in gcc 4.2+

==6026== drd, a thread error detector
==6026== Copyright (C) 2006-2010, and GNU GPL'd, by Bart Van Assche.
==6026== Using Valgrind-3.6.0 and LibVEX; rerun with -h for copyright info
==6026== Command: ./a.out
==6026== 
==6026== The object at address 0x4f0f780 is not a mutex.
==6026==    at 0x4A11932: pthread_mutex_unlock (drd_pthread_intercepts.c:640)
==6026==    by 0x4C6F8BD: __gnu_cxx::free_list::_M_get(unsigned long)
(gthr-default.h:790)
==6026==    by 0x4021E0: __gnu_cxx::bitmap_allocator<int>::_S_refill_pool() (in
/dev/shm/a.out)
==6026==    by 0x401A38:
__gnu_cxx::bitmap_allocator<int>::_M_allocate_single_object() (in
/dev/shm/a.out)
==6026==    by 0x40178E: __gnu_cxx::bitmap_allocator<int>::allocate(unsigned
long) (in /dev/shm/a.out)
==6026==    by 0x400CDD: main (in /dev/shm/a.out)

[Bug libstdc++/47354] [4.3/4.4/4.5/4.6 Regression] bitmap_allocator free_list::_M_get never locks mutex

[redi at gcc dot gnu.org]

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

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-01-19 02:27:49 UTC ---
Author: redi
Date: Wed Jan 19 02:27:45 2011
New Revision: 168980

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168980
Log:
2011-01-19  Graham Reed  <greed@pobox.com>

    PR libstdc++/47354
    * src/bitmap_allocator.cc (free_list::_M_get): Lock mutex.


Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/src/bitmap_allocator.cc

[Bug libstdc++/47354] [4.3/4.4/4.5 Regression] bitmap_allocator free_list::_M_get never locks mutex

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.6.0
            Summary|[4.3/4.4/4.5/4.6            |[4.3/4.4/4.5 Regression]
                   |Regression]                 |bitmap_allocator
                   |bitmap_allocator            |free_list::_M_get never
                   |free_list::_M_get never     |locks mutex
                   |locks mutex                 |

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-01-19 02:29:41 UTC ---
fixed on trunk so far

[Bug libstdc++/47354] [4.3/4.4/4.5 Regression] bitmap_allocator free_list::_M_get never locks mutex

[redi at gcc dot gnu.org]

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

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-01-19 08:50:02 UTC ---
Author: redi
Date: Wed Jan 19 08:49:58 2011
New Revision: 168985

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168985
Log:
2011-01-19  Graham Reed  <greed@pobox.com>

    PR libstdc++/47354
    * src/bitmap_allocator.cc (free_list::_M_get): Lock mutex.


Modified:
    branches/gcc-4_5-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_5-branch/libstdc++-v3/src/bitmap_allocator.cc

[Bug libstdc++/47354] [4.3/4.4/4.5 Regression] bitmap_allocator free_list::_M_get never locks mutex

[redi at gcc dot gnu.org]

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

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-01-19 08:50:33 UTC ---
Author: redi
Date: Wed Jan 19 08:50:29 2011
New Revision: 168986

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168986
Log:
2011-01-19  Graham Reed  <greed@pobox.com>

    PR libstdc++/47354
    * src/bitmap_allocator.cc (free_list::_M_get): Lock mutex.


Modified:
    branches/gcc-4_4-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_4-branch/libstdc++-v3/src/bitmap_allocator.cc

[Bug libstdc++/47354] [4.3 Regression] bitmap_allocator free_list::_M_get never locks mutex

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.4.6, 4.5.3
            Summary|[4.3/4.4/4.5 Regression]    |[4.3 Regression]
                   |bitmap_allocator            |bitmap_allocator
                   |free_list::_M_get never     |free_list::_M_get never
                   |locks mutex                 |locks mutex

--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-01-19 08:56:51 UTC ---
fixed for 4.4.6 and 4.5.3 too

[Bug libstdc++/47354] [4.3 Regression] bitmap_allocator free_list::_M_get never locks mutex

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.4.6

--- Comment #13 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-01-27 14:25:41 UTC ---
I guess we can close this as fixed, at this point nobody really cares about
this rather unused allocator in 4.3.x.

[Bug libstdc++/47354] [4.3 Regression] bitmap_allocator free_list::_M_get never locks mutex

[greed at pobox dot com]

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

--- Comment #14 from Graham Reed <greed at pobox dot com> 2011-01-27 15:22:36 UTC ---
I agree; thanks guys.