Re: Possible bug in shared_ptr(unique_ptr) constructor?

[Jonathan Wakely <jwakely dot gcc at gmail dot com>]

On 7 October 2013 15:57, Jonathan Wakely wrote:
> On 7 October 2013 13:39, Brad Spencer wrote:
>>
>> Am I on to something?
>
> Yes, I think so - please report it to Bugzilla and CC me, I'll analyse
> it properly and deal with it asap.

Here's a testcase to reproduce it using the default new_allocator
configuration (or any other allocator base):

#include <memory>
#include <cassert>

struct X { };

using spcd = std::_Sp_counted_deleter<X*, std::default_delete<X>,
std::allocator<void>, std::__default_lock_policy>;

namespace std
{
  template<>
    struct allocator<spcd>
    {
      using value_type = spcd;

      allocator() = default;

      template<typename U>
        allocator(const allocator<U>&) { }

      value_type* allocate(size_t n)
      {
        if (n != 1)
          throw bad_alloc();
        allocated = true;
        return static_cast<value_type*>((void*)(storage));
      }

      void deallocate(value_type* p, size_t n)
      {
        if (n != 1 || p != (void*)storage || !allocated)
          abort();
        allocated = false;
      }

      static char storage[sizeof(spcd)];
      static bool allocated;
    };

  char allocator<spcd>::storage[];
  bool allocator<spcd>::allocated = false;
}

int main()
{
  {
    std::shared_ptr<X> s( std::unique_ptr<X>(new X) );
    assert( std::allocator<spcd>::allocated );
  }
  assert( !std::allocator<spcd>::allocated );
}


And here's a suggested patch to fix it:

--- include/bits/shared_ptr_base.h.orig 2013-10-07 15:06:49.245991558 +0000
+++ include/bits/shared_ptr_base.h      2013-10-07 15:59:47.940991976 +0000
@@ -653,8 +653,8 @@
          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());
+         using _Sp = _Sp_counted_deleter<_Ptr, _Del, allocator<void>, _Lp>;
+         return _S_do_create_from_up<_Sp>(std::move(__r));
        }

       template<typename _Tp, typename _Del>
@@ -665,8 +665,20 @@
          typedef typename unique_ptr<_Tp, _Del>::pointer _Ptr;
          typedef typename std::remove_reference<_Del>::type _Del1;
          typedef std::reference_wrapper<_Del1> _Del2;
-         return new _Sp_counted_deleter<_Ptr, _Del2, std::allocator<void>,
-           _Lp>(__r.get(), std::ref(__r.get_deleter()));
+         typedef _Sp_counted_deleter<_Ptr, _Del2, allocator<void>, _Lp> _Sp;
+         return _S_do_create_from_up<_Sp>(std::move(__r));
+       }
+
+      template<typename _Sp, typename _Up>
+       static _Sp_counted_base<_Lp>*
+       _S_do_create_from_up(_Up&& __r)
+       {
+         using _Alloc = std::allocator<_Sp>;
+         using _ATr = std::allocator_traits<_Alloc>;
+         _Alloc __a;
+         auto __ptr = _ATr::allocate(__a, 1);
+         _ATr::construct(__a, __ptr, __r.get(), __r.get_deleter());
+         return __ptr;
        }

       _Sp_counted_base<_Lp>*  _M_pi;