Memory heap errors

[darelet]

Hi all,
I've been trying to work on solving this myself but failed short of new ideas. I am experiencing memory heap errors ever since I tried to transfer some functions (for calculating statistics i.e. getMode/getMean/getVariance) into a new class in the spirit of OOP.

Compiler error is...
--> This may be due to a corruption of the heap, and indicates a bug in try.exe or any of the DLLs it has loaded.
The output window may have more diagnostic information

This then takes me to the malloc.c file.

Note...
Whenever I execute the functions within the main() function all goes well, but whenever I transfer them to a separate header file and put up some class wrappings then call them from main() using a class variable, then the above error surfaces.

Weird enough, the error occurs not when I declare a stats (which is the name of the class enclosing the functions, but down the line where my Gabor function is called.

I've researched this on the internet but the answers are too specific for my case. I wanted to continue with the initial error-free approach but realized I cannot build up my C++ proficiency with such style. I'm pasting some of the code below, and have tried to highlight the problem areas (please ignore everything else). Please assist...

int main(int argc, char *argv[])
{
	const int size = 256; 
	void saveMatrix(IplImage *g, dataStructure<uchar> *datastruct);
	void populateMatrix(dataStructure<uchar> *data, IplImage *g, int &Ex);
	int getMode(dataStructure<uchar> *data, int &Ef, int fx[]);
	float getMean(int Efx, int Ef);
	float getVariance(dataStructure<uchar> *data, int Ex, float mean);
	void printMatrix(dataStructure<uchar> *data, haralickStructure *haralick);
	void histogramEqualization(dataStructure<uchar> *data, int fx[]);
	

	dataStructure<uchar> *data = new dataStructure<uchar>; 
	glcmStructure *glcm = new glcmStructure;
	haralickStructure *haralik = new haralickStructure;
		int max=0, min=0, mode=0, Ex=0, fx[size], Ef=0; float mean=0, stdev=0;

		IplImage *a = cvLoadImage("c:\\tit\\lena.bmp");	// lena.bmp glcm.pgm baboon.tif syn.png
		IplImage *g = cvCreateImage(cvGetSize(a),a->depth,1);	// verts.JPG
		cvCvtColor(a,g,CV_RGB2GRAY);
		IplImage *b = cvCloneImage(g);
		cvReleaseImage(&a);

		populateMatrix(data, g, Ex);

		/*haralick *m=new haralick(data);
		m->calculateGLCM(0,1);
		m->calculateHaralick();*/
------------------------------------------------------------------------------------
		stats m(data);       <--- this is where the problem is, but shows up 
		m.getStatistics();   <--  later in Gabor
-------------------------------------------------------------------------------------
		mode = getMode(data, Ef, fx); // get mode
		mean = getMean(Ex, Ef); // get Mean
		stdev = (float) sqrt(getVariance(data,Ex,mean)); // standard deviation
		
		//histogramEqualization(data,fx);

		//saveMatrix(g, data);
		//stats s(data);
		//statistics<float> *r = s.getStatistics();
		
		//Ex = r->Efx;
		//mode = r->mode; mean=r->mean; stdev=r->stdev;
		Console::WriteLine(L"\n Grey-Level Features ...");
		Console::WriteLine(L"Sigma of x: \t\t" + Ex);
		Console::WriteLine("Max Grey: \t\t" + data->maxGrey);
		Console::WriteLine("Minimum Grey:\t\t" + data->minGrey);
		Console::WriteLine("Mode: \t\t\t" + mode);
		Console::WriteLine("Mean: \t\t\t" + mean);
		Console::WriteLine(L"Standard deviation: \t" + stdev);
		//printMatrix(data,haralik);
		dataStructure<uchar> *had;
--------------------------------------------------------------------------------
		Gabor ggb;                   <-- error shows up once this line executes
		had = ggb.getGabor2D(data);
---------------------------------------------------------------------------------

		//saveMatrix(g, data);
		 char *mn = "c:\\tit\\verts.JPG";
		myImageClass dn(mn); 
		//dn.setFileName("c:\\tit\\save.JPG"); dn.saveMatrix();
		//dn.setData(had);
		
		IplImage *ig = dn.getImage();
		//cvNamedWindow("dt2",1); cvShowImage("dt2",ig); 
		//cvWaitKey(0); cvDestroyAllWindows();
		
		system("pause");
		//cvReleaseImage(&g); g=0; ig=0; 
		data=0;
		return 0;
}

Next is the class definition

#pragma once
#include "datastructs.h"

typedef unsigned char classDataType;
typedef float statDataType;
class stats
{
public:
	stats(dataStructure<uchar> *datap);
	statistics<float> * getStatistics();

	// should be called in this order ... order is important
	void getMode();
	void getMean();
	void getVariance();
	statistics<statDataType> *statsData; // variable to hold calculated statistics
	dataStructure<uchar> *d_data;
	~stats(void);
};

I don't know what else you may need, please let me know. Thanks

[Fbody]

What is this "dataStructure" type that you are using? It appears to be a templated class...

Is it something that you defined? If so, how is it defined?

Is your class definition in your main file or in a separate header file?

[StuXYZ]

First off as Fbody points out correctly, you ideally will give us a bit more information particularly on dataStructure.

Second, a large number of "memory" errors come about because of memory freed too early. BUT the error only occurs after the memory is used for something else. This may be why is it only happening when you break the program up, because the compiler puts the memory in a different location, which then gets over written. Use a memory debugger, e.g. valgrind. http://valgrind.org and it will tell you the location. It will also tell you if you overrun an array boundary etc.

However, all of this is conjecture/guess work. So please post dataStructure, then we can try to solve the problem.

[darelet]

@Fbody: No, it is a template structure, defined in another header file that carries my data structures. The header file does not contain any classes, just structures and global constants. Here's the file (I'm currently only using unsigned char for T in all cases where I apply it)

