How to solve problem about "segmentation fault; core dump" in GCC?

[jaswinder]

Hi, folks:

When using GCC to program, I always get the message " segmentation fault
(core dumped)" during running. How can I solve this problem? I test each
function separately, and ok!
Thanks

 

[Shao Miller]

Hi, folks:

When using GCC to program, I always get the message " segmentation fault
(core dumped)" during running. How can I solve this problem? I test each
function separately, and ok!
Thanks

There could be different reasons for your error. If you share some
code, perhaps someone can spot the error in the code... Such as
dereferencing a garbage or null pointer... Or modifying a read-only
array of 'char'... Or something else...

 

[Jens Thoms Toerring]

When using GCC to program,

I guess you mean to compile your programs?

I always get the message " segmentation fault
(core dumped)" during running. How can I solve this problem? I test each
function separately, and ok!

Well, a segmentation fault means that you try to use memory
you don't "own" (it might be writing to or reading from past
the end of an array, having stray pointers etc. just to name
the more common cases). There are countless possibilities to
do that so without seeing the actual source code it is simply
impossible to help you. The worst thing is that the actual
reason for the segmentation fault could be a bug thousands
of lines of code away from the place where the segmentation
fault finally happens, so just posting the functions it's
happening in often doesn't help too much (or the place it
happens might be a completely innocent function like printf).

One thing you could do to get an idea what's actually going
wrong if a code inspection doesn't come up with any clues is
to use a memory debugger like e.g. vallgrind.

Regards, Jens

 

[jaswinder]

There could be different reasons for your error. If you share some
code, perhaps someone can spot the error in the code... Such as
dereferencing a garbage or null pointer... Or modifying a read-only
array of 'char'... Or something else...

Ok here is a typical function with memory and pointers, gettbl is used to
build a Look Up Table used later...

double *mkary(size_t siz)
{
int bytes=(size_t)siz*sizeof(double); // nb double is size 4
double *ary=(double *)malloc(bytes+1);
for(bytes=0;bytes<siz;++bytes) *(ary+bytes)=gettbl(bytes);
return (double*) ary;
}
double *freeary(double *ary)
{
void *vp=(void*)ary;
if(vp!=(void*)NULL) free(vp);
return (double*)vp;
}

 

[Ben Bacarisse]

When using GCC to program, I always get the message " segmentation fault
(core dumped)" during running. How can I solve this problem? I test each
function separately, and ok!

There are some very good tools for tracking down this sort of problem[1]
but they are often very specific to the system you are using. That
makes the details off-topic here. The people who will know the best
tools to use will hang out in a group that discusses programming on your
system.

[1] My favourite is valgrind if that is available to you.

 

[Shao Miller]

Ok here is a typical function with memory and pointers, gettbl is used to
build a Look Up Table used later...

double *mkary(size_t siz)
{
int bytes=(size_t)siz*sizeof(double); // nb double is size 4
double *ary=(double *)malloc(bytes+1);
for(bytes=0;bytes<siz;++bytes) *(ary+bytes)=gettbl(bytes);
return (double*) ary;
}
double *freeary(double *ary)
{
void *vp=(void*)ary;
if(vp!=(void*)NULL) free(vp);
return (double*)vp;
}

double *mkary(size_t siz)
{
/**
* Changed 'bytes' to 'size_t' type.
* Removed redundant cast on 'siz'.
* It looks like 'bytes' is the raw size.
*/
size_t bytes = siz * sizeof (double); // nb double is size 4
/**
* Remove redundant cast from 'malloc'.
* You might want to check for a null pointer returned.
*/
double *ary = malloc(bytes + 1);
for (bytes=0; bytes < siz; ++bytes)
/* Array notation is also possible here. */
*(ary + bytes) = gettbl(bytes);
/* Removed redundant cast on 'ary'. */
return ary;
}

/**
* The next function appears to wrap 'free'
* and return a freed pointer. Why?
*/
double *freeary(double *ary)
{
/**
* Removed intermediary 'void *'.
* Simplified redundant test for null pointer.
*/
free(ary);
/**
* Removed redundant cast on 'vp'.
* Why return this pointer? Who will use it? Why?
*/
return ary;
}

 

[Ian Collins]

Ok here is a typical function with memory and pointers, gettbl is used to
build a Look Up Table used later...

double *mkary(size_t siz)
{
int bytes=(size_t)siz*sizeof(double); // nb double is size 4
double *ary=(double *)malloc(bytes+1);
for(bytes=0;bytes<siz;++bytes) *(ary+bytes)=gettbl(bytes);
return (double*) ary;
}
double *freeary(double *ary)
{
void *vp=(void*)ary;
if(vp!=(void*)NULL) free(vp);
return (double*)vp;
}

