Error freeing memory....

[Sheldon]

Hi everyone,

I have a fairly large program that has a problem that I cannot seem to
solve.
The program runs smoothly but when it tries to free allocated memory
before exiting I get a segmentation fault.
I have checked many times the following:

1. That I have not freed the memory before hand and call free this
way: if(array) free(array);

2. The arrays are vectors that was allocated with error checking:
if(mem == NULL)...

3. There are no conflicts like a double definition of a variable.

4. Variables are assigned in structs

But still there I get his error. Can anyone offer some advice on what
may be the problem?
The code is nearly 1000 lines long. I can post it if anyone wants to
look at it or I can send it via email.
I would greatly appreciate any help with this problem.


/S

 

[Seebs]

I have a fairly large program that has a problem that I cannot seem to
solve.
The program runs smoothly but when it tries to free allocated memory
before exiting I get a segmentation fault.
I have checked many times the following:

1. That I have not freed the memory before hand and call free this
way: if(array) free(array);

2. The arrays are vectors that was allocated with error checking:
if(mem == NULL)...

3. There are no conflicts like a double definition of a variable.

4. Variables are assigned in structs

But still there I get his error. Can anyone offer some advice on what
may be the problem?
The code is nearly 1000 lines long. I can post it if anyone wants to
look at it or I can send it via email.
I would greatly appreciate any help with this problem.

A thousand lines isn't all that much. A couple thoughts:

1. Go through removing functional code to strip it down. If you find
that, after removing something, you suddenly don't have a problem, then
that code is what did it.
2. Once you get it stripped down, it should be a lot shorter.

My guess would be that you're overrunning something -- allocating the
wrong size of space. The last time I had this, I'd allocated enough space
for a pointer, rather than enough space for the thing pointed to...

-s

 

[Eric Sosman]

Hi everyone,

I have a fairly large program that has a problem that I cannot seem to
solve.
The program runs smoothly but when it tries to free allocated memory
before exiting I get a segmentation fault.
I have checked many times the following:

1. That I have not freed the memory before hand and call free this
way: if(array) free(array);

A note in passing: This makes no difference, because
free(NULL) is legal (and does nothing).

2. The arrays are vectors that was allocated with error checking:
if(mem == NULL)...

3. There are no conflicts like a double definition of a variable.

4. Variables are assigned in structs

But still there I get his error. Can anyone offer some advice on what
may be the problem?
The code is nearly 1000 lines long. I can post it if anyone wants to
look at it or I can send it via email.
I would greatly appreciate any help with this problem.

Almost certainly, you have somehow corrupted the "private"
data with which malloc() and free() and so on keep track of what
is allocated and what isn't. Unfortunately, there is a very
large number of ways this corruption could have occurred, and
a lot of them will leave no visible traces until long after the
damage has been done (so, for example, the faulty function may
not appear in a stack trace when the crash occurs). These can
be very difficult to track down, but there are a few patterns
that occur often enough that they're worth checking for:

- Calling free() on a pointer to memory that wasn't obtained
from malloc() (include realloc() and calloc() as appropriate, here
and throughout):

char array[] = "Hello, world!";
...
free (array); /* Bad: not dynamic memory */

- Calling free() with a pointer to a block of dynamic memory,
but not to its start:

char *ptr = malloc(42);
...
++ptr;
...
free (ptr); /* Bad: not the original value */

- Calling free() twice on the same block:

char *ptr = malloc(42);
...
free (ptr); /* No problem */
...
free (ptr); /* Bad: not "your" memory any more */

- Storing outside the bounds of an allocated area, and thus
stomping on whatever is next to it. The classic example in C is:

char *ptr = malloc(strlen(string));
strcpy (ptr, string); /* Bad: no space for the '\0' */

- Using an uninitialized or otherwise invalid pointer or
array index. Several examples:

char *ptr;
...
strcpy (ptr, "Hello"); /* Bad: ptr "points to garbage" */

char *ptr;
...
free (ptr); /* Bad: ptr "points to garbage" */

char array[42];
int idx;
...
array[idx] = 'X'; /* Bad: idx "is garbage" */

If inspection doesn't reveal any of these (or similar) bugs
in your code, you'll have to resort to run-time checking. Tools
to assist with this are platform-specific: Linux has valgrind,
Solaris has libumem, things like dbmalloc and Electric Fence are
fairly widely available and you may be able to get them running
on your system, and there are commercial products like Purify.
The very existence of all these debugging tools shows that the
problem is (1) not uncommon and (2) uncommonly difficult to solve
unaided. Good luck!

 