#include "stdafx.h"
#ifndef datastructs_H
#define datastructs_H

#include "highgui.h"
#include "cv.h"
#include "cxcore.h"
#include <stdlib.h>
#include <stdio.h>
#include <math.h>
#include <iostream>

#define MAX_GREY 255
#define MIN_GREY 0

typedef unsigned char uchar;

	template<class T> struct dataStructure
	{
		int nRows, nCols; 
		T *data, maxGrey, minGrey;
	};

	struct 	glcmStructure{
		int size; float *glcm;
	};
	struct haralickStructure{
	// Contrast group:			contrast, dissimilarity, homogeneity, 
	// Orderliness measures:	ASM(angular second moment)/energy/uniformity, maximum probability, Entropy
	// Descriptive statistics:	Mean of i and j, Variance of i and j, Correlation
	
	float	CON,		DIS,		HOM, 
			ASM,		ENERGY,		MAX,	ENT, 
			MEAN_i,		MEAN_j,		VAR_i,	VAR_j,	STDEV_i, STDEV_j,	COR;			
	};


	template<class T> struct statistics {
		// the template just allows for double/single precision specification
		unsigned long Ef; // summation of frequencies
		// Next is mean --> standard deviation
		// Variance can always be contrived from the standard dev.
		T Efx, mean, mode, median, stdev;
		unsigned long * fx;
	};
	struct GaborStructure{
		float *G_real, *G_imag, *Gy,					// Real, imaginary components; 
			gamma, theta, lambda, bandwidth,			//gamma(aspect ratio), orientation angle and wavelength
			sigma_y, sigma_x,						// Vertical/Horizontal standard deviation
			x0, y0, phase,
			freq_y, freq_x;						// Vertical and horizontal frequency
		int y_filterHalfWidth, x_filterHalfWidth;			// Mask Radius
		int wavelength;
	};
	
  

#endif

