Detecting deleted memory

I think you should try to use VC's debug heap features.  These are features that allow you to store with a memory block the source code location where memory was alocoated, then when the memory block is corrupted in some way, like being deleted a 2nd time, you will be able to "identify" the memory.  It will also detect other errors, like blocks that have been allocated and not delete or attempt to write data past the end of a block.

continues.

You need to call the _CrtSetDbgFlag() procedure and set the _CRTDBG_LEAK_CHECK_DF flag.  With this flag set, you will get a report at the end of your program that lists the errors detected, like times when you wrote past the end of a memory block or memory blocks that were allocated and never deleted.

You can set the flag like

#if _DEBUG
#include <crtdbg.h>
#endif

#if _DEBUG
   int Flg = _CrtSetDbgFlag(_CRTDBG_REPORT_FLAG);
   Flg |= _CRTDBG_LEAK_CHECK_DF;
   _CrtSetDbgFlag(Flg);
#endif

This will give you reports of when errors occur, but won't necessarily help you track down the place where the error occurs.  To help with that you can use the debug allocation procedure _malloc_dbg() to allocate your memory.  This procedure allows you to specify the source code file name and line number of the code requesting the allocation.  This information is then stored with the memory allocated.  Then when an error is dectected with a block, the debug also prints this information which can be invaluable in tracking down the error.  (Just knowing where to start searching makes such a big difference!)


But as C++ programmery, you don't want to use _malloc_dbg(), you want to use "new" and "delete".  No problem.  The basic idea is to overload operator new and delete. with versions that call _malloc_dbg() and _freed_dbg.  _malloc_dbg is like malloc, but allows the file name and line number to be specified.  To make this useful you need to have overloads to operator new (also new[]) that allow the source code file name and line number of the calling code to be passed in.  Now the problem with that is that to use those overloads, the caller would have to use a weird syntax like

int *ptr = new(__FILE__,__LINE__) [100];

that syntax is rather unpleasant and can't be "removed" in a release version.  To get around that we define a macro using #define that allows the overloaded new operators to be called in debug version (passing the file and line number automatically) and allows the regular versions to be called in the release version.

void *
operator new(size_t  Siz,                        // Size of block to allocate.                    //
             char   *FilNam,                     // File name.                                    //
             int     LinNum)                     // Line number.                                  //
{
   return _malloc_dbg(Siz,_NORMAL_BLOCK,FilNam,LinNum);

}

// This overload is needed because A) I have changed
// the standard operator delete to use _free_dbg() and
// B) other code, like from the STL, might allocate memory
// with the regular new (this one) and that memory will be
// deleted with the standard delete, so I have to make this
// new with the delete that I've changed.
void *
operator new [](size_t  Siz)                     // Size of block to allocate.                    //
{
   return _malloc_dbg(Siz,_NORMAL_BLOCK,NULL,123456);
}

void *
operator new [](size_t  Siz,                     // Size of block to allocate.                    //
                char   *FilNam,                  // File name.                                    //
                int     LinNum)                  // Line number.                                  //
{
   return _malloc_dbg(Siz,_NORMAL_BLOCK,FilNam,LinNum);
}



void
operator delete(void *MemPtr,                    // -> memory to free.                            //
                char *FilNam,                    // File name.                                    //
                int   LinNum)                    // Line number.                                  //
{
   _free_dbg(MemPtr,_NORMAL_BLOCK);
}

void
operator delete [](void *MemPtr)                 // -> memory to free.                            //
{
   _free_dbg(MemPtr,_NORMAL_BLOCK);
}
void
operator delete [](void *MemPtr,                 // -> memory to free.                            //
                   char *FilNam,                 // File name.                                    //
                   int   LinNum)                 // Line number.                                  //
{
   _free_dbg(MemPtr,_NORMAL_BLOCK);
}

Final the macros are

#ifdefine _DEBUG
#define QNew    new(__FILE__,__LINE__)
#else
#define QNew    new
#endif
#define QDel    delete

Just use "QNew" everywhee in place of "new" and "QDel"  (for consistency) in place of "delete"  That can be used when allocating arrays (with the [] on the end) and when allocating initialized objects (with the (param,param) at the end.)

Now a couple of points about this.  When you use the debug heap functions to delete memory, if doesn't really delete the memory, it just sets a flag somewhere in the heap to indicate the block was deleted, but the block remains there and the bytes of the block are filled with a constant value (0x1C, I think).  So if you attemt to use the block again you will probably detect it (As its value should stand out) and if you attempt to delete the block a 2nd time the RTL will detect it and warn you of th error.

That will probably help you to track dow the error, it has done very well for me the past.  However another approach is to try to design your application so this is never even an issue.  Wherever possible try to use smart pointers and reference counted objects. Or at least try to design classes that manage the dynamically allocated memory used by your program, this will greatly reduce the number of locating in your code where a mistake can be occuring and also greatly reduces the liklhood of a mistage.  These practices programming much simpler and make programs much more reliable.  

