0

I am wondering why the following code will crash?

My environment is ubuntu64,gcc 4.8.1

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

#define SIZE 1024*1024*1024
int main()
{
    printf("%ld,%ld,%ld\n",sizeof(int),sizeof(long),sizeof(size_t));//output 4,8,8
    printf("%ld\n",SIZE); //output 1073741824

    int *p = (int*)malloc(SIZE);
    if(!p){
            perror("malloc");
            exit(1);
    }

    memset(p,0,SIZE);    //this works fine

    size_t i=0;   
    for(;i<SIZE;++i){
            p[i] = 10;  //gdb shows when crashed i = 268436476
    }    
}
Cœur
  • 37,241
  • 25
  • 195
  • 267
camino
  • 10,085
  • 20
  • 64
  • 115

2 Answers2

3

You're allocating SIZE bytes, not SIZE ints. When you try to write to SIZE ints in your for loop you are therefore writing beyond the end of allocated memory.

Change:

int *p = (int*)malloc(SIZE);

to:

int *p = malloc(SIZE * sizeof(p[0]));

Note also that the int cast which I have removed is both redundant and potentially harmful.

Community
  • 1
  • 1
Paul R
  • 208,748
  • 37
  • 389
  • 560
  • Why would the cast be potentially harmful? – Craig McQueen Nov 14 '13 at 22:06
  • See the question I linked to in the answer - the cast can obscure the fact that you forgot to `#include ` which could result in a truncated (and therefore invalid) pointer. – Paul R Nov 14 '13 at 22:07
  • Thanks, Shall we always use calloc instead of malloc to avoid these silly mistake? – camino Nov 14 '13 at 22:12
  • Don't use `calloc` unless you need to initialise the memory to all zeroes. – Paul R Nov 14 '13 at 22:12
  • OK, maybe always use malloc as "malloc (SIZE * sizeof(unit))" would be safer:) – camino Nov 14 '13 at 22:16
  • when not using `calloc()` you have to make sure that `SIZE * sizeof p[0]` does not overflow. `calloc()` might have no overhead on some platforms (Linux) because underlying memory allocation zeros always the memory due to security considerations. – ensc Nov 14 '13 at 22:17
  • Yes, but use `malloc(SIZE * sizeof(p[0]))` as above, not `malloc(SIZE * sizeof(int))`. – Paul R Nov 14 '13 at 22:17
1

Use

int *p = calloc(SIZE, sizeof p[0]);
ensc
  • 6,704
  • 14
  • 22