C++ new placement operator confuse Valgrind Memcheck

Question

So in my program I have a contiguous array of objects. I am doing this because I want spatial locality between each of these objects. They are really small and in a very large array and are iterated over in order.

As such I initialize them like this

memoryPool = new Object[MAX_OBJECTS];

This of course calls the default initializer on every single one of the objects. I wrote no default initializer And that is completely fine as they will be over written. They cant be initialized to a valid state at the beginning because they need to be initialized with data generated later down the road at various times.

There is therefore a variable objectCount the objectCount has been initialized with the valid initializer. When it is time to increase the objectCount variable

 assert(newObjectCount >= objectCount);
 for (int i = objectCount; i < newObjectCount; i++) {
            new(&(memoryPool[i])) Object(/*Calls the valid initializer of the class*/);
  }
 objectCount = newObjectCount;

This technique which uses the placement new I found here.

So you can see that I maintain spatial locality between these classes like so.

There may be a better way to do this but I dont know of one and I am all ears.

Now on to the actual question. This code when ran through valgrind with leak-check=full generates a LOT of output about Conditional jump or move depends on uninitialized value(s) and it is always on one of the Object objects. Will this sort of allocation be on of the "funny things with pointers" that could throw off Memcheck?

Answer 1

The first problem is that in your case memoryPool[i] already contains a valid object that is overwritten by your placement-new without the old object being properly deleted.

So a slightly better way would then be :

memoryPool[i].~Object();  // first destroy the obl object
new(&(memoryPool[i])) Object (...);  

But use of placement new should be exceptional. If it is just to replace objects, you should not use such delicate technique, but rather use a simple assignment:

memoryPool[i] = Object (...); // just replace the old object with a new one.  

Of course, in both cases you should respect the rule of 3. If you wouldn't, depending on the members of Object, you could still leak memory.

Finally, another problem: since you allocate a dynamic array, you'd need to delete the array as well when you no longer need it:

delete[] memoryPool;   // note the []

A much more flexible and robust approach would be to use a vector<Object> which would allow memoryPool to grow and shrink dynamically, freeing you from the duty of memory management. In addition, you could construct and add elements as needed with emplace_back()

Answer 2

Also note that if you really want to write your own pool allocator then you need to tell Valgrind about the memory operations on the pool. Valgrind already recognizes builtin functions like malloc, operator new etc.

You can instrument your memory pool with the Valgrind client requests, as documented in the manual.

If you do not instrument your pool then Valgrind will not detect errors such as the following scenario

• block M of memory is allocated with new and forms the memory pool. Valgrind marks M as allocated but not initialized.
• block S of memory is suballocated from the pool. Valgrind knows nothing about this operation.
• S is correctly initialized. Valgrind marks S as initialized.
• you do some stuff with S
• S is returned to the pool. Valgrind again knows nothing about this, whereas it should be marking S as free
• block S is again suballocated from the pool. Valgrind should be marking S as allocated but not initialized but instead S is still marked as initialized
• now you do an 'uninitialized read' on S, but Valgrind thinks that S is initialized so does not emit an error. You now have a false negative :-(