diff options
author | Pedro Alves <pedro@palves.net> | 2020-10-30 18:26:15 +0100 |
---|---|---|
committer | Pedro Alves <pedro@palves.net> | 2020-10-30 01:01:12 +0000 |
commit | 79952e69634bd02029e79676a38915de60052e89 (patch) | |
tree | 74f734787fcea919e5f6166aef3775147e7946a7 /gdb/frame.h | |
parent | 4dd5c35212cf4685bb14f7709508de22a36e7dc2 (diff) | |
download | gdb-79952e69634bd02029e79676a38915de60052e89.zip gdb-79952e69634bd02029e79676a38915de60052e89.tar.gz gdb-79952e69634bd02029e79676a38915de60052e89.tar.bz2 |
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
Diffstat (limited to 'gdb/frame.h')
-rw-r--r-- | gdb/frame.h | 51 |
1 files changed, 41 insertions, 10 deletions
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 *); |