How serious is the new/delete operator mismatch error?
Asked Answered
N

4

23

I have discovered the classic new/delete mismatch error in our codebase as follows:

char *foo = new char[10];

// do something

delete foo; // instead of delete[] foo;

Just how serious is this? Does it cause a memory leak or error? What are the consequences. We have some memory issues, but this doesn't seem serious enough to explain all our symptoms (heap corruption etc)

EDIT: extra questions for clarity
Does it just free the first member of the array? or
Does it make the system lose track of the array? or
Corrupt memory is some way?

Nichollenicholls answered 11/2, 2012 at 8:0 Comment(0)
R
22

It's undefined behavior serious (it could work, it could crash, it could do something else).

Randirandie answered 11/2, 2012 at 8:2 Comment(8)
Thanks for the quick answer. So really undefined? As in, no-one can tell what might happen? If that is so, it might explain our observations.Nichollenicholls
@AndrewS. Indeed undefined. I remember it "worked" in Visual Studio for example.Randirandie
Undefined - i.e. depends on the compiler/OS.Mentalism
@EdHeal: No, unspecified depends on compiker/OS. Undefined means depends on compiler/OS/state of the universe.Shelbyshelden
I happens to work with Visual Studio if there are no destructors to call. But just by chance.Smelt
@MooingDuck Sorry, but I disagree with such simplification. It is undefined from the perspective of the C++ Standard. An implementation may very well define some behavior (once the machine code is generated, it defines some behavior). Just the Standard does not care. My point is that sometimes it is useful to see what happens at the machine code level (such as silent overwriting some part of the stack). Once the developers understand these issues, they will less likely create programming bugs.Trenchant
@MooingDuck Moreover, understanding what actually might happen when behavior is undefined might be very helpful for further finding of the cause of problems. If one observes some unexpected behavior (as OP does), the knowledge that it is caused by UB does not help much by itself. Understanding some typical consequences of different UB cases might in practice considerably help to resolve bugs.Trenchant
@DanielLangr: I totally agree that understanding undefined behavior can be helpful. My comment was not a great simplification. I merely meant that "depends on the compiler/OS" is a misleading explanation for "undefined", as that's a better explanation for "unspecified". "Undefined behavior" usually doesn't depend on the compiler much, though you're right that a compiler may choose to define behaviors even there.Shelbyshelden
L
15

At the first sight, calling delete instead of delete[] should not be very bad: you destroy the first object and you provoke some memory leak.

BUT: then, delete (or delete[]) calls free to free the memory. And free needs its originally allocated address, to free the memory correctly. Or, the thing is, while new returns the original adress allocated by malloc, new[] returns a different address.

Calling free on the address returned by new[] provokes a crash (it frees memory chaotically).

See these very instructive links for better understanding:

http://blogs.msdn.com/b/oldnewthing/archive/2004/02/03/66660.aspx#66782

http://web.archive.org/web/20080703153358/http://taossa.com/index.php/2007/01/03/attacking-delete-and-delete-in-c

From these articles it is also obvious why calling delete[] instead of delete is also a very bad idea.

So, to answer: yes, it is a very very serious error. It corrupts memory (after calling the destructor of the first object only).

Levity answered 22/9, 2014 at 20:41 Comment(0)
T
3

It is very serious. With new[], implementations typically store somewhere the number of allocated array elements, since they need to know how many of them will be destructed by delete[].

Compare allocation and deallocations of a single object with new/delete and new[]/delete: https://godbolt.org/z/GYTh7f7Y7. You can clearly see that the machine code in the latter case is much more complicated. Note that new[] stores the number of elements (1) to the beginning of the allocated memory with mov QWORD PTR [rax], 1. delete[] then reads this number with mov rsi, QWORD PTR [rdi-8] to be able to iterate over elements and call their destructors.

Ordinary new does not store this number, so consequently, when you use new together with delete[], delete[] will read some unspecified number and apply destructors to unpredicted memory. This can create serious vulnerability problems.

The opposite new[] plus delete case is also very wrong. Ordinary new expression typically returns a pointer that points exactly to the memory block internally allocated by operator new (which usually calls malloc). This pointer when passed to delete expression is then internally passed as-is to the operator delete deallocation function.

But the same does not hold for new[]. Namely, new[] does not return a pointer obtained internally by operator new. Instead, it returns this pointer increased by 8 bytes (with GCC, but I think the same holds for Clang and MSVC on x86_64). See that lea r12, [rax+8] in the linked assembly. In these 8 bytes, the number of allocated array elements is stored. Consequently, if you apply delete to what you obtained with new[], delete will pass to operator delete a pointer that has not been allocated with operator new, because it will not subtract those 8 bytes from it. This will finally likely cause some like heap corruption.

Trenchant answered 20/5, 2021 at 14:49 Comment(0)
S
1

As pointed out in other answers, mismatched new [] vs delete will potentially crash.

However if the type being new'd or delete'd does not have a destructor, then at least with GCC, MSVC and Clang, with the default implementation of operator new/delete methods, there will be no consequence. This includes basic types int, char etc.

Mismatched new[] vs delete (or new vs delete[]) for complex classes will likely crash.

This is because the compiler does not insert the element count before the data if it does not need to call the destructor.

Modifying @Daniel Langr's godbolt page, note the limited difference between new/delete and new[]/delete[] if the class does not have a destructor: https://godbolt.org/z/hW4f3Ge13 (compared to when it does have a destructor: https://godbolt.org/z/GYTh7f7Y7) The only difference is the calls to operator new vs operator new[] and similar for delete. These functions by default have the same implementation.

This can be useful to know if you're trying to attribute the cause of heap corruption.

Saintebeuve answered 16/1, 2023 at 13:14 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.