60
votes

I'm having a problem with some program, I have searched about segmentation faults, by I don't understand them quite well, the only thing I know is that presumably I am trying to access some memory I shouldn't. The problem is that I see my code and don't understand what I am doing wrong.

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

#define   lambda   2.0
#define   g        1.0
#define   Lx       100
#define   F0       1.0
#define   Tf       10
#define   h       0.1
#define   e       0.00001

FILE   *file;

double F[1000][1000000];

void Inicio(double D[1000][1000000]) {
int i;
for (i=399; i<600; i++) {
    D[i][0]=F0;
}
}

void Iteration (double A[1000][1000000]) {
long int i,k;
for (i=1; i<1000000; i++) {
    A[0][i]= A[0][i-1] + e/(h*h*h*h)*g*g*(A[2][i-1] - 4.0*A[1][i-1] + 6.0*A[0][i-1]-4.0*A[998][i-1] + A[997][i-1]) + 2.0*g*e/(h*h)*(A[1][i-1] - 2*A[0][i-1] + A[998][i-1]) + e*A[0][i-1]*(lambda-A[0][i-1]*A[0][i-1]);
    A[1][i]= A[1][i-1] + e/(h*h*h*h)*g*g*(A[3][i-1] - 4.0*A[2][i-1] + 6.0*A[1][i-1]-4.0*A[0][i-1] + A[998][i-1]) + 2.0*g*e/(h*h)*(A[2][i-1] - 2*A[1][i-1] + A[0][i-1]) + e*A[1][i-1]*(lambda-A[1][i-1]*A[1][i-1]);
    for (k=2; k<997; k++) {
        A[k][i]= A[k][i-1] + e/(h*h*h*h)*g*g*(A[k+2][i-1] - 4.0*A[k+1][i-1] + 6.0*A[k][i-1]-4.0*A[k-1][i-1] + A[k-2][i-1]) + 2.0*g*e/(h*h)*(A[k+1][i-1] - 2*A[k][i-1] + A[k-1][i-1]) + e*A[k][i-1]*(lambda-A[k][i-1]*A[k][i-1]);
    }
    A[997][i] = A[997][i-1] + e/(h*h*h*h)*g*g*(A[0][i-1] - 4*A[998][i-1] + 6*A[997][i-1] - 4*A[996][i-1] + A[995][i-1]) + 2.0*g*e/(h*h)*(A[998][i-1] - 2*A[997][i-1] + A[996][i-1]) + e*A[997][i-1]*(lambda-A[997][i-1]*A[997][i-1]);
    A[998][i] = A[998][i-1] + e/(h*h*h*h)*g*g*(A[1][i-1] - 4*A[0][i-1] + 6*A[998][i-1] - 4*A[997][i-1] + A[996][i-1]) + 2.0*g*e/(h*h)*(A[0][i-1] - 2*A[998][i-1] + A[997][i-1]) + e*A[998][i-1]*(lambda-A[998][i-1]*A[998][i-1]);
    A[999][i]=A[0][i];
}
}

main() {
long int i,j;
Inicio(F);
Iteration(F);
file = fopen("P1.txt","wt");
for (i=0; i<1000000; i++) {
    for (j=0; j<1000; j++) {
        fprintf(file,"%lf \t %.4f \t %lf\n", 1.0*j/10.0, 1.0*i, F[j][i]);
    }
}
fclose(file);
}

Thanks for your time.

4
At what point does the segfault occur?Jeff Mercado
^ Try using Valgrind to get the line number where the segfault occurs. It's generally fairly obvious once you've narrowed it down to one line - if not, post which line it is and we can help.1''
seeing your code,firstly your compiler must have gone segmentation fault...perilbrain
It compiles, It says segmentation fault when I run it, ./ProgramAriaramnes
Your program will segfault if it can't create P1.txt. You should always system fopen (and other routines that can fail) for failure.Jim Balter

4 Answers

125
votes

This declaration:

double F[1000][1000000];

would occupy 8 * 1000 * 1000000 bytes on a typical x86 system. This is about 7.45 GB. Chances are your system is running out of memory when trying to execute your code, which results in a segmentation fault.

40
votes

Your array is occupying roughly 8 GB of memory (1,000 x 1,000,000 x sizeof(double) bytes). That might be a factor in your problem. It is a global variable rather than a stack variable, so you may be OK, but you're pushing limits here.

Writing that much data to a file is going to take a while.

You don't check that the file was opened successfully, which could be a source of trouble, too (if it did fail, a segmentation fault is very likely).

You really should introduce some named constants for 1,000 and 1,000,000; what do they represent?

You should also write a function to do the calculation; you could use an inline function in C99 or later (or C++). The repetition in the code is excruciating to behold.

You should also use C99 notation for main(), with the explicit return type (and preferably void for the argument list when you are not using argc or argv):

