gdb: make thread_info::m_thread_fsm a std::unique_ptr

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).

To ensure consistent use, the field is made a private member
(m_thread_fsm).  The setter method (set_thread_fsm) can then check
that it is incorrect to associate a FSM to a thread_info object if
another one is already in place.  This is ensured by an assertion.

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
This commit is contained in:
Lancelot SIX 2022-01-11 10:10:11 -05:00
parent d08cbc5d32
commit 573269a87c
8 changed files with 96 additions and 67 deletions

View File

@ -10830,8 +10830,10 @@ 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));
tp->set_thread_fsm
(std::unique_ptr<thread_fsm>
(new until_break_fsm (command_interp (), tp->global_num,
std::move (breakpoints))));
if (lj_deleter)
lj_deleter->release ();

View File

@ -108,9 +108,9 @@ should_print_stop_to_console (struct interp *console_interp,
{
if ((bpstat_what (tp->control.stop_bpstat).main_action
== BPSTAT_WHAT_STOP_NOISY)
|| tp->thread_fsm == NULL
|| tp->thread_fsm->command_interp == console_interp
|| !tp->thread_fsm->finished_p ())
|| tp->thread_fsm () == nullptr
|| tp->thread_fsm ()->command_interp == console_interp
|| !tp->thread_fsm ()->finished_p ())
return 1;
return 0;
}

View File

@ -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;
@ -443,6 +444,32 @@ class thread_info : public refcounted_object,
m_suspend.stop_reason = reason;
}
/* Get the FSM associated with the thread. */
struct thread_fsm *thread_fsm () const
{
return m_thread_fsm.get ();
}
/* Get the owning reference to the FSM associated with the thread.
After a call to this method, "thread_fsm () == nullptr". */
std::unique_ptr<struct thread_fsm> release_thread_fsm ()
{
return std::move (m_thread_fsm);
}
/* Set the FSM associated with the current thread.
It is invalid to set the FSM if another FSM is already installed. */
void set_thread_fsm (std::unique_ptr<struct thread_fsm> fsm)
{
gdb_assert (m_thread_fsm == nullptr);
m_thread_fsm = std::move (fsm);
}
int current_line = 0;
struct symtab *current_symtab = NULL;
@ -480,11 +507,6 @@ class thread_info : public refcounted_object,
when GDB gets back SIGTRAP from step_resume_breakpoint. */
int step_after_step_resume_breakpoint = 0;
/* 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;
/* 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
resume of the thread, and not immediately. */
@ -550,6 +572,11 @@ class thread_info : public refcounted_object,
Nullptr if the thread does not have a user-given name. */
gdb::unique_xmalloc_ptr<char> m_name;
/* 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. */
std::unique_ptr<struct thread_fsm> m_thread_fsm;
};
using thread_info_resumed_with_pending_wait_status_node

View File

