delete operator triggering user breakpoint?

chrispauljarram
chrispauljarram used Ask the Experts™
on
Hi,

I've read other people have been had similar problems but can't seem to find a clear solution.  I have an MFC app and suddely seem to be having problems using the delete operator.  Wherever I use it it is causing a user breakpoint to be triggered in free.c at this line:

#endif  /* CRTDLL */
        else    //  __active_heap == __SYSTEM_HEAP
#endif  /* _WIN64 */
        {
            retval = HeapFree(_crtheap, 0, pBlock);
            if (retval == 0)
            {
                errno = _get_errno_from_oserr(GetLastError());
            }
        }
}


I can literally just do this in a function and it causes this to be thrown, I'm not sure what has changed in this project to make this happen :-/...

      char* test = new char[20];
      delete test; // triggers breakpoint.

Can someone please help?  This is strange to me :(

Many thanks in advance,
Chris
Comment
Watch Question

Do more with

Expert Office
EXPERT OFFICE® is a registered trademark of EXPERTS EXCHANGE®
evilrixSenior Software Engineer (Avast)

Commented:
It's going to be because you are running debug build and the C runtime debug heap deallocator has identified heap corruption (you are trying to delete a pointer that is pointing to heap that it thinks should not be freed). Most likely, either the point is not pointing to a valid heap allocated object, the heap has already been deleted or some other code has corrupted the heap.
evilrixSenior Software Engineer (Avast)

Commented:
   char* test = new char[20];
      delete test; // triggers breakpoint.

Is incorrect anyway!

it should be

    char* test = new char[20];
      delete [] test; // triggers breakpoint.

// NOTE the [] are required when deleting a heap allocated array
Introduction to Web Design

Develop a strong foundation and understanding of web design by learning HTML, CSS, and additional tools to help you develop your own website.

AndyAinscowFreelance programmer / Consultant

Commented:
I agree with evilrix - you are missing the brackets.  
to clean up should be
delete [] test;


Is this EXACTLY what gives you problems in your real app ?)

Author

Commented:
OK so that was a bad example (in fact I'm not actually using arrays like that anywhere in my code).  If I call delete on ANYTHING that is a pointer created with new it triggers the breakpoint.  I think evilrix was right in the debug build thing, going to give it a try now... thanks guys will post back shortly.
evilrixSenior Software Engineer (Avast)

Commented:
>> I think evilrix was right in the debug build thing, going to give it a try now... thanks guys will post back shortly.
You need to realize though, this DOES mean you have a problem -- you can't just ignore it because it doesn't happen in release. I'm sure you realise that but I feel I should point it out just in case you don't :)

Somewhere you are doing something improper with the heap, problem is it could be anywhere... you need to check EVERYWHERE you do anything with heap... these types of defects are hard to fix because they are often transient. If you have access to a good memory profiling tool that is the best way to tackle this. If it's a large code base trying to track this down by eye could prove quite hard.
AndyAinscowFreelance programmer / Consultant

Commented:
Are you certain about the anything in your comment ?

This sort of problem is often a simple failure in code when it is specific objects.

eg.

P* p = new P();
...
p = NULL;  //or some other value p = (P*)1234;
...
delete p;  //   Oops - can't delete a pointer to null (or invalid adress)


also
void foo(P* p)
{
...
delete p;
}

elsewhere
P* p = new P();
...
foo(p);
delete p;   // Oops - p already deleted in call to foo
evilrixSenior Software Engineer (Avast)

Commented:
>> Are you certain about the anything in your comment ?

Once the heap is corrupted this is probably the behavior you'd see... and a right royal pain to track down :S
AndyAinscowFreelance programmer / Consultant

Commented:
>>and a right royal pain to track down
I concur ;-)


However a questioner sometimes does say things like everything when they really mean instances of only this object - that is why I would like the questioner to really confirm the anything in the comment
evilrixSenior Software Engineer (Avast)

Commented:
:)

Commented:
I think memory overrun may also cause this assert. So sprintf(), whatever working with arrays, buffers, etc., should be checked. Better with an error detecting tool.
evilrixSenior Software Engineer (Avast)

