This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA/commit] Memory leak in on reading frame register
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: gdb-patches at sourceware dot org, Jerome Guitton <guitton at adacore dot com>
- Date: Mon, 11 May 2015 13:53:12 -0700
- Subject: Re: [RFA/commit] Memory leak in on reading frame register
- Authentication-results: sourceware.org; auth=none
- References: <1431100524-7793-1-git-send-email-brobecker at adacore dot com> <55508A83 dot 3060605 at redhat dot com>
> > When using a conditional breakpoint where the condition evaluated
> > to false a large number of times before the program stopped,
> > a user reported that GDB's memory consumption was growing very
> > quickly until it ran out of memory.
> >
> > The problem was tracked down to temporary struct values being created
> > each time the program stops and we evaluate those conditions. This
> > patch fixes the issue by releasing the temporary values, and adds
> > a comment explaining why we do that.
> >
> > gdb/ChangeLog:
> >
> > Jerome Guitton <guitton@adacore.com>:
> > * findvar.c (read_frame_register_value): Fix a memory leak.
> >
> > Tested on x86_64-linux. No regression.
> >
>
> Not sure about this.
>
> How come this in bpstat_check_breakpoint_conditions didn't
> handle this issue already? :
>
> ...
> /* We use value_mark and value_free_to_mark because it could
> be a long time before we return to the command level and
> call free_all_values. We can't call free_all_values
> because we might be in the middle of evaluating a
> function call. */
> struct value *mark = value_mark ();
>
> ...
> value_free_to_mark (mark);
An excellent question, which I will try to research in the next
couple of days!
...
> Otherwise, what is releasing other kinds of temporary values?
> Are we leaking them? E.g., with:
>
> int global_val;
> void foo () {}
> int main () { while (1) foo (); }
>
> and then:
>
> (gdb) break foo if global_var == 1
>
> an/or:
>
> (gdb) break foo if (global_var + 1) == 2
>
>
> Maybe nothing breaks with this patch as its deleting register lval
> values, but the case above would involve lval_memory values,
> and if we did something for those like in this patch, I fear
> that places that want to walk an expression's value chain,
> like update_watchpoint / can_use_hardware_watchpoint would break.
But I confess I don't quite understand what you mean by the above.
Are you saying that the current patch may be OK (because we're
creating and deleting a value that we know is independent of all
other values), but that it sets a precendent for other forms where
it might not be OK?
--
Joel