[RFA] Fix leaks by clearing registers and frame caches.

  Valgrind reports leaks such as the below in the tests:
gdb.threads/corethreads.exp
gdb.threads/gcore-thread.exp
gdb.ada/task_switch_in_core.exp
gdb.trace/tfile.exp
gdb.base/siginfo-thread.exp

==12701== 1,123 (72 direct, 1,051 indirect) bytes in 1 blocks are definitely lost in loss record 2,928 of 3,247
==12701==    at 0x4C2C4CC: operator new(unsigned long) (vg_replace_malloc.c:344)
==12701==    by 0x5CF771: get_thread_arch_aspace_regcache(ptid_t, gdbarch*, address_space*) (regcache.c:330)
==12701==    by 0x5CF92A: get_thread_regcache (regcache.c:366)
==12701==    by 0x5CF92A: get_current_regcache() (regcache.c:372)
==12701==    by 0x4C7964: get_current_frame() (frame.c:1587)
==12701==    by 0x4C7A3C: get_selected_frame(char const*) (frame.c:1651)
==12701==    by 0x669EAD: print_thread_info_1(ui_out*, char const*, int, int, int) (thread.c:1151)
==12701==    by 0x66A9A1: info_threads_command(char const*, int) (thread.c:1217)
==12701==    by 0x40A878: cmd_func(cmd_list_element*, char const*, int) (cli-decode.c:1892)
...

Fix these leaks by clearing registers and frame caches.
This leak and fix is similar to the leak fixed by 799efbe8e01
---
 gdb/ChangeLog  | 5 +++++
 gdb/corelow.c  | 2 ++
 gdb/inferior.c | 4 ++++
 3 files changed, 11 insertions(+)
  

Philippe Waroquiers <philippe.waroquiers@skynet.be> wrote:

> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index 3ce612d31b..9fcc2d2372 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -535,6 +535,8 @@ core_target::detach (inferior *inf, int from_tty)
>       'this'.  */
>    unpush_target (this);
>  
> +  /* Clear the register cache and the frame cache.  */
> +  registers_changed ();

I'm wondering if it might be better to call

  registers_changed_ptid (ptid_t (inf->pid));

instead?  (I haven't checked to see if this compiles, but hopefully you get
the idea.)

That way, we only touch the register cache for the inferior being
detached.

>    reinit_frame_cache ();
>    maybe_say_no_core_file_now (from_tty);
>  }
> diff --git a/gdb/inferior.c b/gdb/inferior.c
> index e7b49376e9..abfef700c3 100644
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -208,6 +208,10 @@ exit_inferior_1 (struct inferior *inftoex, int silent)
>    inf->pending_detach = 0;
>    /* Reset it.  */
>    inf->control = inferior_control_state (NO_STOP_QUIETLY);
> +
> +  /* Clear the register cache and the frame cache.  */
> +  registers_changed ();

Likewise here.

> +  reinit_frame_cache ();
>  }

It seems okay to me otherwise...

Kevin
  

On Thu, 2019-05-02 at 18:32 -0700, Kevin Buettner wrote:
> Philippe Waroquiers <philippe.waroquiers@skynet.be> wrote:
> 
> > diff --git a/gdb/corelow.c b/gdb/corelow.c
> > index 3ce612d31b..9fcc2d2372 100644
> > --- a/gdb/corelow.c
> > +++ b/gdb/corelow.c
> > @@ -535,6 +535,8 @@ core_target::detach (inferior *inf, int from_tty)
> >       'this'.  */
> >    unpush_target (this);
> >  
> > +  /* Clear the register cache and the frame cache.  */
> > +  registers_changed ();
> 
> I'm wondering if it might be better to call
> 
>   registers_changed_ptid (ptid_t (inf->pid));
> 
> instead?  (I haven't checked to see if this compiles, but hopefully you get
> the idea.)
> 
> That way, we only touch the register cache for the inferior being
> detached.
Yes, I contemplated doing that.
But the detach code is somewhat tricky, as it is not clear up to
what point the 'inf' and/or inf->pid stays valid + the detach
method is target dependent and can do whatever.
(see e.g. the comment about the 'this' that becomes dangling).

So, it looked safer to me to just clear the register and framecache
when an inferior exits and/or detach, as the efficiency for such
operation is not critical (and there are already many places that
are doing that when attaching/detaching/forking/...).


> 
> >    reinit_frame_cache ();
> >    maybe_say_no_core_file_now (from_tty);
> >  }
> > diff --git a/gdb/inferior.c b/gdb/inferior.c
> > index e7b49376e9..abfef700c3 100644
> > --- a/gdb/inferior.c
> > +++ b/gdb/inferior.c
> > @@ -208,6 +208,10 @@ exit_inferior_1 (struct inferior *inftoex, int silent)
> >    inf->pending_detach = 0;
> >    /* Reset it.  */
> >    inf->control = inferior_control_state (NO_STOP_QUIETLY);
> > +
> > +  /* Clear the register cache and the frame cache.  */
> > +  registers_changed ();
> 
> Likewise here.
> 
> > +  reinit_frame_cache ();
> >  }
> 
> It seems okay to me otherwise...
Thanks for the review, I haved pushed the fix with the 'global clear'
(for the reasons explained above) but if there is a need to
have a more 'precise clear', just tell me, and I can rework this
in a follow up.

Philippe