25 replies to this topic

[sasho648]

So this actually is my first solution I made . It is c++ new\delete memory leaks finder. It does an memory check at the end of the program and if some object created with new wasn't deleted it post a message with the object line and source file name. It use an a tree for collecting runtime memory actions. Here is the .cpp and .h file. I will be happy for any opinion and advice about this solution.

MemoryHandler.cpp

#include <iostream>
#include <windows.h>
#include <stdio.h>

#ifdef DEBUG
struct MemoryAllocTree {
	char* File;
	unsigned int Line;
	MemoryAllocTree* next;
	void* MemoryPointer;
} * _gpAllocTree(0) ;
MemoryAllocTree** ppCurAllocTree(&_gpAllocTree);
unsigned int _gCurLine;
char* _gCurFile;
#endif

void* operator new(size_t size)
{
	void* pMem(malloc(size ? size : 1));
	if(!pMem)
	{
		#ifdef WIN32_MEM_LOG
		MessageBoxA(0, "No more memory to use. The app will exit now!", "Error!", MB_OK | MB_ICONERROR);
		#endif
		#ifndef WIN32_MEM_LOG
		std::cout << "No more memory to use. The app will exit now!" << "Error!" << std::endl;
		#endif
		exit(1);
	}
	#ifdef DEBUG
	(*ppCurAllocTree)=(MemoryAllocTree*)malloc(sizeof(MemoryAllocTree));
	if(!(*ppCurAllocTree))
	{
		#ifdef WIN32_MEM_LOG
		MessageBoxA(0, "No more memory to use for the debug version of MemoryHandler. Try compiling the app \
					 in release mode. It will exit now!", "Error!", MB_OK | MB_ICONERROR);
		#endif
		#ifndef WIN32_MEM_LOG
		std::cout << "No more memory to use for the debug version of MemoryHandler. Try compiling the app \
					 in release mode. It will exit now!" << "Error!" << std::endl;
		#endif
		exit(1);
	}
	(*ppCurAllocTree)->Line=_gCurLine;
	(*ppCurAllocTree)->File=_gCurFile;
	(*ppCurAllocTree)->MemoryPointer=pMem;
	(*ppCurAllocTree)->next=0;
	ppCurAllocTree=&(*ppCurAllocTree)->next;
	#endif
	return pMem;
}

void* operator new[](size_t size)
{
	return operator new(size);
}

void operator delete[](void* pMem)
{
	operator delete(pMem);
}

void operator delete(void* pMem)
{
	if(!pMem) return;
	#ifdef DEBUG
	MemoryAllocTree* sourc(_gpAllocTree);
	while(sourc&&sourc->MemoryPointer!=pMem)
		sourc=sourc->next;
	if(!sourc)
	{
		#ifdef DEBUG
		#ifdef WIN32_MEM_LOG
		char str[80];
		sprintf(str, "Invalid delete call at File: %s Line: %d", _gCurFile,
				_gCurLine);
		MessageBoxA(0, str, "Warning!", MB_OK | MB_ICONWARNING);
		#endif
		#ifndef WIN32_MEM_LOG
		std::cout << "Invalid delete call at File: " << _gCurFile << " Line: " << _gCurLine << " Warning!" << std::endl;
		#endif
		#endif
		return;
	}
	sourc->MemoryPointer=0;
	#endif
	free(pMem);
	return;
}

#ifdef DEBUG
void EndOfProgramMemCheck() {
	MemoryAllocTree* pCurAllocTree(_gpAllocTree);
	while(pCurAllocTree) {
		if(pCurAllocTree->MemoryPointer) {
			#ifndef WIN32_MEM_LOG
			std::cout << "Object at line: " << pCurAllocTree->Line << " in file: "
			<< pCurAllocTree->File << " wasn't deleted!" << std::endl;
			#endif
			#ifdef WIN32_MEM_LOG
			char str[80];
			sprintf(str, "Object at line: %d in file: %s wasn't deleted!", pCurAllocTree->Line,
					pCurAllocTree->File);
			MessageBoxA(0, str, "Memory leaked!", MB_OK | MB_ICONWARNING);
			#endif
		}
		MemoryAllocTree* cach(pCurAllocTree);
		pCurAllocTree=pCurAllocTree->next;
		free(cach);
	}
}
#endif