@stuXYZ: I checked for memory being used after being freed, to the extent of eliminating pointer variables to no success. The Valgrind tool you recommended seems to only work with Linux and Mac, I'm using Visual studio 2008 for my project...

[Taywin]

From the look at the code, I believe you are working on computer vision project doing edge detection or such. Anyway, I would take a closer look at how you implement populateMatrix() which is called before your first error you found. The reason is that these functions could have changed or overwritten memory location while it is going through, and you may not realize it. It could be a hidden bug, so maybe you could show us how the function is written? Oh and how you implement constructor for stats class as well.

void saveMatrix(IplImage *g, dataStructure<uchar> *datastruct);
  void populateMatrix(dataStructure<uchar> *data, IplImage *g, int &Ex);
  int getMode(dataStructure<uchar> *data, int &Ef, int fx[]);
  float getMean(int Efx, int Ef);
  float getVariance(dataStructure<uchar> *data, int Ex, float mean);
  void printMatrix(dataStructure<uchar> *data, haralickStructure *haralick);
  void histogramEqualization(dataStructure<uchar> *data, int fx[]);
	

  dataStructure<uchar> *data = new dataStructure<uchar>; 
  glcmStructure *glcm = new glcmStructure;
  haralickStructure *haralik = new haralickStructure;
  int max=0, min=0, mode=0, Ex=0, fx[size], Ef=0; float mean=0, stdev=0;

  IplImage *a = cvLoadImage("c:\\tit\\lena.bmp");
  IplImage *g = cvCreateImage(cvGetSize(a),a->depth,1);

  cvCvtColor(a,g,CV_RGB2GRAY);    // convert 'a' to gray scale and save in 'g'
  IplImage *b = cvCloneImage(g);  // copy image 'g' to 'b'
  cvReleaseImage(&a);  // free memory of a

/************ take a closer look at function below *************/
  populateMatrix(data, g, Ex);
/************ take a closer look at function above *************/

  /*haralick *m=new haralick(data);
  m->calculateGLCM(0,1);
  m->calculateHaralick();*/

------------------------------------------------------------------------------------
		stats m(data);       <--- this is where the problem is, but shows up 
		m.getStatistics();   <--  later in Gabor
-------------------------------------------------------------------------------------

[darelet]

@Taywin: At the risk of cluttering up this thread, let me attach populateMatrix. I've gone through it again and seen nothing wrong. It uses an already declared global variable 'data' and I don't explicitly delete 'data' inside the function. I'm really suspecting the bug to be in 'stats' since I added a new class and had no problems. Let me include the 'stats' constructor as well. Thanks all for your effort

Next: populateMatrix

void populateMatrix(dataStructure<uchar> *data, IplImage *g, int &Ex)
{
	int rows, cols, step, chn;
		rows=data->nRows=g->height; cols=data->nCols=g->width; step=g->widthStep; chn=g->nChannels;

		data->data = new uchar [cols*rows];
		for (int i=0;i<cols*rows;i++) data->data[i] = (uchar) '0';

		data->maxGrey = data->minGrey = (uchar) g->imageData[0];

		for (int i=0;i<rows ;i++)
		{
			for (int j=0;j<cols;j++) 
			{
				data->data[i*cols+j] =  (uchar) g->imageData[i*step + j*chn];
				//Console::Write(" " + data->data[i*cols+j]);
				//std::cout << " int:" << (int) data->data[i*cols+j] << "   char:" << (uchar) data->data[i*cols+j];

				if (data->data[i*cols+j] > data->maxGrey) data->maxGrey=data->data[i*cols+j];
				if (data->data[i*cols+j] < data->minGrey) data->minGrey=data->data[i*cols+j];

				 //Calculate Sigma x (Ex)
				Ex += data->data[i*cols+j];				
			}
		}
}

Next: Constructor for stats...