[Eric Sosman]

Hi everyone,

I have a fairly large program that has a problem that I cannot seem to
solve.
[...]

It's one of the possibilities I suggested you look for:
calling free() on memory not obtained from malloc() and friends.
In the case at hand, you're obtaining memory from something called
smalloc(), which is not part of the Standard C library. We're
not sure exactly what it is, but there's at least one package of
that name floating around the Net -- and that package, according
to its description, allocates from static memory, not from dynamic
memory. So when you pass it to free() (or to Free() -- Lord only
knows that *that* is), you have sinned and you get punished.

Either find and use the smalloc() package's "moral equivalent"
to free() -- maybe sfree()? -- or switch to using true malloc()
throughout.

I also notice that you often fail to check whether an
allocation request succeeded or failed. Yes, I know you *said*
you checked, but in the actual code you frequently don't. This
slipshod practice may eventually bite you in the behind -- and
perhaps it already has.

Go, and sin no more.

 

[Phil Carmody]

Hi everyone,

I have a fairly large program that has a problem that I cannot seem to
solve.
The program runs smoothly but when it tries to free allocated memory
before exiting I get a segmentation fault.
I have checked many times the following:

1. That I have not freed the memory before hand and call free this
way: if(array) free(array);

I know you say you've checked many times, but if you want to
decorate your use of free(), then do
free(array); array=NULL;
instead. This might help your error show itself more quickly.
But setting array to something like 0x00000001 might make it
show itself even quicker still, as a double-free will almost
certainly make itself known immediately. This is called 'poison',
and is a useful technique for debugging.

The code is nearly 1000 lines long. I can post it if anyone wants to
look at it or I can send it via email.
I would greatly appreciate any help with this problem.

Quicker would be for you to link with something like 'electric fence',
and then run it within a debugging environment.

Phil

 

[Ian Collins]

On 03/15/10 10:54 AM, Sheldon wrote:

Did you really have to quote all that again?

smalloc is just a function that I use. Nothing from the net but an
idea I got from a friend:

The other "Free..." is something I got from this site when I wanted to
how to create 2D and 3D arrays with contiguous memory. All of these
functions I've included above - I know, it's a lot to sift through so
here they are:

void FreeCon3DF(float ***array) {
free((void *)array[0]);
free((void *)array);
}

void FreeCon3DD(double ***array) {
free((void *)array[0]);
free((void *)array);
}

void FreeCon3DSI(short int ***array) {
free((void *)array[0]);
free((void *)array);
}

These look well dodgy. The corresponding allocation functions call
smalloc multiple times, so there will be more allocations than frees.

 

[Nick Keighley]

I have a fairly large program that has a problem that I cannot seem to
solve.
The program runs smoothly but when it tries to free allocated memory
before exiting I get a segmentation fault.
I have checked many times the following:

1. That I have not freed the memory before hand and call free this
way: if(array) free(array);

this has no affect if array is not set to NULL (or someother bad
value). You are aware that free() doesn't set array to NULL?

This is erroneous code:-

if (array) free (array);
if (array) free (array);

ass array gets freed twice.

I really don't like your FreeConxxx() functions and similar. Why the
cast to void*? Why do you think freeing array[0] id going to do
anything magic? What about the rest of the array?
You could try adding some logging (this is assuming you aren't just
going to run electric fence or whatever) to you program. Log every
malloc and free call to a file. I suspect the mallocs and fgrees won't
match.

malloc 100 bytes at BFEE0010
malloc 200 bytes at BFEE0114
free at BFEE0010

I bet they won't all match up correctly.

BTW: Windows has stuff to help debug these sorts of problems. Not as
good as valgrind etc. but readily available.

 

[Nick Keighley]

And it also makes no difference because

   if(freed_pointer) free(freed_pointer);

is still undefined.

why?

 

[Sheldon]

I have a fairly large program that has a problem that I cannot seem to
solve.
The program runs smoothly but when it tries to free allocated memory
before exiting I get a segmentation fault.
I have checked many times the following:

1. That I have not freed the memory before hand and call free this
way: if(array) free(array);

this has no affect if array is not set to NULL (or someother bad
value). You are aware that free() doesn't set array to NULL?

