Re: [patch] fixing leaking struct value* in py-value.c

[Phil Muldoon <pmuldoon at redhat dot com>]

On 12/10/2009 11:15 PM, Aman Gupta wrote:
> The gdb process will leak a large amount of memory (>512mb RSS) while
> executing the python loop. Some (but not all) of this memory is
> returned after the loop completes and free_all_values() is called for
> the next command. The following patch against archer-tromey-python
> keeps the memory usage constant during the loop.


On my tests, the consumption noted is far in excess of that, so good
find. 

> 
> diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
> index 14efd79..f305b01 100644
> --- a/gdb/python/py-value.c
> +++ b/gdb/python/py-value.c
> @@ -143,7 +143,7 @@ valpy_new (PyTypeObject *subtype, PyObject *args,
> PyObject *keywords)

Your mailer appears to be wrapping patches.  So this means the patch
command will complain of a malformed patch.  In this case it was
easily fixed in an editor, but a heads up in any case.


>  #define GDB_PY_HANDLE_EXCEPTION(Exception)				\
> +    GDB_PY_HANDLE_EXCEPTION_AND_CLEANUP(Exception, NULL)
> +
> +#define GDB_PY_HANDLE_EXCEPTION_AND_CLEANUP(Exception, mark)				\
>      do {								\
> -      if (Exception.reason < 0)						\
> +      if (Exception.reason < 0) {					\
> +        if (mark)						\
> +          value_free_to_mark (mark);						\
>  	return PyErr_Format (Exception.reason == RETURN_QUIT		\
>  			     ? PyExc_KeyboardInterrupt : PyExc_RuntimeError, \
>  			     "%s", Exception.message);			\
> +      }						\
>      } while (0)

When I try to compile against archer-tromey-python with:

../archer/configure && make CFLAGS="-O0 -g3"

I get a compile error:

cc1: warnings being treated as errors
../../archer/gdb/python/py-symbol.c: In function ‘gdbpy_lookup_symbol’:
../../archer/gdb/python/py-symbol.c:200: error: implicit declaration of function ‘value_free_to_mark’
make[2]: *** [py-symbol.o] Error 1

This is because GDB_PY_HANDLE_EXCEPTION is used in many sources files,
and value_free_to_mark is defined in value.{h|c}. And as you've
taken over the definition of GDB_PY_HANDLE_EXCEPTION the substitution
has some side-effects. So this needs to be fixed first.

Did you spot or note any regressions from a full test run by comparing
the gdb.sum files before and after the patch?  A note indicating the
platform/architecture of testing, and any regressions spotted would be
great, thanks.

Also, this patch needs a ChangeLog.

Cheers,

Phil