stats::stats(dataStructure<uchar> *datap){
	if (datap == 0){
		statsData=0;
		d_data=0;
		return;
	}
	statsData = new statistics<statDataType>;
	statsData->Ef = (unsigned long) 0;
	statsData->Efx = statsData->mean = statsData->median = statsData->mode = statsData->stdev 
		= (statDataType) 0.0;

	d_data=new dataStructure<uchar>;
		d_data->nCols=datap->nCols; d_data->nRows=datap->nRows; d_data->maxGrey=datap->maxGrey;
		d_data->minGrey=datap->minGrey;
		int i=0;
		d_data->data = new uchar[d_data->nCols*d_data->nRows];
		while(i<(d_data->nCols*d_data->nRows)){
			d_data->data[i]=datap->data[i]; 
			i++;
			uchar n = datap->data[i];
			d_data->data[i]=n;
			uchar m = d_data->data[i];
			 
		}
}

[Taywin]

Thanks. I guess my C++ is rusty because I have not been using it for years. I agree that I see nothing wrong with your populateMatrix() function. I could be wrong here... I do not remember comparing a pointer with 0? Wouldn't it be 'null' instead?

Anyway I forgot to suggest you one thing in my earlier post. You could also implement a function printout() for your 'data' before and after the populateMatrix() and stats() to see whether or not your 'data' has changed. I suggested populateMatrix() because it is the initiated step to your 'data'. If there is something wrong with the 'data' as the earliest, it could be in there.

Also, sometimes, a function definition may look fine because everything happens inside is local, but after the function is called by reference (as your 'data' is a pointer), it may not correctly save values to your variables. As a result, the values disappear or change to something else.

[darelet]

@Taywin: Thanks for your prompt reply.
1. Conventionally, I normally instantiate my pointers to 0, as well as set them to 0 in case of a failure of some sort which allows me to check for occurrence of these two conditions. As far as comparing pointers with 0 or NULL, I thought it's the same since NULL is defined as 0 somewhere, though I'm not 100% sure and will consider this.

2. The values after populateMatrix are OK (after all populateM..() is the one filling in the values). I don't get beyond the 'stats' constructor call, it's where the runtime error is coming in.

I'm trying to alter some few things here and there. While I can escape with having the 3 statistic functions (getMode, etc) in the main() file, I'd rather anticipate your help as this will help me prevent similar problems in future.

[Taywin]

You are welcome.

Now, I am going to suggest my old way of debugging when something like this happen. To nail it down a little bit further, you could add a print out (such as std::cout << "1") with different print out value in between each line inside the constructor to see where exactly the code produces the error. This should narrow down where it is even further.

[Taywin]

