Sign in to follow this  
AlexB.hpp

[?++] Memory leak checker

Recommended Posts

AlexB.hpp    201
I've started working on simple and little class for C++ to check some memory leaks. I have made some work on it, but there is a lot of work to do. I'm planing to add cppunit, autoreconf, documentation, static class instread of global functions, different loggers.

Now it has:
shared library,
header with some funcs declarations,
makefile to build it via g++,
simple script to test it,
linux only but I'll port it to win after some researching.

I do it for my own project but I would like to share it with a great pleasure.

Now it looks like this
[CODE]
alexb leak_checker> ./run.sh
MEMORY LEAK! 0x8c1a008 main.cpp 5
MEMORY LEAK! 0x8c1a038 main.cpp 6
[/CODE]

with such main:
[source]
#include <iostream>
#include "allocation_hooks.h"
int main(int argc, char** argv) {
int* num = new int;
num = new int[1];
check_map();
}
[/source]

Repo is [url="https://github.com/AlexBolotsin/memcheck"]here[/url].

PS Warning it has some magic in it! ^_^ Edited by AlexB.hpp

Share this post


Link to post
Share on other sites
AlexB.hpp    201
[quote name='ApochPiQ' timestamp='1354056553' post='5004714']
[code]#include <malloc.h>
#include "allocation_hooks.h"

int main() {
void* foo = malloc(1024);
check_map(); // No leak reported.
}[/code]
[/quote]
Cause there is no malloc overloading. Will add to todo list. Thx

[url="https://github.com/AlexBolotsin/leak_check/issues/1"]1st issue[/url] added. Edited by AlexB.hpp

Share this post


Link to post
Share on other sites
SiCrane    11839
Don't forget calloc() and realloc(). realloc() is always a fun one to deal with because depending on how you call it, it can also act like malloc() or free().

Share this post


Link to post
Share on other sites
AlexB.hpp    201
[quote name='SiCrane' timestamp='1354058338' post='5004731']
Don't forget calloc() and realloc(). realloc() is always a fun one to deal with because depending on how you call it, it can also act like malloc() or free().
[/quote]
Thx for calloc don't even know about it.

Share this post


Link to post
Share on other sites
AlexB.hpp    201
Added malloc, calloc, realloc and free wrappers. Added gitignore.

I have a question about realloc. Does it reallocate of incoming pointer? Is it correct in my code? Edited by AlexB.hpp

Share this post


Link to post
Share on other sites
wqking    761
Good start.

But reporting leaks with only addresses is not quite helpful.
I would like to know the call stack and the class name, if possible.

Share this post


Link to post
Share on other sites
Dynamo_Maestro    769
I forgotten the method (since this technique ironically reports smart pointers as leaks heh) but MFC has a pretty decent way of checking for leaks already even if you arent making MFC apps, heres the link anyway http://msdn.microsoft.com/en-us/library/c99kz476(v=vs.100).aspx

Share this post


Link to post
Share on other sites
AlexB.hpp    201
[quote name='wqking' timestamp='1354096944' post='5004907'] Good start. But reporting leaks with only addresses is not quite helpful. I would like to know the call stack and the class name, if possible. [/quote]
I'm planning such output:
[CODE]
Memory leaks summary:
operator new in main.cpp:34 (0x0fa43d41) allocated 4 bytes
...
[/CODE]

What output would be enough informative for you?
[quote name='Dynamo_Maestro' timestamp='1354102278' post='5004932'] I forgotten the method (since this technique ironically reports smart pointers as leaks heh) but MFC has a pretty decent way of checking for leaks already even if you arent making MFC apps, heres the link anyway http://msdn.microsoft.com/en-us/library/c99kz476(v=vs.100).aspx [/quote]
I'm making it as cross-platform solution. But however thanks.

Share this post


Link to post
Share on other sites
SiCrane    11839
Your realloc() implementation is definitely wrong. As it is, it [i]creates[/i] a memory leak. If realloc() returns a new address, the old allocation needs to be freed. Also, the data copy doesn't properly handle the situation where the new size is greater than the old size.

Share this post


Link to post
Share on other sites
AlexB.hpp    201
[quote name='SiCrane' timestamp='1354113215' post='5004970']
Your realloc() implementation is definitely wrong. As it is, it [i]creates[/i] a memory leak. If realloc() returns a new address, the old allocation needs to be freed. Also, the data copy doesn't properly handle the situation where the new size is greater than the old size.
[/quote]
Many thanks about this issue with old ptr.
Yeah I knew about greater size then old ptr had. Will it be ok if i'll do memset(new_ptr, 0, size)?