This is erroneous code:-

   if (array) free (array);
   if (array) free (array);

ass array gets freed twice.

I really don't like your FreeConxxx() functions and similar. Why the
cast to void*? Why do you think freeing array[0] id going to do
anything magic? What about the rest of the array?
You could try adding some logging (this is assuming you aren't just
going to run electric fence or whatever) to you program. Log every
malloc and free call to a file. I suspect the mallocs and fgrees won't
match.

    malloc 100 bytes at BFEE0010
    malloc 200 bytes at BFEE0114
    free at BFEE0010

I bet they won't all match up correctly.

BTW: Windows has stuff to help debug these sorts of problems. Not as
good as valgrind etc. but readily available.

2. The arrays are vectors that was allocated with error checking:
if(mem == NULL)...

3. There are no conflicts like a double definition of a variable.

4. Variables are assigned in structs

But still there I get his error. Can anyone offer some advice on what
may be the problem?
The code is nearly 1000 lines long. I can post it if anyone wants to
look at it or I can send it via email.
I would greatly appreciate any help with this problem.

/S

Thanks guys for your help. The problem is now solved. I switched from
malloc to calloc and that did it.
I also added some of the suggested improvements but still the error
remained.
Any ideas why calloc worked and malloc didn't?

Thanks for your help and ideas/critiques

/S

 

[Ben Bacarisse]

Thanks guys for your help. The problem is now solved. I switched from
malloc to calloc and that did it.
I also added some of the suggested improvements but still the error
remained.
Any ideas why calloc worked and malloc didn't?

I'd be worried! You may have fixed it, but in my experience that is
very rare. Far more often what you will have done is to mask the
error so that there is no longer any visible sign of it. I'd switch
right back because I don't like to loose track of errors before I've
fixed them.

BTW, since you like to zero out your data, I would switch back to
calloc for the "main" part of the arrays and remove the zeroing loops
once you find the bug.

It's hard for people here to do more than inspect the code because
it is not complete and there is no data to test with. I am not
certain, but might get more help if you posted in a form that was
complete along with a test data set. I'd use a website for this since
the program is just large enough that cutting out the various files
from a linear posting is tedious.

Thanks for your help and ideas/critiques

Just a few thoughts...

Someone else has brought this up, but you certainly have a memory
leak. The FreeCon3Dxx functions don't free up the patterns of
allocation made by the Con3Dxx functions. The 2D ones look OK.

There is much duplication of code. I'd consider switching to C++ just
so you can write all you allocation and stats functions as templates.
This will save you maintaining three versions of each one. This is a
simple use of C++ and there is no need to learn the whole language to
do this.

If you want to stick with C, you could consider using macros to make
the various differently types versions of your utility functions. All
lo priority since the work is done. It's just a tidy up.

Because of the way you allocate 1, 2 and 3D allays, you could write
the various min and max function all in terms of a single helper
function (at least for each base type) that treats the array a 1D.

You will get some simplification if you believe (as you should) that
sizeof(char) is 1. If you worry that you might change the char type
to one that is wider (wchar_t) then the usual:

p = malloc(N * sizeof *p);

idiom that you use elsewhere will do the trick.

 

[Nick Keighley]

I intended for (freed_pointer) to represent a previously freed pointer.

ah, sorry I get you now. I was treating at just an arbitary name

if (zczc_floop) free(zczc_floop);

 

[Keith Thompson]

This is erroneous code:-

if (array) free (array);
if (array) free (array);

ass array gets freed twice.

[...]

In practice, yes, that will probably result in a second call to free()
on the same pointer, which invokes undefined behavior.

In principle, it's the second test ("if (array)") that invokes
undefined behavior. Once a pointer value has been passed to free(),
that pointer value becomes indeterminate, and any reference to it is
undefined.

(Which means that, given some smarts from the compiler, the value of
"array" *could* be set to NULL after the call to free() -- though
I don't think the compiler would be doing you any favors if it
performed this kind of magic.)

I'm assuming, of course, that "array" is the name of a pointer
variable. If so, it should probably have a better name.

 

[Richard Bos]

This is erroneous code:-

if (array) free (array);
if (array) free (array);

ass array gets freed twice.

[...]

In practice, yes, that will probably result in a second call to free()
on the same pointer, which invokes undefined behavior.

