This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] completer.c (expression_completer): Stop memory leak.
- From: Michael Snyder <msnyder at vmware dot com>
- To: Pedro Alves <pedro at codesourcery dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Wed, 09 Mar 2011 12:29:22 -0800
- Subject: Re: [RFA] completer.c (expression_completer): Stop memory leak.
- References: <4D77CDD6.7010700@vmware.com> <201103092007.33388.pedro@codesourcery.com>
Pedro Alves wrote:
On Wednesday 09 March 2011 18:58:30, Michael Snyder wrote:
In this case, it is possible for fieldname to be allocated before an
exception is thrown.
OK?
Notice how `fieldname' is uninitialized by expression_completer.
If an exception is thrown from within parse_field_expression
before writting to `fieldname', you'll be calling `free'
(it should be xfree, btw) on an uninitialized pointer. That's
bad.
Please fix this within parse_field_expression itself.
1) even if what I describe above can't happen as is
today (it may or not, dunno), your change makes the
code quite fragile. 2) any other parse_field_expression call
that isn't wrapped in a TRY_CATCH like this, is a
potential leak.
OK how is this? I'm a little uncertain about the way
"name" is handled there at the end...
2011-03-09 Michael Snyder <msnyder@vmware.com>
* parse.c (parse_field_expression): Clean up memory gracefullly.
Index: parse.c
===================================================================
RCS file: /cvs/src/src/gdb/parse.c,v
retrieving revision 1.108
diff -u -p -r1.108 parse.c
--- parse.c 5 Mar 2011 22:02:47 -0000 1.108
+++ parse.c 9 Mar 2011 20:27:08 -0000
@@ -1211,34 +1211,37 @@ parse_field_expression (char *string, ch
struct value *val;
int subexp;
volatile struct gdb_exception except;
+ struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
TRY_CATCH (except, RETURN_MASK_ERROR)
{
in_parse_field = 1;
exp = parse_exp_in_context (&string, 0, 0, 0, &subexp);
+ make_cleanup (xfree, exp);
}
in_parse_field = 0;
if (except.reason < 0 || ! exp)
return NULL;
if (expout_last_struct == -1)
{
- xfree (exp);
+ do_cleanups (cleanups);
return NULL;
}
*name = extract_field_op (exp, &subexp);
if (!*name)
{
- xfree (exp);
+ do_cleanups (cleanups);
return NULL;
}
+ make_cleanup (xfree, name);
/* This might throw an exception. If so, we want to let it
propagate. */
val = evaluate_subexpression_type (exp, subexp);
/* (*NAME) is a part of the EXP memory block freed below. */
*name = xstrdup (*name);
- xfree (exp);
+ do_cleanups (cleanups);
return value_type (val);
}