Solved

Testing for dangling pointer

Posted on 2004-03-25
17
1,846 Views
Last Modified: 2007-12-19
Hi everybody,

This will be my first posting to this rich resource of knowledge,
thanks to all of you in advance... here we go.

It is well known, that C/C++ offers no native way to test the validity of pointers.
Trying dereference a pointer to deallocated or reallocated memory leaves you with undefined results or acces violations.

But I am wondering. What is wrong with the following code snippet to verify the validity of a possibly dangling pointer:

CMyClass *pMyClass = new CMyClass;

[... some code, that deallocates *pMyClass  ...]

try
{
       pMyCLass->CheckMyself();
}
catch(...)
{
       //** Do nothing
       return;
}

pMyCLass->DoSomething();

Where the call to CheckMyself will certainly fail.
Or even better,

try
{
     if (CMyClass::Validate(pMyClass))
         pMyCLass->DoSomething();
}
catch(...)
{
       //** Do nothing
}


Where CMyClass::Validate() is some static member function to perform consistency checking on the object and will
again fail/throw, if pMyClass is invalid.

It's certainly no elegant, but it does seem to work.


Cheers,
Sebastian
 

 
0
Comment
Question by:bastibartel
  • 5
  • 4
  • 3
  • +4
17 Comments
 
LVL 23

Expert Comment

by:chensu
Comment Utility
This can be a debugging practice. But don't use this in your product code. It hides the bug. The problem will eventually show up but in somewhere else, which makes debugging more difficult. You should fix the bug. It would be better to use assert to check the validity.
0
 
LVL 86

Expert Comment

by:jkr
Comment Utility
>>It's certainly no elegant, but it does seem to work.

Sure it will. But, why not using e.g. 'IsBadReadPtr()' or 'IsBadWritePtr()'? See also http://msdn.microsoft.com/library/default.asp?url=/library/en-us/memory/base/access_validation_functions.asp ("Access Validation Functions")
0
 
LVL 23

Expert Comment

by:chensu
Comment Utility
>why not using e.g. 'IsBadReadPtr()' or 'IsBadWritePtr()'?

That's what the MFC ASSERT_POINTER macro does.
0
 
LVL 1

Expert Comment

by:GJanusz
Comment Utility
The only aforementioned practice I've used is to assert on pointer value == NULL.  It is good programming practice to set the pointer value to NULL immediately after all instances of code that frees the memory pointed at by the pointer.  This will help in debugging, because it will tell you that the object was freed somewhere beforehand.  (You may need to initialize it at program startup, although this has never been an issue for me since VC++ does this for you in debug mode.)

Obviously, the norm should be that you are not getting to code that expects the pointer to be valid when it is not, unless you are also using the pointer in the dual function of a flag (i.e., pMyClass == NULL vs. pMyClass != NULL).  But if your code is program is large or the pointer is used all over the place, you're better off using a separate variable as a flag.  That way you will know what is going on when you inspect the value of the pointer at a break point.
0
 
LVL 5

Author Comment

by:bastibartel
Comment Utility
@chensu :
IsBadReadPtr() does not fail on deallocated space.

@chensu
It is of yourse good practice to have all invalid pointers point to NULL.
And not beeing able to track these invalid pointers seems to be sloppy programming.

However I am presenting array'd objects in a CListCtrl Object via LVN_GETDISPINFO, i.e. on the fly while the ctrl draws itself.
The drawing utilizes pointers to the actual objects in the array (stored in LVITEM.lParam). So do all the mouse messages to the CListCtrl.

(Originally, I have been using indecees into the array to do some range checking, but the code looked a bit messy...and I it seems to be common practice to assign object pointers, rather than indecees to the data-member of the LVITEM struct in CListCtrls)

Now the array grows, shrinks, is displaced by an operation asynchronous with the GUI. As soon as this operation terminates I ensure that the List Ctrl is updated. However, before this updating takes place, the user might fiddle with his mouse and raise MouseMove messages to the not yet updated CListctrl.
And I did not want to introduce the overhead of thread synchronization for the objects beeing display in the CListCtrl.

Mmmmhh, now that I'm laying it out like this... you are right, it can be merely a debugging practice, since the access to array'd elements is undefined while this array grows or shrinks.

BTW debugging was pretty much my intention. I was just wondering, if there is anything wrong with leaving this sort of code in the release to catch any unforeseen events.


