2

I'm working on an existing c project (spglib on sourceforge), and I'm running into the following problem after cleaning up some array initializations:

* glibc detected * tests/spglibtest: free(): invalid next size (fast): 0x08ab46e0 ***

The backtrace is:

#0  0xb7fe1424 in __kernel_vsyscall ()
#1  0xb5cfdd61 in raise () from /lib/libc.so.6
#2  0xb5cff5ee in abort () from /lib/libc.so.6
#3  0xb5d397ed in ?? () from /lib/libc.so.6
#4  0xb5d3f7b1 in ?? () from /lib/libc.so.6
#5  0xb5d4052b in ?? () from /lib/libc.so.6
#6  0xb5d441cd in free () from /lib/libc.so.6
#7  0xb6681484 in sym_get_multiplicity (cell=0xbfffe1f0, symprec=0.050000000000000003) at /git/xtalopt-public/src/spglib/symmetry.c:168
#8  0xb6680550 in spg_find_primitive (lattice=0xbfffe2a8, position=0x813c6f0, types=0x813c700, num_atom=2, symprec=0.050000000000000003)
    at /git/xtalopt-public/src/spglib/spglib.c:253

The error is in the "free(trans)" line below:

int sym_get_multiplicity(const Cell *cell, const double symprec)
{
  int i, rc;
  double **trans;
  trans = (double**)malloc(cell->size * sizeof(double*));
  for (i = 0; i < cell->size; i++) {
    trans[i] = (double*)malloc(3 * sizeof(double));
  }

  rc = get_translation(&trans[0][0], identity, cell, symprec);

  for (i = 0; i < cell->size; i++) {
    free(trans[i]);
  }
  free(trans);

  return rc;
}

get_translation assigns values to trans like so:

static int get_translation(double trans[][3], const int rot[3][3], const Cell *cell,
                           const double symprec)
{
...
  for (j = 0; j < 3; j++) {
    trans[num_trans][j] = someDouble;
  }
...
}

Valgrind is showing the following when writing to the array in get_translation:

==17929== Invalid write of size 8
==17929==    at 0x56BE8A7: get_translation (symmetry.c:285)
==17929==    by 0x56BE44B: sym_get_multiplicity (symmetry.c:163)
...
==17929==  Address 0x9cb5868 is 0 bytes after a block of size 8 alloc'd
==17929==    at 0x4024918: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==17929==    by 0x56BE3F7: sym_get_multiplicity (symmetry.c:158)
....

This suggests to me that it's trying to write past the end of the allocated memory for trans, but it's writing to trans[0][0], and trans is of dimension [2][3]. This should work, AFAIK, can anyone see something that I'm missing?

dlonie
  • 575
  • 2
  • 7
  • 12
  • So while not an answer, trans isn't strictly the same as something defined like double trans[2][3]. In particular trans[0][2] and trans[1][0] aren't placed sequentially in memory in your example, something that is necessary true for something like dobule trans[2][3]. – Aurojit Panda Sep 15 '10 at 22:17
  • You should be getting an "argument passed from incompatible pointer type" warning on the call to `get_translation()` - I suggest turning up your compiler warning level. – caf Sep 15 '10 at 23:38
  • Really not sure why this was marked as a duplicate. The incredibly-generic error message is the same, but the underlying problem is quite different... – dlonie Jul 18 '14 at 16:51

2 Answers2

6

Your types are wrong, you can't pass a pointer to an array of pointers to a function expecting an array of arrays (i.e. a pointer to an array).

For the signature of get_translation that you have, you need:

double (*trans)[3] = malloc(cell->size * sizeof(double[3]));
CB Bailey
  • 755,051
  • 104
  • 632
  • 656
  • From a memory-management standpoint, this does make sense to me, and it works when I make the change -- Thanks! However, I'm have some trouble wrapping my head around the syntax in the declaration, and googling hasn't turned up much info on this. Can you recommend a reference on this technique? Thanks again! – dlonie Sep 15 '10 at 23:30
  • @dlonie: A couple of recent questions on this subject are: http://stackoverflow.com/questions/3707096/ and http://stackoverflow.com/questions/3706704/ . – CB Bailey Sep 16 '10 at 06:44
  • Ah, I see now. I was thrown off by seeing a pointer dereferenced in it's declaration -- rather unintuitive based on my c++ experience! The tip about the spiral rule helped clear this up. – dlonie Sep 16 '10 at 12:07
2

SO here is perhaps one problem. Your function seems to assume (based on) trans[num_trans][j] = someDouble; that trans is in fact an array laid out sequentially which as I mentioned above is not true in this case. You are allocating an array of pointers rather than a 2 dimensional array. Something similar to

double* trans = malloc(cell->size * 3); might be better. In general you might want to use a 1d array instead of a 2d one and just use it as 2 dimensional array.

Aurojit Panda
  • 909
  • 6
  • 11