Heap corruption in MFC program

Question

I am having a problem with a heap corruption issue in a program. In the program I am reading a block of data and performing FFT and IFFT on it. I am doing it for 2 images, master and slave. The EXACT same code works fine for the master but shows a heap corruption for the slave file when I try to delete the slave buffer.

fcomplex is defined as:

typedef struct {float real, imag;}fcomplex;

A snippet of relevant parts of the code is attached: Full code: http://sharetext.org/7xXe

The error does not occur if I do not call the fft and ifft functions for the slave image. (Everything works fine for the master though)

To debug the error I installed Application verifier but I was not able to decode the log file. Its here: http://sharetext.org/Y2ji (XML file copy pasted)

The error visual studio give is: Heap corruption Detected: after normal block (#194456) at 0x062C0040

CCoarseFun::fcomplex * slave_bfr;
CCoarseFun::fcomplex * slave_col;


slave_bfr = Pcoarse.init_1Dcmplx(SIZE*s_cols); 
slave_col = Pcoarse.init_1Dcmplx(SIZE);

Pcoarse.cfft1d_(&SIZE,slave_col,&FFTdir); // This function causes a problem
Pcoarse.complex_mult_col(filter, slave_col, SIZE, slave_col)
Pcoarse.cfft1d_(&SIZE,slave_col,&FFTdir); // As does this one

// delete memory related to slave
delete [] slave_bfr;    // Heap corruption here
delete [] slave_col;

What is baffleing me is that the code is pretty simple and it works 100% for only the master files. Why is it crashing for the slave?

Can some one guide me to a solution or maybe a tutorial on how to use the Application verifier as well?

Thanks, Shaunak

EDIT: Using Win7 x64 - VS2010

EDIT 2: Definition for init_1Dcmplx

CCoarseFun::fcomplex* CCoarseFun::init_1Dcmplx(int n)
{
  fcomplex *a;
  a=new fcomplex[n];
  for(int i=0;i<n;i++)
  {
    a[i].real=float(0.0);
    a[i].imag=float(0.0);
  }
  return a;
}

EDIT3: COde for cfft1D_ : http://sharetext.org/hzIg

EDIT4: Code for mem.delfloat()

void CMemAlloc::del_float(float *a)
{
  if (a!=NULL)
  {
    delete[] a;
    a=NULL;
  }
  else
  {
    return;
  }
}

How is `init_1Dcmplx` defined? Does it use `new ...[]` to allocate the structures? — nneonneo, Aug 28 '12 at 07:38
BTW, why does `cfft1d_()` take pointers to integers? They seem to be just input parameters. — Lubo Antonov, Aug 28 '12 at 08:17
@LuboAntonov EDIT 4 - if(a!=NULL) { delete[] a; a=NULL; } else { return; } — shaunakde, Aug 28 '12 at 08:17
@LuboAntonov - The fft functions were written by some one else. The documentation only said that it took pointers. I cant explain why. — shaunakde, Aug 28 '12 at 08:18
Can you post the actual definition? The parameter declarations are important for this one. — Lubo Antonov, Aug 28 '12 at 08:18
@LuboAntonov - void cfft1d_(int *np, fcomplex *c, int *dir); — shaunakde, Aug 28 '12 at 08:19
@LuboAntonov void del_float(float* a); - Thanks for trying to help BTW. Im very badly stuck — shaunakde, Aug 28 '12 at 08:22
Well, del_float is not quite correct. It is deleting the array alright, but it is not setting the pointer back to NULL, like it seems. That's because it is operating on a copy of the pointer. But the pointer is not used after, so I don't think that's the ultimate cause. — Lubo Antonov, Aug 28 '12 at 08:25
@LuboAntonov Should i simply replace that with a delete statement instead? Please do post an answer, so if its correct, I can credit you for it. :) — shaunakde, Aug 28 '12 at 08:26
You can try, in any case it needs to be fixed - but I think it is something else. — Lubo Antonov, Aug 28 '12 at 08:28
@LuboAntonov - replacing it with a delete [] statement causes no ill effects I can perceive, so I did it. Doesnt solve the problem though — shaunakde, Aug 28 '12 at 08:31
Not solving your problem, but maybe of interest for you: You can always delete a null ptr, `delete` allows this (other than `free`), so no need to check it. And setting a to `NULL` in `del_float` is only setting a local variable, which is destroyed upon return, so has no real effect. — Werner Henze, Aug 28 '12 at 08:48
@WernerHenze - Thanks for the info. As you can see im very new to this and to top that, the old code was written by someone apparently as new as I am. Every bit of info helps. — shaunakde, Aug 28 '12 at 08:50
cfft1d_ doesn't need to take np and dir as pointers. they should just be 'int's. Every time you use a pointer unnecessarily, a fairy dies ;-) — Roddy, Aug 28 '12 at 08:58