Why is you code so full of superfluous casts? Why does freeary return a
freed pointer? Any attempt to use that retuned pointer will end in tears.

 

[Ben Bacarisse]

Shao Miller wrote:

Ok here is a typical function with memory and pointers, gettbl is used to
build a Look Up Table used later...

The chances that this snippet has the bug is, of course, low but I note
that you use 6 casts in 7 lines and that is a bad sign. casts often
mask serious errors so it is wise to use them only when they are
needed. None of yours are needed.

double *mkary(size_t siz)
{
int bytes=(size_t)siz*sizeof(double); // nb double is size 4
double *ary=(double *)malloc(bytes+1);
for(bytes=0;bytes<siz;++bytes) *(ary+bytes)=gettbl(bytes);
return (double*) ary;
}
double *freeary(double *ary)
{
void *vp=(void*)ary;
if(vp!=(void*)NULL) free(vp);
return (double*)vp;
}

Rather than go though line by line, here's what I'd rewrite the above:

double *mkary(size_t siz)
{
size_t bytes = siz * sizeof(double);
double *ary = malloc(bytes + 1);
for (bytes = 0; bytes < siz; ++bytes)
ary[bytes] = gettbl(bytes);
return ary;
}

double *freeary(double *ary)
{
if (ary != NULL) free(ary);
return ary;
}

Note that it is safe to free a pointer that is NULL, but I've kept the
test since you may be relying on setting some pointers to NULL to
prevent double freeing.

And, please, white space is cheap but priceless.

 

[Ian Collins]

Ok here is a typical function with memory and pointers, gettbl is used to
build a Look Up Table used later...

