This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCH] Replace finish_thread_state_cleanup with a RAII class
- From: Pedro Alves <palves at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Wed, 28 Mar 2018 15:44:28 +0100
- Subject: [PATCH] Replace finish_thread_state_cleanup with a RAII class
A small patch I was sitting on in the multi-target branch.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* gdbthread.h (finish_thread_state_cleanup): Delete declaration.
(scoped_finish_thread_state): New class.
* infcmd.c (run_command_1): Use it instead of finish_thread_state
cleanup.
* infrun.c (proceed, prepare_for_detach, wait_for_inferior)
(fetch_inferior_event, normal_stop): Likewise.
* thread.c (finish_thread_state_cleanup): Delete.
---
gdb/gdbthread.h | 31 +++++++++++++++++++++++++++----
gdb/infcmd.c | 13 +++++--------
gdb/infrun.c | 52 +++++++++++++++++++++++++---------------------------
gdb/thread.c | 10 ----------
4 files changed, 57 insertions(+), 49 deletions(-)
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 9b468dbda7..09ea5baf23 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -567,10 +567,33 @@ extern int threads_are_executing (void);
Notifications are only emitted if the thread state did change. */
extern void finish_thread_state (ptid_t ptid);
-/* Same as FINISH_THREAD_STATE, but with an interface suitable to be
- registered as a cleanup. PTID_P points to the ptid_t that is
- passed to FINISH_THREAD_STATE. */
-extern void finish_thread_state_cleanup (void *ptid_p);
+/* Calls finish_thread_state on scope exit, unless release() is called
+ to disengage. */
+class scoped_finish_thread_state
+{
+public:
+ explicit scoped_finish_thread_state (ptid_t ptid)
+ : m_ptid (ptid)
+ {}
+
+ ~scoped_finish_thread_state ()
+ {
+ if (!m_released)
+ finish_thread_state (m_ptid);
+ }
+
+ /* Disengage. */
+ void release ()
+ {
+ m_released = true;
+ }
+
+ DISABLE_COPY_AND_ASSIGN (scoped_finish_thread_state);
+
+private:
+ bool m_released = false;
+ ptid_t m_ptid;
+};
/* Commands with a prefix of `thread'. */
extern struct cmd_list_element *thread_cmd_list;
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 9c236b8e70..bacc10144b 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -565,8 +565,6 @@ static void
run_command_1 (const char *args, int from_tty, enum run_how run_how)
{
const char *exec_file;
- struct cleanup *old_chain;
- ptid_t ptid;
struct ui_out *uiout = current_uiout;
struct target_ops *run_target;
int async_exec;
@@ -655,11 +653,10 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)
events --- the frontend shouldn't see them as stopped. In
all-stop, always finish the state of all threads, as we may be
resuming more than just the new process. */
- if (non_stop)
- ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
- else
- ptid = minus_one_ptid;
- old_chain = make_cleanup (finish_thread_state_cleanup, &ptid);
+ ptid_t finish_ptid = (non_stop
+ ? pid_to_ptid (current_inferior ()->pid)
+ : minus_one_ptid);
+ scoped_finish_thread_state finish_state (finish_ptid);
/* Pass zero for FROM_TTY, because at this point the "run" command
has done its thing; now we are setting up the running program. */
@@ -680,7 +677,7 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)
/* Since there was no error, there's no need to finish the thread
states here. */
- discard_cleanups (old_chain);
+ finish_state.release ();
}
static void
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 6648698df6..fc0aac0f82 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2979,7 +2979,6 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
ptid_t resume_ptid;
struct execution_control_state ecss;
struct execution_control_state *ecs = &ecss;
- struct cleanup *old_chain;
int started;
/* If we're stopped at a fork/vfork, follow the branch set by the
@@ -3043,7 +3042,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
/* If an exception is thrown from this point on, make sure to
propagate GDB's knowledge of the executing state to the
frontend/user running state. */
- old_chain = make_cleanup (finish_thread_state_cleanup, &resume_ptid);
+ scoped_finish_thread_state finish_state (resume_ptid);
/* Even if RESUME_PTID is a wildcard, and we end up resuming fewer
threads (e.g., we might need to set threads stepping over
@@ -3198,7 +3197,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
target_commit_resume ();
- discard_cleanups (old_chain);
+ finish_state.release ();
/* Tell the event loop to wait for it to stop. If the target
supports asynchronous execution, it'll do this from within
@@ -3641,7 +3640,6 @@ prepare_for_detach (void)
while (!ptid_equal (displaced->step_ptid, null_ptid))
{
- struct cleanup *old_chain_2;
struct execution_control_state ecss;
struct execution_control_state *ecs;
@@ -3663,14 +3661,13 @@ prepare_for_detach (void)
/* If an error happens while handling the event, propagate GDB's
knowledge of the executing state to the frontend/user running
state. */
- old_chain_2 = make_cleanup (finish_thread_state_cleanup,
- &minus_one_ptid);
+ scoped_finish_thread_state finish_state (minus_one_ptid);
/* Now figure out what to do with the result of the result. */
handle_inferior_event (ecs);
/* No error, don't finish the state yet. */
- discard_cleanups (old_chain_2);
+ finish_state.release ();
/* Breakpoints and watchpoints are not installed on the target
at this point, and signals are passed directly to the
@@ -3696,7 +3693,6 @@ void
wait_for_inferior (void)
{
struct cleanup *old_cleanups;
- struct cleanup *thread_state_chain;
if (debug_infrun)
fprintf_unfiltered
@@ -3709,7 +3705,7 @@ wait_for_inferior (void)
/* If an error happens while handling the event, propagate GDB's
knowledge of the executing state to the frontend/user running
state. */
- thread_state_chain = make_cleanup (finish_thread_state_cleanup, &minus_one_ptid);
+ scoped_finish_thread_state finish_state (minus_one_ptid);
while (1)
{
@@ -3740,7 +3736,7 @@ wait_for_inferior (void)
}
/* No error, don't finish the state yet. */
- discard_cleanups (thread_state_chain);
+ finish_state.release ();
do_cleanups (old_cleanups);
}
@@ -3859,7 +3855,6 @@ fetch_inferior_event (void *client_data)
struct execution_control_state ecss;
struct execution_control_state *ecs = &ecss;
struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
- struct cleanup *ts_old_chain;
int cmd_done = 0;
ptid_t waiton_ptid = minus_one_ptid;
@@ -3911,14 +3906,12 @@ fetch_inferior_event (void *client_data)
/* If an error happens while handling the event, propagate GDB's
knowledge of the executing state to the frontend/user running
state. */
- if (!target_is_non_stop_p ())
- ts_old_chain = make_cleanup (finish_thread_state_cleanup, &minus_one_ptid);
- else
- ts_old_chain = make_cleanup (finish_thread_state_cleanup, &ecs->ptid);
+ ptid_t finish_ptid = !target_is_non_stop_p () ? minus_one_ptid : ecs->ptid;
+ scoped_finish_thread_state finish_state (finish_ptid);
/* Get executed before make_cleanup_restore_current_thread above to apply
still for the thread which has thrown the exception. */
- make_bpstat_clear_actions_cleanup ();
+ struct cleanup *ts_old_chain = make_bpstat_clear_actions_cleanup ();
make_cleanup (delete_just_stopped_threads_infrun_breakpoints_cleanup, NULL);
@@ -3973,9 +3966,11 @@ fetch_inferior_event (void *client_data)
}
}
- /* No error, don't finish the thread states yet. */
discard_cleanups (ts_old_chain);
+ /* No error, don't finish the thread states yet. */
+ finish_state.release ();
+
/* Revert thread and frame. */
do_cleanups (old_chain);
@@ -8164,7 +8159,6 @@ normal_stop (void)
{
struct target_waitstatus last;
ptid_t last_ptid;
- struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
ptid_t pid_ptid;
get_last_target_status (&last_ptid, &last);
@@ -8175,8 +8169,11 @@ normal_stop (void)
propagate GDB's knowledge of the executing state to the
frontend/user running state. A QUIT is an easy exception to see
here, so do this before any filtered output. */
+
+ ptid_t finish_ptid = null_ptid;
+
if (!non_stop)
- make_cleanup (finish_thread_state_cleanup, &minus_one_ptid);
+ finish_ptid = minus_one_ptid;
else if (last.kind == TARGET_WAITKIND_SIGNALLED
|| last.kind == TARGET_WAITKIND_EXITED)
{
@@ -8185,14 +8182,15 @@ normal_stop (void)
"checkpoint", when the current checkpoint/fork exits,
linux-fork.c automatically switches to another fork from
within target_mourn_inferior. */
- if (!ptid_equal (inferior_ptid, null_ptid))
- {
- pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
- make_cleanup (finish_thread_state_cleanup, &pid_ptid);
- }
+ if (inferior_ptid != null_ptid)
+ finish_ptid = ptid_t (inferior_ptid.pid ());
}
else if (last.kind != TARGET_WAITKIND_NO_RESUMED)
- make_cleanup (finish_thread_state_cleanup, &inferior_ptid);
+ finish_ptid = inferior_ptid;
+
+ gdb::optional<scoped_finish_thread_state> maybe_finish_thread_state;
+ if (finish_ptid != null_ptid)
+ maybe_finish_thread_state.emplace (finish_ptid);
/* As we're presenting a stop, and potentially removing breakpoints,
update the thread list so we can tell whether there are threads
@@ -8224,7 +8222,7 @@ normal_stop (void)
after this event is handled, so we're not really switching, only
informing of a stop. */
if (!non_stop
- && !ptid_equal (previous_inferior_ptid, inferior_ptid)
+ && previous_inferior_ptid != inferior_ptid
&& target_has_execution
&& last.kind != TARGET_WAITKIND_SIGNALLED
&& last.kind != TARGET_WAITKIND_EXITED
@@ -8265,7 +8263,7 @@ normal_stop (void)
}
/* Let the user/frontend see the threads as stopped. */
- do_cleanups (old_chain);
+ maybe_finish_thread_state.reset ();
/* Select innermost stack frame - i.e., current frame is frame 0,
and current location is based on that. Handle the case where the
diff --git a/gdb/thread.c b/gdb/thread.c
index 99f3f5be63..c1a8174e96 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1040,16 +1040,6 @@ finish_thread_state (ptid_t ptid)
gdb::observers::target_resumed.notify (ptid);
}
-void
-finish_thread_state_cleanup (void *arg)
-{
- ptid_t *ptid_p = (ptid_t *) arg;
-
- gdb_assert (arg);
-
- finish_thread_state (*ptid_p);
-}
-
/* See gdbthread.h. */
void
--
2.14.3