Lubo Antonov · Accepted Answer · 2012-08-28T08:39:52.560

1

The mem_float() function is not correct. It looks like it is setting the pointer to NULL after the delete, but it is only working on a copy of the pointer, so the caller's copy is still pointing to deleted memory block.

You can just do

delete [] cf;
cf = NULL;

You have a couple of lines that look like this:

four1(cf-1,nn,isign);

I think this is accessing memory before the beginning of the array.

Beyond this, the indexing inside four1() is insanely complicated - you will have to step through it with the debugger to check the edge cases.

edited Aug 28 '12 at 08:39

answered Aug 28 '12 at 08:30

Lubo Antonov

2,301
14
18

I will look into its behavior. This function however works perfectly well the first call, for the master file though. And thank you so much for your continued help. – shaunakde Aug 28 '12 at 08:35
Nevermind, I was wrong about the indexing, I've removed that part. – Lubo Antonov Aug 28 '12 at 08:36
I am look to see what changes I can do to that function. I do not see the problem though: The function initializes cf for 2*n values [say 32]. "i" will go till 30, so the index 31 will be valid. The constraint on n is that it must be a power of 2, so the code will be valid IMO. edit: Sorry ! did not see your update – shaunakde Aug 28 '12 at 08:42
Sorry! I typed that as you posted the edit! Il look into the new issue. F11'ing :) – shaunakde Aug 28 '12 at 08:42
Changing that too four1(cf,nn,isign) gives a new heap error. But I think this solution is correct, and at-least points me in the right direction. I may not be able to debug the fft code just yet. Can I work around it in some way? – shaunakde Aug 28 '12 at 08:49
Well, just changing the `cf-1` parameter will create problems in the `four1()` function (it is now probably accessing beyond the end of cf), which is why I was suggesting you debug it. You can also figure it out with pen and paper, since you know the actual values for the parameters. – Lubo Antonov Aug 28 '12 at 08:50
Inside the function - for (i=1;i – shaunakde Aug 28 '12 at 08:56

score 0 · Answer 2 · edited May 23 '17 at 11:56

Firstly it's important to understand what the heap corruption error is telling you. When you run a Debug build in Visual Studio it uses the Debug version of the run-time library which has a debug heap. Whenever you allocate some memory with new there are some extra guard bytes either side of it. When you delete it the debug heap checks that those guard bytes are intact, and if not then it issues this warning.

Assuming that Pcoarse.init_1Dcmplx() allocates memory on the heap then the two calls to it will most likely allocate memory one after the other:

XXXXXX - guard bytes
slave_bfr
XYXYXY - these are the guard bytes that are probably being corrupted
slave_cols
XXXXXX

You perform the operation Pcoarse.cfft1d_() on slave_cols but then you get the heap corruption error when deleting slave_bfr. This suggests that cfft1d_() is overwriting memory before the start of slave_cols so that it's corrupting the guard bytes of slave_bfr.

So I would look through your code for cfft1d_() for places where you might have negative array indexes as this could cause the memory in slave_bfr to be stomped on.

Also check some of the SO answers with useful tips on how to make the most of the debug heap, in particular this one about how to enable more intensive memory checks.

Thanks for the informative post. I am trying to debug this via the links you have provided and suggestions from others on this page — shaunakde, Aug 28 '12 at 08:51

score 0 · Answer 3 · answered Aug 28 '12 at 08:55

0

"Heap corruption Detected: after normal block (#194456) at 0x062C0040" says that you allocated a block, got the block #194456 with address 0x062C0040 and you wrote more memory bytes than you allocated. So you have a classical buffer overflow. You should consider replacing pointers with STL containers. In your case I'ld prefer using std::vector over a raw float array allocated with new float[]. The STL containers can help you detect writing beyond boundaries immediately at the wrong access, not only after deleting the memory block.

answered Aug 28 '12 at 08:55

Werner Henze

16,404
12
44
69

I personally would have liked to, since im not very comfortable with pointers / raw arrays, but all the old code uses that structure, inclusive of functions that i need to call like fft etc. Maybe I can try allocating a bigger block - or my allocation size is wrong, because code runs flawlessly for master. – shaunakde Aug 28 '12 at 09:18
If i replace slave_bfr = Pcoarse.init_1Dcmplx(SIZE*s_cols) with slave_bfr = Pcoarse.init_1Dcmplx(SIZE*s_cols+1) it does not give me any errors. But I have a 30mb chunk left after the delete statement. – shaunakde Aug 28 '12 at 09:46

Heap corruption in MFC program

3 Answers3