MemoryHandler.h

#pragma once

/*Sasho 648 Memory Handle functions and memory leaks finder
- define DEBUG to active it and add at the function EndOfProgramMemCheck()
before exit your app. Define a WIN32_MEM_LOG to active an Win32 Messages.
Type MEM_FUNC_CALL at the line begin where new\delete \[] function is called.
Example: MEM_FUNC_CALL char* ptr = new char[50];
*/

#ifdef DEBUG
extern unsigned int _gCurLine;
extern char* _gCurFile;
#endif

#ifdef DEBUG
#define MEM_FUNC_CALL _gCurLine=__LINE__; _gCurFile=(char*)__FILE__;
#endif

#ifndef DEBUG
#define MEM_FUNC_CALL
#endif

void* operator new(size_t size) ;
void* operator new[](size_t size) ;
void operator delete[](void* pMem) ;
void operator delete(void* pMem) ;

#ifdef DEBUG
void EndOfProgramMemCheck() ;
#endif

You need to make compiling debug define (DEBUG) at the compiler options and for win32 message posting you need to define WIN32_MEM_LOG. Before each new\delete call you need to type MEM_FUNC_CALL.

Here is a simple code using my solution:

class My {
public:
	My() {std::cout<<"My::My()"<<std::endl;}
	~My() {std::cout<<"My::~My()"<<std::endl;}
} ;

int main() {
	MEM_FUNC_CALL My* ptr=new My[23];
	MEM_FUNC_CALL delete[] ptr;
	MEM_FUNC_CALL delete ptr;
	size_t* My_Size_T=new size_t;
	My_Size_T=(size_t*)0xbff7ffff;
	delete My_Size_T;
	#ifdef DEBUG
	EndOfProgramMemCheck();
	#endif
	system("PAUSE");
	
	return 0;
}

Here will print warning messages for invalid delete call (2) and a memory leak.

Edited by sasho648, 10 September 2012 - 11:58 AM.

[M3Gamer]

Funny how i just read about comments and the next program i see hasnt got any i skimmed over i tbut i cant understand whats going on exactly(not because of the comments!) because im still going through the c++ documentation.

[fastcall22]

extern unsigned int _gCurLine;
extern char* _gCurFile;

#define MEM_FUNC_CALL _gCurLine=__LINE__; _gCurFile=(char*)__FILE__;

For instance, try the following code:

#include "MemoryHandler.h"
#include <iostream>

class A { };

class B {
public:
    B() { MEM_FUNC_CALL a = new A(); }
    ~B() { MEM_FUNC_CALL delete a; }

private:
   B( const B& );
   B& operator= ( const B& );

private:
	A* a;
};

int main( int, char*[] ) {
    B* b = 0;
    if ( false )
        MEM_FUNC_CALL b = new B;

    EndOfProgramMemCheck();
}

You can also override new and new[] to include additional parameters and use a macro to hijack all calls to new and delete to call your overloaded version, without needing to remember to use MEM_FUNC_CALL. (Though, you do need to make sure that the macros are defined in each compilation unit.) Take a look at a demonstration of memory leak detection for Visual Studio on MSDN. It should give you some ideas.

Also:
Those aren't trees, those are linked lists :|

Edited by fastcall22, 11 September 2012 - 10:51 AM.

[Cornstalks]

