Segmentation fault when malloc/free appear in loop in C

Question

Hi there,

I have a program that basically looks like:

    typedef struct cpl_def
            {
            int A;
            int B;
            int OK; 
            struct cpls *link; 
            }cpls;


        int main(void)
            {
            int n1, n2;
            int num = 300; /* say */
            int *a;
            a = NULL;
            int *apt;

            int *b;
            b = NULL;
            int *bpt;

            cpls *cplptr; 
            cplptr = NULL; 

                int i, j;
                for (i=0; i < 2000; i++) 
                    {

                    if (i == 0)
                      {
                        cplptr = (cpls *) malloc(num*sizeof(cpls) ); /* create the structure */ 
                        initalize(cplptr);
                        }
    /*
                    ...operations on cplptr ...  */  


                   FOO(cplptr);
     /*
               ...determine I need a subset of size n1 (a positive integer of size n1 which changes during the loop) entries from cplptr ...  */

                 n1 = FOO2(cplptr);
                 n2 = FOO3(cplptr);

 /*
               ...figure out the values of A, B for additional n2 entries into cplptr ...
      */        
    cplptr2 = (cpls *) malloc(n2*sizeof(cpls) ); /* a second structure to store additional entries */
            /* .... operations on cplptr2 ...*/

    /* ...copy subset of n1 entries from cplptr into dynamically allocated arrays a,b of size n1... */  

            a = malloc(n1 * sizeof(int));
            apt = &a[0];

            b = malloc(n1 * sizeof(int));
            bpt = &b[0];


            for (j=0; j < num; j++)
                 {
                 if (cplptr[j].OK==1)
                            {
                            (*apt++) = cplptr[j].a;
                            (*bpt++) = cplptr[j].b;
                            }
                 }
               free(cplptr); /* free the first structure */

               cplptr = (cpls *) malloc((n1+n2)*sizeof(cpls) ); /* redeclare the first structure to reflect the proper sizes */

               for (j = 0; j < n1; j++) /* transfer a subset of size n1 to the first structure */
                      {
                       cplptr[j].a = a[j];
                       cplptr[j].b = b[j];
                       cplptr[j].OK = 1;
                      }
               for (j = n1; j < n1 + n2; j++) /* transfer things to the first structure */
                      {
                       cplptr[j].a = cplptr2[j].a;
                       cplptr[j].b = cplptr2[j].b;
                       cplptr[j].OK = cplptr2[j].OK;
                      }

               free(a)
               free(b)

               free(cplptr2); /* free the second structure */
             } /* End iteration i
    } /* End main() */

This is just the skeletal form but it hopefully provides enough of a picture. ANyhow it generally runs fine, but for some values of n1, n2, the free(cplptr) seems to cause a segmentation fault. It's only called once, and I check the address after the malloc() call to cplptr and before the corresponding free() for cplptr.

.... 
cplptr = (cpls *) malloc(num*sizeof(cpls) );
printf("fine to this %p\n", &cplptr[0]);
...
printf("fine to this %p\n", &cplptr[0]);
free(cplptr) <- segmentation fault happens here.

The addresses match, meaning free() should be freeing what it's supposed to, right?? gdb gives Program received signal SIGSEGV, Segmentation fault. 0xb7ce179b in ?? () from /lib/tls/i686/cmov/libc.so.6 and step Cannot find bounds of current function

Is there another way to implement something like that avoids the segmentation faults?

Thanks a million for your suggestions! Any idea what's going on??

Answer 1

If you're hitting a crash on free(), this is most likely due to heap corruption - you are writing beyond the end of an allocated block of memory, freeing a pointer twice, or similar.

Valgrind is a great tool to debug these sort of issues on Linux.

Answer 2

I presume that in the real program, you set n1 and n2 somewhere?

Also, allocate cplptr outside the loop instead of checking i==0.

You may have a typo, you never set bpt, but set apt twice.

Answer 3

You haven't initialized n1 or n2 anywhere, they could be zero or even negative.

Answer 4

  a = malloc(n1 * sizeof(int));
  apt = &a[0];

  b = malloc(n1 * sizeof(int));
  apt = &b[0]; // <-- should be bpt, bug ?

Answer 5

It looks like from this, you are trying to create a linked list:

typedef struct cpl_def
{
  int A;
  int B;
  int OK; 
  struct cpls *link;      
} cpls;

But then, the way you use it, you are creating an array:

cplptr = (cpls *) malloc(num*sizeof(cpls) );
...
cplptr[j].a = a[j];

This makes it unclear to me what you are really trying to do. You never seem to initialise the link member either, so maybe you are trying to treat it like a linked list and walking off the end into hyperspace?

Other things that look weird to me from your code:

1. n1 and n2 never receive a value
2. you don't seem to use the apt variable for anything
3. a and b are never freed
4. you never check the return value from malloc, maybe you are failing an allocation?

None of this is really a "smoking gun" though, maybe you should try to narrow down your sample to something that works, but still causes the issue?

Answer 6

Set the flags

Before posting, consider setting the following flags: cc -Werror -Wall smth.c

smth.c: In function 'main':
smth.c:13: error: 'NULL' undeclared (first use in this function)
smth.c:13: error: (Each undeclared identifier is reported only once
smth.c:13: error: for each function it appears in.)
cc1: warnings being treated as errors
smth.c:29: warning: implicit declaration of function 'malloc'
smth.c:29: warning: incompatible implicit declaration of built-in function 'malloc'
smth.c:37: error: 'cplptr2' undeclared (first use in this function)
smth.c:37: warning: incompatible implicit declaration of built-in function 'malloc'
smth.c:53: error: 'struct cpl_def' has no member named 'a'
smth.c:54: error: 'struct cpl_def' has no member named 'b'
smth.c:57: warning: implicit declaration of function 'free'
smth.c:63: error: 'struct cpl_def' has no member named 'a'
smth.c:64: error: 'struct cpl_def' has no member named 'b'
smth.c:69: error: 'struct cpl_def' has no member named 'a'
smth.c:70: error: 'struct cpl_def' has no member named 'b'
smth.c:76:12: error: "/*" within comment
smth.c:77: warning: control reaches end of non-void function

Next thing what you need is a dynamic array Please structure your code! The functions to build such an array could be abstracted.

Do you read from right to left? No offense, but comments go before the code.

/* comment */
code

Don't be a star-programmer! If something needs more than one star, make a function.