double *mkary(size_t siz)
{
int bytes=(size_t)siz*sizeof(double); // nb double is size 4
double *ary=(double *)malloc(bytes+1);

I didn't comment on this before, but malloc(bytes+1) is very suspicions,
why the +1?

 

[Jens Thoms Toerring]

Ok here is a typical function with memory and pointers, gettbl is used to
build a Look Up Table used later...

double *mkary(size_t siz)
{
int bytes=(size_t)siz*sizeof(double); // nb double is size 4

Why cast 'siz' to a type it already has? Moreover, when you
emphasize the "size_t-ness" that much, why isn't 'bytes'
then also a 'size_t'? And that the size of double is 4 is
specific to your platform, on others it may have a comple-
tely different value, so your comment is confusing at best.

double *ary=(double *)malloc(bytes+1);

Why the cast of the return value of malloc() (or did you forget
to include <stdlib.h> and got a compiler warning)? And why adding
1 to 'bytes' - this is an array of doubles and not a string where
may you have to add 1 for the trailing '\0' character when you
calculate the length with strlen()? I would write that as

double *ary = malloc( siz * sizeof *ary );

and drop the line that calculates 'bytes'. (Of course, you
should always test if malloc() succeeded, if it didn't all
bets are off since then 'ary' will be just a NULL pointer).

for(bytes=0;bytes<siz;++bytes)
*(ary+bytes)=gettbl(bytes);

Mmm, why use '*(ary+bytes)' when 'ary[bytes]' would do equally
well? I know they are exactly identical, but I would consider
the second form to be easier to read. My personal feeling is
also that using 'bytes' both as a variable for the number of
bytes you want to malloc() and later as an index into an array
can be confusing, I would prefer to use two distinct variables
and let the compiler worry about optimizing that.

return (double*) ary;

Why cast 'ary' to a type it already has? I would strongly
recommend to avoid using casts unless they're really needed.
I consider a cast as a red flag that something unusual is
going on that may need closer scrutiny - if everything works
according to the normal typing rules a cast wouldn't be needed.

}
double *freeary(double *ary)
{
void *vp=(void*)ary;
if(vp!=(void*)NULL) free(vp);
return (double*)vp;
}

Not a single cast in this function is needed. They really only
make reading the code much harder. And you don't need to check
if 'ary' (or 'vp') is NULL before calling free() on it - calling
free() on a NULL pointer is just a NOP. In case that 'ary' isn't
a pointer you got from malloc(), realloc() or calloc() you're
screwed anyway and comparing it to NULL won't help in the least.

But this function invokes a very sneaky form of undefined be-
haviour. free() doesn't set the pointer you call it on to NULL
(I guess that's what you assume it would do, otherwise why re-
turn it's value?). But using a pointer after calling free() on
it actually does invoke undefined behaviour (that's right, using
the value of a pointer after free() does that, not just derefe-
rencing it). What I guess you intended to do in that function is
probably just

double * freeary( double * ary )
{
free( ary );
return NULL;
}

But it's rather unlikely that any of this is the actual culprit
(except maybe returning the now useless value of 'ary' if you
later make decisions based on its values).

Regards, Jens

 

[Keith Thompson]

Ok here is a typical function with memory and pointers, gettbl is used to
build a Look Up Table used later...

double *mkary(size_t siz)
{
int bytes=(size_t)siz*sizeof(double); // nb double is size 4
double *ary=(double *)malloc(bytes+1);
for(bytes=0;bytes<siz;++bytes) *(ary+bytes)=gettbl(bytes);
return (double*) ary;
}
double *freeary(double *ary)
{
void *vp=(void*)ary;
if(vp!=(void*)NULL) free(vp);
return (double*)vp;
}

Every cast in your code is unnecessary. I'm also going to add some
white space to make it more legible:

double *mkary(size_t siz)
{
int bytes = siz * sizeof(double); // nb double is size 4
double *ary = malloc(bytes + 1);
for (bytes=0; bytes < siz; ++bytes)
*(ary + bytes) = gettbl(bytes);
return ary;
}

double *freeary(double *ary)
{
void *vp = ary;
if (vp != NULL)
free(vp);
return vp;
}

It's very unlikely that sizeof(double) is 4; it's far more likely
to be 8. Fortunately, your code doesn't actually appear to depend
on that assumption, so removing the comment would solve that.

I can't think of any reason not to make "bytes" a size_t rather than an
int.

You should *always* check whether malloc returned a null pointer,
and take some corrective action if it did (even if you just
abort the program with an error message).

*(ary + bytes) is equivalent to ary[bytes]; I find the index form
clearer.

Now we get to the heart of the problem. Since ary is a pointer to
double, adding an integer to it advances that number of *doubles*.
On the first iteration of the loop, you take the result of
gettbl(bytes) (whatever that may be), convert it to double if it
isn't already, and store the result in ary[0]. On the second,
you do the same thing to ary[1]. But how many elements do you store?

Let's say sizeof(double) is 8 and siz is 10. Then bytes is 80.
You attempt to store 80 double values in a 10-element array. Kaboom.
(Or rather, kaboom if you're lucky. The behavior is undefined;
you could easily have written garbage over some other object and
caused arbitrarily bad behavior later on.)

Your freeary() function is not particularly useful (and you don't
call it). The test for "vp != NULL" is not necessary; free(NULL)
quietly does nothing. After the call to free, if vp was non-NULL in
the first place, you return its value -- which is an indeterminate
pointer to memory that you no longer own. There is nothing you can
do with that value; even examining it invokes undefined behavior.
That's why free() doesn't return a value. Drop the freeary()
function and replace any calls with a simple "free(ary)".
(The implicit conversion between void* and other pointer types
means you don't need either a cast or a temporary void* object.)

Recommended reading: Sections 4 and 6 of the comp.lang.c FAQ,
<http://c-faq.com/>. Of course, don't think I'm discouraging you
from reading the rest of it, but those sections are most relevant
to what you're doing.

 

[Jens Thoms Toerring]

Now we get to the heart of the problem. Since ary is a pointer to
double, adding an integer to it advances that number of *doubles*.
On the first iteration of the loop, you take the result of
gettbl(bytes) (whatever that may be), convert it to double if it
isn't already, and store the result in ary[0]. On the second,
you do the same thing to ary[1]. But how many elements do you store?

Let's say sizeof(double) is 8 and siz is 10. Then bytes is 80.
You attempt to store 80 double values in a 10-element array.

Actually, if you look at the code again, he uses only 'siz'
elements, and he has allocated enough for that (actually one
byte too much):

|> int bytes = siz * sizeof(double); // nb double is size 4
|> double *ary = malloc(bytes + 1);
|> for (bytes=0; bytes < siz; ++bytes)
|> *(ary + bytes) = gettbl(bytes);

I guess your impression is just an effect of using the 'bytes' va-
riable for two completely unrelated purposes, first for calculating
the number of bytes required and later on as an index into the array
- that's why I would propose to use different variables (as I also
got confused by it;-).
Regards, Jens

 

[Shao Miller]

Now we get to the heart of the problem. Since ary is a pointer to
double, adding an integer to it advances that number of *doubles*.
On the first iteration of the loop, you take the result of
gettbl(bytes) (whatever that may be), convert it to double if it
isn't already, and store the result in ary[0]. On the second,
you do the same thing to ary[1]. But how many elements do you store?

Let's say sizeof(double) is 8 and siz is 10. Then bytes is 80.
You attempt to store 80 double values in a 10-element array.

Actually, if you look at the code again, he uses only 'siz'
elements, and he has allocated enough for that (actually one
byte too much):

|> int bytes = siz * sizeof(double); // nb double is size 4
|> double *ary = malloc(bytes + 1);
|> for (bytes=0; bytes < siz; ++bytes)
|> *(ary + bytes) = gettbl(bytes);

I guess your impression is just an effect of using the 'bytes' va-
riable for two completely unrelated purposes, first for calculating
the number of bytes required and later on as an index into the array
- that's why I would propose to use different variables (as I also
got confused by it;-).

It was re-purposed, perhaps, but a minimally clever compiler could
likely take care of efficiency concerns for a separate variable by
noting that 'bytes' is never used again.

'siz' might be enjoyable with the name 'element_count'. 'bytes' might
be enjoyable as 'size'/'siz'/'sz'. 'element' might be a good variable
for the 'for' loop. Heheheh.

 

[Keith Thompson]

Now we get to the heart of the problem. Since ary is a pointer to
double, adding an integer to it advances that number of *doubles*.
On the first iteration of the loop, you take the result of
gettbl(bytes) (whatever that may be), convert it to double if it
isn't already, and store the result in ary[0]. On the second,
you do the same thing to ary[1]. But how many elements do you store?

Let's say sizeof(double) is 8 and siz is 10. Then bytes is 80.
You attempt to store 80 double values in a 10-element array.

Actually, if you look at the code again, he uses only 'siz'
elements, and he has allocated enough for that (actually one
byte too much):

|> int bytes = siz * sizeof(double); // nb double is size 4
|> double *ary = malloc(bytes + 1);
|> for (bytes=0; bytes < siz; ++bytes)
|> *(ary + bytes) = gettbl(bytes);

I guess your impression is just an effect of using the 'bytes' va-
riable for two completely unrelated purposes, first for calculating
the number of bytes required and later on as an index into the array
- that's why I would propose to use different variables (as I also
got confused by it;-).