• Identifiers starting with an underscore in the global scope are reserved for the implementation, so I wouldn't prefix globals with "_". Namespaces are your friend.
• <stdio.h> is a C header. The C++ equivalent is <cstdio>. I'm actually not entirely sure why you include it.
• You should be reporting errors to std::cerr, not std::cout.
• Don't exit() when an allocation fails. Let the user determine if it's fatal. Throw a std::bad_alloc exception, like the normal new operator does.
• Any planned future support for multithreading (it'll wreck havoc with your current global state)?
• It would be useful if it didn't call any Windows specific functions so that it would be cross platform. Maybe output results/leaks to a log file?

Overall, it's a cool idea, and I think with some changes suggested by fastcall22 and me (and others who contribute), it could be a cool little project.

Edited by Cornstalks, 11 September 2012 - 10:33 AM.

[BinaryPhysics]

MessageBoxA(0, str, "Memory leaked!", MB_OK | MB_ICONWARNING);

You make this call multiple times. Don't. Just use 'MessageBox'. The reason that there's a macro defining the name is for character width (I think).

[Brother Bob]

MessageBoxA(0, str, "Memory leaked!", MB_OK | MB_ICONWARNING);

You make this call multiple times. Don't. Just use 'MessageBox'. The reason that there's a macro defining the name is for character width (I think).

MessageBoxA is technically the correct function to call for narrow width strings, which is what he has. If you use MessageBox you also have to, perhaps unnecessarily for cases like this, decorate the string to ensure that the string is actually compatible with the function that the macro expands to.

Not saying one or the other is better, just that the macro needs more support code, or unnecessary support code depending on your point of view, to function properly.

[sasho648]

Thanks for the advices!
@fastcall22 I try adding the new operator additional parameters but it have ambigouos meaning. Here is what I mean:
[source lang="cpp"]void* operator new(size_t size, const char* szFile=__FILE__, unsigned int nLineNo=__LINE__) _GLIBCXX_THROW (std::bad_alloc) __attribute__((__externally_visible__));[/source]

The error message is for the ambigouos function meaning (void* operator new(std::size_t) is the other). I tried using namespace to fix this but I think this is impossible.

[brx]

Actually, your new operator is amibouos because of the default parameters. It now has the same signature as the regular new operator (the first parameter is filled by the compiler). Leave out the default parameters and you'll be fine. Then use macros (yeah, I know... bad time ;) ) to replace all occurences of new with your version:

void* operator new(size_t size, const char* szFile, unsigned int nLineNo);
#define MY_NEW new(__FILE__, __LINE__)
#define new MY_NEW

Edited by brx, 20 September 2012 - 01:46 AM.

[sasho648]

Yeah I do this and fix the problem minutes ago (forgoting to edit the post) but the problem now is that I can't find I way to do the same with the delete operator:

#define delete delete ( __FILE__ , __LINE__ ) //for my own delete function
//void operator delete(void* pMem, const char* szFile, unsigned int nLineNo)

This define doesn't work. I need the line and file for this function because it will output an error when you wrong call the function.

[RulerOfNothing]

You need to get rid of the space between delete and (.

[sasho648]

You need to get rid of the space between delete and (.

This doesn't work for me. Can you give me an example code or explaining me more clearly.

Edited by sasho648, 20 September 2012 - 11:10 AM.

[fastcall22]

Just a shot in the dark, but it's possible that the macro is expanding indefinitely, since the macro definition contains a reference to itself. It may be why memory leak detection in Visual Studio uses two macros instead of the one:

#define DBG_NEW new ( _NORMAL_BLOCK, __FILE__, __LINE__ )
#define new DBG_NEW

So your delete may look something like:

#define DBG_DELETE delete ( __FILE__, __LINE__ )
#define delete DBG_DELETE

[sasho648]

This doesn't work.

[Brother Bob]

I believe that you can overload the delete operator to take additional parameters but it cannot be called except for one particular situation: when the constructor fails for an object allocated with a new taking multiple parameters, the corresponding delete with multiple parameters are called. Otherwise, a delete with custom parameter cannot be called.

Thus, if you overload delete like you do, the only way to have it called is if new succeeds but the constructor of the allocated object fails.

[sasho648]

Ok I found a way to pass the arguments in the delete operator by a global variables like this:

#define DELETE_OLD delete

#define delete 0; while(bIsDeleteParamsPassing); bIsDeleteParamsPassing=true; chFile=(char*)__FILE__; inLineNo=__LINE__; DELETE_OLD

Here is the final version:

https://rapidshare.com/files/476519966/MemoryHandler.cpp

https://rapidshare.com/files/493613952/MemoryHandler.hpp

and example for used this (cmd) in main.cpp

https://rapidshare.com/files/1234352939/main.cpp

Edited by sasho648, 21 September 2012 - 01:09 PM.

[Cornstalks]

Ugh... rapidshare...

Ok I found a way to pass the arguments in the delete operator by a global variables like this:

#define DELETE_OLD delete
#define delete 0; while(bIsDeleteParamsPassing); bIsDeleteParamsPassing=true; chFile=(char*)__FILE__; inLineNo=__LINE__; DELETE_OLD

Please don't.

#define DBG_DELETE delete(__FILE__, __LINE__)
#define delete DBG_DELETE

should work (edit: turns out C++ doesn't support such syntax). If it doesn't, people can help you fix it... you didn't post the actual error when you said "This doesn't work." If you do, we can help you figure out what's up with it.

If you want some reasoning why I say the above code is bad, all you have to do is look at bIsDeleteParamsPassing... is that supposed to be some kind of mutex? You realize it doesn't work to protect your code in a multithreaded environment, right? I'm also unsure of why the 0; is there...

And just a few more critiques on your code:

• Be consistent with the std namespace. Sometimes you just say size_t instead of std::size_t. You should always use the fully qualified std:: prefix in header files (and also in source files, unless you do using namespace std).
• If you're going to use global variables (which you shouldn't be), put them in a namespace
• Be consistent in your type names. Sometimes you use unsigned int, sometimes UINT
• Consider using proper types for booleans (i.e. use false instead of 0 when assigning to a bool)

Edited by Cornstalks, 26 September 2012 - 11:33 AM.

[sasho648]

Thank yoy very much for the critiques, this is exactly what I need because I learn c++ only from books and internet. The error behind "This doesn't work." is :
||=== Memw, Debug ===|
C:\Projects\Memw\main.cpp||In function 'int main()':|
C:\Projects\Memw\main.cpp|21|warning: left operand of comma operator has no effect [-Wunused-value]|
C:\Projects\Memw\main.cpp|21|error: type 'int' argument given to 'delete', expected pointer|
C:\Projects\Memw\main.cpp|21|error: expected ';' before 'my'|
||=== Build finished: 2 errors, 2 warnings ===|

at code:

using namespace std;
struct Some {
    Some() {std::cout<<"constructor" <<std::endl;}
    ~Some() {std::cout<<"destructor" <<std::endl;}
    unsigned long hash;
    unsigned long hash1;
} ;
int main()
{
    Some* my=new Some[2];
    cout << "Hello world!" << endl;
    delete my; //line 21
    #ifdef DEBUG
    EndOfProgramMemCheck();
    #endif
    system("PAUSE");
    return 0;
}

with defined

#define DBG_DELETE delete(__FILE__, __LINE__)
#define delete DBG_DELETE

This means that the delete define expect pointer, not function call.

At Last: Can you give me better idea for the multithreaded environment importing (from bIsDeleteParamsPassing)?

[sasho648]

So here is my last update of this solution. Now it really support multithreading ( I use __thread variables ). Here is the source:

MemoryHandler.cpp

#include <iostream>
#include <windows.h>
#include <cstring>
#include <cstdio>
struct MemoryAllocList {
	char* File;
	unsigned int Line;
	MemoryAllocList* next;
	void* MemoryPointer;
} ;
struct LineFile {
	unsigned int nLineNo;
	char* szFile;
} ;
MemoryAllocList* & GetMemoryList()
{
	static __thread MemoryAllocList* pAllocList(0);
	return pAllocList;
}
MemoryAllocList** & GetMemoryCurrPtrList()
{
	static __thread MemoryAllocList** ppCurAllocList(0);
	return ppCurAllocList;
}
LineFile & GetDeleteParams()
{
	static __thread LineFile Obj;
	return Obj;
}
void* operator new(size_t size, const char* szFile, unsigned int nLineNo) _GLIBCXX_THROW (std::bad_alloc)
{
	void* pMem;
	while(true) {
		pMem=malloc(size ? size : 1);
		if(!pMem)
		{
			#ifdef WIN32_MEM_LOG
			MessageBoxA(0, "No more memory to use!", "Warning!", MB_OK | MB_ICONWARNING);
			#endif
			#ifndef WIN32_MEM_LOG
			std::cerr << "No more memory to use!" << "Warning!" << std::endl;
			#endif
			std::new_handler cach=std::set_new_handler(0);
			if(cach) (*cach)();
			else throw std::bad_alloc();
			std::set_new_handler(cach);
		}
		else
			break;
	}
	MemoryAllocList** & ppCurAllocList=GetMemoryCurrPtrList();
	if(!ppCurAllocList) //Thread first new call
	{
		ppCurAllocList=&GetMemoryList();
	}
	while(*ppCurAllocList);
	(*ppCurAllocList)=(MemoryAllocList*)malloc(sizeof(MemoryAllocList));
	if(!(*ppCurAllocList))
	{
		#ifdef WIN32_MEM_LOG
		MessageBoxA(0, "No more memory to use for the debug version of MemoryHandler. Try compiling the app \
					 in release mode. It will exit now!", "Error!", MB_OK | MB_ICONERROR);
		#endif
		#ifndef WIN32_MEM_LOG
		std::cerr << "No more memory to use for the debug version of MemoryHandler. Try compiling the app \
					 in release mode. It will exit now!" << "Error!" << std::endl;
		#endif
		exit(1);
	}
	(*ppCurAllocList)->Line=nLineNo;
	(*ppCurAllocList)->File=(char*)szFile;
	(*ppCurAllocList)->MemoryPointer=pMem;
	(*ppCurAllocList)->next=0;
	ppCurAllocList=&(*ppCurAllocList)->next;
	return pMem;
}
void* operator new[](size_t size, const char* szFile, unsigned int nLineNo) _GLIBCXX_THROW (std::bad_alloc)
{
	return operator new(size, szFile, nLineNo);
}
void operator delete[](void* pMem) _GLIBCXX_USE_NOEXCEPT
{
	operator delete(pMem);
}
bool PassDeleteParams (const char* szFile, unsigned int nLineNo)
{
	LineFile & Obj(GetDeleteParams());
	Obj.nLineNo=nLineNo;
	Obj.szFile=(char*)szFile;
	return false;
}
void operator delete(void* pMem) _GLIBCXX_USE_NOEXCEPT
{
	LineFile CurrPos(GetDeleteParams());
	if(!pMem) return;
	MemoryAllocList* sourc(GetMemoryList());
	while(sourc&&sourc->MemoryPointer!=pMem)
		sourc=sourc->next;
	if(!sourc)
	{
		#ifdef WIN32_MEM_LOG
		char str[80];
		sprintf(str, "Invalid delete call at File: %s Line: %d", CurrPos.szFile,
				CurrPos.nLineNo);
		MessageBoxA(0, str, "Warning!", MB_OK | MB_ICONWARNING);
		#endif
		#ifndef WIN32_MEM_LOG
		std::cerr << "Invalid delete call at File: " << CurrPos.szFile << " Line: " << CurrPos.nLineNo << " Warning!" << std::endl;
		#endif
		return;
	}
	sourc->MemoryPointer=0;
	free(pMem);
	return;
}
void EndThreadMemCheck() {
	MemoryAllocList* pCurAllocTree(GetMemoryList());
	while(pCurAllocTree) {
		if(pCurAllocTree->MemoryPointer) {
			#ifndef WIN32_MEM_LOG
			std::cerr << "Object at line: " << pCurAllocTree->Line << " in file: "
			<< pCurAllocTree->File << " wasn't deleted!" << std::endl;
			#endif
			#ifdef WIN32_MEM_LOG
			char str[80];
			sprintf(str, "Object at line: %d in file: %s wasn't deleted!", pCurAllocTree->Line,
					pCurAllocTree->File);
			MessageBoxA(0, str, "Memory leaked!", MB_OK | MB_ICONWARNING);
			#endif
		}
		MemoryAllocList* cach(pCurAllocTree);
		pCurAllocTree=pCurAllocTree->next;
		free(cach);
	}
}

MemoryHandler.hpp

#pragma once
/*Sasho 648 Memory leaks and inavlid delete calls finder (MultiThread support)
Set the MemoryHandler.cpp file to compile only in debug mode
and include the MemoryHandler.hpp only when the DEBUG symbol is defined. Then
at the end of a thread call EndThreadMemCheck() too when the DEBUG symbol is defined.
Define WIN32_MEM_LOG for Windows Messages.
*/
void* operator new(size_t size, const char* szFile, unsigned int nLineNo) _GLIBCXX_THROW (std::bad_alloc);
void* operator new[](size_t size, const char* szFile, unsigned int nLineNo) _GLIBCXX_THROW (std::bad_alloc);
void operator delete[](void* pMem) _GLIBCXX_USE_NOEXCEPT;
void operator delete(void* pMem) _GLIBCXX_USE_NOEXCEPT;
bool PassDeleteParams (const char* szFile, unsigned int nLineNo);
#define new new ( __FILE__ , __LINE__ )
#define DEL_OLD delete
#define delete if(PassDeleteParams(__FILE__, __LINE__)); else DEL_OLD
void EndThreadMemCheck() ;

Edited by sasho648, 26 September 2012 - 11:07 AM.

[Brother Bob]

You should wrap any statement-like macro into an single-looping do-while loop to ensure that the macro uses the exact same syntax and code paths as what it's trying to replace (the delete-statement in this case).

Consider things like this with your current code:

if(obj->isDead())
    delete obj;
else
    obj->update();

The standard procedure to wrap macros that require a semicolon at the end is like this:

#define FOO do { /* code here, single or multiline-code */ } while(false)

What this macro does is forcing the user to end the macro call with a semicolon, just like you would have to do with the normal delete statement. Leaving out the semicolon after the macro call will produce a compiler error; the semicolon is not optional anymore as with regular macros. Furthermore, it does not introduce the possibility of an additional code path, and can therefore be used safely in nested constructs like for-loops and if-statements without introducing unexpected and unintended code paths.

An extra bonus is that the do-while loop has its own "private" scope for you to define variables in without affecting outside scope.

[brx]

Thank yoy very much for the critiques, this is exactly what I need because I learn c++ only from books and internet. The error behind "This doesn't work." is :
||=== Memw, Debug ===|
C:\Projects\Memw\main.cpp||In function 'int main()':|
C:\Projects\Memw\main.cpp|21|warning: left operand of comma operator has no effect [-Wunused-value]|
C:\Projects\Memw\main.cpp|21|error: type 'int' argument given to 'delete', expected pointer|
C:\Projects\Memw\main.cpp|21|error: expected ';' before 'my'|
||=== Build finished: 2 errors, 2 warnings ===|
[...]
with defined

#define DBG_DELETE delete(__FILE__, __LINE__)
#define delete DBG_DELETE

The first parameter for delete is a void*. That's what the message tells you. So the call to your overriden delete (which should probably look something like this:

void* operator delete(void*, const char* szFile, unsigned int nLineNo);

) you'd need to write

delete(myObject, __FILE__, __LINE__);

However your macro will do this:

delete myObject;
// will be expanded to
delete(__FILE__, __LINE__) myObject;

Apparently this is not correct.

I just checked, and even the

delete(myObject, __FILE__, __LINE__);

gives a compile error in visual studio ("Cannot delete objects that are no pointers") for whatever reason.

The code

delete(__FILE__, __LINE__, myObject);

Compiles but doesn't call my delete, but the default delete operator.

So anyway. Here's my suggestion:

#include <iostream>
class Foo {
public:
Foo() {}
};
void* operator new(size_t size, const char* f, unsigned int line) {
std::cout << "new: " << f << ":" << line << std::endl;
return malloc(size);
}
// just here to avoid the warning "'void *operator new(size_t,const char *,unsigned int)' : no matching operator delete found; memory will not be freed if initialization throws an exception"
void operator delete(void* ptr, const char* f, unsigned int line) {
std::cout << "delete: " << f << ":" << line << std::endl;
return free(ptr);
}
void Destroy(void* ptr, const char* f, unsigned int line) {
std::cout << "delete: " << f << ":" << line << std::endl;
return free(ptr);
}
#define DBG_NEW new(__FILE__, __LINE__)
#define new DBG_NEW
#define DBG_DELETE(p) Destroy(p, __FILE__, __LINE__)
#define delete(p) DBG_DELETE(p)
int main() {
Foo* myFoo = new Foo();
delete(myFoo);
return 0;
}