The problem isn't that we're trying to delete it a second time.  The problem is that we delete it using one pointer.

Then we try to access it with a different pointer to the same area of memory.  The function trying to access the memory doesn't know that another function deleted that memory with a pointer that was named something else.

I really need to be able to detect whether or not the memory has been deleted.

The problem isn't that we're trying to delete it a second time.  The problem is that we delete it using one pointer.

Then we try to access it with a different pointer to the same area of memory.  The function trying to access the memory doesn't know that another function deleted that memory with a pointer that was named something else.

I really need to be able to detect whether or not the memory has been deleted.

>> I really need to be able to detect whether
>> or not the memory has been deleted.
Do you mean during debugging?   If so the debug heap can help you in this, as I sort of outlined in my last comment.  Or do you mean during run-time of a release version.  If that is what you mean, it sounds like you need to recosider your design.  For example if you have two pointers that point to 1 memory block and you don't want the block deleted until both pointers are no longer using it, then you need to use smart pointers and reference counting.  Are you familar with these concepts?

I read the additional comment.  However, I'm already including overriden debug delete operators to report memory leaks and the line of allocation.

I also know where the line of allocation occurs for this memory block.

What I don't know is how to change the other twenty pointers to NULL.  Some of the other pointers point to this area of memory at one point, but get moved to another place in memory at another point.  And I don't know how many pointers there are to the memory.

There are so many functions where it is being passed and re-passed and captured into another pointer that I'm going to lose my hair if I have to spend days to debug it to the nitty gritty details of each line of code ten times until I finally luck up and re-produce it.

I would be flogged for suggesting that we re-write the tree code to use smart pointers.  We are on a tight schedule, and re-writing this code is out of the question.

Here's my beef:

{
    CTree * pTree = m_pTree->GetBase();
    pTree->m_pOptionalData = NULL;

}

the pTree pointer's memory has been deleted (who knows where?).

So naturally, pTree->m_pOptionalData = NULL; causes a GPF.

If I could do something like:
{
    CTree * pTree = m_pTree->GetBase();
   
    if (IsMemoryAllocated(pTree))
    {
        pTree->m_pOptionalData = NULL;
    }
}

that would fix my problem.

I would be flogged for suggesting that we re-write the tree code to use smart pointers.  We are on a tight schedule, and re-writing this code is out of the question.

Here's my beef:

{
    CTree * pTree = m_pTree->GetBase();
    pTree->m_pOptionalData = NULL;

}

the pTree pointer's memory has been deleted (who knows where?).

So naturally, pTree->m_pOptionalData = NULL; causes a GPF.

If I could do something like:
{
    CTree * pTree = m_pTree->GetBase();
   
    if (IsMemoryAllocated(pTree))
    {
        pTree->m_pOptionalData = NULL;
    }
}

that would fix my problem.

Nietod:

You have been very detailed in your answers, but I still don't have an answer that can get this fixed without spending days debugging it.

I'll raise the points since you've spent so much time if you can help me on this.

