On 08/02/2013 09:18 PM, Mathieu Desnoyers wrote:
> Please have a look at my implementation of quick and dirty malloc/free
> tracker here:
>
> https://github.com/efficios/memleak-finder
>
> file: memleak-finder.c
>
> Implementing a work-around for calloc vs dlsym was indeed tricky. The
> solution I got there works fine so far (static array used for this
> special-case). I think we might get the free() case to be more solid by
> checking if the address being freed is in the static buffer range.

I played a little with this code (e.g. by removing the "constructor"
attribute from the do_init() function, to increase the chances that I
could see static_calloc() being used in my own code).
I found one bug (where the static buffer size adjustment was wrong, so
the second time static_calloc was called a pointer pointing into the
previous buffer would be returned).
And I did observe a crash without the appropriate adjustment to free().

> If you could submit a patch that improves memleak-finder.c to get this
> right, it would be very much appreciated.

Please find attached a small patch for this. As you can see, if the
pointer falls into the range of the static buffer, free() does nothing.
("freeing" the buffer properly would require much more work,
and arguably falls outside the scope of what is needed here, since
static_calloc() is really only required during startup, and thus should
never run out of space.


> Then, I recommend porting the code from memleak-finder.c to UST
> liblttng-ust-libc-wrapper. The approach I used in memleak-finder.c seems
> to work quite well, and is clearly more complete that what we find in
> liblttng-ust-libc-wrapper currently.

Yes, with the adjusted free() implementation as per this patch, I think
this is a robust solution. Do you think we need any form of multi-thread
protection ? Or can we assume that library initialization will have
completed by the time multiple threads could possibly call calloc() ?

    Stefan

-- 
Stefan Seefeld
CodeSourcery / Mentor Graphics
http://www.mentor.com/embedded-software/

>From 858ae5805f3bea303edfcec8156494be2fa3580d Mon Sep 17 00:00:00 2001
From: Stefan Seefeld <[email protected]>
Date: Mon, 5 Aug 2013 10:54:24 -0400
Subject: [PATCH] Fix typo and robustify free() implementation.

Signed-off-by: Stefan Seefeld <[email protected]>
---
 memleak-finder.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/memleak-finder.c b/memleak-finder.c
index 2f8b996..d3755ff 100644
--- a/memleak-finder.c
+++ b/memleak-finder.c
@@ -160,7 +160,7 @@ void *static_calloc(size_t nmemb, size_t size)
 	if (nmemb * size > sizeof(static_calloc_buf) - static_calloc_len)
 		return NULL;
 	prev_len = static_calloc_len;
-	static_calloc_len += nmemb + size;
+	static_calloc_len += nmemb * size;
 	return &static_calloc_buf[prev_len];
 }
 
@@ -341,6 +341,15 @@ free(void *ptr)
 {
 	const void *caller = __builtin_return_address(0);
 
+	/* Check whether the memory was allocated with
+	 * static_calloc, in which case there is nothing
+	 * to free.
+	 */
+	if ((char*)ptr >= static_calloc_buf &&
+	    (char*)ptr < static_calloc_buf + STATIC_CALLOC_LEN) {
+		return;
+	}
+
 	do_init();
 
 	if (thread_in_hook) {
-- 
1.8.3.1

_______________________________________________
lttng-dev mailing list
[email protected]
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Reply via email to