2

I'm trying to create scalable pool allocator (for now only for built-in data types). And when I'm scanning it with valgrind, I'm getting warning:

Invalid write of size 8

The program it self compiles and works correctly

Pool Allocator code:

#include "pool_allocator.h"
//Pool allocator reference: http://www.thinkmind.org/download.php?articleid=computation_tools_2012_1_10_80006

namespace sc2d::memory {

void pool_allocator::create(size_t block_size, size_t blocks_numb, size_t alignment)
{
    size_of_block = block_size;
    num_of_blocks = blocks_numb;
    p_start = reinterpret_cast<unsigned char*>(aligned_malloc(block_size * blocks_numb, alignment));
    num_of_free_blocks = num_of_blocks;
    p_next = p_start;
}

void pool_allocator::destroy()
{
    aligned_free(p_start);
    p_start = nullptr;
}

void* pool_allocator::allocate()
{
    if(num_of_initialized < num_of_blocks)
    {
        size_t* ptr = (size_t*)addr_from_index(num_of_initialized);
        *ptr = num_of_initialized + 1;
        num_of_initialized++;
    }

    void* ptr = nullptr;
    if(num_of_free_blocks > 0)
    {
        ptr = (void*)p_next;
        --num_of_free_blocks;
        if(num_of_free_blocks != 0)
        {
            p_next = addr_from_index(*(size_t*)p_next);
        }
    }
    else
    {
        resize(num_of_blocks << 1);
        ptr = addr_from_index(num_of_initialized);
        num_of_initialized++;
        num_of_free_blocks--;
        p_next = addr_from_index(num_of_initialized);
    }

    return ptr;
}

void pool_allocator::resize(size_t new_size)
{
    num_of_free_blocks = num_of_blocks;
    num_of_blocks = new_size;
    if(void* p_new_start = realloc(p_start, size_of_block * num_of_blocks))
        p_start = reinterpret_cast<unsigned char*>(p_new_start);
    else
    {
        // TODO: Error handling
    }
}

void pool_allocator::deallocate(void* ptr)
{
    if(p_next != nullptr)
    {
        (*(size_t*)ptr) = index_from_addr(p_next);
        p_next = (unsigned char*)ptr;
    }
    else
    {
        (*(size_t*)ptr) = num_of_blocks;
        p_next = (unsigned char*)ptr;
    }
    num_of_free_blocks++;
    num_of_initialized--;
}

unsigned char* pool_allocator::addr_from_index(size_t index) const
{
    return p_start + index * size_of_block;
}

size_t pool_allocator::index_from_addr(const unsigned char* ptr) const
{
    return ((size_t)(ptr - p_start) / size_of_block);
}
}

Usage (code for testing):

double *d_p1 = nullptr;
double *d_p2 = nullptr;
double *d_p3 = nullptr;
double *d_p4 = nullptr;
double *d_p5 = nullptr;
double *d_p6 = nullptr;
double *d_p7 = nullptr;
double *d_p8 = nullptr;
double *d_p9 = nullptr;

using namespace sc2d::memory;
std::unique_ptr<pool_allocator> p_alloc = std::make_unique<pool_allocator>();

p_alloc->create(sizeof(double), 2, alignof(double*));

d_p1 = (double*)p_alloc->allocate();
d_p2 = (double*)p_alloc->allocate();
d_p3 = (double*)p_alloc->allocate();
d_p4 = (double*)p_alloc->allocate();
d_p5 = (double*)p_alloc->allocate();
d_p6 = (double*)p_alloc->allocate();
d_p7 = (double*)p_alloc->allocate();
d_p8 = (double*)p_alloc->allocate();
d_p9 = (double*)p_alloc->allocate();


*d_p1 = 1.99;
*d_p2 = 2.99;
*d_p3 = 3.99;
*d_p4 = 4.99;
*d_p5 = 5.99;
*d_p6 = 6.99;
*d_p7 = 7.99;
*d_p8 = 8.99;
*d_p9 = 9.99;

Valgrind log:

Invalid write of size 8
       at 0x10F45A: main (main.cpp:142)