int main(void)

Out of idle curiosity, I took a copy of your code, changed all occurrences of 1000 to ROWS, all occurrences of 1000000 to COLS, and then created enum { ROWS = 1000, COLS = 10000 }; (thereby reducing the problem size by a factor of 100). I made a few minor changes so it would compile cleanly under my preferred set of compilation options (nothing serious: static in front of the functions, and the main array; file becomes a local to main; error check the fopen(), etc.).

I then created a second copy and created an inline function to do the repeated calculation, (and a second one to do subscript calculations). This means that the monstrous expression is only written out once — which is highly desirable as it ensure consistency.

#include <stdio.h>

#define   lambda   2.0
#define   g        1.0
#define   F0       1.0
#define   h        0.1
#define   e        0.00001

enum { ROWS = 1000, COLS = 10000 };

static double F[ROWS][COLS];

static void Inicio(double D[ROWS][COLS])
{
    for (int i = 399; i < 600; i++) // Magic numbers!!
        D[i][0] = F0;
}

enum { R = ROWS - 1 };

static inline int ko(int k, int n)
{
    int rv = k + n;
    if (rv >= R)
        rv -= R;
    else if (rv < 0)
        rv += R;
    return(rv);
}

static inline void calculate_value(int i, int k, double A[ROWS][COLS])
{
    int ks2 = ko(k, -2);
    int ks1 = ko(k, -1);
    int kp1 = ko(k, +1);
    int kp2 = ko(k, +2);

    A[k][i] = A[k][i-1]
            + e/(h*h*h*h) * g*g * (A[kp2][i-1] - 4.0*A[kp1][i-1] + 6.0*A[k][i-1] - 4.0*A[ks1][i-1] + A[ks2][i-1])
            + 2.0*g*e/(h*h) * (A[kp1][i-1] - 2*A[k][i-1] + A[ks1][i-1])
            + e * A[k][i-1] * (lambda - A[k][i-1] * A[k][i-1]);
}

static void Iteration(double A[ROWS][COLS])
{
    for (int i = 1; i < COLS; i++)
    {
        for (int k = 0; k < R; k++)
            calculate_value(i, k, A);
        A[999][i] = A[0][i];
    }
}

int main(void)
{
    FILE *file = fopen("P2.txt","wt");
    if (file == 0)
        return(1);
    Inicio(F);
    Iteration(F);
    for (int i = 0; i < COLS; i++)
    {
        for (int j = 0; j < ROWS; j++)
        {
            fprintf(file,"%lf \t %.4f \t %lf\n", 1.0*j/10.0, 1.0*i, F[j][i]);
        }
    }
    fclose(file);
    return(0);
}

This program writes to P2.txt instead of P1.txt. I ran both programs and compared the output files; the output was identical. When I ran the programs on a mostly idle machine (MacBook Pro, 2.3 GHz Intel Core i7, 16 GiB 1333 MHz RAM, Mac OS X 10.7.5, GCC 4.7.1), I got reasonably but not wholly consistent timing:

Original   Modified
6.334s      6.367s
6.241s      6.231s
6.315s     10.778s
6.378s      6.320s
6.388s      6.293s
6.285s      6.268s
6.387s     10.954s
6.377s      6.227s
8.888s      6.347s
6.304s      6.286s
6.258s     10.302s
6.975s      6.260s
6.663s      6.847s
6.359s      6.313s
6.344s      6.335s
7.762s      6.533s
6.310s      9.418s
8.972s      6.370s
6.383s      6.357s

However, almost all that time is spent on disk I/O. I reduced the disk I/O to just the very last row of data, so the outer I/O for loop became:

for (int i = COLS - 1; i < COLS; i++)

the timings were vastly reduced and very much more consistent:

Original    Modified
0.168s      0.165s
0.145s      0.165s
0.165s      0.166s
0.164s      0.163s
0.151s      0.151s
0.148s      0.153s
0.152s      0.171s
0.165s      0.165s
0.173s      0.176s
0.171s      0.165s
0.151s      0.169s

The simplification in the code from having the ghastly expression written out just once is very beneficial, it seems to me. I'd certainly far rather have to maintain that program than the original.

3
votes

What system are you running on? Do you have access to some sort of debugger (gdb, visual studio's debugger, etc.)?

That would give us valuable information, like the line of code where the program crashes... Also, the amount of memory may be prohibitive.

Additionally, may I recommend that you replace the numeric limits by named definitions?

As such:

#define DIM1_SZ 1000
#define DIM2_SZ 1000000

Use those whenever you wish to refer to the array dimension limits. It will help avoid typing errors.

1
votes

Run your program with valgrind of linked to efence. That will tell you where the pointer is being dereferenced and most likely fix your problem if you fix all the errors they tell you about.