Share this post


Link to post
Share on other sites
SiCrane    11839
Probably not, but I wouldn't know for sure unless I saw the complete context for your code. Either way it doesn't address the issue of properly copying the data in the buffer that realloc() is supposed to do.

Share this post


Link to post
Share on other sites
AlexB.hpp    201
[quote name='SiCrane' timestamp='1354124673' post='5005006']
Probably not, but I wouldn't know for sure unless I saw the complete context for your code. Either way it doesn't address the issue of properly copying the data in the buffer that realloc() is supposed to do.
[/quote]
To prevent all misunderstandings I've implemented it via original funcs.

Also I have improved output:
[CODE]
Memory leak summary:
operator new (0x83af008) missed 4 bytes in main.cpp:5
operator new[] (0x83af040) missed 4 bytes in main.cpp:6
malloc (0x83af078) missed 4 bytes in main.cpp:7
calloc (0x83af0b0) missed 4 bytes in main.cpp:8
realloc (0x83af0e8) missed 8 bytes in main.cpp:10
Total leaked 24 bytes
[/CODE]

Share this post


Link to post
Share on other sites
SiCrane    11839
That still doesn't handle realloc() properly. You aren't taking into account instances when size is equal to zero and you are also aren't handling null return values properly for what looks like any of the allocation functions. Your code also looks like it blows up messily if your header is included before code that uses placement new (which includes large chunks of the standard library).

Share this post


Link to post
Share on other sites
the_edd    2109
Some other problems:[list]
[*]void* operator new[] is missing a 'return'.
[*]Not exception safe (meaning that somewhat ironically, it can cause leaks).
[*]Causes problems with existing placement-new overloads (though working around that is probably very tricky).
[/list]

Share this post


Link to post
Share on other sites
AlexB.hpp    201
[quote name='SiCrane' timestamp='1354131396' post='5005045'] That still doesn't handle realloc() properly. You aren't taking into account instances when size is equal to zero and you are also aren't handling null return values properly for what looks like any of the allocation functions. Your code also looks like it blows up messily if your header is included before code that uses placement new (which includes large chunks of the standard library). [/quote]
Hm. I've read it already. Tomorrow I'll fix it. That bastard is really tricky, too many behaviors for just one func...
About including my header - yeah it has a bug here, I know about it. I'll describe it in docs. Have no ideas now. Thank you.

[quote name='e?dd' timestamp='1354134791' post='5005058'] Some other problems:[list] [*]void* operator new[] is missing a 'return'. [*]Not exception safe (meaning that somewhat ironically, it can cause leaks). [*]Causes problems with existing placement-new overloads (though working around that is probably very tricky). [/list] [/quote]
Many thanks. Still need some tests. It's in todo list.
About placement new... I'll keep it in mind.

Share this post


Link to post
Share on other sites
SiCrane    11839
I told you that realloc() was a fun one to deal with. As for the placement new problems, there aren't a lot of good options when you're avoiding platform dependent code. One approach used in Paul Nettle's mmgr is to create a header that undefined the macros in the memory manager. So there were a pair of headers, both without header guards, that could be used to toggle on and off the macros so it wouldn't screw up third party headers. I'm not a big fan of this technique. It's annoyingly manual and has the possibility of introducing ODR errors. More importantly, the information from the macros isn't that useful by itself. Sure it's better than nothing, but I'd much have a stack trace to see not only where the leaked allocation was done, but what called the function that leaked the allocation and so on.

Share this post


Link to post
Share on other sites
Hodgman    51223
[i]This isn't exactly helpful for the purposes of a general purpose leak-detector, so feel free to disregard[/i] [img]http://public.gamedev.net//public/style_emoticons/default/wink.png[/img]

The way that I've seen most engine code-bases deal with these issues, is to simply ban the use of standard allocation routines ([i]new/malloc/etc[/i]) as part of their coding guidelines. The only '[font=courier new,courier,monospace]new[/font]' that is allowed is placement-new, and it's probably never used directly, but via an engine macro.
If you want memory, you get it from the engine's allocation API, which becomes the single place where tracking code needs to be inserted.

Share this post


Link to post
Share on other sites
hupsilardee    491
[quote name='Hodgman' timestamp='1354157135' post='5005178']
[i]This isn't exactly helpful for the purposes of a general purpose leak-detector, so feel free to disregard[/i] [img]http://public.gamedev.net//public/style_emoticons/default/wink.png[/img]