Address 0x71b9e40 is 0 bytes inside a block of size 16 free'd
       at 0x4C31D2F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
       by 0x151EF7: sc2d::memory::pool_allocator::resize(unsigned long) (pool_allocator.cpp:62)
       by 0x151E3C: sc2d::memory::pool_allocator::allocate() (pool_allocator.cpp:48)
       by 0x10F3B9: main (main.cpp:133)
Block was alloc'd at
       at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
       by 0x151C6D: sc2d::memory::aligned_malloc(unsigned long, unsigned long) (aligned_allocator.cpp:22)
       by 0x151CFA: sc2d::memory::pool_allocator::create(unsigned long, unsigned long, unsigned long) (pool_allocator.cpp:16)
       by 0x10F375: main (main.cpp:129)

Invalid write of size 8
       at 0x10F469: main (main.cpp:143)
Address 0x71b9e48 is 8 bytes inside a block of size 16 free'd
       at 0x4C31D2F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
       by 0x151EF7: sc2d::memory::pool_allocator::resize(unsigned long) (pool_allocator.cpp:62)
       by 0x151E3C: sc2d::memory::pool_allocator::allocate() (pool_allocator.cpp:48)
       by 0x10F3B9: main (main.cpp:133)
Block was alloc'd at
       at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
       by 0x151C6D: sc2d::memory::aligned_malloc(unsigned long, unsigned long) (aligned_allocator.cpp:22)
       by 0x151CFA: sc2d::memory::pool_allocator::create(unsigned long, unsigned long, unsigned long) (pool_allocator.cpp:16)
       by 0x10F375: main (main.cpp:129)

Invalid write of size 8
      at 0x10F478: main (main.cpp:144)
Address 0x71b9ea0 is 16 bytes inside a block of size 32 free'd
      at 0x4C31D2F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
      by 0x151EF7: sc2d::memory::pool_allocator::resize(unsigned long) (pool_allocator.cpp:62)
      by 0x151E3C: sc2d::memory::pool_allocator::allocate() (pool_allocator.cpp:48)
      by 0x10F3E9: main (main.cpp:135)
Block was alloc'd at
      at 0x4C31D2F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
      by 0x151EF7: sc2d::memory::pool_allocator::resize(unsigned long) (pool_allocator.cpp:62)
      by 0x151E3C: sc2d::memory::pool_allocator::allocate() (pool_allocator.cpp:48)
      by 0x10F3B9: main (main.cpp:133)

......

HEAP SUMMARY:
in use at exit: 0 bytes in 0 blocks
total heap usage: 9 allocs, 9 frees, 73,112 bytes allocated
All heap blocks were freed -- no leaks are possible 
For counts of detected and suppressed errors, rerun with: -v
ERROR SUMMARY: 8 errors from 8 contexts (suppressed: 0 from 0)

I know that there is some issues with this pool allocator implementation:

  • realloc may return pointer to a new memory location (new start).
  • I'm using aligned allocator but realloc doesn't guarantee proper alignment after reallocation, so maybe I need solution described here.

Thank you!

UPD: Link to godbolt compiler explorer

UPD 2: Aligned malloc code:

#include <memory>

#if defined(__GLIBC__) && ((__GLIBC__>=2 && __GLIBC_MINOR__>=8) || 
 __GLIBC__>2) \
 && defined(__LP64__)
 #define GLIBC_MALLOC_ALREADY_ALIGNED 1
#else
  #define GLIBC_MALLOC_ALREADY_ALIGNED 0
#endif

#if defined(__FreeBSD__) && !defined(__arm__) && !defined(__mips__)
  #define FREEBSD_MALLOC_ALREADY_ALIGNED 1
#else
  #define FREEBSD_MALLOC_ALREADY_ALIGNED 0
#endif

#if (defined(__APPLE__) \
  || defined(_WIN64) \
  || GLIBC_MALLOC_ALREADY_ALIGNED \
  || FREEBSD_MALLOC_ALREADY_ALIGNED)
#define MALLOC_ALREADY_ALIGNED 1
#else
 #define MALLOC_ALREADY_ALIGNED 0
#endif

#if ((defined __QNXNTO__) || (defined _GNU_SOURCE) || ((defined 
_XOPEN_SOURCE) && (_XOPEN_SOURCE >= 600))) \
&& (defined _POSIX_ADVISORY_INFO) && (_POSIX_ADVISORY_INFO > 0)
    #define HAS_POSIX_MEMALIGN 1
