Why does my program crash when I increment a pointer and then delete it?

Question

I was solving some programming exercise when I realised that I have a big misunderstanding about pointers. Please could someone explain the reason that this code causes a crash in C++.

#include <iostream>

int main()
{
    int* someInts = new int[5];

    someInts[0] = 1; 
    someInts[1] = 1;

    std::cout << *someInts;
    someInts++; //This line causes program to crash 

    delete[] someInts;
    return 0;
}

P.S I am aware that there is no reason to use "new" here, I am just making the example as small as possible.

Gobetween

Can you please explain why do you think that this code should work and what do you expect it to produce? Maybe you think that it's possible to delete only part of the allocated memory? Or maybe that calling the delete operator will work with any pointer that is pointing inside the allocated object (no matter where exactly)? Maybe something else, elaborate please. – Kaseykasha

Everyone so far is explaining correct, but sortof opaque memory semantics. Unfortunate, since this is clearly a beginner question, and the OP almost certainly meant (*someInts)++;. – Coplin

Why wouldn't it crash? – Heppman

@imallett: You might have a point there. Suggest you answer? – Vadnais

@imallett I think you may be right. I certainly did not interpret the question in that way however! Some explanation by the OP of what the program is attempting to do would be nice... – Irkutsk

@philipxy Actually, their code is a proper MCVE and demonstrates the exact issue, and the line with the comment is the one that's directly responsible for the crash. (But, as you pointed out, it's not the line where it crashes.) delete[] expects a pointer that was obtained from a corresponding array new. Incrementing a pointer obtained from array new, then passing the resulting pointer to delete[], is invalid and causes UB. As they correctly pointed out, someInts++; is the problem line. – Crescint

Although, if we want to be pedantic, it's not a minimal example. That would be int main() { int* ptr = new int[5]; delete ++ptr; } Same crash for the same reason, the only difference is that it doesn't output 1 first. – Crescint

@JustinTime An MCVE involves a lot more than just code. Maybe the delete is the problem and maybe it is an artifact of their example. They specifically say 'there is no reason to use "new" here', and maybe they mean their problem occurs with a new/delete that has a reason to be used, but maybe they mean they are using new/delete only to make the example small, with the problem being something else. Had they given a proper MCVE we might have a better idea. – Dissolvent

Sorry for not being totally clear. I thought that because someInts was pointing to the first element of the array then I could increment someInts to point to the next element of the array and then dereference it to cout the next element. I overestimated the magic properties of delete to keep track of this, but it is obvious when pointed out by someone else! Also to clarify my original code was an array of structures the size of which was decided by the user at runtime hence why I was using new. Since I am a beginner all of the answers and comments helped me but I am only able to choose one. – Gobetween

You are basically trying to delete memory that has never been allocated. This is undefined behavior; you will often wind up corrupting the program's memory management data structures in doing this and crash the program as a result. In some cases, this can lead to a security vulnerability. – Amabelle

@Dissolvent Assuming the example is complete, then the problem is specifically passing delete [] a pointer that wasn't obtained by new []; see [expr.delete/2] in the standard, where it mentions delete array. So, no, there really isn't any 'maybe' at all. Furthermore, if you assume that any given user's example code doesn't actually reflect their actual code, then you logically can't accept anything as an MCVE, because you can't guarantee that they just made it up. No offense. – Crescint

