This is the mail archive of the
mailing list for the GDB project.
Re: [patch] Memory leak fixes
On Tue, Oct 5, 2010 at 3:47 PM, Tom Tromey <firstname.lastname@example.org> wrote:
> Ali> diff -ru gdb-7.1/gdb/dwarf2read.c gdb-7.1/gdb/dwarf2read.c
> Ali> --- gdb-7.1/gdb/dwarf2read.c ? ? ? 2010-09-14 16:59:40.000000000 -0700
> Ali> +++ gdb-7.1/gdb/dwarf2read.c ? ? ? 2010-06-15 14:53:54.000000000 -0700
> Ali> @@ -5462,6 +5462,7 @@
> Ali> ? ?set_descriptive_type (type, die, cu);
> Ali> ? ?do_cleanups (back_to);
> Ali> + ?xfree (range_types);
> This one seems to be fixed in CVS.
> There is a cleanup covering range_types.
Are you referring to the call to make_cleanup() ? That gets called
only if ndim == 0 and I can't really figure out what it is supposed to
do with the arg other than saving it in a cleanup struct and returning
> Actually, I see this cleanup in 7.1 as well.
> How did you find this leak?
> Maybe the logic around creating the cleanup is wrong.
I found the leak using a static checker. It is known to have false
> Ali> diff -ru gdb-7.1/gdb/printcmd.c gdb-7.1/gdb/printcmd.c
> Ali> --- gdb-7.1/gdb/printcmd.c 2010-09-14 17:03:28.000000000 -0700
> Ali> +++ gdb-7.1/gdb/printcmd.c 2010-06-15 14:53:54.000000000 -0700
> Ali> @@ -608,6 +608,7 @@
> Ali> ? ? ?fputs_filtered (">", stream);
> Ali> ? ?do_cleanups (cleanup_chain);
> Ali> + ?xfree (filename);
> Ali> ?}
> This one is also covered by a cleanup.
I don't see any cleanup calls. filename gets allocated by
build_address_symbolic() and nothing is freeing it as far as I can
> Ali> diff -ru gdb-7.1/gdb/python/py-type.c gdb-7.1/gdb/python/py-type.c
> Ali> --- gdb-7.1/gdb/python/py-type.c ? 2010-09-14 17:03:28.000000000 -0700
> Ali> +++ gdb-7.1/gdb/python/py-type.c ? 2010-06-14 14:04:28.000000000 -0700
> Ali> @@ -580,6 +580,7 @@
> Ali> ? ? ? ?type_print (type_object_to_type (self), "", stb, -1);
> Ali> + ? ? ?xfree (thetype);
> Ali> ? ? ? ?thetype = ui_file_xstrdup (stb, &length);
> Ali> ? ? ? ?do_cleanups (old_chain);
> I didn't check this one, but if you want to insert a free before a call
> to do_cleanups, usually that means you should create a cleanup at the
> point of allocation.
The allocation is happing at the very next line at: ui_file_xstrdup()
and the variable is used after the loop (I'm assuming TRY_CATCH macro
can loop). So, it was safer to free to memory before the leak happens
rather than after allocation,