Valgrind does not detect dangerous freeing memory

Question

I am learning valgrind framework and I decided to run it on my own minor test case. Here is following program, which forces deletion extra object from heap (I running it on AMD64/LINUX):

#include <iostream>
using namespace std;

struct Foo
{
    Foo(){ cout << "Creation Foo" << endl;}
    ~Foo(){ cout << "Deletion Foo" << endl;}
};

int main()
{
    Foo* ar = new Foo[3];
    *(reinterpret_cast<int*>(ar)-2) = 4;
    delete[] ar;
    return 0;
}

But result of execution of valgrind really confused me:

$ valgrind --leak-check=full ./a.out -v

==17649== Memcheck, a memory error detector

==17649== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.

==17649== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info

==17649== Command: ./a.out -v

==17649==

Creation Foo

Creation Foo

Creation Foo

Deletion Foo

Deletion Foo

Deletion Foo

Deletion Foo

==17649==

==17649== HEAP SUMMARY:

==17649== in use at exit: 72,704 bytes in 1 blocks

==17649== total heap usage: 3 allocs, 2 frees, 73,739 bytes allocated

==17649==

==17649== LEAK SUMMARY:

==17649== definitely lost: 0 bytes in 0 blocks

==17649== indirectly lost: 0 bytes in 0 blocks

==17649== possibly lost: 0 bytes in 0 blocks

==17649== still reachable: 72,704 bytes in 1 blocks

==17649== suppressed: 0 bytes in 0 blocks

==17649== Reachable blocks (those to which a pointer was found) are not shown.

==17649== To see them, rerun with: --leak-check=full --show-leak-kinds=all

==17649==

==17649== For counts of detected and suppressed errors, rerun with: -v

==17649== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

It seems like that valgrind (version 3.13.0)did not detect any memory corruption?

UPD: I compiled main.cpp with command g++ -g main.cpp

Answer 1

Valgrind does not detect array "prefix" change probably since it is a valid part of memory. Even though it is not supposed to be directly changed by user code it is still accessed and modified by array constructor code and valgrind does not provide such fine access check separation. Also note that this corruption does not seem to corrupt heap so deallocation succeeds.

Valgrid does not detect destructor call on invalid object probably because this call does not actually access invalid storage. Adding some class field will change the situation:

struct Foo
{
    int i;
    Foo(): i(0) { cout << i << "Creation Foo" << endl;}
   ~Foo(){ cout << i << "Deletion Foo" << endl;}
};

Invalid read of size 4

Answer 2

Valgrind doesn't detect a problem with memory because there is none.

Let's go through your program one step after another (this is implementation dependent, but it is basically how it works for gcc and other major compilers):

Calling new Foo[3]:

1. Memory of 8+3*sizeof(Foo) bytes is allocated, lets call it pointer p. 8 bytes are needed to store the number of elements in the array. We will need this number, when delete is called.
2. The number of objects in the array is saved to p[0]=3.
3. Placement new-operator Foo() is called for memory-addresses p+8, p+8+sizeof(Foo) and p+8+2*sizeof(Foo), i.e. 3 objects are created.
4. ar has the address p+8 and points to the first Foo-object.

Manipulating number of objects *(reinterpret_cast<int*>(ar)-2) = 4

1. Ok, p[0] is now 4. Everybody thinks there are 4 objects in the array (but in reality only 3)

NB: If Foo would have a trivial destructor (one like for example int has), the situation would be a little bit different and accessing ar-8 would be an invalid access.

In this case the compiler optimizes out the calls of destructor, because nothing must be done. But then there is no need to remember the number of elements - so p is actually ar and there are no offset/additional 8 bytes in the beginning.

This is the reason, why for most compilers the actually wrong code:

int *array=new int[10];
delete array;//should be delete [] array;

works without problems: the memory manager doesn't need to know how much memory is behind a pointer, whether it is only one int or multiple - it just frees the memory.

Calling delete [] ar

1. Destructor is called p[0]=4 times, also for arr[0], arr[1], arr[2] and arr[3]. Calling it for arr[3] is undefined behavior, but nothing bad happens: calling the destructor doesn't not free the memory (or even touch it in your case). It only prints something - nothing wrong about it.
2. Freeing the array-memory. Actually p- pointer is freed and not ar because the memory-manager "knows" only p - we can calculate p from ar. Somewhere down the hole free(p) is called - nobody cares how many memory it holds - and the used operator delete(*void) doesn't provide it.

Nothing there, what is a problem from Valgrind's point of view.

---

To make my point clearer (see resulting assembler here):

Foo f;

would results in calling only the destructor (no memory accesses) but not freeing the memory - that is what happens in your program for objects arr[0], arr[1], arr[2] and arr[3]

call    Foo::~Foo()

but

Foo *f=new Foo();
delete f;

would result in calling the destructor and operator delete, which would delete the memory on the heap:

    call    Foo::~Foo()
    movq    %rbp, %rdi
    call    operator delete(void*) ; deletes memory, which was used for f

Yet operator delete is not called for every object in your case, because the memory was also not allocated in bits but as whole memory-chunk, i.e. p.

---

If you would call delete ar; instead of delete [] ar; you can see what happens:

1. Destructor is called only for the first Foo-object.
2. Program will try to free pointer arr instead of pointer p. Yet pointer ar is unknown to memory-manager (it only knows p) and this is problematic.

---

As VTT points out, you will see invalid memory access to memory beyond the array if destructor touches some memory in the object.

You would get errors, if your destructor have to free some memory (e.g. having a vector as member) and thus interpreting random memory-content as addresses and calling operator delete for those random addresses.