[Bug libstdc++/58659] New: Construction of shared_ptr from unique_ptr mismatches new/delete and std::allocator for __shared_ptr_count

[Bug libstdc++/58659] New: Construction of shared_ptr from unique_ptr mismatches new/delete and std::allocator for __shared_ptr_count

[spencer at starscale dot com]

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

            Bug ID: 58659
           Summary: Construction of shared_ptr from unique_ptr mismatches
                    new/delete and std::allocator for __shared_ptr_count
           Product: gcc
           Version: 4.8.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: spencer at starscale dot com
                CC: jwakely.gcc at gmail dot com

Using gcc-4.8.1 configured with --enable-threads=single
--enable-libstdcxx-allocator=mt, I'm seeing the following tiny program
crash:

int
main()
{
  std::unique_ptr<X> u = std::unique_ptr<X>(new X);
  std::shared_ptr<X> s(std::move(u));
  return 0;
}

Note that I am using a different default std::allocator base class
(not new/delete).

I think the bug is that the shared_ptr count object is being allocated
with new and deleted with std::allocator.  This will work when
std::allocator is new/delete, but not otherwise.

I haven't had time to prove it or work up a patch yet; I can if that
helps: please let me know.

Look at __shared_ptr_base's __S_create_from_up:

template<typename _Tp, typename _Del>
  static _Sp_counted_base<_Lp>*
  _S_create_from_up(std::unique_ptr<_Tp, _Del>&& __r,
    typename std::enable_if<!std::is_reference<_Del>::value>::type* =
  0)
  {
    typedef typename unique_ptr<_Tp, _Del>::pointer _Ptr;
    return new _Sp_counted_deleter<_Ptr, _Del, std::allocator<void>,
      _Lp>(__r.get(), __r.get_deleter());
  }

It always uses "new" to allocate an _Sp_counted_deleter (in both
overloads).  I think this is the bug, because _Sp_counted_deleter
always uses std::allocator to dispose of itself:

virtual void
_M_destroy() noexcept
{
  typedef typename allocator_traits<_Alloc>::template
    rebind_traits<_Sp_counted_deleter> _Alloc_traits;
  typename _Alloc_traits::allocator_type __a(_M_del);
  _Alloc_traits::destroy(__a, this);
  _Alloc_traits::deallocate(__a, this, 1);
}

CC Jonathan Wakely who has already analyzed this with a test case for the stock
std::allocator on the mailing list.

[Bug libstdc++/58659] Construction of shared_ptr from unique_ptr mismatches new/delete and std::allocator for __shared_ptr_count

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2013-10-07
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
   Target Milestone|---                         |4.8.2
     Ever confirmed|0                           |1

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Thanks for the report (it's easier to track and find in future if it's in
bugzilla)

I'll fix this asap.

[Bug libstdc++/58659] Construction of shared_ptr from unique_ptr mismatches new/delete and std::allocator for __shared_ptr_count

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Author: redi
Date: Tue Oct  8 12:33:37 2013
New Revision: 203274

URL: http://gcc.gnu.org/viewcvs?rev=203274&root=gcc&view=rev
Log:
    PR libstdc++/58659
    * include/bits/shared_ptr_base.h (__shared_count::__shared_count(P,D)):
    Delegate to constructor taking allocator.
    (__shared_count::_S_create_from_up): Inline into ...
    (__shared_count::__shared_count(unique_ptr<Y,D>&&): Here. Use
    std::conditional instead of constrained overloads. Allocate memory
    using the allocator type that will be used for deallocation.
    * testsuite/20_util/shared_ptr/cons/58659.cc: New.
    * testsuite/20_util/shared_ptr/cons/43820_neg.cc: Adjust.

Added:
    trunk/libstdc++-v3/testsuite/20_util/shared_ptr/cons/58659.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/bits/shared_ptr_base.h
    trunk/libstdc++-v3/testsuite/20_util/shared_ptr/cons/43820_neg.cc

[Bug libstdc++/58659] Construction of shared_ptr from unique_ptr mismatches new/delete and std::allocator for __shared_ptr_count

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Author: redi
Date: Tue Oct  8 13:38:21 2013
New Revision: 203277

URL: http://gcc.gnu.org/viewcvs?rev=203277&root=gcc&view=rev
Log:
    PR libstdc++/58659
    * include/bits/shared_ptr_base.h (__shared_count::__shared_count(P,D)):
    Delegate to constructor taking allocator.
    (__shared_count::_S_create_from_up): Inline into ...
    (__shared_count::__shared_count(unique_ptr<Y,D>&&): Here. Use
    std::conditional instead of constrained overloads. Allocate memory
    using the allocator type that will be used for deallocation.
    * testsuite/20_util/shared_ptr/cons/58659.cc: New.
    * testsuite/20_util/shared_ptr/cons/43820_neg.cc: Adjust.

Added:
   
branches/gcc-4_8-branch/libstdc++-v3/testsuite/20_util/shared_ptr/cons/58659.cc
Modified:
    branches/gcc-4_8-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_8-branch/libstdc++-v3/include/bits/shared_ptr_base.h
   
branches/gcc-4_8-branch/libstdc++-v3/testsuite/20_util/shared_ptr/cons/43820_neg.cc

[Bug libstdc++/58659] Construction of shared_ptr from unique_ptr mismatches new/delete and std::allocator for __shared_ptr_count

[redi at gcc dot gnu.org]

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

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

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I'm not going to backport this to the 4.7 branch, as it works fine in the
default configuration.