2

Why is this code for the computation of the inner product of two vectors yields a double free or corruption error, when compiled with:

ejspeiro@Eduardo-Alienware-14:~/Dropbox/HPC-Practices$ gcc --version
gcc (Ubuntu 4.8.4-2ubuntu1~14.04) 4.8.4

The code comes from this reference.

// Computation of the inner product of vectors aa and bb.

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

int main() {

  size_t nn = 100000000;
  size_t total_mem_array = nn*sizeof(double);

  double *aa;
  double *bb;
  double ss = 0.0;

  aa = (double *) malloc(total_mem_array);
  bb = (double *) malloc(total_mem_array);

  int ii = 0;

  for (ii = 0; ii < nn; ++ii) {
    aa[ii] = 1.0;
    bb[ii] = 1.0;
  }

  double sum1 = 0.0;
  double sum2 = 0.0;

  for (ii = 0; ii < nn/2 - 1; ++ii) {
    sum1 += (*(aa + 0))*(*(bb + 0));
    sum2 += (*(aa + 1))*(*(bb + 1));
    aa += 2;
    bb += 2;
  }
  ss = sum1 + sum2;

  free(aa);
  free(bb);

  return 0;
}
Cong Ma
  • 10,692
  • 3
  • 31
  • 47
Eduardo
  • 697
  • 8
  • 26
  • 3
    Once you do aa += ... your call to free() isn't getting the pointer that malloc() returned. Same for bb. – Joshua Taylor Jan 10 '16 at 01:45
  • 4
    Possible duplication of https://stackoverflow.com/questions/5414549/c-free-routine-and-incremented-array-pointers – Cong Ma Jan 10 '16 at 01:47

2 Answers2

6

The error is caused because the value passed to free() is not the same value returned by malloc(), as you increment aa and bb.

To correct it you could, for example, define two additional pointer variables that are used only for memory management, i.e. allocation and deallocation. Once memory is acquired by them, assign it to aa and bb.

Ziezi
  • 6,375
  • 3
  • 39
  • 49
  • 2
    Even better might be to make a single call to malloc and a single call to free. The memory for aa and bb could be in a single chunk. It's a micro optimization, but it could improve memory and would be two fewer library calls. – Joshua Taylor Jan 10 '16 at 02:10
0

You can simplify:

for (ii = 0; ii < nn/2 - 1; ++ii) {
    sum1 += (*(aa + 0))*(*(bb + 0));
    sum2 += (*(aa + 1))*(*(bb + 1));
    aa += 2;
    bb += 2;
}

to:

for (ii = 0; ii < nn/2 - 1; ++ii) {
    sum1 += aa[ii * 2]     * bb[ii * 2];
    sum2 += aa[ii * 2 + 1] * bb[ii * 2 + 1];
}

which has the dual benefits of avoiding incrementing your pointers which causes your problem, and making your code a whole lot clearer.

Crowman
  • 25,242
  • 5
  • 48
  • 56
  • 1
    To make the symmetry even clearer, I'd probably use `sum0 += aa[ii * 2 + 0] * bb[ii * 2+ 0]; sum1 += aa[ii * 2 + 1] * bb[ii * 2 + 1];` knowing that even the dumbest compiler with optimization turned off is unlikely to actually add 0. Note the renaming of the sum variables too. – Jonathan Leffler Jan 10 '16 at 03:04