Segmentation Fault in C

[philstewart]

Hi,

Can any one help me spot the error in this snippit from my code, it compiles fine but produces a segmentation fault when run. The offending line appears to be the fprintf line near the end - when I remove it, I get no segmentation fault, but I'm having a complete brain freeze and can't spot what's wrong!

#include <stdio.h>
#include <stdlib.h>
int main()
{
FILE *in, *out;
int i, j;
double vmax, vmin, temp;
double c_states[4][720000];
double states[4][720000];

vmax = -100000000.;
vmin = 100000000.;

in = fopen("../ctrl/states.dat", "rt");
for (i = 0; i <= 719999; ++i)
{
for (j = 0; j <= 3; ++j)
{
fscanf(in, "%lf", &temp);
c_states[j][i]=temp;
}
fscanf(in, "\n");
}
fclose(in);

in = fopen("states.dat", "rt");
for (i = 0; i <= 719999; ++i)
{
for (j = 0; j <= 3; ++j)
{
fscanf(in, "%lf", &temp);
states[j][i]=temp;
}
fscanf(in, "\n");
}
fclose(in);

for (i = 0; i <= 719999; ++i)
{
if (states[1][i] > vmax)
vmax = states[1][i];
if (c_states[1][i] > vmax)
vmax = c_states[1][i];
if (states[1][i] < vmin)
vmin = states[1][i];
if (c_states[1][i] < vmin)
vmin = c_states[1][i];
}

out = fopen("overdrive.plot", "wt");
fprintf(out,"set yrange [%4.10lf:%4.10lf]\n",vmin*0.99,vmax*1.01);
fclose(out);
return 0;
}

Thanks,

Phil

[gerard4143]

Is fopen returning a valid pointer to out?

[philstewart]

I assume so, prior to that particular line, there are more outputs to the same file (e.g. fprintf(out,"set tmargin 1") which work perfectly fine. Though I don't know how to check if it returns a valid pointer.

[Margrad]

try something like this...


if ((out = fopen("overdrive.plot", "wt")) != NULL)
{
fprintf(out,"set yrange [%4.10lf:%4.10lf]\n",vmin*0.99,vmax*1.01);
fclose(out);
}
else
printf("\n error...\n");

[philstewart]

Thanks for your response, I've tested your code and I still get a segmentation fault (it doesn't reach printf error line).

The file appears to open ok, but there must be something on the fprintf line that's wrong!

[Mariano Suárez-]

Have you tried running your program inside a debugger, such as gdb?
It will instantly tell you where the segfault is.

I can't show you because to run your code one needs files which you did not provide.

But, in any case: if you haven't already, invest time learning how to use gdb.

[philstewart]

Hi Mariano,

I didn't know about gdb so thanks for the pointer, it gave me the output;

