[PATCH] gdb: make thread_info::thread_fsm a std::unique_ptr
Lancelot SIX
lancelot.six@amd.com
Wed Jan 19 11:51:35 GMT 2022
While working on function calls, I realized that the thread_fsm member
of struct thread_info is a raw pointer to a resource it owns. This
commit changes the type of the thread_fsm member to a std::unique_ptr in
order to signify this ownership relationship and slightly ease resource
management (no need to manually call delete).
The function run_inferior_call takes an argument as a pointer to a
call_thread_fsm and installs it in it in a thread_info instance. Also
change this function's signature to accept a unique_ptr in order to
signify that the ownership of the call_thread_fsm is transferred during
the call.
No user visible change expected after this commit.
Tested on x86_64-linux with no regression observed.
Change-Id: Ia1224f72a4afa247801ce6650ce82f90224a9ae8
---
gdb/breakpoint.c | 5 +++--
gdb/gdbthread.h | 3 ++-
gdb/infcall.c | 23 ++++++++++-------------
gdb/infcmd.c | 9 ++++++---
gdb/infrun.c | 20 +++++++-------------
gdb/thread.c | 3 +--
6 files changed, 29 insertions(+), 34 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 1812bfe42f9..b2821042303 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -10838,8 +10838,9 @@ until_break_command (const char *arg, int from_tty, int anywhere)
breakpoints.emplace_back (std::move (location_breakpoint));
}
- tp->thread_fsm = new until_break_fsm (command_interp (), tp->global_num,
- std::move (breakpoints));
+ gdb_assert (tp->thread_fsm == nullptr);
+ tp->thread_fsm.reset (new until_break_fsm (command_interp (), tp->global_num,
+ std::move (breakpoints)));
if (lj_deleter)
lj_deleter->release ();
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 9921dae7a71..735f37300f6 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -34,6 +34,7 @@ struct symtab;
#include "gdbsupport/forward-scope-exit.h"
#include "displaced-stepping.h"
#include "gdbsupport/intrusive_list.h"
+#include "thread-fsm.h"
struct inferior;
struct process_stratum_target;
@@ -483,7 +484,7 @@ class thread_info : public refcounted_object,
/* Pointer to the state machine manager object that handles what is
left to do for the thread's execution command after the target
stops. Several execution commands use it. */
- struct thread_fsm *thread_fsm = NULL;
+ std::unique_ptr<struct thread_fsm> thread_fsm = nullptr;
/* This is used to remember when a fork or vfork event was caught by
a catchpoint, and thus the event is to be followed at the next
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 05cf18f0a7f..a178051391c 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -574,7 +574,7 @@ call_thread_fsm::should_notify_stop ()
thrown errors. The caller should rethrow if there's an error. */
static struct gdb_exception
-run_inferior_call (struct call_thread_fsm *sm,
+run_inferior_call (std::unique_ptr<call_thread_fsm> sm,
struct thread_info *call_thread, CORE_ADDR real_pc)
{
struct gdb_exception caught_error;
@@ -599,7 +599,7 @@ run_inferior_call (struct call_thread_fsm *sm,
/* Associate the FSM with the thread after clear_proceed_status
(otherwise it'd clear this FSM), and before anything throws, so
we don't leak it (and any resources it manages). */
- call_thread->thread_fsm = sm;
+ call_thread->thread_fsm = std::move (sm);
disable_watchpoints_before_interactive_call_start ();
@@ -1251,12 +1251,10 @@ call_function_by_hand_dummy (struct value *function,
just below is the place to chop this function in two.. */
{
- struct thread_fsm *saved_sm;
- struct call_thread_fsm *sm;
-
/* Save the current FSM. We'll override it. */
- saved_sm = call_thread->thread_fsm;
- call_thread->thread_fsm = NULL;
+ std::unique_ptr<thread_fsm> saved_sm
+ = std::move (call_thread->thread_fsm);
+ struct call_thread_fsm *sm;
/* Save this thread's ptid, we need it later but the thread
may have exited. */
@@ -1274,14 +1272,15 @@ call_function_by_hand_dummy (struct value *function,
return_method != return_method_normal,
struct_addr);
- e = run_inferior_call (sm, call_thread.get (), real_pc);
+ e = run_inferior_call (std::unique_ptr<call_thread_fsm> (sm),
+ call_thread.get (), real_pc);
gdb::observers::inferior_call_post.notify (call_thread_ptid, funaddr);
if (call_thread->state != THREAD_EXITED)
{
/* The FSM should still be the same. */
- gdb_assert (call_thread->thread_fsm == sm);
+ gdb_assert (call_thread->thread_fsm.get () == sm);
if (call_thread->thread_fsm->finished_p ())
{
@@ -1300,8 +1299,7 @@ call_function_by_hand_dummy (struct value *function,
/* Clean up / destroy the call FSM, and restore the
original one. */
call_thread->thread_fsm->clean_up (call_thread.get ());
- delete call_thread->thread_fsm;
- call_thread->thread_fsm = saved_sm;
+ call_thread->thread_fsm = std::move (saved_sm);
maybe_remove_breakpoints ();
@@ -1316,8 +1314,7 @@ call_function_by_hand_dummy (struct value *function,
/* Didn't complete. Clean up / destroy the call FSM, and restore the
previous state machine, and handle the error. */
call_thread->thread_fsm->clean_up (call_thread.get ());
- delete call_thread->thread_fsm;
- call_thread->thread_fsm = saved_sm;
+ call_thread->thread_fsm = std::move (saved_sm);
}
}
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 9f4ed8bff13..af8b6c3eb46 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -848,7 +848,8 @@ step_1 (int skip_subroutines, int single_inst, const char *count_string)
steps. */
thr = inferior_thread ();
step_sm = new step_command_fsm (command_interp ());
- thr->thread_fsm = step_sm;
+ gdb_assert (thr->thread_fsm == nullptr);
+ thr->thread_fsm.reset (step_sm);
step_command_fsm_prepare (step_sm, skip_subroutines,
single_inst, count, thr);
@@ -1355,7 +1356,8 @@ until_next_command (int from_tty)
delete_longjmp_breakpoint_cleanup lj_deleter (thread);
sm = new until_next_fsm (command_interp (), tp->global_num);
- tp->thread_fsm = sm;
+ gdb_assert (tp->thread_fsm == nullptr);
+ tp->thread_fsm.reset (sm);
lj_deleter.release ();
proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
@@ -1762,7 +1764,8 @@ finish_command (const char *arg, int from_tty)
sm = new finish_command_fsm (command_interp ());
- tp->thread_fsm = sm;
+ gdb_assert (tp->thread_fsm == nullptr);
+ tp->thread_fsm.reset (sm);
/* Finishing from an inline frame is completely different. We don't
try to show the "return value" - no way to locate it. */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 2e7ed15723f..e3bc0706c68 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -698,7 +698,7 @@ follow_fork ()
int current_line = 0;
symtab *current_symtab = NULL;
struct frame_id step_frame_id = { 0 };
- struct thread_fsm *thread_fsm = NULL;
+ std::unique_ptr<struct thread_fsm> thread_fsm = nullptr;
if (!non_stop)
{
@@ -755,7 +755,7 @@ follow_fork ()
step_frame_id = tp->control.step_frame_id;
exception_resume_breakpoint
= clone_momentary_breakpoint (tp->control.exception_resume_breakpoint);
- thread_fsm = tp->thread_fsm;
+ thread_fsm = std::move (tp->thread_fsm);
/* For now, delete the parent's sr breakpoint, otherwise,
parent/child sr breakpoints are considered duplicates,
@@ -767,7 +767,6 @@ follow_fork ()
tp->control.step_range_end = 0;
tp->control.step_frame_id = null_frame_id;
delete_exception_resume_breakpoint (tp);
- tp->thread_fsm = NULL;
}
parent = inferior_ptid;
@@ -809,7 +808,8 @@ follow_fork ()
tp->control.step_frame_id = step_frame_id;
tp->control.exception_resume_breakpoint
= exception_resume_breakpoint;
- tp->thread_fsm = thread_fsm;
+ gdb_assert (tp->thread_fsm == nullptr);
+ tp->thread_fsm = std::move (thread_fsm);
}
else
{
@@ -2651,8 +2651,7 @@ clear_proceed_status_thread (struct thread_info *tp)
if (!signal_pass_state (tp->stop_signal ()))
tp->set_stop_signal (GDB_SIGNAL_0);
- delete tp->thread_fsm;
- tp->thread_fsm = NULL;
+ tp->thread_fsm.reset ();
tp->control.trap_expected = 0;
tp->control.step_range_start = 0;
@@ -4103,13 +4102,8 @@ fetch_inferior_event ()
delete_just_stopped_threads_infrun_breakpoints ();
- if (thr != NULL)
- {
- struct thread_fsm *thread_fsm = thr->thread_fsm;
-
- if (thread_fsm != NULL)
- should_stop = thread_fsm->should_stop (thr);
- }
+ if (thr != nullptr && thr->thread_fsm != nullptr)
+ should_stop = thr->thread_fsm->should_stop (thr);
if (!should_stop)
{
diff --git a/gdb/thread.c b/gdb/thread.c
index 611be3f4633..b13a066e7f5 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -163,8 +163,7 @@ thread_cancel_execution_command (struct thread_info *thr)
if (thr->thread_fsm != NULL)
{
thr->thread_fsm->clean_up (thr);
- delete thr->thread_fsm;
- thr->thread_fsm = NULL;
+ thr->thread_fsm.reset ();
}
}
base-commit: 8ffb6df2aa60af2fae5378e2520fbf6bdd0b0962
--
2.25.1
More information about the Gdb-patches
mailing list