Commented:
>> I think memory overrun may also cause this assert. So sprintf(), whatever working with arrays, buffers, etc., should be checked. Better with an error detecting tool.
it is unlikely to be a factor with C style arrays created on the stack, which this is a completely different memory area. It could; however, be caused by overrunning a heap allocated buffer... like Andy and I already said, this is almost certainly  due to some for of heap corruption.
evilrixSenior Software Engineer (Avast)

Commented:
...oh and I guess we should also not rule out the possibility of an under-run.

BTW: If it was a C style array overrun the debugger would actually give a completely different error since Visual Studio >= 2005  instruments debug builds (unless the option is disabled) to provide bounds checking.

Author

Commented:
AndyAinscow:

Yes I understand those fundamental principals :)  When I say anything, I mean anything - not only this object or instances of this class.

Using the release variant prevents the problem, but I still have a number of issues with char*'s which are a little strange and can be worked around with CStrings (which arent causing the same problems).

Will spend a little longer here and get back to you.

Chris
AndyAinscowFreelance programmer / Consultant

Commented:
>>Using the release variant prevents the problem

NO, it just hides it until it comes back to bite you (as evilrix has already said).
evilrixSenior Software Engineer (Avast)

Commented:
>> Using the release variant prevents the problem

No it doesn't it just does this....

----------------------------------------------
if we're in a debug build
   tell the user something bad is about to happen
endif

do something bad
----------------------------------------------

In a release build the bad thing still happens, you don't don't see it (immediately).

>> but I still have a number of issues with char*'s which are a little strange and can be worked around with CStrings (which arent causing the same problems).

CStrings are MFC... are you using MFC? If not (or even if!) consider using the standard C++ string type std::string.
http://www.cplusplus.com/reference/string/string/
AndyAinscowFreelance programmer / Consultant

Commented:
>>but I still have a number of issues with char*'s

Could that be the cause of your problem, or even just another symptom ?
(A CString is a sort of wrapper for a character buffer, albeit with quite a lot of functionality added to stop you reinventing the wheel).
trinitrotolueneDirector - Software Engineering

Commented:
chrispauljarram:

You could try using Rational Purify to help you identify the issue....a trial version should just do fine.

http://www14.software.ibm.com/webapp/download/search.jsp?q=purify&x=48&y=9

Be sure to adhere to IBM's terms and conditions for usage of the trial version

Author

Commented:
Hi,

Thanks will take a look at RationalPurify.  
I tried switching to CStrings and it didnt really solve the issue - however now I have a new one which has only just started happening despite NO changes to this section of code (I am usually a java programmer although C++ taught to start with - I forgot just what a bunch of crap C++ and MFC is!).

It is in my block which uses the save dialog:

            OPENFILENAME    ofn;
            TCHAR szFileName[MAX_PATH];
            // These should be destructed automatically on function exit.
            TCHAR szFileTitle[MAX_PATH];
            // display save dialog
            memset(&ofn, 0, sizeof(ofn)); // initialize structure to 0/NULL
            szFileName[0] = '\0';
            szFileTitle[0] = '\0';

            ofn.lStructSize = sizeof(ofn);
            ofn.lpstrFile = szFileName;
            ofn.nMaxFile = MAX_PATH;
            ofn.lpstrDefExt = NULL;
            ofn.Flags =  OFN_EXPLORER | OFN_OVERWRITEPROMPT;
            ofn.lpstrFilter = "User Files (*.usr)\0*.usr;*.CXX\0All Files (*.*)\0*.*\0";
            ofn.nFilterIndex = 0;
            ofn.hwndOwner = m_hWnd;
            ofn.lpstrDefExt = ".usr";
            ofn.lpstrInitialDir = ".";

            if (GetSaveFileName(&ofn))
            {
                  strncpy(szFileTitle, szFileName + ofn.nFileOffset, ofn.nFileExtension - (ofn.nFileOffset + 1));
                  szFileTitle[ofn.nFileExtension - (ofn.nFileOffset + 1)] = '\0';
                  //GetFileTitle((LPCSTR)szFileName, (LPTSTR) szFileTitle, sizeof(szFileName));
                  CUser* usr = m_pUserSettingsView->getCurrentUser();
                  usr->setFilename(szFileName);
                  usr->setName(szFileTitle);
                  usr->write(szFileName);
                  m_pUserSettingsView->setUsers(m_pUsers);
                  m_pUserSettingsView->RedrawWindow();

                  if(closingUser)
                  {
                        OnCloseUser();
                        closingUser = false;
                  }                  
            }


