[1/3] gdb: add inferior_target_stack_changed observer, use it to clear auxv cache

  A following patch expects to be able to read the core target's auxv
data right after pushing it to the inferior's target stack.  It is not
the case today, because the read requests would hit the auxv cache,
filled earlier using (empty) auxv data obtained from the exec target.
The auxv cache is only cleared a bit later in the core target
initialization, when the inferior_appeared observers are notified.

I think it would make sense to flush an inferiors auxv cache as soone as
its target stack is modified.  The auxv cache should reflect what
reading an inferior's auxv data would look like at any point in time,
and simply speed up that process.  In other words, it should work the
same with the cache as it would work without the cache.  It seems
obvious that pushing/popping a target from the stack may change what
reading auxv for that inferior would return, and that we should
therefore flush the cache at that point.

Add an inferior_target_stack_changed observable, and attach
invalidate_auxv_cache_inf to it.  Notify this observable in the
push_target and unpush_target methods of inferior.

Change-Id: I76b00cebe933e3b26620eda39f57425aaba928e5
---
 gdb/auxv.c       |  2 ++
 gdb/inferior.c   | 23 ++++++++++++++++++++++-
 gdb/inferior.h   |  9 ++-------
 gdb/observable.c |  1 +
 gdb/observable.h |  3 +++
 5 files changed, 30 insertions(+), 8 deletions(-)


base-commit: d0a2cfbd3141dae38498fa077b01ae6bb394462b
  

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> Add an inferior_target_stack_changed observable, and attach
Simon> invalidate_auxv_cache_inf to it.  Notify this observable in the
Simon> push_target and unpush_target methods of inferior.

This looks good to me, thanks.

It seems like overall it would be better to have the auxv cache just be
some member of 'inferior'.  Then these indirections and observers
wouldn't be needed -- the inferior could just manage it directly.

Tom
  

On 11/30/22 10:44, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> Add an inferior_target_stack_changed observable, and attach
> Simon> invalidate_auxv_cache_inf to it.  Notify this observable in the
> Simon> push_target and unpush_target methods of inferior.
> 
> This looks good to me, thanks.

Thanks, will push once the rest of the series is approved.

> It seems like overall it would be better to have the auxv cache just be
> some member of 'inferior'.  Then these indirections and observers
> wouldn't be needed -- the inferior could just manage it directly.

I kind of like observers, it gives the impression that things are
decoupled... but yeah in practice it just adds unnecessary layers of
indirection.

Simon
  

>> It seems like overall it would be better to have the auxv cache just be
>> some member of 'inferior'.  Then these indirections and observers
>> wouldn't be needed -- the inferior could just manage it directly.

Simon> I kind of like observers, it gives the impression that things are
Simon> decoupled... but yeah in practice it just adds unnecessary layers of
Simon> indirection.

I like them depending on the situation.  For me it's sort of like
attaching a registry entry.

If there are "optional" modules that need to attach data to a core data
structure (e.g., something arch-specific attaching to an objfile) then a
registry entry makes sense.  But for things that are inherit attributes
of an object, a registry entry would be sort of absurd.

I see observers the same way.  If two modules "should be" decoupled,
like if one is a core bit of gdb and another is some thing that might be
not compiled in, or only useful in some situations, then an observer is
a good way to convey state changes.

For auxv, IIUC, it's intended to always cache this data, which is
somewhat intrinsic to the inferior and the module must track some
aspects of the inferior's lifetime.  To me this says, just make some
auxv cache object as a field of inferior and let the inferior tell it
directly.

That said I wouldn't expect a change here since it's already written.
Just wouldn't object; and I guess it's worthwhile to try to articulate
the principle for new code.

thanks,
Tom