Buffer overflow with memory_pool_collection

[athanase]

Hello,

memory_pool_collection reports a buffer overflow on deallocation. I might be doing something wrong...

Here is the code that fails on my machine (linux x64)

int32_t main( int32_t argc, char* argv[] )
{
	namespace memory = foonathan::memory;
	using namespace memory::literals;

	using pools = memory::memory_pool_collection< memory::node_pool,
						      memory::identity_buckets,
						      memory::allocator_reference< memory::malloc_allocator > >;

	const auto max_size = 64u;
	pools pool( max_size, 200_MiB, memory::malloc_allocator{} );

	memory::vector< int32_t, decltype( pool ) > vec( { 1, 2, 3, 4 }, pool );
	return 0;
}

the output of the program:

/home/test/build/debug/bin/sandbox
[foonathan::memory] Buffer overflow at address 0x7f0863cf65b8 detected, corresponding memory block 0x7f0863cf65a0 has only size 16.Signal: SIGABRT (Aborted)

the stacktrace:

raise 0x00007f08919e9615
abort 0x00007f08919d2862
(anonymous namespace)::default_buffer_overflow_handler debugging.cpp:95
foonathan::memory::detail::debug_fill_free debug_helpers.cpp:69
foonathan::memory::detail::free_memory_list::deallocate free_list.cpp:219
foonathan::memory::memory_pool_collection<foonathan::memory::node_pool, foonathan::memory::identity_buckets, foonathan::memory::allocator_storage<foonathan::memory::reference_storage<foonathan::memory::detail::lowlevel_allocator<foonathan::memory::detail::malloc_allocator_impl> >, foonathan::memory::no_mutex> >::deallocate_array memory_pool_collection.hpp:238
foonathan::memory::allocator_traits<foonathan::memory::memory_pool_collection<foonathan::memory::node_pool, foonathan::memory::identity_buckets, foonathan::memory::allocator_storage<foonathan::memory::reference_storage<foonathan::memory::detail::lowlevel_allocator<foonathan::memory::detail::malloc_allocator_impl> >, foonathan::memory::no_mutex> > >::deallocate_array memory_pool_collection.hpp:451
foonathan::memory::allocator_storage<foonathan::memory::reference_storage<foonathan::memory::memory_pool_collection<foonathan::memory::node_pool, foonathan::memory::identity_buckets, foonathan::memory::allocator_storage<foonathan::memory::reference_storage<foonathan::memory::detail::lowlevel_allocator<foonathan::memory::detail::malloc_allocator_impl> >, foonathan::memory::no_mutex> > >, foonathan::memory::no_mutex>::deallocate_array allocator_storage.hpp:201
foonathan::memory::std_allocator<int, foonathan::memory::memory_pool_collection<foonathan::memory::node_pool, foonathan::memory::identity_buckets, foonathan::memory::allocator_storage<foonathan::memory::reference_storage<foonathan::memory::detail::lowlevel_allocator<foonathan::memory::detail::malloc_allocator_impl> >, foonathan::memory::no_mutex> > >::deallocate_impl std_allocator.hpp:275
foonathan::memory::std_allocator<int, foonathan::memory::memory_pool_collection<foonathan::memory::node_pool, foonathan::memory::identity_buckets, foonathan::memory::allocator_storage<foonathan::memory::reference_storage<foonathan::memory::detail::lowlevel_allocator<foonathan::memory::detail::malloc_allocator_impl> >, foonathan::memory::no_mutex> > >::deallocate std_allocator.hpp:203
std::allocator_traits<foonathan::memory::std_allocator<int, foonathan::memory::memory_pool_collection<foonathan::memory::node_pool, foonathan::memory::identity_buckets, foonathan::memory::allocator_storage<foonathan::memory::reference_storage<foonathan::memory::detail::lowlevel_allocator<foonathan::memory::detail::malloc_allocator_impl> >, foonathan::memory::no_mutex> > > >::deallocate alloc_traits.h:341
std::_Vector_base<int, foonathan::memory::std_allocator<int, foonathan::memory::memory_pool_collection<foonathan::memory::node_pool, foonathan::memory::identity_buckets, foonathan::memory::allocator_storage<foonathan::memory::reference_storage<foonathan::memory::detail::lowlevel_allocator<foonathan::memory::detail::malloc_allocator_impl> >, foonathan::memory::no_mutex> > > >::_M_deallocate stl_vector.h:354
std::_Vector_base<int, foonathan::memory::std_allocator<int, foonathan::memory::memory_pool_collection<foonathan::memory::node_pool, foonathan::memory::identity_buckets, foonathan::memory::allocator_storage<foonathan::memory::reference_storage<foonathan::memory::detail::lowlevel_allocator<foonathan::memory::detail::malloc_allocator_impl> >, foonathan::memory::no_mutex> > > >::~_Vector_base stl_vector.h:335
std::vector<int, foonathan::memory::std_allocator<int, foonathan::memory::memory_pool_collection<foonathan::memory::node_pool, foonathan::memory::identity_buckets, foonathan::memory::allocator_storage<foonathan::memory::reference_storage<foonathan::memory::detail::lowlevel_allocator<foonathan::memory::detail::malloc_allocator_impl> >, foonathan::memory::no_mutex> > > >::~vector stl_vector.h:683
main main.cpp:67
__libc_start_main 0x00007f08919d4152
_start free_list_array.hpp:98

and the memory content:

Thanks in advance for the help.

[foonathan]

So sorry for the delay, feel free to ping me in the future after a week or so. Issue is now fixed. Note that memory_pool and also memory_pool_collection aren't ideal for array allocations as done by std::vector. They're best if you need to allocate many individual objects, like in a linked list, for example. It still works, but is not as efficient.

BTW, you don't need memory::allocator_reference, memory::malloc_allocator alone is enough.

[athanase]

Thanks a lot, the fix works perfectly fine.

@foonathan what would you consider the best practise with your library for std::vector array allocations ?

I know that the first thing I should do is measure and I also understand that in general a temporary_allocator is a good match for std::vector. But if neither temporary_allocator nor stack_allocator can be used, what would you recommend ?

Wouldn't memory_pool be a good option compared to directly use std::allocator wich will then call std::malloc ?

Or would you say that the code is badly written and I should avoid such situations and make sure I am always using a temporary or stack allocator ?

[foonathan]

It depends on your use case.

• if your vector is usually small (let's say < 16 elements), employing a small buffer optimization (where small memory is available from the stack) might be useful.
• if you can somehow use a stack allocator (e.g. because the vector is only used inside a function), you should prefer to do that. Note that std::vector isn't ideal there either, as it will waste the previous memory on resize - the container doesn't know that it might be able to simple expand the existing memory, but it will allocate new memory, copy over, and deallocate the old one (which does nothing for a stack).
• if neither of the above cases works, using the default allocator is fine: if you're allocating big arrays of varying size, that's exactly what it was designed for.

Note that small buffer optimization is not possible with allocators alone. You might find https://github.com/foonathan/array interesting.

[athanase]

Clear explanation, thanks a lot for your help !