Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#10389 closed Bugs (fixed)

[container] Double free problem on boost::containers::vector<std::unique_ptr<T>>

Reported by: kbinani <kbinani.bt@…> Owned by: John Maddock
Milestone: To Be Determined Component: type_traits
Version: Boost 1.56.0 Severity: Problem
Keywords: Cc:

Description

Using boost::container::vector with std::unique_ptr causes double-free problem.

Compiler

clang++ on Mac OSX 10.9.4

$ clang++ --version
$ Apple LLVM version 5.0 (clang-500.2.79) (based on LLVM 3.3svn)
$ Target: x86_64-apple-darwin13.3.0
$ Thread model: posix

Boost version

1.54, 1.55, 1.56, HEAD(d811937 on https://github.com/boostorg/boost.git)

How to reproduce

Compile the sample code below and run.

//file: container_vector_double_free.cpp
//clang++ container_vector_double_free.cpp -std=c++11 -I/Users/kbinani/Documents/github/boostorg/boost
#include <boost/container/vector.hpp>
#include <memory>

int main()
{
        typedef std::unique_ptr<int> object_ptr_t;
        typedef boost::container::vector<object_ptr_t> container_t;

        container_t c;

        // if the number of object to 'push_back' is less than 3, double-free problem does not occur.
        int const kNum = 10;

        // when 'reserve' called before 'push_back', double-free problem does not occur.
        //c.reserve(kNum);

        for (int i = 0; i < kNum; ++i) {
                object_ptr_t item(new int(0));
                c.push_back(boost::move(item));
        }

        return 0;
}

Result:

a.out(31615,0x7fff79acb310) malloc: *** error for object 0x7ff170403980: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Abort trap: 6

Stack trace:

#3      0x00007fff8da6707f in free ()
#4      0x0000000100002560 in std::__1::default_delete<int>::operator()(int*) const [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2488
#5      0x0000000100002539 in std::__1::unique_ptr<int, std::__1::default_delete<int> >::reset(int*) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2687
#6      0x00000001000024e3 in std::__1::unique_ptr<int, std::__1::default_delete<int> >::~unique_ptr() [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2655
#7      0x00000001000024e3 in std::__1::unique_ptr<int, std::__1::default_delete<int> >::~unique_ptr() [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2655
#8      0x00000001000024e3 in std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > >::destroy(std::__1::unique_ptr<int, std::__1::default_delete<int> >*) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:1739
#9      0x00000001000024af in void boost::container::allocator_traits<std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > > >::priv_destroy<std::__1::unique_ptr<int, std::__1::default_delete<int> > >(boost::integral_constant<bool, true>, std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > >&, std::__1::unique_ptr<int, std::__1::default_delete<int> >*) at /Users/kbinani/Documents/github/boostorg/boost/libs/container/boost_container_vector_test/../../../boost/container/allocator_traits.hpp:300
#10     0x0000000100002471 in void boost::container::allocator_traits<std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > > >::destroy<std::__1::unique_ptr<int, std::__1::default_delete<int> > >(std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > >&, std::__1::unique_ptr<int, std::__1::default_delete<int> >*) at /Users/kbinani/Documents/github/boostorg/boost/libs/container/boost_container_vector_test/../../../boost/container/allocator_traits.hpp:242
#11     0x0000000100002259 in void boost::container::destroy_alloc_n<std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > >, std::__1::unique_ptr<int, std::__1::default_delete<int> >*>(std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > >&, std::__1::unique_ptr<int, std::__1::default_delete<int> >*, std::__1::iterator_traits<std::__1::unique_ptr<int, std::__1::default_delete<int> >*>::difference_type, boost::container::container_detail::enable_if_c<!(boost::has_trivial_destructor<std::__1::iterator_traits<std::__1::unique_ptr<int, std::__1::default_delete<int> >*>::value_type>::value), void>::type*) at /Users/kbinani/Documents/github/boostorg/boost/libs/container/boost_container_vector_test/../../../boost/container/detail/utilities.hpp:1073
#12     0x0000000100001f2d in void boost::container::vector<std::__1::unique_ptr<int, std::__1::default_delete<int> >, std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > > >::priv_forward_range_insert_new_allocation<boost::container::container_detail::insert_move_proxy<std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > >, std::__1::unique_ptr<int, std::__1::default_delete<int> >*> >(std::__1::unique_ptr<int, std::__1::default_delete<int> >*, unsigned long, std::__1::unique_ptr<int, std::__1::default_delete<int> >*, unsigned long, boost::container::container_detail::insert_move_proxy<std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > >, std::__1::unique_ptr<int, std::__1::default_delete<int> >*>) at /Users/kbinani/Documents/github/boostorg/boost/libs/container/boost_container_vector_test/../../../boost/container/vector.hpp:2417
#13     0x0000000100001aee in boost::container::container_detail::vec_iterator<std::__1::unique_ptr<int, std::__1::default_delete<int> >*, false> boost::container::vector<std::__1::unique_ptr<int, std::__1::default_delete<int> >, std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > > >::priv_forward_range_insert_no_capacity<boost::container::container_detail::insert_move_proxy<std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > >, std::__1::unique_ptr<int, std::__1::default_delete<int> >*> >(std::__1::unique_ptr<int, std::__1::default_delete<int> >* const&, unsigned long, boost::container::container_detail::insert_move_proxy<std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > >, std::__1::unique_ptr<int, std::__1::default_delete<int> >*>, boost::container::container_detail::integral_constant<unsigned int, 1u>) at /Users/kbinani/Documents/github/boostorg/boost/libs/container/boost_container_vector_test/../../../boost/container/vector.hpp:2045
#14     0x000000010000193c in void boost::container::vector<std::__1::unique_ptr<int, std::__1::default_delete<int> >, std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > > >::priv_push_back<std::__1::unique_ptr<int, std::__1::default_delete<int> > >(std::__1::unique_ptr<int, std::__1::default_delete<int> >&&) at /Users/kbinani/Documents/github/boostorg/boost/libs/container/boost_container_vector_test/../../../boost/container/vector.hpp:1944
#15     0x0000000100001810 in boost::container::vector<std::__1::unique_ptr<int, std::__1::default_delete<int> >, std::__1::allocator<std::__1::unique_ptr<int, std::__1::default_delete<int> > > >::push_back(std::__1::unique_ptr<int, std::__1::default_delete<int> >&&) at /Users/kbinani/Documents/github/boostorg/boost/libs/container/boost_container_vector_test/../../../boost/container/vector.hpp:1467
#16     0x0000000100001581 in main at /Users/kbinani/Documents/github/boostorg/boost/libs/container/boost_container_vector_test/boost_container_vector_test/main.cpp:14

Change History (5)

comment:1 by Ion Gaztañaga, 8 years ago

Component: containertype_traits
Owner: changed from Ion Gaztañaga to John Maddock

I've found boost::has_trivial_copy returns true for unique_ptr in Clang. I've found related issues here:

http://stackoverflow.com/questions/12754886/has-trivial-copy-behaves-differently-in-clang-and-gcc-whos-right

and

http://stackoverflow.com/questions/22812183/a-deleted-default-constructor-could-still-be-trivial

the problem is that it's not clear if types with deleted copy constructor can still be trivially copy constructible.

I don't know if boost::has_trivial_copy has defined behaviour on this and this might be a bug, I'm reassining to TypeTraits (John) in case it's considered a bug.

If not, please reassign it to Container. The workaround is to use is_copy_constructible && has_trivial_copy to know if a type has a copy constructor that is trivial, and memcpy can be used to move objects.

comment:2 by John Maddock, 8 years ago

Resolution: fixed
Status: newclosed

I believe this is now fixed in https://github.com/boostorg/type_traits/commit/df74811a4c996f0e5fd5551622aefed803db27d4

Note that has_trivial_assign and has_trivial_construct very likely have the same issues, but I don't know how to fix those!

comment:3 by Ion Gaztañaga, 8 years ago

I think we are missing is_copy_assignable in Boost. The implementation seems to be possible only in C++11 using decltype but Boost could support noncopyable and Boost.Move, just like is_copy_constructible does.

Once we have it, we can apply a similar patch to has_trivial_assign. Does this make sense?

comment:4 by John Maddock, 8 years ago

If you can implement it, then yes, lets add it.

comment:5 by Ion Gaztañaga, 8 years ago

Ok, I'll create a pull request when I manage to implement it.

Note: See TracTickets for help on using tickets.