Sometimes this line:

if (GetSaveFileName(&ofn))

causes a breakpoint to be triggered after the filename has been filled in and the ok button pressed (the next line is not reached).  This is sporadic, I can use the same filename once and it is fine then try it again and it causes this crash. I cannot for the life of me see the reason for this and am hoping someone can restore my faith in this farcical language by giving me some clear answers!  I just cannot understand why these breakpoints are being triggered with seemingly no problems in the code (some of it is even copied from M$'s examples on their site without changes!).

Can anyone help, please?

Thanks,
Chris J
evilrixSenior Software Engineer (Avast)

Commented:
>>  I just cannot understand why these breakpoints are being triggered
I've already told you what the problem most likely is... heap corruption. And this is probably happening somewhere else in your code. Once the heap (or stack) have been corrupted all bets are off regarding sensible behavior. Transient errors like these are symptomatic of the problem I have described. You need to go back and analyze the code base (using a tool, such as rational) to try and find out where the problem starts from. I'm afraid there is no easy simple fix anyone can give you for this. This is where your hard code debugging skills will be tested fully.

Some articles on heap corruption that might hep you understand the problem better
http://windowsitpro.com/article/articleid/22275/heap-corruption-part-1.html
http://www.efnetcpp.org/wiki/Heap_Corruption

This MSDN article describes how you can use the debug CRT (C Run Time) to try and resolve heap related issues.
http://msdn.microsoft.com/en-us/library/974tc9t1.aspx

-Rx.

Commented:
Please be sure that (ofn.nFileExtension - (ofn.nFileOffset + 1)) is less than MAX_PATH. Or just make szFileName, szFileTile longer.
Please put all the code retrieving the file name in a separate function. In the worst case you can test this function seperaetely in another small and simple project. If a problem there - you will see it.
In the code you posted, you can remove everything about openfilename and replace it with the hardcoded values - in order to test second half of the posted code.
Just split your code, remove parts of it, till the memory corruption will disappear. Than add slowly - you will find, here is not a lot of source code to test.
AndyAinscowFreelance programmer / Consultant

Commented:
lpstrFilter Pointer to a buffer containing pairs of null-terminated filter strings. The  last string in the buffer must be terminated by two NULL characters.

This is a fault in your code - whether it causes your problem is another matter.

Author

Commented:
pgnatyuk, thanks - the string length was the first thing I tried.  I'll try to break down the function as you suggested but I strongly suspect the heap corruption is the issue here as this code was working 100% fine until recently (without changes) and I am getting similar unexplained behaviour elsewhere.  In terms of the actual faliure, it is happening on return from the GetSaveFileName function, the next line (strncpy) is not even hit.  The problem is sproradic, it sometimes happens and sometimes not (even when tested with the exact same values).

AndyAinscow - I'll change that but yes I don't think that is the cause of it as the breakpoint is hit after the filename has been entered and the dialog closed.

Rational Purify - thanks for the suggestion guys, I took a look at this (and installed the evalutaion version) but ended up frustrated that most of the functionality was missing, and a license is 1000ukp (about 100x more than I can afford to spend on this project at the moment ;).  Are there any other free tools or much cheaper similar ones that may help?

Thanks again,
Chris J

evilrixSenior Software Engineer (Avast)

Commented:
>> I strongly suspect the heap corruption is the issue here
Me too because a stack over-run is highly unlikely to manifest as heap related errors... liek I said above, the errors you'd get would be different.

>> In terms of the actual faliure, it is happening on return from the GetSaveFileName function
Does it do any heap manipulation, either directly or indirectly?

Commented:
Right now you said that there is a heap corruption somewhere in your app and we have no idea where. It is a wasting of time to think that there is a leak in GetSaveFileName.
evilrixSenior Software Engineer (Avast)

Commented:
>> It is a wasting of time to think that there is a leak in GetSaveFileName.
I don't think anyone said that pgnatyuk, did they?

Commented:
I'd say without BoundChecker or Purify+, you have not many ways to find and fix the problem. If the code is not too big, you can just check all memory allocations with the standard recommendations - all arrays deleted as "delete []", you use free afer malloc, you don't overrun the memory blocks.
If you don't see, devide your code - comment a half of it and test. if yout cannot reproduce the issue - the problem is in the commented part. And so on.
evilrixSenior Software Engineer (Avast)

Commented:
>> If you don't see, devide your code - comment a half of it and test. if yout cannot reproduce the issue - the problem is in the commented part

I'm sorry but this is really poor advice. Heap corruption will cause transient issues... just because they don't manifest doesn't mean you have successfully comment the cause out. The only safe way to do this is using a memory profiler.

Author

Commented:
Hi evilrix,

Yes from what I've been seeing I totally agree - the mere fact these issues are compeltely unpredictable would have been looking all day if I were to comment sections of the code out.

One thing I am doing is using new and delete, not malloc and free - surely this makes no difference?

It is really frustrating as I am generally very careful with memory allocation / deallocation. I have a computer science degree and have been a professional software engineer for 10 years, however almost exclusively Java where all I've had to worry about is nullifying references and letting the garbage collector do its thing.  I am sure to deallocate everything allocated in my C++ code and I do not do any heap allocation that could otherwise be taken care of with stack vars local to methods.  In my app there arent a great deal of objects allocated either; I'm going to try and replace my CStrings with std::strings today at least to eliminate any problems there, after that I'll look for a few more profilers and let you know what I find.

Thanks so much for your time and comments guys, you are being a big help and I'm sure I'll be able to iron this out and close this off shortly.

Cheers
Chris J

Commented:
yes.
If I can imagine that I don't have a memory profiler, I'd try to check the code with my eyes and with the comment/uncomment different parts of it.
So the solution is to use BoundChecker. :)
Can we recomment to make an own memory profiler with VirtualQuery/VirtualQueryEx/ReadProcessMemory/etc? :)
Anyway, he needs to find oput which part of code has the leak. So the right way is to keep testing.
 
 
evilrixSenior Software Engineer (Avast)

