Why is this segfault in C?

Question

I can't understand why this tiny C segfaults program:

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

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

    int in = atoi(argv[1]);
    printf("Input %d\n",in);

    int *n = (int *)malloc(in);
    int j;

    for (j=0;j<in;j++)
        n[j] = j;

    printf("Sanity check...\n");

    char *c = (char *)malloc(1024*1024*20);
    int i;
    for (i=0; i<20*1024*1024;i++)
        c[i] = i;

    printf("No segfault. Yay!\n");

    return 0;
}

Compiled with

$ gcc -O0 test.c -o run

Conclusion:

$. / run 1000

$ Entrance 1000

$ Health Check ...

$ [1] 17529 Segmentation error (kernel reset) ./ run 1000

Now, if I moved one of the for-loops loops like this:

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

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

    int in = atoi(argv[1]);
    printf("Input %d\n",in);

    int *n = (int *)malloc(in);
    int j;

    printf("Sanity check...\n");

    char *c = (char *)malloc(1024*1024*20);
    int i;
    for (i=0; i<20*1024*1024;i++)
        c[i] = i;

    printf("No segfault. Yay!\n");

    for (j=0;j<in;j++)
        n[j] = j;

    return 0;
}

everything works .. same build phase, this is the conclusion:

$. / run 1000

$ Entrance 1000

$ Health Check ...

$ No segfault. Hooray!

The reason I am making a large 20 megabyte malloc is to try to remove the cache effects from the code that I am profiling. It looks like both implementations should work, but the first one works when the malloc-ing array is 20MB. Did I miss something obvious here?

Thank.

Answer 1

int in = atoi(argv[1]);
int *n = (int *)malloc(in);

in , in . :

malloc(sizeof(int) * in);

, sizeof(char) - 1.

Answer 2

int *n = (int *)malloc(in);

.

,

int *n = (int *)malloc(in * sizeof( int ) );

Answer 3

given the posted code,
lets just run down the reasons it seg faults.

my comments are interspersed with the code

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

// argc is not used, so the compiler will raise a warning
int main(int argc, char *argv[])
{

    // the value of argc was not checked to assure a parameter is present
    // so a 'usage' message was not output when no parameter is present
    // or too many parameters are present
    // note: 'in' is (IMO) a poor name for a local variable
    int in = atoi(argv[1]);
    // missing check to assure the value if 'in' is greater than 0
    // and handling the error when the value is <= 0

    printf("Input %d\n",in);

    // in C, the returned value from calls to malloc(), and family,
    //       should not be cast
    // the returned value is (per the definition of 'n')
    // to be a pointer to an array of int's
    // which should be written as:
    // int *n = malloc( in * sizeof int );
    // the returned value from malloc(), and family of functions,
    //     needs to always be checked (!= NULL)
    //     to assure the operation was successful
    int *n = (int *)malloc(in);
    int j;

    // because the malloc call did not ask for a memory allocation
    // of 'in' int's, so this code block corrupts the heap
    // by writing past the end of the allocated memory
    for (j=0;j<in;j++)
        n[j] = j;

    printf("Sanity check...\n");

    // in C, the returned value from calls to malloc(), and family of functions,
    //       should not be cast
    // the returned value needs to be checked (!= NULL) to assure the operation
    //       was successful
    // the calculated value: 1024*1024*20 should be calculated by the
    //       compiler, not repeatedly at run time.
    //       so insert 'const int twentyMillion = 1024*1024*20;'
    //       at beginning of program
    char *c = (char *)malloc(1024*1024*20);
    int i;

    // to avoid repeatedly calculating the 20million value, use a const (as above)
    // so this line would be: for ( i=0; i<twentyMillion; i++ )
    for (i=0; i<20*1024*1024;i++)
        // the value of 'i' will massively exceed the value that can be kept in a char
        //  I.E. 255 so this loses data
        //  suggest replacing following line with: j = i%256;  c[i] = j;
        // to make it obvious what is going on
        c[i] = i;

    printf("No segfault. Yay!\n");

    return 0;
} // end function: main