Eek, forget to tell you another thing about doing the print out for each line. Please do not add any kind of that line in a loop. It would just confuse you instead of give you more info. What you should do when there is a loop is to print out some results you may want to see at the end of the loop. The constructor has the part that copies image data from one array to the other and that should not have print out inside. Sorry, my bad. :(

[Fbody]

If data is a global, why are you passing it in the first place? It should already exist. Also, because it's a global, the name "data" is globally-scoped. You can't reuse the name without causing conflicts.

I'm guessing that since you are creating a pointer named "data" as a parameter of populateMatrix() that it is causing the "arrow" operator to dereference to the wrong memory address. Either change the name of the parameter, or remove it...

[darelet]

Hi all again,
Please persevere with me through this lengthy explanation, maybe it's the best way to be exhaustive. I've been trying to implement your recommendations to no avail. What beats me completely is the shifty nature of these run-time errors. After commenting/uncommenting certain lines in a number of tries (guess-work to be honest), the compiler has thrown errors with reference to onexit.c and (notoriously) malloc.c; but all to do with the memory heap.

So as to be specific, I've decided to strip my code to its problematic skeleton, by commenting until an error is thrown. The code is attached below (the functions remain unchanged from what I posted earlier in this thread).

Explanation..
The following executes without error. However, the only commented line leads to an exception if I uncomment it. It being a mere declaration makes it useless in helping me diagnose the cause. After inserting break-points, I discovered that the error is being thrown after 'Gabor ggb;', and leading to _heap_alloc (size_t size) in malloc.c. d at the line 'return HeapAlloc(_crtheap, 0, size ? size : 1)'
For convenience, the error is: Windows has triggered a breakpoint in try.exe. This may be due to a corruption of the heap, and indicates a bug in try.exe or any of the DLLs it has loaded. The output window may have more diagnostic information
Heap corruption detected at 03E90478
HEAP[try.exe]: HEAP: Free Heap block 3e90470 modified at 3e90480 after it was freed

FYI: if also I comment 'stats m(data); m.getStatistics();' the error is not thrown. Thanks in advance

int main(int argc, char *argv[])
{
	const int size = 256; 
	void saveMatrix(IplImage *g, dataStructure<uchar> *datastruct);
	void populateMatrix(dataStructure<uchar> *dataP, IplImage *g, int &Ex);
	int getMode(dataStructure<uchar> *dataP, int &Ef, int fx[]);
	float getMean(int Efx, int Ef);
	float getVariance(dataStructure<uchar> *dataP, int Ex, float mean);
	void printMatrix(dataStructure<uchar> *dataP, haralickStructure *haralick);
	void histogramEqualization(dataStructure<uchar> *dataP, int fx[]);
	

	dataStructure<uchar> *data = new dataStructure<uchar>; 
	//glcmStructure *glcm = new glcmStructure; <-- problem after uncommenting this
	haralickStructure *haralik = new haralickStructure;
		int max=0, min=0, mode=0, Ex=0, fx[size], Ef=0; float mean=0, stdev=0;

		IplImage *a = cvLoadImage("c:\\tit\\lena.bmp");	
		IplImage *g = cvCreateImage(cvGetSize(a),a->depth,1);	
		cvCvtColor(a,g,CV_RGB2GRAY);
		IplImage *b = cvCloneImage(g);
		cvReleaseImage(&a);

		populateMatrix(data, g, Ex);

		stats m(data); // no error if I comment these two lines
		m.getStatistics();


		dataStructure<uchar> *had;
		Gabor ggb;
// error is being thrown after the immediate above line

		data=0;
		return 0;
}

[mitrmkar]

At least the stats constructor needs some attention, see the comments below.

<snip>

while(i<(d_data->nCols*d_data->nRows)){
  d_data->data[i]=datap->data[i]; 

  // 'i' gets incremented too early here
  i++;	

  // Upon last iteration; i == d_data->nCols*d_data->nRows, which is wrong

  uchar n = datap->data[i];

  // .. then, the damage is done here ...
  d_data->data[i]=n;

  uchar m = d_data->data[i];			 
}

[darelet]

Thanks for that,
Actually I had initially implemented it as a for loop but changed it to what it is out of desperation (and to check that the values were being copied to the new structure). Well, I've changed it back to this, but it hasn't changed anything.

for(int i=0; i<(d_data->nCols*d_data->nRows); i++)
  {
     d_data->data[i]=datap->data[i]; 
  }

Anything else?

[darelet]

Just wondering if I'll finally have to dig out the contents of 03E90478, 3e90470 and 3e90480 for analysis as indicated by the compiler. I'm not any good in assembly programming stuff.

[mitrmkar]

Perhaps rather try _CrtCheckMemory() to narrow down the problematic part(s).
I.e. in the simplest form of its usage, your code might look like ..

#include <crtdbg.h>
...
// Is the memory still good?
_CrtCheckMemory();
populateMatrix(data, g, Ex);
// Is the memory still good?
_CrtCheckMemory();
stats m(data);
// Is the memory still good?
_CrtCheckMemory();
m.getStatistics();
// Is the memory still good?
_CrtCheckMemory();
...

Run your (Debug build) program in the debugger, when _CrtCheckMemory() detects corruption, by default it displays an Assertion Failure message box of some sort - at that point, hopefully you'll be enlightened.

[Taywin]

Nice catch mitrmkar. Didn't see that. :(

I doubt that you would get much out of assembly if you do not know how your compiler actually did to your code. What I mean is that each compiler may produce different assembly code; besides, code optimization may cause the code to be much different than your own code.

Have you tried to put more comments inside your stat constructor as I suggested earlier to see where it breaks (see code comment below)? You could group the print out to narrow it down.

stats::stats(dataStructure<uchar> *datap) {
  if (datap == 0) {
    statsData=0;
    d_data=0;
    return;
  }

  // add print out to console here 1
  statsData = new statistics<statDataType>;
  statsData->Ef = (unsigned long) 0;
  statsData->Efx = statsData->mean = statsData->median = statsData->mode = statsData->stdev = (statDataType) 0.0;
  // add print out to console here 2

  d_data=new dataStructure<uchar>;
  d_data->nCols=datap->nCols; d_data->nRows=datap->nRows;
  d_data->maxGrey=datap->maxGrey;
  d_data->minGrey=datap->minGrey;
  d_data->data = new uchar[d_data->nCols*d_data->nRows];
  // add print out to console here 3

  for(int i=0; i<(d_data->nCols*d_data->nRows); i++) {
     d_data->data[i]=datap->data[i]; 
  }
  // add print out to console here 4
}

[darelet]

Thanks to all participants for your great input. I traced the error to the getMode function in stats.h. (Look at the code below) I solved by setting a fixed histogram size of 256 in line 6. In a sample run, the Max-Min method was giving me 245(max value)-25 (min image pixel value in lena.jpg), leading to an array size of 221, so the highest index would be 220. But then the highest pixel value being 245 means the histogram index would at some point hit 245, which would be out-of-range with respect to 220, the highest index permitted.

void stats::getMode()
{
	classDataType mode=0; 
	int r = d_data->nRows, c = d_data->nCols;
	int size /*= (d_data->maxGrey-d_data->minGrey)+1; size*/ = 256;
	statsData->fx = new unsigned long [size];
	for (int i=0; i<size; i++) 
		statsData->fx[i]=0;
	
	for(int i=0;i<r;i++) 
		for(int j=0;j<c;j++)
		{
			uchar glevel = d_data->data[i*c+j];
			statsData->fx[glevel] += 1;// fx[] = UnNormalized Grey level Histogram
		}

		unsigned long f = 0;
		for (int i=0; i<size; i++)
		{
			if (f < statsData->fx[i]) 
			{
				f = statsData->fx[i]; mode = i;
			}
			statsData->Ef += statsData->fx[i];
		}	
		statsData->mode = mode;
}

What I don't understand up to this point is:
1. Why couldn't the compiler throw and immediately highlight whenever an index beyond 220 was accessed.
2. Why was it running OK when within the main() function as opposed to independently in the stats class?
3. Why it was working well with (comparatively) smaller images?

I guess these questions could as well be rhetoric, although I would mind someone furnishing me with the answers. In the meantime, I'll try my hands on the _crtcheckmemory() as memory will be a big issue in my image processing project. Otherwise, I would consider this thread resolved.

Thanks all..

[mitrmkar]

>> 1. Why couldn't the compiler throw and immediately highlight whenever an index beyond 220 was accessed.
Your compiler does not generate code for validating each and every memory read/write - you just have to be very careful.

>> >> Why it was working well with (comparatively) smaller images?
What did d_data->maxGrey-d_data->minGrey yield with those particular images?

>> Why was it running OK when within the main() function as opposed to independently in the stats class?
When you are corrupting memory, generally you might think of it as 'all-bets-are-off'.
You were lucky because the Debug heap management kicked in and spotted errors.

Note that if e.g. your memory block indices are 'suitably off', you may actually be writing to another (valid, allocated) block, silently corrupting data - this is hard to detect and the debug heap management routines are of no help here.

PS. Maybe consider using std::vector s instead of dynamic allocation?