The way that I've seen most engine code-bases deal with these issues, is to simply ban the use of standard allocation routines ([i]new/malloc/etc[/i]) as part of their coding guidelines. The only '[font=courier new,courier,monospace]new[/font]' that is allowed is placement-new, and it's probably never used directly, but via an engine macro.
If you want memory, you get it from the engine's allocation API, which becomes the single place where tracking code needs to be inserted.
[/quote]

Or you could make as many objects as possible exist/allocated on the stack, maybe using 2 step initialisation, or use a factory pattern?

Share this post


Link to post
Share on other sites
Matias Goldberg    9573
Your system won't work (& may crash) if the application is allocating memory from multiple threads. Writing a scalable memory tracker that handles multiple threads is your next real challenge.

[b]Edit: [/b]Also, you need to handle the nothrow variations of new & delete. There is a total of 4 news & 4 deletes to overload. Edited by Matias Goldberg

Share this post


Link to post
Share on other sites
dave    2187
[quote name='Hodgman' timestamp='1354157135' post='5005178']
[i]This isn't exactly helpful for the purposes of a general purpose leak-detector, so feel free to disregard[/i] [img]http://public.gamedev.net//public/style_emoticons/default/wink.png[/img]

The way that I've seen most engine code-bases deal with these issues, is to simply ban the use of standard allocation routines ([i]new/malloc/etc[/i]) as part of their coding guidelines. The only '[font=courier new,courier,monospace]new[/font]' that is allowed is placement-new, and it's probably never used directly, but via an engine macro.
If you want memory, you get it from the engine's allocation API, which becomes the single place where tracking code needs to be inserted.
[/quote]

I agree, reducing memory leaks is best prevented than fixed later. It's an architecture design really.

Share this post


Link to post
Share on other sites
AlexB.hpp    201
Damn it, I've stuck a little.

Meet cool bug when some containers of STL use my funcs and do stucking in infinite loop.

You know how it happens - you read about 1000 books of cpp and still doing this stupid mistakes in you code. :D

Share this post


Link to post
Share on other sites
SiCrane    11839
Well if you want help with that one you should probably post some code that demonstrates the problem; it doesn't seem like you've checked that into your repository. However, of the code that you have updated in your repository I noticed that you updated your realloc() implementation, but it still does't handle null returns from realloc() properly, and that you've added exception specifications on some of your functions. [url=http://www.gotw.ca/publications/mill22.htm]Using C++ exception specifications is a bad idea[/url].

Share this post


Link to post
Share on other sites
AlexB.hpp    201
Oh, many thanks again.

Looks like I've fixed the old one problem with infinite loop in stl. So... Actually I've done major work on it.

Ok I pretty sure that realloc is ok now.
It does;
[CODE]
new_ptr = realloc(old_ptr, size)
if (!new_ptr) return old_ptr
return new_ptr
[/CODE]

I'll read stuff about exceptions. And figure out how it should be.

I have funny stuff as one of my friends said. I have calls of delete on unregistered ptrs :)
[CODE]
Try to erase unregistered pointer 0x82f10f8!
Memory leak summary:
operator new (0x82f1008) missed 4 bytes in main.cpp:5
operator new [] (0x82f1040) missed 4 bytes in main.cpp:6
malloc (0x82f1078) missed 4 bytes in main.cpp:7
calloc (0x82f10b0) missed 4 bytes in main.cpp:8
realloc (0x82f10e8) missed 8 bytes in main.cpp:10
Total leaked 24 bytes
Try to erase unregistered pointer 0x82f10f8!
Try to erase unregistered pointer 0x82f10c0!
Try to erase unregistered pointer 0x82f1088!
Try to erase unregistered pointer 0x82f1050!
Try to erase unregistered pointer 0x82f1018!
[/CODE] Edited by AlexB.hpp

Share this post


Link to post
Share on other sites
SiCrane    11839
realloc() is still wrong. What happens when realloc() is given an existing buffer to expand, but there isn't enough room to expand it? It leaves the existing memory allocation alone and returns null. What happens when your function encounters that situation? It marks the memory block as freed and then returns a non-null pointer signalling that the resize succeeded. So not only do you have an allocated memory block that won't show up in your leak tracker, you've just told the user that his memory block is larger than it actually is, so he probably is going to write past the end of allocated memory.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this