Program received signal SIGSEGV, Segmentation fault.
0x00000000004006f3 in main (argc=Cannot access memory at address 0x7fff1bbcaf4c
) at plot.c:13
13 {

I previously thought it was the fprintf line that caused it, apparently due to the fact I wasn't using gcc to compile at the time. So it would appear the problem is actually with the arguments I understand this to say? This is the full top of my code, have I made an error?

#include <stdio.h>
#include <stdlib.h>

int main(int argc, char **argv)
{
FILE *in, *out;
int i, j, n;
double vmax, vmin, naimax, naimin, apdmax, apdmin, caimax, caimin, nakmax, nakmin, calmax, calmin, nacamax, nacamin;
double temp;

double c_states[4][720000];
double c_currents[4][720000];
double states[4][720000];
double currents[4][720000];
double apd[5][600];
double c_apd[5][600];

vmax = naimax = apdmax = caimax = nakmax = calmax = nacamax = -100000000.;
vmin = naimin = apdmin = caimin = nakmin = calmin = nacamin = 100000000.;

if (argc > 1)
n = atoi(argv[1]);
else
{
printf("Arguments: Number of lines in APD.dat (int)\n");
exit(1);
}

Thanks for all your help so far, a fresh mind helps loads!

[philstewart]

I've just used the intel debugger too and it adds a little extra info about the seg fault;

Intel(R) Debugger for applications running on Intel(R) 64, Version 10.1-32 , Build 20070828
------------------
object file name: generate_plot
Reading symbols from generate_plot...done.
(idb) run 480
Starting program: generate_plot
Program received signal SIGSEGV
main (argc=2, argv=0x7fff87945288) at plot.c:30
30 n = atoi(argv[1]);

[dshaw256]

So it's dying at line 13 where you are declaring a number of HUGE static arrays... I think you have over 100 Mbytes of static arrays being created (4*720000 = 2.88 million cells * 8 bytes per cell for a double(?) would be 23 Mbytes per array, and there are 4 of those, pluse more....). I'm not big on Intel architecture internals, but you just might be running past an internal limit there?

Try allocating those arrays dynamically using malloc. Just a thought.

[Margrad]

what are the arguments you're parsing?

[dr death]

dshaw256 is spot on here. You are trying to put too much on the stack. Either malloc the variables (preferred) or increase the stack size.

I compiled your program as test:


$ gcc -fstack-check -o test test.c
test.c: In function ‘main’:
test.c:32: warning: frame size too large for reliable stack checking
test.c:32: warning: try reducing the number of local variables


Increasing the stack size:


$ ulimit -s
10240
$ ./test
Segmentation fault
$ ulimit -s 102400
$ ulimit -s
102400
$ ./test
Arguments: Number of lines in APD.dat (int)


To set it programatically try


$ man 2 setrlimit

[philstewart]

what are the arguments you're parsing?

I parse only one argument: the number of lines in the file APD.dat. In the cases above, I used the value 480.

[philstewart]

dshaw256 and dr_death, thanks for your comments, sorry I'm not an expert on these sorts of technicalities, for some reason I don't have a man page about this, but from the info page I gather this is a limit on the resources available to running programs? I will your suggestions to see if it gets round the problem, many thanks.

[dr death]

I wouldn't worry about the details too much. Basically if you define an array with a fixed size e.g.

int main()
{
double a[100];
/* rest of code here */
return 0;
}

it is allocated memory on the stack, whereas if you dynamically allocate it e.g.

int main()
{
double *a;
a = malloc(100 * sizeof(double));
/* rest of code here */
free(a);
return 0;
}

it is allocated memory on the heap. For more details see e.g.
http://en.wikipedia.org/wiki/Call_stack
Now the size of the heap is determined by the amount of RAM you have in your machine (possibly including swap), while in linux / unix the size of the stack is determined by the shell. If you are using bash, the command to increase the stack size is "ulimit". As in my example above

$ ulimit -s 102400

will give you 102400 kB of stack (100MB) which should be enough for the variables that you are using. Note that this setting is for the current shell only, so when you close the xterm and open another, you will have to type in this command again. For this reason I would recommend using malloc, although as an alternative you could add the ulimit command to you ~/.bashrc

[philstewart]

Thanks dr_death, I understand the difference a lot better now and malloc certainly seems a lot more appealing, but I'm not sure how to malloc a 2D array correctly, so far my attempts have failed, I'll continue trawling google to get a working prototype for one.

Once again, many thanks for your help!

[Margrad]

for 2D use something like this:


int nl=4, /*number of lines*/
nc=720000, /*number of colunes */
c=0,
double **ptr=NULL; /*we will need a pointer to others pointers, ie, **ptr*/

/*start by allocating the lines */
ptr=(double**)calloc(nl,sizeof(double*));

/* then allocate the coluns*/
for(c=0;c<nl;c++)
ptr[c]=(double*)calloc(nc,sizeof(double));

/*don't forget to free all memory in the end*/
for(c=0;c<nc;c++)
free(ptr[c]);
free(ptr);
}

[dr death]

For these sorts of problems, the comp.lang.c FAQ is your friend (no need to google for random sites)
http://c-faq.com/
So e.g. your question on multidimensional arrays (which was answered by Margrad) is explained here: http://c-faq.com/aryptr/dynmuldimary.html

[philstewart]

Margrad, dr_death, thanks a lot, I've got it working now! Thanks so much for all your help, and thanks for the tip on c-faq, I'll definately be using this as my first port of call in the future!

Cheers,
Phil

[cable_txg]

I just tried ulimit -s 102400, I still got segmentation fault. Trying ulimit -s 1024000 worked.
I'm not a big fan of allocation large portions of memory at once (always on the fly), I usually program in C++.

[dshaw256]

Not to go on and on with this, but I was thinking that the whole block of doubles could be dynamically allocated in one lump if you used a struct. Then the way the individual variables are referenced in the remainder of the code is largely unchanged; you just need to add a pointer-> reference in front of the fields from the dynamically-allocated block.

The following compiles & runs correctly on my FC6 system, with no changes to the stack frame size, or any other environmental changes at all.


#include <stdio.h>
#include <stdlib.h>

// define (don't allocate!) dynamic variable storage

typedef struct {
double c_states [4][720000];
double c_currents[4][720000];
double states [4][720000];
double currents [4][720000];

double apd [5][600];
double c_apd [5][600];
} bigArrays;

// allocate a pointer to dynamic variables

bigArrays *myArrays;

int main () {
int i, j, count=0;

// allocate storage for the big variables

myArrays = (bigArrays *) malloc( sizeof( bigArrays ));

// initialize 4x720000 arrays

for ( i=0; i<4; ++i )
for ( j=0; j<720000; ++j ) {

myArrays->c_states [i][j] = 0.0;
myArrays->c_currents[i][j] = 0.0;
myArrays->states [i][j] = 0.0;
myArrays->currents [i][j] = 0.0;

++count;
}

// initialize 5x600 arrays

for ( i=0; i<5; ++i )
for ( j=0; j<600; ++j ) {

(*myArrays).apd [i][j] = 0.0;
(*myArrays).c_apd[i][j] = 0.0;

++count;
}

// ( rest of code goes here )

free( myArrays );

// just to prove that we successfully executed all those loops...

printf( "We successfully ran %d loops (should be 2883000)\n", count );

return( 0 );
}

Phil, if you haven't played much with pointers and structures, they take a little getting used to. I still get confused by the things. :o)

There are two basic ways to dereference an element in a dynamically-allocated structure: pointer->field, or (*pointer).field. I used both in the code above. I think everybody in the world except me prefers the first style. I got used to the second style when writing Macintosh OS9 code, where I was required to dereference OS structure elements through "handles" (pointers to pointers). The (**handle).field notation was simple, and pretty much the only syntax I could make work anyway. And the notation is closer to that used when you are dereferencing a single value through a pointer, which I think has mnemonic value.

Anyway, hope this helps.

[sideways]

Nice thread, good info and problem clearly resolved. I like your struct solution dshaw256, it is particularly simple. (You do not need to make bigArrays a global though, it can be defined local)

[mwette]

The best way to track this down is to use the debugger.

$ gcc -g -o prog prog.c
$ gdb prog
gdb) run
Segmentation fault
gdb) info stack
...