@ -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;
@ -597,9 +597,8 @@ run_inferior_call (struct call_thread_fsm *sm,
clear_proceed_status (0);
/* 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;
(otherwise it'd clear this FSM). */
call_thread->set_thread_fsm (std::move (sm));
disable_watchpoints_before_interactive_call_start ();
@ -1251,12 +1250,9 @@ 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 = call_thread->release_thread_fsm ();
struct call_thread_fsm *sm;
/* Save this thread's ptid, we need it later but the thread
may have exited. */
@ -1273,17 +1269,19 @@ call_function_by_hand_dummy (struct value *function,
values_type,
return_method != return_method_normal,
struct_addr);
e = run_inferior_call (sm, call_thread.get (), real_pc);
{
std::unique_ptr<call_thread_fsm> sm_up (sm);
e = run_inferior_call (std::move (sm_up), 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 () == sm);
if (call_thread->thread_fsm->finished_p ())
if (call_thread->thread_fsm ()->finished_p ())
{
struct value *retval;
@ -1297,11 +1295,16 @@ call_function_by_hand_dummy (struct value *function,
/* Get the return value. */
retval = sm->return_value;
/* 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;
/* Restore the original FSM and clean up / destroh the call FSM.
Doing it in this order ensures that if the call to clean_up
throws, the original FSM is properly restored. */
{
std::unique_ptr<thread_fsm> finalizing
= call_thread->release_thread_fsm ();
call_thread->set_thread_fsm (std::move (saved_sm));
finalizing->clean_up (call_thread.get ());
}
maybe_remove_breakpoints ();
@ -1315,9 +1318,13 @@ 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;
{
std::unique_ptr<thread_fsm> finalizing
= call_thread->release_thread_fsm ();
call_thread->set_thread_fsm (std::move (saved_sm));
finalizing->clean_up (call_thread.get ());
}
}
}

View File

@ -848,7 +848,7 @@ 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;
thr->set_thread_fsm (std::unique_ptr<thread_fsm> (step_sm));
step_command_fsm_prepare (step_sm, skip_subroutines,
single_inst, count, thr);
@ -865,7 +865,7 @@ step_1 (int skip_subroutines, int single_inst, const char *count_string)
/* Stepped into an inline frame. Pretend that we've
stopped. */
thr->thread_fsm->clean_up (thr);
thr->thread_fsm ()->clean_up (thr);
proceeded = normal_stop ();
if (!proceeded)
inferior_event_handler (INF_EXEC_COMPLETE);
@ -1355,7 +1355,7 @@ 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;
tp->set_thread_fsm (std::unique_ptr<thread_fsm> (sm));
lj_deleter.release ();
proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
@ -1762,7 +1762,7 @@ finish_command (const char *arg, int from_tty)
sm = new finish_command_fsm (command_interp ());
tp->thread_fsm = sm;
tp->set_thread_fsm (std::unique_ptr<thread_fsm> (sm));
/* Finishing from an inline frame is completely different. We don't
try to show the "return value" - no way to locate it. */

View File

@ -698,7 +698,6 @@ follow_fork ()
int current_line = 0;
symtab *current_symtab = NULL;
struct frame_id step_frame_id = { 0 };
struct thread_fsm *thread_fsm = NULL;
if (!non_stop)
{
@ -741,6 +740,7 @@ follow_fork ()
case TARGET_WAITKIND_VFORKED:
{
ptid_t parent, child;
std::unique_ptr<struct thread_fsm> thread_fsm;
/* If the user did a next/step, etc, over a fork call,
preserve the stepping state in the fork child. */
@ -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 = tp->release_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,7 @@ follow_fork ()
tp->control.step_frame_id = step_frame_id;
tp->control.exception_resume_breakpoint
= exception_resume_breakpoint;
tp->thread_fsm = thread_fsm;
tp->set_thread_fsm (std::move (thread_fsm));
}
else
{
@ -2651,8 +2650,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->release_thread_fsm ();
tp->control.trap_expected = 0;
tp->control.step_range_start = 0;
@ -3935,24 +3933,24 @@ reinstall_readline_callback_handler_cleanup ()
static void
clean_up_just_stopped_threads_fsms (struct execution_control_state *ecs)
{
if (ecs->event_thread != NULL
&& ecs->event_thread->thread_fsm != NULL)
ecs->event_thread->thread_fsm->clean_up (ecs->event_thread);
if (ecs->event_thread != nullptr
&& ecs->event_thread->thread_fsm () != nullptr)
ecs->event_thread->thread_fsm ()->clean_up (ecs->event_thread);
if (!non_stop)
{
for (thread_info *thr : all_non_exited_threads ())
{
if (thr->thread_fsm == NULL)
if (thr->thread_fsm () == nullptr)
continue;
if (thr == ecs->event_thread)
continue;
switch_to_thread (thr);
thr->thread_fsm->clean_up (thr);
thr->thread_fsm ()->clean_up (thr);
}
if (ecs->event_thread != NULL)
if (ecs->event_thread != nullptr)
switch_to_thread (ecs->event_thread);
}
}
@ -4103,13 +4101,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)
{
@ -4122,8 +4115,9 @@ fetch_inferior_event ()
clean_up_just_stopped_threads_fsms (ecs);
if (thr != NULL && thr->thread_fsm != NULL)
should_notify_stop = thr->thread_fsm->should_notify_stop ();
if (thr != nullptr && thr->thread_fsm () != nullptr)
should_notify_stop
= thr->thread_fsm ()->should_notify_stop ();
if (should_notify_stop)
{
@ -8340,13 +8334,13 @@ print_stop_event (struct ui_out *uiout, bool displays)
}
tp = inferior_thread ();
if (tp->thread_fsm != NULL
&& tp->thread_fsm->finished_p ())
if (tp->thread_fsm () != nullptr
&& tp->thread_fsm ()->finished_p ())
{
struct return_value_info *rv;
rv = tp->thread_fsm->return_value ();
if (rv != NULL)
rv = tp->thread_fsm ()->return_value ();
if (rv != nullptr)
print_return_value (uiout, rv);
}
}

View File

@ -630,12 +630,12 @@ mi_on_normal_stop_1 (struct bpstat *bs, int print_frame)
tp = inferior_thread ();
if (tp->thread_fsm != NULL
&& tp->thread_fsm->finished_p ())
if (tp->thread_fsm () != nullptr
&& tp->thread_fsm ()->finished_p ())
{
enum async_reply_reason reason;
reason = tp->thread_fsm->async_reply_reason ();
reason = tp->thread_fsm ()->async_reply_reason ();
mi_uiout->field_string ("reason", async_reason_lookup (reason));
}

View File

@ -160,11 +160,10 @@ thread_has_single_step_breakpoint_here (struct thread_info *tp,
void
thread_cancel_execution_command (struct thread_info *thr)
{
if (thr->thread_fsm != NULL)
if (thr->thread_fsm () != nullptr)
{
thr->thread_fsm->clean_up (thr);
delete thr->thread_fsm;
thr->thread_fsm = NULL;
std::unique_ptr<thread_fsm> fsm = thr->release_thread_fsm ();
fsm->clean_up (thr);
}
}