Commented:
>> One thing I am doing is using new and delete, not malloc and free - surely this makes no difference?

Well, as long as you pair them up correctly and use the right then no, it makes no difference. Since you are writing C++ though, you should use new and delete (and new [] and delete []) for other good reasons that I won't explain here unless you want me too.

Commented:
you are wasting time with CString replacing. Of course it is just my opinion.
evilrixSenior Software Engineer (Avast)

Commented:
>> Anyway, he needs to find oput which part of code has the leak. So the right way is to keep testing.
With all due respect pgnatyuk, I'm not 100% sure you have a handle on this problem. By all means contribute but please be sure you know what you are saying is right before you do. This problem is complex enough already without us throwing arbitrary suggestions into the pot.
trinitrotolueneDirector - Software Engineering

Commented:
Well if you find Purify unsuitable then there aren't many tools out there which are more helpful.

The best thing to do would be to do a line by line step through of your code and debug it. Set up a watch on the variables you suspect and inspect their values as you step through the code.

The next brute force technique would be to revisit your code and review it thoroughly to ensure that you are not leaking memory anywhere. Also check to see whether any dynamically allocated objects are not going out of scope...
evilrixSenior Software Engineer (Avast)

Commented:
>> The best thing to do would be to do a line by line step through of your code and debug it
Unless the code is just a few hundred lines or less, this is really not an option... there are so many ways this problems could happen and a lot of them will be completely innocuous. Although I don't want to throw cold water on the power of a good debug session, I have dealt with enough of these kinds of issues (not caused by me I hasten to add!) to know that the only serious way to resolve them is through the use of a good memory profiler. Unfortunately, nearly all code I work on will build on Linux so I use Valgrind for all my profiling needs. Unfortunately, it does not work on Windows.