I was starting to wonder why I was the only one who noticed that
problem!

Yes, using "bytes" as a size and then as an index is really poor
style. It doesn't even make sense as a name for the index, since
it's iterating by doubles, not by bytes.

I'd also suggest that "siz" is a poor name for the number of (double)
elements. Since C built-in sizeof operator counts bytes, the word
"size" strongly tends to imply a size in bytes, not in elements.
Use something like "count" or "length" instead.

And I'm not crazy about abbreviating "size" as "siz", and "array" as
"ary". I'm not 100% sure what bothers me about it, but I think part of
it is that there's no sensible pronunciation that's distinct from that
of the whole word. If it really represents a size, why not call it
"size"?

 

[Jens Thoms Toerring]

If possible, compile with debugging information (-g on gcc) and run it on a
debugger (such as gdb if available). Usually this will point you right at
the error and the call stack. At that point you have to deduce what chain of
events led you to that.

A word of caution: while using a debugger can be helpful in a lot
of cases it may also lead you astray - when the real reason for
the segmentation fault and the actual manifestation of its effect
are near to each other a debugger can be very useful. But if that
is not the case what you describe as "deduce what chain of events"
can be a the real hard thing to do - at the place where the segmen-
tation fault happens nothing really wrong is to be found and too
much checking just there may lead you to spend insane amounts of
time following up on red herings (been there and done that;-). So
stepping back a bit and, if nothing else helps, using a memory de-
bugger can safe a lot of time and effort more effectively spend on
other things...

If the error moves everytime you compile with different options or
small code tweaks, you're storing through an invalid pointer.

I am not sure I would subscribe to "Heisenbugs" being only a re-
sult of that - e.g. writing past the end of an array and having
the memory layout changed by different compiler options comes to
mind.
Regards, Jens

 

[Barry Schwarz]

Ok here is a typical function with memory and pointers, gettbl is used to
build a Look Up Table used later...

double *mkary(size_t siz)
{
int bytes=(size_t)siz*sizeof(double); // nb double is size 4
double *ary=(double *)malloc(bytes+1);
for(bytes=0;bytes<siz;++bytes) *(ary+bytes)=gettbl(bytes);
return (double*) ary;
}
double *freeary(double *ary)
{
void *vp=(void*)ary;
if(vp!=(void*)NULL) free(vp);
return (double*)vp;

On those occasions when ary was not NULL at the start of this
function, this statement invokes undefined behavior. Once the memory
has been freed, the value of any pointers which pointed to that memory
become indeterminate. Any attempt to evaluate an indeterminate
pointer is prohibited.