Access Errors When Freeing Memory Anybody ever see this?

[mitchellmckain 0]

I am getting access violations when I free up (using free) memory blocks (integer arrays) that have been allocated using malloc. This has only recently started happening in the project I have been working on for years so it is not because I do not know how to use free or malloc. I am wondering if this can occur due to a previous write to the block using an array index which is out of bounds, even though the write itself does not generate an error.

[Khymnon 0]

It might help a lot if you could provide a snippet of the relevant code. After all, with C, a thing as trivial as not properly ending an iteration can cause violations such as this one.(I assumed you were using C, since you didn't use NEW and DELETE).

[grib 0]

Almost always this is a double free(). There are often some heap related structures that can cause problems if they are courrupted, but usually these are just before the pointer that malloc() returns.

[mitchellmckain 0]

A code snippet is not going to help here since there is just a bunch of free statements called when the program is getting ready to shut down, but I must assume the problem occurs somewhere in the 16,000 lines of code where the memory blocks are used. If I could isolate the problem to snippet I could figure out the problem myself. I have put the free statements in try catch statements to manage the problem but sometimes even this fails (pretty strange huh?).I plan to test my hypothesis that out of bounds references are causing the problem by suppressing the usage of the offending memory block and seeing that also eliminates the access violation. Also thanks to gribs suggestion I will check for array overruns in the preceeding memory allocation. I can even test the idea with either usage suppression or by rearranging the memory allocations.Thanks for the suggestions.

[grib 0]

new will throw if it has a problem, but malloc and free don't throw to my knowlage. The std::vector .at() method will throw on a boundry violation. Without using vectors there is not much the language can do for you. Note that you can overwrite heap structures at any time. It's probably not going to be just before the next malloc. malloc(16)-1 might be a pointer that the heap uses as part of a linked list to track blocks when they are freed, but this can change often.

The thing is it's not just a matter of getting it to work, undefined behavior often works, you really need to go through and track down all your bounds violations. Wrapping your pointers in non-copyable(make the copy ctor private) classes is usually the cheapest solution. the boost libraries shared_ptr will track down most of the rest. Boost also provides a standard non_copyable class that tends to be cleaner that rolling your own.

[mitchellmckain 0]

Well I did narrow it down, at least aparently.

 

The access violation occurs in free(isg)

void exit_program( int err ){#define tfree(a) {try{free(a);}catch(...){continue;}}    tfree(texture_id);    if(mxmp)tfree(moonp);    if(mxpp)tfree(planp);    tfree(sgi);    tfree(lvmsg);    tfree(isg);//followed by a lot more of these

But the offending code is probably in the following snippet. lsg is initialized to -1, and sgi is an indexer, bubble sorted by partition. The indices of the partitions are in isg (shifted by isg[0] to save memory in the allocation of sgi). lvmsg and isg are allocated as nsg+1 intergers.

   if(stargroups)for(i=0;i<nsg;i++)lvmsg[i]=isg[i+1]-1;  if(stargroups&&ii<mxo){    if(ii>=isg[lsg+1])lsg++;    //if in stargroup get object in luminosity order    if(lsg>=0)i=sgi[ii-isg[0]];    else i=ii;    //if in stargoup then not visible if last was not visible    if(lsg>-1)if(ii>lvmsg[lsg]){vis[i]=false;fard[i]=false;continue;}}  else {i=ii;if(ii==mxo)lf=lumf;}

Setting stargroups to false eliminates the access violation but i=ii; after this section which eliminates all significant effectiveness of the code in the program does not eliminate the access error. The behavior of the access violation has been fairly predictable in this manner remaining unchanged after many changes in the code. So I have a good chance at least that the bug is right there in front of me and easy to fix.

 

In fact while I was writing this post I narrowed it down to

   if(stargroups)for(i=0;i<nsg;i++)lvmsg[i]=isg[i+1]-1;

This makes a kind of sense since it is almost the only place in this portion of code where an assignment to these allocated blocks are made. lvmsg is allocated just after isg is allocated and freed just before isg is.

 

The thing is it's not just a matter of getting it to work, undefined behavior often works, you really need to go through and track down all your bounds violations.  Wrapping your pointers in non-copyable(make the copy ctor private) classes is usually the cheapest solution.  the boost libraries shared_ptr will track down most of the rest.  Boost also provides a standard non_copyable class that tends to be cleaner that rolling your own.

1064335470[/snapback]

You are quite right of course and I may be chasing after a ghost. My code will always be part C rather than fully C++ but using more classes would help. This will be a longer term goal for me I think. I have been doing a lot of house cleaning recently and this may be a good time to try the stuff at boost. However speed is a critical issue in this program. In fact the code above was itself an important speed improvement.

[mitchellmckain 0]

Found it. Later on I was assigning something to lvmsg[lsg] while lsg was still -1.

[mitchellmckain 0]

In conclusion, I think we can say that writing outside the bounds of an array in C can cause just about anything to happen. Since this can be devilishly hard to track back to its cause, then, if at all possible, it is best to prevent this from ever happening using something like grib's idea of non-copyable classes.