Cheers,
Sebastian
0
 
LVL 5

Author Comment

by:bastibartel
Comment Utility
Sorry, my first remark goes of course to jkr primarily
0
 
LVL 5

Author Comment

by:bastibartel
Comment Utility
@ GJanusz

Keeping track of the reference count of dynamically allocated object would than be called smart pointers I suppose.
Ii never used them and I was hoping to do without.


The question is, is the sample code, i.e. raising an catching an ACCESS_VIOLATION exception proper means to verify the validity of a pointer. Will it always work and is it sort of supported by c++.


Sebastian
0
 
LVL 4

Expert Comment

by:void_main
Comment Utility
Hi!

The book "C++ in 24 hours" by Jesse Liberty says, that deallocated pointers are timebombs (if you use them after deallocation), and that they can be disarmed by setting the pointer to zero after deallocation. When using them then C++ throws an exception. If not set to zero there can happen several things:
1. Nothing (the memory was not used by another object)
2. Immeadetly crash
3. Delayed crash or strange behaviour

Its good to avoid all of theese points.

nice weekend and best regards
void_main
0
Maximize Your Threat Intelligence Reporting

Reporting is one of the most important and least talked about aspects of a world-class threat intelligence program. Here’s how to do it right.

 
LVL 22

Expert Comment

by:grg99
Comment Utility
if you have some asynchronous thread changing pointers on you, you need an interlock of some sort on the structure to prevent multiple threads from reading and writing in there.

In Windows yo can use Enter/ExitCriticalRegion around the fiddly bits.

0
 
LVL 1

Expert Comment

by:GJanusz
Comment Utility
@ Author

I don't use MFC's ASSERT() macro, because anywhere I think is worth ASSERTing in a Debug version, I think is worth ASSERTing in a Release version.  I've written my own macro ("RASSERT") that appears in my programs.  I use RASSERT simply to verify something important that I expect to always evaluate to true.  If I am aware of any situation when it might not be, I handle it in a more friendly way, such as an error dialog.

However, a rather proper solution to your situation -- with the good added information you have provided -- is to isolate the code that modifies your array/list control from your other code.  Write a function that adds to your array AND does whatever list control lParam modification is needed.  Or, write a function that inserts an item into the list control AND allocates the memory that should be pointed at by lParam.  A function that deletes a list box item AND deallocates the lParam memory.  And so on.

Have a nice day!
0
 
LVL 86

Expert Comment

by:jkr
Comment Utility
>>IsBadReadPtr() does not fail on deallocated space.

Neither will yours unless deallocated space is cleaned up...
0
 
LVL 2

Accepted Solution

by:
guntherothk earned 125 total points
Comment Utility
C++ does not change the value of p when you delete p; There is no way to tell by looking at a pointer if it points to allocated memory or free memory or a random trash location, except that a pointer value of 0 is guaranteed to be invalid by the standard.

delete p; doesn't zero p. You aren't guaranteed an access violation when you dereference p, so the try/catch scheme won't necessarily work. Furthermore, in VC 6 at least, an access violation isn't automatically translated to a C++ exception, so you'd get an unhandled structured exception. To do the translation, you need to call _set_se_translator() near the top of your program and supply a translator function. You also need to compile with /EHa to be sure C++ is prepared to catch such an asynchronously generated exception.

My personal practice is to follow every delete by an assignment to zero, as in delete p; p = 0; so that future dereferences of p are more likely to generate the hoped-for access violation.

delete p may or may not zero the pointed-to memory block. In VC 6 for instance, the debug version zeros the block but the release version does not, because it's quicker. Your Validate() function might find the class instance zero'd in the debug version, but not in the release version. Some compilers will set freed memory to a non-zero but easy-to-recognize value (0xDEADC0DE, that is "dead code" is one I remember). People have thought to mark objects as deleted by setting a variable in the destructor, but as you can see, both the standard and common implementations can prevent this from working.

Furthermore, if the freed space is reallocated to another object before you call Validate(), Validate() might return true when it shouldn't. Worst case would be if the storage was reallocated to an instance of the same type. This could make you crazy in debugging because you'd have a pointer to a deleted object that referred to a different, validly allocated object of the same type. Thus, a tool like your Validate() method cannot be safely written.

