[PATCHv3 0/3] Restore thread and frame patches

[Andrew Burgess <andrew.burgess@embecosm.com>]

This is a third attempt to get some, or maybe all of this work merged.
Or to get some idea on what might be seen as an acceptable direction
to take this work.

I originally posted this here (v2):

  https://sourceware.org/pipermail/gdb-patches/2020-April/167984.html

and (v1):

  https://sourceware.org/pipermail/gdb-patches/2020-February/166202.html
  https://sourceware.org/pipermail/gdb-patches/2020-April/167215.html

Changes since V2:

 - Rebase to current master.

 - Fixed minor coding style issues, and improved a comment as pointed
   out by Baris.

 - Reordered the patches so #2 is now #1.  I think that current #1
   which is really a restructure might be worth merging even if #2 and
   #3 never get merged, hence placing it first.

 - A few minor tweaks to take acount of general code changes since v2.

There was some initial positive feedback on some of the v1 patches,
but Pedro was not convinced:

  https://sourceware.org/pipermail/gdb-patches/2020-April/167223.html

I'll quote his feedback here, and reply to it inline:

> Frankly, I'm not really sure I like this.  It seems like a can of worms to
> me...  It's going to cause us to have to decide whether it's a good idea to
> save/restore all kinds of state in the objects hierarchy.  E.g., if this is
> reasonable, then it would also be reasonable to restore the selected
> Ada task.  And frames.  And maybe the selected source file and line for
> list.  Once we gain support for fibers, coroutines, etc. we'll
> then need to apply the same logic.  Etc.  And then maybe we'll need
> some way to query the selected thread of a given inferior.  Etc.

This feedback was on v1 of the patch where I changed the default
behaviour to be restore thread/frame, since v2 the default is to
maintain GDB's current behaviour and have the restore be a feature a
user must turn on.

I think this addresses the concern Pedro raised as we no longer have
an inconsistent position, some things are restored while others are
not, instead we have a switch to allow somethings to be restored while
we lack a switch to allow other things to be restored.

While I agree with Pedro's original feedback that a user might be
confused, "why is XXX restored, but not YYY", now they will simply be
left wondering, "Why is there no switch to restore YYY?".  Though they
might wonder why the switch doesn't exist, and might be disapointed
even, I don't think it will leave them confused with the actual
behaviour of GDB.

Further, though "restoring stuff" as a broad category clearly covers
all the things Pedro mentions, restoring of each thing will require
its own piece of work.  I don't think we should prevent merging this
just because there might be some other (similar, but unrelated) part
of GDB that could also be saved and restored.

> 
> The current rule is quite simple.  If you select a object then its container
> is implicitly selected.  So if you select a thread, you implicitly select
> its inferior, and implicitly select its target.

OK, but that's looking upward, a frame is in a thread, a thread is in
an inferior, an inferior is in a target.  These changes are about lookig down.

>                                                   And the first/initial container
> object is selected.

I'm confused!  Above you talk about containers as looking upward
thread -> inferior -> target, but I think you're now talking about
things as looking down, in which case talking about containers seems
like poor terminology.

When we switch to an inferior then its containing target is
automatically selected, but there's only one of those to select.

>                        It seems very natural to me that "inferior 2" ends up
> selecting the initial thread of inferior 2.  I.e., normally, thread 2.1.

I'd certainly never want to suggest you're wrong for preferring a
particular behaviour.  For me though the choice of the first thread
seems pretty arbitrary, instead the idea of restoring the selected
object within a container seems more consistent.

However, I feel I've addressed this concern by making both of the
save/restore features being off by default.

Thoughts, or feedback on any or all of these patches is always
welcome.

Thanks,
Andrew

---

Andrew Burgess (3):
  gdb: unify two copies of restore_selected_frame
  gdb: Restore previously selected thread when switching inferior
  gdb: Track the current frame for each thread

 gdb/ChangeLog                                 |  53 +++
 gdb/NEWS                                      |  21 ++
 gdb/doc/ChangeLog                             |  14 +
 gdb/doc/gdb.texinfo                           |  42 ++-
 gdb/frame.c                                   |  84 +++++
 gdb/frame.h                                   |  85 +++++
 gdb/gdbthread.h                               |  15 +-
 gdb/inferior.c                                |  58 ++-
 gdb/inferior.h                                |  10 +
 gdb/infrun.c                                  |  25 +-
 gdb/testsuite/ChangeLog                       |  10 +
 .../gdb.threads/restore-selected-frame.c      |  85 +++++
 .../gdb.threads/restore-selected-frame.exp    | 336 ++++++++++++++++++
 gdb/testsuite/gdb.threads/restore-thread.c    | 248 +++++++++++++
 gdb/testsuite/gdb.threads/restore-thread.exp  | 219 ++++++++++++
 gdb/thread.c                                  | 128 +++----
 16 files changed, 1331 insertions(+), 102 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/restore-selected-frame.c
 create mode 100644 gdb/testsuite/gdb.threads/restore-selected-frame.exp
 create mode 100644 gdb/testsuite/gdb.threads/restore-thread.c
 create mode 100644 gdb/testsuite/gdb.threads/restore-thread.exp

-- 
2.25.4