>> review it thoroughly to ensure that you are not leaking memory anywhere
Why is everyone obsessed with memory leaks? This is NOT a memory leak problem. It is a heap corruption problem (almost certainly) and the two problems are NOT the same.

Please please... unless you understand heap corruption and can offer some real solution for the asker, there really is no point in posting your favorite memory leak tracking tip (mainly because it's not a memory leak issue!).

@chrispauljarram,

I've been doing some digging and I've found a Microsoft tool called PageHeap, which claims to aid debugging heap corruption problems. Details of how to use it and where to download it from can be found at the following link.
http://support.microsoft.com/default.aspx?scid=kb;en-us;286470

Commented:
evilrix, I ask your permission to post here an info about the Microsoft Security Development Lifecycle. :) If we already the intensive bombardment...
http://msdn.microsoft.com/en-us/security/cc448177.aspx
I have to say it is a very boring subject. But there are few tools used by the developers inside MS. Here are the downloading page:
 http://www.microsoft.com/whdc/devtools/debugging/default.mspx
AppVerifier:
http://msdn.microsoft.com/en-us/library/ms220948(VS.80).aspx
http://technet.microsoft.com/en-us/library/bb457063.aspx
http://msdn.microsoft.com/en-us/library/aa480483.aspx
 Personally I don't use these tools (I have another set), but I saw people in MS and other companies using it.
@evilrix - Of course everyone prefers own way and wants to show how smart he is - so we have this site. You accept it. So am I.

Author

Commented:
Hi evilrix:

Well this is where I am...

I installed boundschecker and after a few headaches finally got it working.  I run my app, create a new user, then go to save it using the function above and bang, it falls over.  I cannot continue (just repeats the access violation message every time I try) and boundschecker reports NO errors whatsoever.

I am not sure of the best way to approach this problem now :(  Is there any more help you can offer?

Thanks,
Chris J
Senior Software Engineer (Avast)
Commented:
>> Is there any more help you can offer?
I've never used BoundsChecker but according to the blurb on the front page this is what it does...

* Detection of memory leaks and other memory-related problems using BoundsChecker technology
* Code performance and code coverage analysis
* Ability to display code call graphs
* Ability to compare two performance profiling runs for differences
* Thread deadlock detection

I have no idea whether that covers monitoring for heap corruption scenarios. I can't advise further.

>> Is there any more help you can offer?
Well, we know that PageHeap is specifically designed for catching heap corruption (again I've not used it but that is what the MSDN says) so that would be the tool I would be using if I was you.

http://support.microsoft.com/kb/286470
http://support.microsoft.com/kb/264471

Also, since you are able to reproduce this issue with a set of very simple steps you should be able to sketch out the code path taken. That being the case you can start looking at all the code you've travelled through to get the the point of failure, with specific interest on any heap related activities. Specifically, look for pointers that you might delete twice or modify before delete and then pass to delete (so you are trying to delete the wrong memory block).

Also check for situations where you might ++ or -- the pointer and then pass it to delete (again, deleting the wrong memory block). Also, look for situations where you might perform a buffer over or under run when writing to the memory.

IIRC the normal way for a heap allocation to store metadata (although this is compiler dependent) about the allocation (such as how big the block is so when delete is call it knows what to do) is to store it before the actual pointer returned, so an underrun is probably more likely to be the cause than an overrun.

e.g. [metadata]*this is the pointer you get*[this is the block you requested]

When you call delete the compiler looks back X bytes from the pointer to see how much to free. Other ways this is done is to use an index table. Like I note above; however, this is a compiler specific thing and is not defined in the C++ standard... it's just generally how it's done (much like the vftable).

-Rx.
evilrixSenior Software Engineer (Avast)

Commented:
^^^ typed quick as I am at work -- please pardon any really bad grammar or typos :)

Author

Commented:
Hi evilrix (and all others who have helped here).

I'm awarding the points now to evilrix as he has been the most help, but thanks to all who have contributed to this thread.  I'll look into the tools and techniques recommended and you've definately all set me on the right path.

Much obliged!

Chris J

Do more with

Expert Office
Submit tech questions to Ask the Experts™ at any time to receive solutions, advice, and new ideas from leading industry professionals.

Start 7-Day Free Trial