If delete happened to zero the block, You might get an access violation inside Validate() in the debug version (if it happened to dereference any pointers that had been zero'd) but not in the release version. Some O/S's might return a sufficiently big block to the system so that the virtual memory hardware subsequently reports the address as invalid, but I don't think that happens in NT.

What happens to freed memory is undefined by the standard, and varies within an individual compiler/OS, so you cannot rely on any specific behavior.

I second the recommendation to use smart pointers. If all your pointers that own data are smart pointers, you don't have to write destructors anymore because the C++ default destructor action does what you want. template auto_ptr is very useful and easy to understand. Its biggest limitation is you can't use it with STL containers. template smart_ptr is more expensive because it keeps reference counts, but works everywhere. I don't have memory problems anymore since I started using smart pointers.
0
 
LVL 1

Expert Comment

by:GJanusz
Comment Utility
Smart pointers are the right solution when program design demands them.  In those cases, you will find that one part of the program that uses the pointer doesn't know what another part is doing (perhaps intentionally for program robustness).  The program design will need to incorporate friendly handling of when a pointer is invalid when you really needed it to be valid, which is not the situation in your case.  So, I think reference counted pointers are the wrong solution in this case.  It would hide bad programming practice, and would also make it harder for you to detect, and possibly find, pointer allocation/deallocation bugs.
0
 
LVL 23

Expert Comment

by:chensu
Comment Utility
GJanusz>because anywhere I think is worth ASSERTing in a Debug version, I think is worth ASSERTing in a Release version.

Say I have a linked list of memory buffers. And the data in those memory buffers follows a certain pattern if nothing goes wrong. Because the data is so important, the application will not function properly if there is a corruption. I would create a debug-only function to check the integrity of the linked list and the data in the buffers for every single operation. I can catch bugs by testing the debug version. But if I put this check in the release version, two things would happen for the end user.

1. The performance of your application is poor if nothing goes wrong.

2. If there is a corruption in the linked list and the data, the end user may get a "friendly" error dialog. But subsequently the application behaves strangely due to the corrupted data and eventually crashes/hangs.

I don't think either of these would help the end user.
0
 
LVL 1

Expert Comment

by:GJanusz
Comment Utility
chensu> I'm not saying that my approach for ASSERT usage is right for everyone.  From what I gather from your posed scenario, I might wrap some of my code in "#ifdef _DEBUG"s.

However, I think the performance of the application is probably not going to be poor from even having many "if" statements scattered throughout the program, except when in extremely CPU intensive loops (which are going to take a relatively long time to go through regardless), or when the values that need to be evaluated in the "if" statement are themselves the CPU hogs.
0
 
LVL 5

Author Comment

by:bastibartel
Comment Utility
@ jkr

>>>>IsBadReadPtr() does not fail on deallocated space.

>> Neither will yours unless deallocated space is cleaned up...

... unless I call a member function of the deallocated object. THAT will certainly fail, won't it.

Sebastian
0
 
LVL 5

Author Comment

by:bastibartel
Comment Utility
Thanks to everybody.

It was hard to pick The answer, as there where several putting me on the right track.

What I conclude from this...
Don't do what I am trying to do.
 ...plus...
Don't rely on any defined behaviour from deallocated mem.
Use smart pointers only if you expect and can sensibly handle invalid pointers, .... or if you are too lazy to clean up after yourself. ;-)
Use thread syncs if you really need to access items from several threads.

Thanks.
0

Featured Post

Highfive + Dolby Voice = No More Audio Complaints!

Poor audio quality is one of the top reasons people don’t use video conferencing. Get the crispest, clearest audio powered by Dolby Voice in every meeting. Highfive and Dolby Voice deliver the best video conferencing and audio experience for every meeting and every room.

Join & Write a Comment

C++ Properties One feature missing from standard C++ that you will find in many other Object Oriented Programming languages is something called a Property (http://www.experts-exchange.com/Programming/Languages/CPP/A_3912-Object-Properties-in-C.ht…
Many modern programming languages support the concept of a property -- a class member that combines characteristics of both a data member and a method.  These are sometimes called "smart fields" because you can add logic that is applied automaticall…
The goal of the tutorial is to teach the user how to use functions in C++. The video will cover how to define functions, how to call functions and how to create functions prototypes. Microsoft Visual C++ 2010 Express will be used as a text editor an…
The viewer will be introduced to the member functions push_back and pop_back of the vector class. The video will teach the difference between the two as well as how to use each one along with its functionality.

763 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

9 Experts available now in Live!

Get 1:1 Help Now