[Mariano Suárez-]

Writing (*pointer).field will get you kicked out of any self-respecting group of people ;-)

[dr death]

If you're really looking to zero all those arrays, then try memset or bzero, or even calloc. Also note that you don't have to typecast the result of malloc (this is the whole point of void*), although this is personal preference (and the topic of many a flame war).

myArrays = malloc( sizeof( bigArrays ) );
memset(myArrays, 0, sizeof(bigArrays));

or

myArrays = malloc( sizeof( bigArrays ) );
bzero(myArrays, sizeof(bigArrays));

or

myArrays = calloc( sizeof( bigArrays ), 1 );

[Mariano Suárez-]

memset or calloc (or bzero, which you should not use) are not a good way to zero out memory which contains doubles/floats (or pointers, for that matter) since there is no guarantee that the bit representation of the zero double/float (or the NULL pointer) has all its bits set to zero.

[dr death]

Technically you're right, but I've never seen such an instance on any architecture. However in the interests of promoting correctness in coding, I'll agree with you.

[cable_txg]

I consider myself part of the old school of thought:

Just because I have 2Gb of Memory does not mean I should start creating variables that require a lot memory above 1Mb.

In short, find a different way to dynamically allocate small amounts of memory at a time - rather than loading a whopping array > 5Kb . I've seen programs created with the attitude of "PC/Server has tons of memory, so it does not matter", crash and halt the system.