In principle, it's the second test ("if (array)") that invokes
undefined behavior. Once a pointer value has been passed to free(),
that pointer value becomes indeterminate, and any reference to it is
undefined.

(Which means that, given some smarts from the compiler, the value of
"array" *could* be set to NULL after the call to free() -- though
I don't think the compiler would be doing you any favors if it
performed this kind of magic.)

Not within reason, it couldn't, since the bit pattern, as seen through
an unsigned char pointer, has to remain the same. You could possibly
argue that, after free()ing, that unaltered bit pattern could now
_compare_ equal to a null pointer when used as a pointer, but that is
not the same thing, IMO, as _setting_ it to NULL. Specifically, after

int *pointer;
unsigned memory_null[sizeof pointer], memory_ptr[sizeof pointer];

pointer=NULL;
memcpy(memory_null, &pointer, sizeof pointer);
pointer=malloc(42);
if (pointer==NULL)
puts("There's bugger all down here on Earth.");
else {
memcpy(memory_ptr, &pointer, sizeof pointer);
free(pointer);
if (memcmp(memory_null, &pointer, sizeof pointer)==0)
puts("Run away! Run away!");
if (memcmp(memory_ptr, &pointer, sizeof pointer)!=0)
puts("I soiled my armour...");
}

your implementation must _not_ have quoted from Monty Python and the
Holy Grail (though it may have quoted from The Meaning of Life).

Richard

 

[Keith Thompson]

This is erroneous code:-

if (array) free (array);
if (array) free (array);

ass array gets freed twice.

[...]

In practice, yes, that will probably result in a second call to free()
on the same pointer, which invokes undefined behavior.

In principle, it's the second test ("if (array)") that invokes
undefined behavior. Once a pointer value has been passed to free(),
that pointer value becomes indeterminate, and any reference to it is
undefined.

(Which means that, given some smarts from the compiler, the value of
"array" *could* be set to NULL after the call to free() -- though
I don't think the compiler would be doing you any favors if it
performed this kind of magic.)

Not within reason, it couldn't, since the bit pattern, as seen through
an unsigned char pointer, has to remain the same.

[...]

Yes, good point, I should have remembered that.

Ok, if the compiler can prove that you don't happen to examine the
pointer's representation as an array of characters, then it can do
anything it likes with it. For that matter, if it can prove that you
do examine it as a pointer, it can do anything it likes with it, since
the program's behavior is undefined.

For example, this program:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(void)
{
int *p = malloc(sizeof *p);
int *null_pointer = NULL;
if (p == NULL) {
exit(EXIT_FAILURE);
}
free(p);
if (p == NULL) {
puts("Hmm, that's odd");
}
if (memcmp(&p, &null_pointer, sizeof p) == 0) {
puts("Hmm, that's really odd");
}
return 0;
}

can legally print one or both of "Hmm, that's odd" and "Hmm, that's
really odd". Of course, it can also legally print "Whiskey Tango
Foxtrot".

But I think I've gone rather far afield from the actual point, which
is that once you attempt to access the value of a free()d pointer,
all bets are off. In practice, non-DS9K compilers aren't likely to
go out of their way to make such a program behave in an unexpected
manner, but optimization can still do some very surprising things.

 

[David Thompson]

A thousand lines isn't all that much. A couple thoughts:

1. Go through removing functional code to strip it down. If you find
that, after removing something, you suddenly don't have a problem, then
that code is what did it.

Warning: If it's a memory clobber bug, removing some (good) code that
doesn't contain the bug may merely hide the symptom(s) because the
clobber moves to a different place that doesn't (currently) matter, or
disguise them because the clobber only causes slightly wrong output
rather than a crash. For example if it now damages your PRNG and
(only) causes the output to be less random than it should, you have
practically no chance of noticing that by looking at output. This can
mislead you into looking at the good code for a bug that isn't there.
You may need to test a whole lattice of cuts to find, with any
confidence, which ones actually remove the problem.

2. Once you get it stripped down, it should be a lot shorter.

That's true. If you are able to find a small(er) version that DOES
still have the symptom, you have a smaller haystack to deal with.

My guess would be that you're overrunning something -- allocating the
wrong size of space. The last time I had this, I'd allocated enough space
for a pointer, rather than enough space for the thing pointed to...

As others have said, the easiest way to look for this is one of the
several tools that actually checks for overruns. I don't know which
(or if any) also catch wild or stale pointers, though.