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

[Pedro Alves <palves@redhat.com>]

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