Re: [RFA/commit] Memory leak in on reading frame register

Pedro Alves
2015-05-11T10:55:08Z

On 05/08/2015 04:55 PM, Joel Brobecker wrote:
> [On behalf of Jerome Guitton]
>
> 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);
...

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.

Thanks,
Pedro Alves