[PATCHv3 1/3] gdb: unify two copies of restore_selected_frame
Andrew Burgess
andrew.burgess@embecosm.com
Fri Sep 25 22:24:07 GMT 2020
GDB currently has two copies of restore_selected_frame, this commit
unifies these two, and moves the implementation into frame.c.
The only possible user visible change after this commit is if GDB is
unable to restore the currently selected stack after performing an
inferior call.
Previously, in any situation, if GDB failed to find a frame with a
matching frame id, then the following warning would be issued, and GDB
would be left with the innermost stack frame selected:
Unable to restore previously selected frame.
After this commit, then GDB will give a slightly different warning:
Couldn't restore frame #%d in current thread. Bottom (innermost) frame selected:
....
Where '....' is a summary of the frame that was selected, and '%d' is
the frame level GDB was looking for. Additionally, this warning is
not given if the previously selected frame was at level 0, in that
case GDB will just silently selected the innermost frame.
If this difference is a problem then it should be easy enough to have
restore_selected_frame take an 'always-warn' flag parameter, but I
haven't done that in this commit.
Beyond this there should be no other user visible changes with this
commit.
gdb/ChangeLog:
* frame.c (restore_selected_frame): Moved from thread.c.
* frame.h (restore_selected_frame): Declare.
(struct frame_id_and_level): New struct.
* gdbthread.h (scoped_restore_current_thread)
<m_selected_frame_id>: Delete. <m_selected_frame_level>: Delete.
<m_selected_frame_info>: New member variable.
* infrun.c (infcall_control_state) <selected_frame_id>: Delete.
<selected_frame_info>: New member variable.
(save_infcall_control_state): Update to use selected_frame_info.
(restore_selected_frame): Delete.
(restore_infcall_control_state): Update to use selected_frame_info.
* thread.c (restore_selected_frame): Delete.
(scoped_restore_current_thread::restore): Update for changes to
member variables.
(scoped_restore_current_thread::scoped_restore_current_thread):
Likewise.
---
gdb/ChangeLog | 20 ++++++++++++
gdb/frame.c | 58 +++++++++++++++++++++++++++++++++
gdb/frame.h | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
gdb/gdbthread.h | 3 +-
gdb/infrun.c | 25 +++------------
gdb/thread.c | 67 ++------------------------------------
6 files changed, 171 insertions(+), 87 deletions(-)
diff --git a/gdb/frame.c b/gdb/frame.c
index 0b708e66827..eba383f9877 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2972,6 +2972,64 @@ frame_prepare_for_sniffer (struct frame_info *frame,
frame->unwind = unwind;
}
+/* See frame.h. */
+
+void
+restore_selected_frame (frame_id a_frame_id, int frame_level)
+{
+ /* This means there was no selected frame. */
+ if (frame_level == -1)
+ {
+ select_frame (NULL);
+ return;
+ }
+
+ gdb_assert (frame_level >= 0);
+
+ /* Restore by level first, check if the frame id is the same as
+ expected. If that fails, try restoring by frame id. If that
+ fails, nothing to do, just warn the user. */
+
+ int count = frame_level;
+ frame_info *frame = find_relative_frame (get_current_frame (), &count);
+ if (count == 0
+ && frame != NULL
+ /* The frame ids must match - either both valid or both outer_frame_id.
+ The latter case is not failsafe, but since it's highly unlikely
+ the search by level finds the wrong frame, it's 99.9(9)% of
+ the time (for all practical purposes) safe. */
+ && frame_id_eq (get_frame_id (frame), a_frame_id))
+ {
+ /* Cool, all is fine. */
+ select_frame (frame);
+ return;
+ }
+
+ frame = frame_find_by_id (a_frame_id);
+ if (frame != NULL)
+ {
+ /* Cool, refound it. */
+ select_frame (frame);
+ return;
+ }
+
+ /* Nothing else to do, the frame layout really changed. Select the
+ innermost stack frame. */
+ select_frame (get_current_frame ());
+
+ /* Warn the user. */
+ if (frame_level > 0 && !current_uiout->is_mi_like_p ())
+ {
+ warning (_("Couldn't restore frame #%d in "
+ "current thread. Bottom (innermost) frame selected:"),
+ frame_level);
+ /* For MI, we should probably have a notification about
+ current frame change. But this error is not very
+ likely, so don't bother for now. */
+ print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
+ }
+}
+
static struct cmd_list_element *set_backtrace_cmdlist;
static struct cmd_list_element *show_backtrace_cmdlist;
diff --git a/gdb/frame.h b/gdb/frame.h
index 3ceb7b32eff..0e668f00be8 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -960,5 +960,90 @@ extern void set_frame_previous_pc_masked (struct frame_info *frame);
extern bool get_frame_pc_masked (const struct frame_info *frame);
+/* When GDB needs to backup and then later restore the currently selected
+ frame this is done by storing the frame id, and then looking up a frame
+ with that stored frame id.
+
+ However, if the previously selected frame can't be restored then GDB
+ should give the user a warning in most cases. If the previously
+ selected frame was level 0 then GDB will just reselect the innermost
+ frame silently without a warning.
+
+ And so, when we backup and restore the currently selected frame we need
+ to track both the frame id, and the frame level, so GDB knows if a
+ warning should be given or not. */
+
+struct frame_id_and_level
+{
+ /* Setup this structure to track FI as the previously selected frame.
+ Any errors thrown while collecting the frame_id or the frame level
+ are rethrown from this function. */
+
+ void reset (struct frame_info *fi)
+ {
+ try
+ {
+ m_id = get_frame_id (fi);
+ m_level = frame_relative_level (fi);
+ }
+ catch (const gdb_exception_error &ex)
+ {
+ /* Place the object into a known state. */
+ m_id = null_frame_id;
+ m_level = -1;
+
+ /* And rethrow the exception. */
+ throw;
+ }
+ }
+
+ /* Reset this structure to indicate there was no previously selected
+ frame. */
+ void reset ()
+ {
+ m_id = null_frame_id;
+ m_level = -1;
+ }
+
+ /* The frame id of the previously selected frame. This value is only
+ defined when LEVEL() is greater than -1. */
+ frame_id id () const
+ {
+ return m_id;
+ }
+
+ /* The level of the previously selected frame, or -1 if no frame was
+ previously selected. */
+ int level () const
+ {
+ return m_level;
+ }
+
+private:
+ /* The frame id. */
+ frame_id m_id;
+
+ /* The level at which ID was found. Set to -1 to indicate that this
+ structure is uninitialised. */
+ int m_level = -1;
+};
+
+/* Try to find a previously selected frame described by A_FRAME_ID and
+ select it. If no matching frame can be found then the innermost frame
+ will be selected and a warning printed. FRAME_LEVEL is the level at
+ which A_FRAME_ID was previously found, and can be -1 to indicate no
+ frame was previously selected, in which case the innermost frame will
+ be selected (without a warning). */
+
+extern void restore_selected_frame (struct frame_id a_frame_id, int frame_level);
+
+/* A wrapper that unpacks a frame_id_and_level and calls the version of
+ restore_selected_frame above. */
+
+inline void
+restore_selected_frame (const frame_id_and_level &fal)
+{
+ restore_selected_frame (fal.id (), fal.level ());
+}
#endif /* !defined (FRAME_H) */
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index ab5771fdb47..a814227b378 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -670,8 +670,7 @@ class scoped_restore_current_thread
thread_info_ref m_thread;
inferior_ref m_inf;
- frame_id m_selected_frame_id;
- int m_selected_frame_level;
+ frame_id_and_level m_selected_frame_info;
bool m_was_stopped;
};
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 6532b06ae52..b1069ea7f76 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -9002,8 +9002,9 @@ 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. */
- struct frame_id selected_frame_id {};
+ /* ID and level of the selected frame when the inferior function call was
+ made. */
+ struct frame_id_and_level selected_frame_info;
};
/* Save all of the information associated with the inferior<==>gdb
@@ -9032,27 +9033,11 @@ 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));
+ inf_status->selected_frame_info.reset (get_selected_frame (NULL));
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
@@ -9085,7 +9070,7 @@ restore_infcall_control_state (struct infcall_control_state *inf_status)
error() trying to dereference it. */
try
{
- restore_selected_frame (inf_status->selected_frame_id);
+ restore_selected_frame (inf_status->selected_frame_info);
}
catch (const gdb_exception_error &ex)
{
diff --git a/gdb/thread.c b/gdb/thread.c
index 0217f3b54f7..9124fbf56d2 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1325,65 +1325,6 @@ switch_to_thread (process_stratum_target *proc_target, ptid_t ptid)
switch_to_thread (thr);
}
-static void
-restore_selected_frame (struct frame_id a_frame_id, int frame_level)
-{
- struct frame_info *frame = NULL;
- int count;
-
- /* This means there was no selected frame. */
- if (frame_level == -1)
- {
- select_frame (NULL);
- return;
- }
-
- gdb_assert (frame_level >= 0);
-
- /* Restore by level first, check if the frame id is the same as
- expected. If that fails, try restoring by frame id. If that
- fails, nothing to do, just warn the user. */
-
- count = frame_level;
- frame = find_relative_frame (get_current_frame (), &count);
- if (count == 0
- && frame != NULL
- /* The frame ids must match - either both valid or both outer_frame_id.
- The latter case is not failsafe, but since it's highly unlikely
- the search by level finds the wrong frame, it's 99.9(9)% of
- the time (for all practical purposes) safe. */
- && frame_id_eq (get_frame_id (frame), a_frame_id))
- {
- /* Cool, all is fine. */
- select_frame (frame);
- return;
- }
-
- frame = frame_find_by_id (a_frame_id);
- if (frame != NULL)
- {
- /* Cool, refound it. */
- select_frame (frame);
- return;
- }
-
- /* Nothing else to do, the frame layout really changed. Select the
- innermost stack frame. */
- select_frame (get_current_frame ());
-
- /* Warn the user. */
- if (frame_level > 0 && !current_uiout->is_mi_like_p ())
- {
- warning (_("Couldn't restore frame #%d in "
- "current thread. Bottom (innermost) frame selected:"),
- frame_level);
- /* For MI, we should probably have a notification about
- current frame change. But this error is not very
- likely, so don't bother for now. */
- print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
- }
-}
-
void
scoped_restore_current_thread::restore ()
{
@@ -1408,7 +1349,7 @@ scoped_restore_current_thread::restore ()
&& target_has_registers
&& target_has_stack
&& target_has_memory)
- restore_selected_frame (m_selected_frame_id, m_selected_frame_level);
+ restore_selected_frame (m_selected_frame_info);
}
scoped_restore_current_thread::~scoped_restore_current_thread ()
@@ -1455,14 +1396,10 @@ scoped_restore_current_thread::scoped_restore_current_thread ()
try
{
- m_selected_frame_id = get_frame_id (frame);
- m_selected_frame_level = frame_relative_level (frame);
+ m_selected_frame_info.reset (frame);
}
catch (const gdb_exception_error &ex)
{
- m_selected_frame_id = null_frame_id;
- m_selected_frame_level = -1;
-
/* Better let this propagate. */
if (ex.error == TARGET_CLOSE_ERROR)
throw;
--
2.25.4
More information about the Gdb-patches
mailing list