If you must use linked lists ( in C) or structs with unions, then by all means do so.

[Mariano Suárez-]

Sinec IEEE is hard to implement in software efficiently, lots of embedded hardware does not use IEEE floating point, resulting in variance there.
(Even IEEE has two zeros (!), one of which is not all zero-bits)

[dshaw256]

Writing (*pointer).field will get you kicked out of any self-respecting group of people ;-)
Ha! Well, I'm a well-known nut-bag and probably DESERVE to be kicked out of any self-respecting group of people. :o)

Also note that you don't have to typecast the result of malloc (this is the whole point of void*), although this is personal preference (and the topic of many a flame war).
Wouldn't want a flame war! I got into the habbit of typecasting voids when writing for Macintosh. Apple's PPC C compiler would issue a warning on any uncast void assignment, and I'm kind of anal about The Clean Compile. I have no better reason than that to do it.

[philstewart]

dshaw256: Thanks, your method seems very elegant in my opinion and very easy to follow, I've never used structs before however I believe they are similar to classes in c++? And though I've used pointers in the past, I still get a little lost with them sometimes!!

cable_txg: I agree you shouldn't use memory just because it's there, however in my field it is often required to use vast amounts of memory given the nature of the problem involved.

[dshaw256]

Phil, I've never used c++, nor have I written more than a small amount of object-oriented code in any language, so your guess is as good as mine as to whether structures and classes are similar. I always think of structures as "records" (flash back to my very-long-ago mainframe programming career). Or to make it very generic, call them "a collection of related fields." If you are describing some kind of object, then they could represent a collection of the object's attributes, I'd guess. But if I understand this correctly, a class is usually a collection of both methods and attributes. Methods can't be expressed in a struct, unless you want to start talking about bizarre stuff like pointers to functions... and let's just not go there! I believe that's why they invented c++. :o)

[skoona]

philstewart,

You might want to look at an utility library for your C programming. I recommend GLIB2.0 and personally use it all the time for non-GTK application. It contains a diverse set of programming constructs for C, which do come in handy and prevent you from re-creating the wheel. Assuming you are a Fedora user, the 'devhelp' documentation tool is already loaded on your machine -- open it and lookup GLIB; otherwise here is a link: http://library.gnome.org/devel/glib/unstable/index.html

[philstewart]

dshaw256, yes I think you are correct about classes and structs, thanks for your analogies, I find them very useful to think about these concepts!

skoona, thanks for the info on GLIB, I notice GArray which would be very useful here. I'll try to use GLIB a lot more now I notice it exists, it appears to have a lot which would be useful to me!

Many thanks to both of you!

[bitwiseb]

Would making the arrays "static" have helped? Any reason you didn't?
static double c_states[4][720000];
Etc. It'd get them off the stack and avoid malloc. With 7 days of irrelevance, maybe you don't care.

There are structs in C++. They're classes but with public access and inheritance by default instead of private. They can have member functions too.

C structs have no member functions. Fun.

[dshaw256]

Actually, making the arrays static does work. The following compiles clean and runs clean. Removing "static" gives a runtime segmentation fault. Good idea.
#include <stdio.h>
#include <stdlib.h>