There is no way (and basically can be no way) to do this--at least not in a
release version.  Why?  For it to work, the memory address specified to
IsMemoryAllocated() could never be used again.  Why?  Consider what would
happen if the memory was allocated, copies of the pointer were made, then the
memory was deleted using 1 of the pointers.  Now what if another block was
allocated at the same address (for a different purpose).  (Note this is not
unlikely, one hopes that this goes on a lot.)  Well in that case the
IsMemoryAllocated() function still returns true, but now when you access the
memory it will be being used for the wrong purpose.  So you will still crash,
so a function like IsMemoryAllocated() would do you know good, at least not
if you ever delete memory.  (This is why the VC debug heap delete procedure
doesn't actually delete the memory.  The memory must never be recycled.  That
is fine for a short-lived debugging test, but is out fo the question for most
real-world program uses.

The fact that you are asking for something like this should be waving red
flags in frong of your eyes.  This says "bad program design" in big letters.
There is no quick fix for this, the fault is inherint in the design.

Fortunately C++ has features to help you make it work.  Things like reference
counting.  I suspect that there are a limited number of distinct places in
your program where it suffers from this, right?  So it should not be too hard
to redesign to use reference counting.

>> What I don't know is how to change the other
>> twenty pointers to NULL.  Some of the other
>> pointers point to this area of memory at one
>> point, but get moved to another place in memory at
>> another point.  And I don't know how many pointers
>> there are to the memory.

>> There are so many functions where it is being passed
>> and re-passed and captured into another pointer that
>> I'm going to lose my hair if I have to spend days to
>> debug it to the nitty gritty details of each line of code
>> ten times until I finally luck up and re-produce it.
That sounds so much like the introductory paragraph for a text book chapter on reference counting.  Are you famialr with it?

If so, yes it will take some time to impliment, but probably not that long.  And as should be clear to you now, there is no way you can have anything even akin to IsMemoryAllocated(), that is a dead-end.  So you are facing a few days of debugging OR a few days of programming that will almost certainly solve the problem, make the program much simpler, make the program far more robust, and virtually elliminate any future occurance of the problem.  Decisions, decissions.

I converted my library code so that basically all objects were reference counted, probably about 300 classes in 30+ source code files in about 200,000  lines of code.  the change (which was part of a group of changes) took about 1 - 2 weeks.  Since that time, I have basically never had a memory leak or access to deleted memory.  I never ever think about new and delete or the lifetime constraints of memory.  And data can be passed around and "copied" freely with no concerns of efficiency.  It is increadible.  

I'm with you on this one nietod.

along the original lines of set the pointers to be NULL, but with a twist - use pointers to pointers...

i.e.

pData = new char[1000];

ppData1 = &pData;
ppData2 = &pData;
ppData3 = ppData1;

// Free using

free( *ppData2 );
*ppData2 = NULL;

// Access using

if( *ppData1 )
{
  // the memory hasn't been free'd
}
else
{
  // the memory has gone!
}

Prob'ly best to wrap this in a class and have reference counting etc. (so it's not free'd until all pointers have let go of it)

Great discussion, but I think it's important to answer the original question. If you're working on anything other than small non-critical software, you absolutely _must_ adopt one of the following two tools for run-time memory checking:

Rational Purify:
http://www.rational.com/products/purify_nt/index.jtmpl

Numega BoundsChecker:
http://www.numega.com/products/aed/vc.shtml

Have a look around the base URLs for other similar products, i.e. Unix or VB versions. I like Rational's purify because it is cross-platform but BoundsChecker is also very good.

Steve

p.s. Just to join the off-topic discussion, I agree that I would be investing some time in cleaning up my design here. With multiple pointers to a single space, "finding the bug" isn't going to be so trivial. You will have to do full code- and scenario-coverage as your  memory deletion is probably not going to happen the same way every time your software is run and may be "sporadically" happening depending on a combination of states and circumstances.

Well if you're going to go down that route then I'd suggest Purify if you can afford it.

Bounds Checker is cheaper and obviously so.

>> one of the following
>> two tools for run-time memory checking:
Neither tool with solve this problem.  No tool could.  There is a flaw in the program design, the program must be redesinged.  The tools could find the problem, but that isn't needed, as DonERogers pointed out, he has foudn the problem already.  He now needs to correct the design flaw that produces the problem.

Agreed. But, the original discussion pointed to the fact that he needs a short-term solution. I would still recommend grabbing Rational Purify - the nice thing is that you can evaluate it for 30 days without committing to a purchase. That should give you a good idea of what it can accomplish.

Having said that, I agree that the original poster is looking for a design strategy, not just a single "I have a GPF somewhere, please help" problem.

Steve

Thank you for all your input.  Even though it doesn't fix the problem, I'm giving you the 200 points for all your time.

You don't understand.  I work for a cheap company.  Let me re-emphasize CHEAP.  We are not willing to take two weeks to re-write any code.

We do have Bounds Checker, and it is the first to complain about the deleted memory.  Then the same line GPFs.

I think I may have found a way to detect it if it was just deleted.  The memory is marked as 0xDDDDDD.  I have verified in a test program that detecting that 0xDDDDDD can work in debug and release.

It is not flawless, but everytime (so far) that the GPF has happened, that is what the memory at the pointer is.

So I think that is the route I'm going to take to see if it resolves the problem.

Trust me.  Nobody hates kludges more than I do. But in this company they are required since we are not willing to pay extra time to get jobs done right.

>> he needs a short-term solution. I
>> would still recommend grabbing Rational Purify
That doesn't solve the problem.  That helps you locate the problem in your code so you can change the code to prevent the problem.  But that is NOT what he wants.  He has already located the problem.  What he wants to do is "cheat" and have a run-time solution that looks to see if the problem has occured and, if so, to avoid the problem.  Purify or any other debugger isn't going to help in that.  He wants his program to USE the information at run-time, not to find problems and fix them a debug time.

>> The memory is marked as 0xDDDDDD.  I have
>> verified in a test program that detecting that 0xDDDDDD
>> can work in debug and release.
That is exactly what I was referfing to before in my argument saying what you wanted was impossible.  Please read that again.  What happens when that memory is reused and therefore no longer set to oxDDDDDDD?  (And this should happen very quickly, its not like it is a rare thing, heaps are designed to reuse memory as much as possible.)

You're right.  

I think I've found another way.

This all happens in destruction of several classes.  I think at the point where the memory gets deleted at a point later in the stack, I can fix it.  I haven't looked at the code yet, but maybe when the memory gets deleted, I can check and see if the other pointer that will be used is pointing to the same area of memory and set it to NULL.  I'm not sure.

But thanks for all your help.

If this is as specific a case as that, you can probably convert it to use reference counting in an hour.

Blimey DonERogers you are a real cowboy!  Dare I ask who you work for?  I hope it's not Boeing.

Blimey DonERogers you are a real cowboy!  Dare I ask who you work for?  I hope it's not Boeing.

I can't say which company I do work for, but it's not Boeing.

I can't say which company I do work for, but it's not Boeing.

Reference counting is not so complex to implement.


It might be possible to add reference counting to your code with very few changes if you use smart pointers. All you need to change would be the pointer type.

Replace all instances of

CBlob* ptr;

with

smartPtr<CBlob> ptr;


A smartPtr is a small objet hoding one pointer on a dynamic object hiding a counter and a pointer on the real object.

You may still use *ptr or ptr->val; So this part of the code doesn't change. All you need to care for is the type of the variable holding references to objects.

Smart ptr do all the work for you. The object is only deleted when the last reference to it is removed. ptr may be a local or global variable, it will work.

The only case where it won't work is with cyclic references. So the reference counting is not the universal solution. But with classes with potential cyclic references, when you don't need an object anymore just write
NULL in the variable to brake any potential cycle. Happily cyclic references are not so common.

The advantage is that there is a limited number of changes to do to the code. The price to pay is that you add an indirection to access the class variables or methods.

The other solution is the one I suspect implemented by nietod which uses intrusive counters. Every classes inherits from a base class that holds the refeerence count and methods to update them and delete it'self when the reference reaches 0.

This will save you indirection performance penalty. But for this solution to work you need to change smartPtr  _and_ classes inheritance so classes no also inherit reference counting.

Antway this is really the way to go if one want's to avoid falling in the type of traps you currently are in.

Good Luck

It would be better to use a searchable data structure, like map<> instead of a log file.  Still it seems like a very poor design for this case especially when better ones are available.

What I did to fix it was this:  When I delete the memory the first time, I check the other pointer (which, thankfully I had access to in the function that deleted the memory) and see if it is pointing to the same memory.  If so, I set the it to null.

I just hope that one of the other twenty pointers that can point to it or somewhere else don't try to access that block of memory.  

Oh well, I'll be finding out on the GPF.

If it doesn't work, it sounds like you are dealing with a single instance, that is there is one "type of memory block in your program that has the problem.   (There might be multiple blocks, but they are all allocated in one place and deleted in one place, or at least limited numbers of places.)  That sounds like a good candidate for reference counting.  It should be a very minimimal amount of work to impliment.

Map structures would be highly library dependent.I suggesyed log because of portability and moreover every thing is visible through the file.

Dear DonERogers,
I feel that that should not be the way it's done.I feel what nietod said is correct.

So please think over it again.

re: gaurangbhatt's answer...

The same functionality is available under Visual C++ with:
_malloc_dbg( size, _NORMAL_BLOCK, __FILE__, __LINE__ );
(equivelent to malloc( size );)

This'll chuck stuff out to the debug output also...

>> Map structures would be highly library
>> dependent. .I suggesyed log because
>> of portability
That makes no sense.  An STL map is completely portable.

>> every thing is visible through the file
"Visible" in what sense?  This is not information to be used in debugging, he wants this information to be used at run-time.  That means the program needs to access the information.  What you are proposing would require him to continueally search this file, While that would work, it would be very inefficient.   using a map would be much more efficient, but still complex and inefficient compared to the alternatives.

>> The same functionality is available under
>> Visual C++ with:
>> _malloc_dbg( size, _NORMAL_BLOCK, __FILE__, __LINE__ );

Not really.  Have you read the question history?

>> The same functionality is available under Visual C++ with:
>> _malloc_dbg( size, _NORMAL_BLOCK, __FILE__, __LINE__ );

>Not really.  Have you read the question history?

Maybe I should've been more clear, I was writing with response to:
"Any allocation or deallocation is done through my_*** functions only.This can be done in a debug mode.This functions write into a file called a log file and we are able to track down any failures."

Althoguh I can't remember the behaviour for memory that's already been free()'d (nice and easy to find memory leaks tho')

I see I thought you were referring to the problem in the question, which cannot be solved with the debug heaps.

>> I can't remember the behaviour for memory
>> that's already been free()'d
Its not actually freed.  The block is marked as deleted and is filled with a particlar byte value, I think 0xDD.  Any attempt to access the memory will fidn it corrupted by this byte value (usually)  and any 2nd attempt to delete the memory will cause a debug report as the memory is marked as deleted.