#else
    #define HAS_POSIX_MEMALIGN 0
#endif
namespace sc2d::memory {

void* _aligned_malloc(size_t size, size_t alignment)
{
    void* res = nullptr;
    void* ptr = malloc(size + alignment);
    if (ptr!=nullptr) {
        res = reinterpret_cast<void*>((reinterpret_cast<size_t>(ptr) & ~(size_t(alignment-1))) + alignment);
        *(reinterpret_cast<void**>(res) - 1) = ptr;
    }
    return res;
}

void* aligned_malloc(size_t size, size_t alignment)
{
#if MALLOC_ALREADY_ALIGNED
    return malloc(size);
#elif HAS_POSIX_MEMALIGN
    void* res;
    const int failed = posix_memalign(&res,size,alignment);
    if(failed) res = 0;
    return res;
#elif (defined _MSC_VER)
    return _aligned_malloc(size, alignment);
#else
    return sc2d::memory::_aligned_malloc(size, alignment);
#endif
}

void _aligned_free(void* ptr)
{
    if (ptr != nullptr)
        free(*(reinterpret_cast<void**>(ptr) - 1));
}

void aligned_free(void* ptr)
{
#if MALLOC_ALREADY_ALIGNED
    free(ptr);
#elif HAS_POSIX_MEMALIGN
    free(ptr);
#elif defined(_MSC_VER)
    _aligned_free(ptr);
#else
    sc2d::memory_aligned_free(ptr);
#endif
}
}
NovaSurfer
  • 49
  • 1
  • 4
  • Regarding your last bullet, there may be an `aligned_realloc` that's a better fit. – user4581301 Jun 11 '19 at 00:26
  • How can this work at all with `realloc` relocating the block into which you have handed out pointers? – Davis Herring Jun 11 '19 at 04:44
  • Your godbolt link uses simple `malloc`. Your code example uses `aligned_malloc`, which isn't standard AFAICS, so you're either using the Microsoft extension `_aligned_malloc` or have coded your own, and not shown it. Which is it? – Useless Jun 11 '19 at 08:57
  • OK, on further reading, your problem is: `realloc` will quite frequently return a new address and invalidate your existing pointers. You _cannot_ keep pointers from before calling realloc, and use them after. Your options are to switch to a blocklist instead of a single-block allocator, or investigate mmap. NB. You can trivially see this by printing the addresses. – Useless Jun 11 '19 at 09:03
  • @Useless , For aligned_malloc I'm using macros: ` #if (defined(__APPLE__) \ || defined(_WIN64) \ || GLIBC_MALLOC_ALREADY_ALIGNED \ || FREEBSD_MALLOC_ALREADY_ALIGNED) #define MALLOC_ALREADY_ALIGNED 1 #else #define MALLOC_ALREADY_ALIGNED 0 #endif ` if MALLOC_ALREADY_ALIGNED I'm using default *malloc* if not, my own implementation. On my system malloc is already aligned – NovaSurfer Jun 11 '19 at 09:55
  • Returning to `realloc`. What if I will use `aligned_malloc` + `memcpy` + `aligned_free` for resizing? – NovaSurfer Jun 11 '19 at 10:02
  • I'm really don't want to use linked list for that allocator, I'm worrying about memory usage, or it's okay :) ? – NovaSurfer Jun 11 '19 at 10:07
  • Please just edit your question so it contains the [minimal _complete_ example](https://stackoverflow.com/help/minimal-reproducible-example). I'm not going to assemble it myself from linked external sources, macros in comments, and that godbolt example. – Useless Jun 11 '19 at 14:09
  • @Useless, sorry. As soon I will be at home, I will add aligned_alloc code. – NovaSurfer Jun 11 '19 at 16:38
  • I'm thinking about switching to C++ 17 `std::aligned_alloc`. But I don't know how to resize pool. I've tried to allocate new chunk of memory using `std::aligned_alloc`, then copy old data to a new one using `memcpy`, but I'm getting Valgrind errors. – NovaSurfer Jun 11 '19 at 18:35

0 Answers0