diff options
| -rw-r--r-- | gdb/ChangeLog | 39 | ||||
| -rw-r--r-- | gdb/blockframe.c | 6 | ||||
| -rw-r--r-- | gdb/frame.c | 117 | ||||
| -rw-r--r-- | gdb/frame.h | 51 | ||||
| -rw-r--r-- | gdb/gdbthread.h | 4 | ||||
| -rw-r--r-- | gdb/infrun.c | 40 | ||||
| -rw-r--r-- | gdb/stack.c | 9 | ||||
| -rw-r--r-- | gdb/thread.c | 64 |
8 files changed, 206 insertions, 124 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 8be91cd..978576a 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,42 @@ +2020-10-30 Pedro Alves <pedro@palves.net> + + * 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. + 2020-10-29 Simon Marchi <simon.marchi@polymtl.ca> * gdbarch.sh (displaced_step_hw_singlestep): Adjust diff --git a/gdb/blockframe.c b/gdb/blockframe.c index 80b7695..80537e0 100644 --- a/gdb/blockframe.c +++ b/gdb/blockframe.c @@ -464,14 +464,10 @@ find_gnu_ifunc_target_type (CORE_ADDR resolver_funaddr) struct frame_info * block_innermost_frame (const struct block *block) { - struct frame_info *frame; - if (block == NULL) return NULL; - frame = get_selected_frame_if_set (); - if (frame == NULL) - frame = get_current_frame (); + frame_info *frame = get_selected_frame (); while (frame != NULL) { const struct block *frame_block = get_frame_block (frame, NULL); diff --git a/gdb/frame.c b/gdb/frame.c index 15168fb..bb835e2 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -317,17 +317,15 @@ frame_stash_invalidate (void) /* See frame.h */ scoped_restore_selected_frame::scoped_restore_selected_frame () { - m_fid = get_frame_id (get_selected_frame (NULL)); + m_lang = current_language->la_language; + save_selected_frame (&m_fid, &m_level); } /* See frame.h */ scoped_restore_selected_frame::~scoped_restore_selected_frame () { - frame_info *frame = frame_find_by_id (m_fid); - if (frame == NULL) - warning (_("Unable to restore previously selected frame.")); - else - select_frame (frame); + restore_selected_frame (m_fid, m_level); + set_language (m_lang); } /* Flag to control debugging. */ @@ -1685,10 +1683,63 @@ get_current_frame (void) } /* The "selected" stack frame is used by default for local and arg - access. May be zero, for no selected frame. */ - + access. + + The "single source of truth" for the selected frame is the + SELECTED_FRAME_ID / SELECTED_FRAME_LEVEL pair. + + Frame IDs can be saved/restored across reinitializing the frame + cache, while frame_info pointers can't (frame_info objects are + invalidated). If we know the corresponding frame_info object, it + is cached in SELECTED_FRAME. + + If SELECTED_FRAME_ID / SELECTED_FRAME_LEVEL are null_frame_id / -1, + and the target has stack and is stopped, the selected frame is the + current (innermost) frame. This means that SELECTED_FRAME_LEVEL is + never 0 and SELECTED_FRAME_ID is never the ID of the innermost + frame. + + If SELECTED_FRAME_ID / SELECTED_FRAME_LEVEL are null_frame_id / -1, + and the target has no stack or is executing, then there's no + selected frame. */ +static frame_id selected_frame_id = null_frame_id; +static int selected_frame_level = -1; + +/* The cached frame_info object pointing to the selected frame. + Looked up on demand by get_selected_frame. */ static struct frame_info *selected_frame; +/* See frame.h. */ + +void +save_selected_frame (frame_id *frame_id, int *frame_level) + noexcept +{ + *frame_id = selected_frame_id; + *frame_level = selected_frame_level; +} + +/* See frame.h. */ + +void +restore_selected_frame (frame_id frame_id, int frame_level) + noexcept +{ + /* save_selected_frame never returns level == 0, so we shouldn't see + it here either. */ + gdb_assert (frame_level != 0); + + /* FRAME_ID can be null_frame_id only IFF frame_level is -1. */ + gdb_assert ((frame_level == -1 && !frame_id_p (frame_id)) + || (frame_level != -1 && frame_id_p (frame_id))); + + selected_frame_id = frame_id; + selected_frame_level = frame_level; + + /* Will be looked up later by get_selected_frame. */ + selected_frame = nullptr; +} + bool has_stack_frames () { @@ -1716,9 +1767,7 @@ has_stack_frames () return true; } -/* Return the selected frame. Always non-NULL (unless there isn't an - inferior sufficient for creating a frame) in which case an error is - thrown. */ +/* See frame.h. */ struct frame_info * get_selected_frame (const char *message) @@ -1727,24 +1776,14 @@ get_selected_frame (const char *message) { if (message != NULL && !has_stack_frames ()) error (("%s"), message); - /* Hey! Don't trust this. It should really be re-finding the - last selected frame of the currently selected thread. This, - though, is better than nothing. */ - select_frame (get_current_frame ()); + + lookup_selected_frame (selected_frame_id, selected_frame_level); } /* There is always a frame. */ gdb_assert (selected_frame != NULL); return selected_frame; } -/* If there is a selected frame, return it. Otherwise, return NULL. */ - -struct frame_info * -get_selected_frame_if_set (void) -{ - return selected_frame; -} - /* This is a variant of get_selected_frame() which can be called when the inferior does not have a frame; in that case it will return NULL instead of calling error(). */ @@ -1757,12 +1796,42 @@ deprecated_safe_get_selected_frame (void) return get_selected_frame (NULL); } -/* Select frame FI (or NULL - to invalidate the current frame). */ +/* Select frame FI (or NULL - to invalidate the selected frame). */ void select_frame (struct frame_info *fi) { selected_frame = fi; + selected_frame_level = frame_relative_level (fi); + if (selected_frame_level == 0) + { + /* Treat the current frame especially -- we want to always + save/restore it without warning, even if the frame ID changes + (see lookup_selected_frame). E.g.: + + // The current frame is selected, the target had just stopped. + { + scoped_restore_selected_frame restore_frame; + some_operation_that_changes_the_stack (); + } + // scoped_restore_selected_frame's dtor runs, but the + // original frame_id can't be found. No matter whether it + // is found or not, we still end up with the now-current + // frame selected. Warning in lookup_selected_frame in this + // case seems pointless. + + Also get_frame_id may access the target's registers/memory, + and thus skipping get_frame_id optimizes the common case. + + Saving the selected frame this way makes get_selected_frame + and restore_current_frame return/re-select whatever frame is + the innermost (current) then. */ + selected_frame_level = -1; + selected_frame_id = null_frame_id; + } + else + selected_frame_id = get_frame_id (fi); + /* NOTE: cagney/2002-05-04: FI can be NULL. This occurs when the frame is being invalidated. */ diff --git a/gdb/frame.h b/gdb/frame.h index 3ceb7b3..f3ec784 100644 --- a/gdb/frame.h +++ b/gdb/frame.h @@ -186,8 +186,14 @@ public: private: - /* The ID of the previously selected frame. */ + /* The ID and level of the previously selected frame. */ struct frame_id m_fid; + int m_level; + + /* Save/restore the language as well, because selecting a frame + changes the current language to the frame's language if "set + language auto". */ + enum language m_lang; }; /* Methods for constructing and comparing Frame IDs. */ @@ -316,24 +322,49 @@ extern bool has_stack_frames (); modifies the target invalidating the frame cache). */ extern void reinit_frame_cache (void); -/* 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, +/* Return the selected frame. Always returns non-NULL. If there + isn't an inferior sufficient for creating a frame, an error is + thrown. When MESSAGE is non-NULL, use it for the error message, otherwise 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); - -/* If there is a selected frame, return it. Otherwise, return NULL. */ -extern struct frame_info *get_selected_frame_if_set (void); +extern struct frame_info *get_selected_frame (const char *message = nullptr); -/* Select a specific frame. NULL, apparently implies re-select the - inner most frame. */ +/* Select a specific frame. NULL implies re-select the inner most + frame. */ extern void select_frame (struct frame_info *); +/* Save the frame ID and frame level of the selected frame in FRAME_ID + and FRAME_LEVEL, to be restored later with restore_selected_frame. + + This is preferred over getting the same info out of + get_selected_frame directly because this function does not create + the selected-frame's frame_info object if it hasn't been created + yet, and thus is more efficient and doesn't throw. */ +extern void save_selected_frame (frame_id *frame_id, int *frame_level) + noexcept; + +/* Restore selected frame as saved with save_selected_frame. + + Does not try to find the corresponding frame_info object. Instead + the next call to get_selected_frame will look it up and cache the + result. + + This function does not throw. It is designed to be safe to called + from the destructors of RAII types. */ +extern void restore_selected_frame (frame_id frame_id, int frame_level) + noexcept; + +/* Lookup the frame_info object for the selected frame FRAME_ID / + FRAME_LEVEL and cache the result. + + If FRAME_LEVEL > 0 and the originally selected frame isn't found, + warn and select the innermost (current) frame. */ +extern void lookup_selected_frame (frame_id frame_id, int frame_level); + /* Given a FRAME, return the next (more inner, younger) or previous (more outer, older) frame. */ extern struct frame_info *get_prev_frame (struct frame_info *); diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h index ab5771f..da20dd4 100644 --- a/gdb/gdbthread.h +++ b/gdb/gdbthread.h @@ -673,6 +673,10 @@ private: frame_id m_selected_frame_id; int m_selected_frame_level; bool m_was_stopped; + /* Save/restore the language as well, because selecting a frame + changes the current language to the frame's language if "set + language auto". */ + enum language m_lang; }; /* Returns a pointer into the thread_info corresponding to 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; diff --git a/gdb/stack.c b/gdb/stack.c index 4ba5500..83c3366 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -1842,9 +1842,9 @@ trailing_outermost_frame (int count) static void select_frame_command_core (struct frame_info *fi, bool ignored) { - struct frame_info *prev_frame = get_selected_frame_if_set (); + frame_info *prev_frame = get_selected_frame (); select_frame (fi); - if (get_selected_frame_if_set () != prev_frame) + if (get_selected_frame () != prev_frame) gdb::observers::user_selected_context_changed.notify (USER_SELECTED_FRAME); } @@ -1863,10 +1863,9 @@ select_frame_for_mi (struct frame_info *fi) static void frame_command_core (struct frame_info *fi, bool ignored) { - struct frame_info *prev_frame = get_selected_frame_if_set (); - + frame_info *prev_frame = get_selected_frame (); select_frame (fi); - if (get_selected_frame_if_set () != prev_frame) + if (get_selected_frame () != prev_frame) gdb::observers::user_selected_context_changed.notify (USER_SELECTED_FRAME); else print_selected_thread_frame (current_uiout, USER_SELECTED_FRAME); diff --git a/gdb/thread.c b/gdb/thread.c index d832443..193f9d4 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -1325,20 +1325,26 @@ 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) +/* See frame.h. */ + +void +lookup_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. */ + /* This either means there was no selected frame, or the selected + frame was the current frame. In either case, select the current + frame. */ if (frame_level == -1) { - select_frame (NULL); + select_frame (get_current_frame ()); return; } - gdb_assert (frame_level >= 0); + /* select_frame never saves 0 in SELECTED_FRAME_LEVEL, so we + shouldn't see it here. */ + 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 @@ -1409,64 +1415,28 @@ scoped_restore_current_thread::restore () && target_has_stack () && target_has_memory ()) restore_selected_frame (m_selected_frame_id, m_selected_frame_level); + + set_language (m_lang); } scoped_restore_current_thread::~scoped_restore_current_thread () { if (!m_dont_restore) - { - try - { - restore (); - } - catch (const gdb_exception &ex) - { - /* We're in a dtor, there's really nothing else we can do - but swallow the exception. */ - } - } + restore (); } scoped_restore_current_thread::scoped_restore_current_thread () { m_inf = inferior_ref::new_reference (current_inferior ()); + m_lang = current_language->la_language; + if (inferior_ptid != null_ptid) { m_thread = thread_info_ref::new_reference (inferior_thread ()); - struct frame_info *frame; - m_was_stopped = m_thread->state == THREAD_STOPPED; - if (m_was_stopped - && target_has_registers () - && target_has_stack () - && target_has_memory ()) - { - /* When processing internal events, there might not be a - selected frame. If we naively call get_selected_frame - here, then we can end up reading debuginfo for the - current frame, but we don't generally need the debuginfo - at this point. */ - frame = get_selected_frame_if_set (); - } - else - frame = NULL; - - try - { - m_selected_frame_id = get_frame_id (frame); - m_selected_frame_level = frame_relative_level (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; - } + save_selected_frame (&m_selected_frame_id, &m_selected_frame_level); } } |