Make scoped_restore_current_thread's cdtors exception free (RFC)

If the remote target closes while we're reading registers/memory for
restoring the selected frame in scoped_restore_current_thread's dtor,
the corresponding TARGET_CLOSE_ERROR error is swallowed by the
scoped_restore_current_thread's dtor, because letting exceptions
escape from a dtor is bad.  It isn't great to lose that errors like
that, though.  I've been thinking about how to avoid it, and I came up
with this patch.

The idea here is to make scoped_restore_current_thread's dtor do as
little as possible, to avoid any work that might throw in the first
place.  And to do that, instead of having the dtor call
restore_selected_frame, which re-finds the previously selected frame,
just record the frame_id/level of the desired selected frame, and have
get_selected_frame find the frame the next time it is called.  In
effect, this implements most of Cagney's suggestion, here:

  /* On demand, create the selected frame and then return it.  If the
     selected frame can not be created, this function prints then throws
     an error.  When MESSAGE is non-NULL, use it for the error message,
     otherwize use a generic error message.  */
  /* FIXME: cagney/2002-11-28: At present, when there is no selected
     frame, this function always returns the current (inner most) frame.
     It should instead, when a thread has previously had its frame
     selected (but not resumed) and the frame cache invalidated, find
     and then return that thread's previously selected frame.  */
  extern struct frame_info *get_selected_frame (const char *message);

The only thing missing to fully implement that would be to make
reinit_frame_cache just clear selected_frame instead of calling
select_frame(NULL), and the call select_frame(NULL) explicitly in the
places where we really wanted reinit_frame_cache to go back to the
current frame too.  That can done separately, though, I'm not
proposing to do that in this patch.

Note that this patch renames restore_selected_frame to
lookup_selected_frame, and adds a new restore_selected_frame function
that doesn't throw, to be paired with the also-new save_selected_frame
function.

There's a restore_selected_frame function in infrun.c that I think can
be replaced by the new one in frame.c.

Also done in this patch is make the get_selected_frame's parameter be
optional, so that we don't have to pass down nullptr explicitly all
over the place.

lookup_selected_frame should really move from thread.c to frame.c, but
I didn't do that here, just to avoid churn in the patch while it
collects comments.  I did make it extern and declared it in frame.h
already, preparing for the move.  I will do the move as a follow up
patch if people agree with this approach.

Incidentally, this patch alone would fix the crashes fixed by the
previous patches in the series, because with this,
scoped_restore_current_thread's constructor doesn't throw either.

gdb/ChangeLog:

	* blockframe.c (block_innermost_frame): Use get_selected_frame.
	* frame.c
	(scoped_restore_selected_frame::scoped_restore_selected_frame):
	Use save_selected_frame.  Save language as well.
	(scoped_restore_selected_frame::~scoped_restore_selected_frame):
	Use restore_selected_frame, and restore language as well.
	(selected_frame_id, selected_frame_level): New.
	(selected_frame): Update comments.
	(save_selected_frame, restore_selected_frame): New.
	(get_selected_frame): Use lookup_selected_frame.
	(get_selected_frame_if_set): Delete.
	(select_frame): Record selected_frame_level and selected_frame_id.
	* frame.h (scoped_restore_selected_frame) <m_level, m_lang>: New
	fields.
	(get_selected_frame): Make 'message' parameter optional.
	(get_selected_frame_if_set): Delete declaration.
	(select_frame): Update comments.
	(save_selected_frame, restore_selected_frame)
	(lookup_selected_frame): Declare.
	* gdbthread.h (scoped_restore_current_thread) <m_lang>: New field.
	* infrun.c (struct infcall_control_state) <selected_frame_level>:
	New field.
	(save_infcall_control_state): Use save_selected_frame.
	(restore_selected_frame): Delete.
	(restore_infcall_control_state): Use restore_selected_frame.
	* stack.c (select_frame_command_core, frame_command_core): Use
	get_selected_frame.
	* thread.c (restore_selected_frame): Rename to ...
	(lookup_selected_frame): ... this and make extern.  Select the
	current frame if the frame level is -1.
	(scoped_restore_current_thread::restore): Also restore the
	language.
	(scoped_restore_current_thread::~scoped_restore_current_thread):
	Don't try/catch.
	(scoped_restore_current_thread::scoped_restore_current_thread):
	Save the language as well.  Use save_selected_frame.

Change-Id: I73fd1cfc40d8513c28e5596383b7ecd8bcfe700f

1 files changed, 7 insertions, 33 deletions

diff --git a/gdb/infrun.c b/gdb/infrun.c
index e8c8fc0..b007af0 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -9003,8 +9003,10 @@ struct infcall_control_state
   enum stop_stack_kind stop_stack_dummy = STOP_NONE;
   int stopped_by_random_signal = 0;
 
-  /* ID if the selected frame when the inferior function call was made.  */
+  /* ID and level of the selected frame when the inferior function
+     call was made.  */
   struct frame_id selected_frame_id {};
+  int selected_frame_level = -1;
 };
 
 /* Save all of the information associated with the inferior<==>gdb
@@ -9033,27 +9035,12 @@ save_infcall_control_state ()
   inf_status->stop_stack_dummy = stop_stack_dummy;
   inf_status->stopped_by_random_signal = stopped_by_random_signal;
 
-  inf_status->selected_frame_id = get_frame_id (get_selected_frame (NULL));
+  save_selected_frame (&inf_status->selected_frame_id,
+		       &inf_status->selected_frame_level);
 
   return inf_status;
 }
 
-static void
-restore_selected_frame (const frame_id &fid)
-{
-  frame_info *frame = frame_find_by_id (fid);
-
-  /* If inf_status->selected_frame_id is NULL, there was no previously
-     selected frame.  */
-  if (frame == NULL)
-    {
-      warning (_("Unable to restore previously selected frame."));
-      return;
-    }
-
-  select_frame (frame);
-}
-
 /* Restore inferior session state to INF_STATUS.  */
 
 void
@@ -9081,21 +9068,8 @@ restore_infcall_control_state (struct infcall_control_state *inf_status)
 
   if (target_has_stack ())
     {
-      /* The point of the try/catch is that if the stack is clobbered,
-         walking the stack might encounter a garbage pointer and
-         error() trying to dereference it.  */
-      try
-	{
-	  restore_selected_frame (inf_status->selected_frame_id);
-	}
-      catch (const gdb_exception_error &ex)
-	{
-	  exception_fprintf (gdb_stderr, ex,
-			     "Unable to restore previously selected frame:\n");
-	  /* Error in restoring the selected frame.  Select the
-	     innermost frame.  */
-	  select_frame (get_current_frame ());
-	}
+      restore_selected_frame (inf_status->selected_frame_id,
+			      inf_status->selected_frame_level);
     }
 
   delete inf_status;