int main ( int argc, char *argv[] ) {

static double c_states [4][720000];
static double c_currents[4][720000];
static double states [4][720000];
static double currents [4][720000];

static double apd [5][600];
static double c_apd [5][600];

int i, j, count=0;

// initialize 4x720000 arrays

for ( i=0; i<4; ++i )
for ( j=0; j<720000; ++j ) {

c_states [i][j] = 0.0;
c_currents[i][j] = 0.0;
states [i][j] = 0.0;
currents [i][j] = 0.0;

++count;
}

// initialize 5x600 arrays

for ( i=0; i<5; ++i )
for ( j=0; j<600; ++j ) {

apd [i][j] = 0.0;
c_apd[i][j] = 0.0;

++count;
}

// ( rest of code goes here )


// just to prove that we successfully executed all those loops...

printf( "We successfully ran %d loops - should come out to 2883000.\n", count );

return( 0 );
}

[bitwiseb]

Is that a typo on line 22 of the code you just pasted?

for ( i=0; i<4; ++i )
for ( j=0; j<720000; ++j ) {

c_states [i][j] = 0.0;
currents [i][j] = 0.0; // Should this be c_currents ?
states [i][j] = 0.0;
currents [i][j] = 0.0;

++count;
}

Also, I'm unsure how standard it is, but there's a chance you can (possibly portably) initialise all of a huge array at compile-time simply in the definition.
static double c_states [4][720000] = {{0.0}};
static double c_currents[4][720000] = {{0.0}};
static double states [4][720000] = {{0.0}};
static double currents [4][720000] = {{0.0}};

static double apd [5][600] = {{0.0}};
static double c_apd [5][600] = {{0.0}};
...and now you can remove all that nested for-loop initialisation code too.

I'll try to verify its standardness (googling wildly), but it compiles and I think you can initialise an entire array (compile-time) by specifying only the first (if the values are all the same).

[bitwiseb]

I was almost right. The remaining elements are initialised to their default value. So luckily, as you're after 0.0 in all elements, it will work for you.

Edit: I'm an idiot. In many ways.

My typo comment from the other reply (which I sent just then, and you've hopefully recieved) still stands.

Since this is all potentially data segment data, all zeroed from the start; it makes no sense to initialise it at run-time, and certainly makes no sense to allocate it at run-time.

I wish I had the C spec though, to point to... for credibility.

Edit:

Apparently you don't even need the initialisation at all, now that the arrays are static.
10 If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate. If an object that has static storage duration is not initialized explicitly, then:

- if it has pointer type, it is initialized to a null pointer;
- if it has arithmetic type, it is initialized to (positive or unsigned) zero;
- if it is an aggregate, every member is initialized (recursively) according to these rules;

[philstewart]

Would making the arrays "static" have helped? Any reason you didn't?

Other than the fact I have never used static before! :p

Thanks for following up the thread, I still appreciate comments even after a week, it helps me learn! I'll definately keep it in mind for the future!

[dshaw256]

Yep, bitwiseb, that was a type-o. I was removing the pointers from the previously posted code and and went overboard. Thanks for the catch. I edited the post and fixed the code.

I didn't know the statics would initialize to default -- I really didn't think about it as I would have used dynamically-allocated arrays anyway. And I would have initialized everything anyway because that would be the anal retentive thing to do. But, anyway, I left the init code in there to make sure that the program stub wrote to all those doubles to ensure that the program actually worked without a segmentation fault.

[bitwiseb]

Actually yeah, that makes sense (the segfault-testing loops). Maybe it's apparent now that I lost track of who the OP was and assumed that code was more than a test...

But since then, I've installed Fedora smoothly on my laptop and am now running it. Smoothly. :D I have Fedora again!

[sideways]

Since this is all potentially data segment data, all zeroed from the start; it makes no sense to initialise it at run-time, and certainly makes no sense to allocate it at run-time.


Hmm, I'd rather not rely on my compiler conforming to the latest C spec to ensure correct initialisation of large arrays, especially if the initialisation is critical. The struct declaration method has the advantage of easily passing pointers to the arrays to other functions.

However, thanks for pointing out that static arrays can be auto-initialised in C99, that's useful to know. :)