[Also, it says there's no reason to use new "here" because the example given doesn't actually need dynamic memory allocation. This indicates that new was used specifically to demonstrate the problem, not that the MCVE shouldn't contain new.] – Crescint

@JustinTime The question is poorly written. It currently has 32 upvotes and 14 downvotes, and my comments are essentially an alternate expression of the first comment re lack of clarity, with 26 upvotes. I don't find that your comments reflect a close reading of my comments, the question or the OP's comment. – Dissolvent

@Dissolvent Yes, it could be better written, but that doesn't change the accuracy of the code itself. Furthermore, the first comment also mentions that the issue is specifically passing a pointer that wasn't obtained by new[] to delete[], albeit in a roundabout way. To quote: "Maybe you think that it's possible to delete only part of the allocated memory? Or maybe that calling the delete operator will work with any pointer that is pointing inside the allocated object (no matter where exactly)?" – Crescint

The code itself is very clear about what's being done, and why it doesn't work; it's the wording that could stand to be improved. And considering that you claimed that it wasn't an MCVE (despite being minimal, complete, and verifiable; the last part could stand to be improved, as it doesn't show the error message caused by the crash, but it's fully reproducible with the given code), I took objection on the grounds that it's: minimal, complete, and fully verifiable, even if not worded as well as it could be. – Crescint

Also, the OP's comment says, and I quote, "I thought that because someInts was pointing to the first element of the array then I could increment someInts to point to the next element of the array and then dereference it to cout the next element. I overestimated the magic properties of delete to keep track of this, but it is obvious when pointed out by someone else!" From reading the post itself, looking at the code, or reading the OP's comment, there's no reason to think it's anything other than passing the wrong pointer to delete[]. – Crescint

Also note that the accepted answer states that the crash is a result of passing a pointer that wasn't obtained by new[] to delete[], indicating that it was indeed the exact issue with the code in question. Perhaps I'm not the one that didn't read closely? – Crescint

Answer 1

It's actually the statement after the one you mark as causing the program to crash that causes the program to crash!

You must pass the same pointer to delete[] as you get back from new[].

Otherwise the behaviour of the program is undefined.

Vadnais

Previous comments alluded to the C standard, but they have been removed. – Vadnais

Yes, my reply was to a removed comment that basically said 'C works, why doesn't C++?' I can remove these comments now if you wish. – Rye

In C++, not only it has to be the same address but it must be the same pointer type (or a base class with a virtual destructor). – Chally

@Phil1970: For array delete (delete[]) it has to be the exact same pointer type, no polymorphism allowed. – Sudor

For reference purposes, most compilers store some data just in front of your data that was allocated with new, and use that data in delete to manage the memory properly. When you incremented the pointer and then deleteed that, the compiler started looking in the wrong place for its bookkeeping data. It used whatever data it found there, which lead to your segfault. – Cupro

The "delete[]" statement is where the program crashes, but the "someInts++" statement is the statement that causes the crash. – Yellowbird

Answer 2

The problem is that with the someInts++; you are passing the address of the second element of an array to your delete[] statement. You need to pass the address of the first (original) element:

int* someInts = new int[5];
int* originalInts = someInts; // points to the first element
someInts[0] = 1;
someInts[1] = 1;

std::cout << *someInts;
someInts++; // points at the second element now

delete[] originalInts;

Chanukah

Answer 3

Without going into the specifics of a specific implementation here, the intuitive reason behind the crash can be explained simply by considering what delete[] is supposed to do:

Destroys an array created by a new[]-expression

You give delete[] a pointer to an array. Among other things, it has to free the memory it allocated to hold the contents of that array after.

How is the allocator know what to free? It uses the pointer you gave it as a key to look up the data structure which contains the bookkeeping information for the allocated block. Somewhere, there is a structure which stores the mapping between pointers to previously allocated blocks and the associated bookkeeping operation.

You may wish this lookup to result in some kind of friendly error message if the pointer you pass to delete [] was not one returned by a corresponding new[], but there is nothing in the standard that guarantees that.

So, it is possible, given a pointer which had not been previously allocated by new[], delete[] ends up looking at something that really is not a consistent bookkeeping structure. Wires get crossed. A crash ensues.

Or, you might wish that delete[] would say "hey, it looks like this pointer points to somewhere inside a region I allocated before. Let me go back and find the pointer I returned when I allocated that region and use that to look up the bookkeeping information" but, again, there is no such requirement in the standard:

For the second (array) form, expression must be a null pointer value or a pointer value previously obtained by an array form of new-expression. If expression is anything else, including if it's a pointer obtained by the non-array form of new-expression, the behavior is undefined. [emphasis mine]

In this case, you are lucky because you found out you did something wrong instantaneously.

PS: This is a hand-wavy explanation

Mirk

Nothing wrong with a hand-wavy explanation sometimes. The other answers are concise, but I finally got why it happens from reading this one. (and @plugwash's :) – Rubenrubens

Normally the information about allocations is not stored in a seperate data structure but instead stored immediately prior to the allocated memory. – Endoscope

It matters a lot in terms of what happens when things go wrong. An actual lookup can return a "not found" error. A simple subtraction will just try to access whatever happens to be in the memory below the user's pointer. – Endoscope

You may wish this lookup to result in some kind of friendly error message if the pointer you pass to delete [] was not one returned by a corresponding new[], but there is nothing in the standard that guarantees that. – Dunant

Answer 4

You can increment a pointer within the block and use that incremented pointer to access different parts of the block, that is fine.

However you must pass Delete the pointer you got from New. Not an incremented version of it, not a pointer that was allocated by some other means.

Why? well the cop-out answer is because that is what the standard says.

The practical answer is because to free a block of memory the memory manager needs information about the block. For example where it starts and ends, and whether adjacent chunks are free (normally a memory manager will combine adjacent free chunks) and what arena it belongs to (important for locking in multithreaded memory managers).

This information is typically stored immediately prior to the allocated memory. The Memory manager will subtract a fixed value from your pointer and look for a structure of allocation metadata at that location.

If you pass a pointer that does not point to the start of a block of allocated memory then the memory manager tries to perform the subtraction and read it's control block but what it ends up reading is not a valid control block.

If you are lucky then the code crashes quickly, if you are unlucky then you can end up with subtle memory corruption.

Endoscope