* [PATCH 1/7] Merge async and sync code paths some more
2015-08-12 17:02 [PATCH 0/7] Replace continuations with an extendable "class" Pedro Alves
@ 2015-08-12 17:02 ` Pedro Alves
2015-08-12 19:48 ` Simon Marchi
` (2 more replies)
2015-08-12 17:02 ` [PATCH 2/7] Replace "struct continuation" mechanism by something more extensible Pedro Alves
` (6 subsequent siblings)
7 siblings, 3 replies; 26+ messages in thread
From: Pedro Alves @ 2015-08-12 17:02 UTC (permalink / raw)
To: gdb-patches
This patch makes the execution control code use largely the same
mechanisms in both sync- and async-capable targets. This means using
continuations and use the event loop to react to target events on sync
targets as well. The trick is to immediately mark infrun's event loop
source after resume instead of calling wait_for_inferior. Then
fetch_inferior_event is adjusted to do a blocking wait on sync
targets.
Tested on x86_64 Fedora 20, native and gdbserver, with and without
"maint set target-async off".
gdb/ChangeLog:
2015-08-12 Pedro Alves <palves@redhat.com>
* breakpoint.c (bpstat_do_actions_1, until_break_command): Don't
check whether the target can async.
* inf-loop.c (inferior_event_handler): Only call target_async if
the target can async.
* infcall.c: Include top.h and interps.h.
(run_inferior_call): For the interpreter to sync mode while
running the infcall. Call wait_sync_command_done instead of
wait_for_inferior plus normal_stop.
* infcmd.c (prepare_execution_command): Don't check whether the
target can async when running in the foreground.
(step_1): Delete synchronous case handling.
(step_once): Always install a continuation, even in sync mode.
(until_next_command, finish_forward): Don't check whether the
target can async.
(attach_command_post_wait, notice_new_inferior): Always install a
continuation, even in sync mode.
* infrun.c (mark_infrun_async_event_handler): New function.
(proceed): In sync mode, mark infrun's event source instead of
waiting for events here.
(fetch_inferior_event): If the target can't async, do a blocking
wait.
(prepare_to_wait): In sync mode, mark infrun's event source.
(infrun_async_inferior_event_handler): No longer bail out if the
target can't async.
* infrun.h (mark_infrun_async_event_handler): New declaration.
* linux-nat.c (linux_nat_wait_1): Remove calls to
set_sigint_trap/clear_sigint_trap.
(linux_nat_terminal_inferior): No longer check whether the target
can async.
* mi/mi-interp.c (mi_on_sync_execution_done): Update and simplify
comment.
(mi_execute_command_input_handler): No longer check whether the
target is async. Update and simplify comment.
* target.c (default_target_wait): New function.
* target.h (struct target_ops) <to_wait>: Now defaults to
default_to_wait.
(default_target_wait): Declare.
* top.c (wait_sync_command_done): New function, factored out from
...
(maybe_wait_sync_command_done): ... this.
* top.h (wait_sync_command_done): Declare.
---
gdb/breakpoint.c | 4 +-
gdb/inf-loop.c | 2 +-
gdb/infcall.c | 36 ++++++--------
gdb/infcmd.c | 142 +++++++++++++++++------------------------------------
gdb/infrun.c | 37 +++++++-------
gdb/infrun.h | 4 ++
gdb/linux-nat.c | 23 ---------
gdb/mi/mi-interp.c | 32 +++---------
gdb/target.c | 11 +++++
gdb/target.h | 8 ++-
gdb/top.c | 16 ++++--
gdb/top.h | 3 ++
12 files changed, 126 insertions(+), 192 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index a6994c7..dbf69da 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4662,7 +4662,7 @@ bpstat_do_actions_1 (bpstat *bsp)
if (breakpoint_proceeded)
{
- if (interpreter_async && target_can_async_p ())
+ if (interpreter_async)
/* If we are in async mode, then the target might be still
running, not stopped at any breakpoint, so nothing for
us to do here -- just return to the event loop. */
@@ -11590,7 +11590,7 @@ until_break_command (char *arg, int from_tty, int anywhere)
be deleted when the target stops. Otherwise, we're already
stopped and delete breakpoints via cleanup chain. */
- if (target_can_async_p () && is_running (inferior_ptid))
+ if (is_running (inferior_ptid))
{
struct until_break_command_continuation_args *args;
args = xmalloc (sizeof (*args));
diff --git a/gdb/inf-loop.c b/gdb/inf-loop.c
index eed881d..7b1f724 100644
--- a/gdb/inf-loop.c
+++ b/gdb/inf-loop.c
@@ -73,7 +73,7 @@ inferior_event_handler (enum inferior_event_type event_type,
/* Unregister the inferior from the event loop. This is done
so that when the inferior is not running we don't get
distracted by spurious inferior output. */
- if (target_has_execution)
+ if (target_has_execution && target_can_async_p ())
target_async (0);
}
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 139c361..b19028d 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -36,6 +36,8 @@
#include "gdbthread.h"
#include "event-top.h"
#include "observer.h"
+#include "top.h"
+#include "interps.h"
/* If we can't find a function's name from its address,
we print this instead. */
@@ -388,10 +390,13 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
ptid_t call_thread_ptid = call_thread->ptid;
int saved_sync_execution = sync_execution;
int was_running = call_thread->state == THREAD_RUNNING;
+ int saved_interpreter_async = interpreter_async;
/* Infcalls run synchronously, in the foreground. */
- if (target_can_async_p ())
- sync_execution = 1;
+ sync_execution = 1;
+ /* So that we don't print the prompt prematurely in
+ fetch_inferior_event. */
+ interpreter_async = 0;
call_thread->control.in_infcall = 1;
@@ -404,25 +409,11 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
TRY
{
- int was_sync = sync_execution;
-
proceed (real_pc, GDB_SIGNAL_0);
/* Inferior function calls are always synchronous, even if the
- target supports asynchronous execution. Do here what
- `proceed' itself does in sync mode. */
- if (target_can_async_p ())
- {
- wait_for_inferior ();
- normal_stop ();
- /* If GDB was previously in sync execution mode, then ensure
- that it remains so. normal_stop calls
- async_enable_stdin, so reset it again here. In other
- cases, stdin will be re-enabled by
- inferior_event_handler, when an exception is thrown. */
- if (was_sync)
- async_disable_stdin ();
- }
+ target supports asynchronous execution. */
+ wait_sync_command_done ();
}
CATCH (e, RETURN_MASK_ALL)
{
@@ -430,6 +421,13 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
}
END_CATCH
+ /* If GDB was previously in sync execution mode, then ensure that it
+ remains so. normal_stop calls async_enable_stdin, so reset it
+ again here. In other cases, stdin will be re-enabled by
+ inferior_event_handler, when an exception is thrown. */
+ sync_execution = saved_sync_execution;
+ interpreter_async = saved_interpreter_async;
+
/* At this point the current thread may have changed. Refresh
CALL_THREAD as it could be invalid if its thread has exited. */
call_thread = find_thread_ptid (call_thread_ptid);
@@ -470,8 +468,6 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
if (call_thread != NULL)
call_thread->control.in_infcall = saved_in_infcall;
- sync_execution = saved_sync_execution;
-
return caught_error;
}
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 82399a4..3faff72 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -504,13 +504,12 @@ prepare_execution_command (struct target_ops *target, int background)
if (background && !target->to_can_async_p (target))
error (_("Asynchronous execution not supported on this target."));
- /* If we don't get a request of running in the bg, then we need
- to simulate synchronous (fg) execution. */
- if (!background && target->to_can_async_p (target))
+ if (!background)
{
- /* Simulate synchronous execution. Note no cleanup is necessary
- for this. stdin is re-enabled whenever an error reaches the
- top level. */
+ /* If we get a request for running in the fg, then we need to
+ simulate synchronous (fg) execution. Note no cleanup is
+ necessary for this. stdin is re-enabled whenever an error
+ reaches the top level. */
async_disable_stdin ();
}
}
@@ -937,44 +936,15 @@ step_1 (int skip_subroutines, int single_inst, char *count_string)
make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
}
- /* In synchronous case, all is well; each step_once call will step once. */
- if (!target_can_async_p ())
- {
- for (; count > 0; count--)
- {
- step_once (skip_subroutines, single_inst, count, thread);
-
- if (!target_has_execution)
- break;
- else
- {
- struct thread_info *tp = inferior_thread ();
+ /* do only one step for now, before returning control to the event
+ loop. Let the continuation figure out how many other steps we
+ need to do, and handle them one at the time, through
+ step_once. */
+ step_once (skip_subroutines, single_inst, count, thread);
- if (!tp->control.stop_step || !tp->step_multi)
- {
- /* If we stopped for some reason that is not stepping
- there are no further steps to make. */
- tp->step_multi = 0;
- break;
- }
- }
- }
-
- do_cleanups (cleanups);
- }
- else
- {
- /* In the case of an asynchronous target things get complicated;
- do only one step for now, before returning control to the
- event loop. Let the continuation figure out how many other
- steps we need to do, and handle them one at the time, through
- step_once. */
- step_once (skip_subroutines, single_inst, count, thread);
-
- /* We are running, and the continuation is installed. It will
- disable the longjmp breakpoint as appropriate. */
- discard_cleanups (cleanups);
- }
+ /* We are running, and the continuation is installed. It will
+ disable the longjmp breakpoint as appropriate. */
+ discard_cleanups (cleanups);
}
struct step_1_continuation_args
@@ -1033,6 +1003,7 @@ step_once (int skip_subroutines, int single_inst, int count, int thread)
if (count > 0)
{
+ struct step_1_continuation_args *args;
/* Don't assume THREAD is a valid thread id. It is set to -1 if
the longjmp breakpoint was not required. Use the
INFERIOR_PTID thread instead, which is the same thread when
@@ -1116,21 +1087,14 @@ step_once (int skip_subroutines, int single_inst, int count, int thread)
tp->control.stepping_command = 1;
proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
- /* For async targets, register a continuation to do any
- additional steps. For sync targets, the caller will handle
- further stepping. */
- if (target_can_async_p ())
- {
- struct step_1_continuation_args *args;
-
- args = xmalloc (sizeof (*args));
- args->skip_subroutines = skip_subroutines;
- args->single_inst = single_inst;
- args->count = count;
- args->thread = thread;
+ /* Register a continuation to do any additional steps. */
+ args = xmalloc (sizeof (*args));
+ args->skip_subroutines = skip_subroutines;
+ args->single_inst = single_inst;
+ args->count = count;
+ args->thread = thread;
- add_intermediate_continuation (tp, step_1_continuation, args, xfree);
- }
+ add_intermediate_continuation (tp, step_1_continuation, args, xfree);
}
}
@@ -1442,7 +1406,7 @@ until_next_command (int from_tty)
proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
- if (target_can_async_p () && is_running (inferior_ptid))
+ if (is_running (inferior_ptid))
{
struct until_next_continuation_args *cont_args;
@@ -1801,8 +1765,6 @@ finish_forward (struct symbol *function, struct frame_info *frame)
proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
discard_cleanups (old_chain);
- if (!target_can_async_p ())
- do_all_continuations (0);
}
/* "finish": Set a temporary breakpoint at the place the selected
@@ -2532,8 +2494,7 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
/* The user requested a plain `attach', so be sure to leave
the inferior stopped. */
- if (target_can_async_p ())
- async_enable_stdin ();
+ async_enable_stdin ();
/* At least the current thread is already stopped. */
@@ -2663,6 +2624,7 @@ attach_command (char *args, int from_tty)
E.g. Mach 3 or GNU hurd. */
if (!target_attach_no_wait)
{
+ struct attach_command_continuation_args *a;
struct inferior *inferior = current_inferior ();
/* Careful here. See comments in inferior.h. Basically some
@@ -2672,25 +2634,19 @@ attach_command (char *args, int from_tty)
STOP_QUIETLY_NO_SIGSTOP is for. */
inferior->control.stop_soon = STOP_QUIETLY_NO_SIGSTOP;
- if (target_can_async_p ())
- {
- /* sync_execution mode. Wait for stop. */
- struct attach_command_continuation_args *a;
-
- a = xmalloc (sizeof (*a));
- a->args = xstrdup (args);
- a->from_tty = from_tty;
- a->async_exec = async_exec;
- add_inferior_continuation (attach_command_continuation, a,
- attach_command_continuation_free_args);
-
- /* Done with ARGS. */
- do_cleanups (args_chain);
-
- return;
- }
-
- wait_for_inferior ();
+ /* sync_execution mode. Wait for stop. */
+ a = xmalloc (sizeof (*a));
+ a->args = xstrdup (args);
+ a->from_tty = from_tty;
+ a->async_exec = async_exec;
+ add_inferior_continuation (attach_command_continuation, a,
+ attach_command_continuation_free_args);
+ /* Done with ARGS. */
+ do_cleanups (args_chain);
+
+ if (!target_is_async_p ())
+ mark_infrun_async_event_handler ();
+ return;
}
/* Done with ARGS. */
@@ -2731,6 +2687,7 @@ notice_new_inferior (ptid_t ptid, int leave_running, int from_tty)
if (is_executing (inferior_ptid))
{
+ struct attach_command_continuation_args *a;
struct inferior *inferior = current_inferior ();
/* We're going to install breakpoints, and poke at memory,
@@ -2741,22 +2698,15 @@ notice_new_inferior (ptid_t ptid, int leave_running, int from_tty)
inferior->control.stop_soon = STOP_QUIETLY_REMOTE;
/* Wait for stop before proceeding. */
- if (target_can_async_p ())
- {
- struct attach_command_continuation_args *a;
+ a = xmalloc (sizeof (*a));
+ a->args = xstrdup ("");
+ a->from_tty = from_tty;
+ a->async_exec = async_exec;
+ add_inferior_continuation (attach_command_continuation, a,
+ attach_command_continuation_free_args);
- a = xmalloc (sizeof (*a));
- a->args = xstrdup ("");
- a->from_tty = from_tty;
- a->async_exec = async_exec;
- add_inferior_continuation (attach_command_continuation, a,
- attach_command_continuation_free_args);
-
- do_cleanups (old_chain);
- return;
- }
- else
- wait_for_inferior ();
+ do_cleanups (old_chain);
+ return;
}
async_exec = leave_running;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index a695f2e..195ee81 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -131,6 +131,14 @@ infrun_async (int enable)
}
}
+/* See infrun.h. */
+
+void
+mark_infrun_async_event_handler (void)
+{
+ mark_async_event_handler (infrun_async_inferior_event_token);
+}
+
/* When set, stop the 'step' command if we enter a function which has
no line number information. The normal behavior is that we step
over such function. */
@@ -3090,15 +3098,11 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
discard_cleanups (old_chain);
- /* Wait for it to stop (if not standalone)
- and in any case decode why it stopped, and act accordingly. */
- /* Do this only if we are not using the event loop, or if the target
- does not support asynchronous execution. */
+ /* Tell the event loop to wait for it to stop. If the target
+ supports asynchronous execution, it'll do this from within
+ target_resume. */
if (!target_can_async_p ())
- {
- wait_for_inferior ();
- normal_stop ();
- }
+ mark_async_event_handler (infrun_async_inferior_event_token);
}
\f
@@ -3762,7 +3766,8 @@ fetch_inferior_event (void *client_data)
make_cleanup_restore_integer (&execution_direction);
execution_direction = target_execution_direction ();
- ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws, TARGET_WNOHANG);
+ ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws,
+ target_can_async_p () ? TARGET_WNOHANG : 0);
if (debug_infrun)
print_target_wait_results (waiton_ptid, ecs->ptid, &ecs->ws);
@@ -7512,10 +7517,10 @@ prepare_to_wait (struct execution_control_state *ecs)
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: prepare_to_wait\n");
- /* This is the old end of the while loop. Let everybody know we
- want to wait for the inferior some more and get called again
- soon. */
ecs->wait_some_more = 1;
+
+ if (!target_is_async_p ())
+ mark_infrun_async_event_handler ();
}
/* We are done with the step range of a step/next/si/ni command.
@@ -8808,14 +8813,6 @@ static const struct internalvar_funcs siginfo_funcs =
static void
infrun_async_inferior_event_handler (gdb_client_data data)
{
- /* If the target is closed while this event source is marked, we
- will reach here without execution, or a target to call
- target_wait on, which is an error. Instead of tracking whether
- the target has been popped already, or whether we do have threads
- with pending statutes, simply ignore the event. */
- if (!target_is_async_p ())
- return;
-
inferior_event_handler (INF_REG_EVENT, NULL);
}
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 6108648..1af64a6 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -192,6 +192,10 @@ enum gdb_signal gdb_signal_from_command (int num);
/* Enables/disables infrun's async event source in the event loop. */
extern void infrun_async (int enable);
+/* Call infrun's event handler the next time through the event
+ loop. */
+extern void mark_infrun_async_event_handler (void);
+
/* The global queue of threads that need to do a step-over operation
to get past e.g., a breakpoint. */
extern struct thread_info *step_over_queue_head;
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index be7a915..fa8e5e3 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3442,12 +3442,6 @@ linux_nat_wait_1 (struct target_ops *ops,
target_pid_to_str (lp->ptid));
}
- if (!target_is_async_p ())
- {
- /* Causes SIGINT to be passed on to the attached process. */
- set_sigint_trap ();
- }
-
/* But if we don't find a pending event, we'll have to wait. Always
pull all events out of the kernel. We'll randomly select an
event LWP out of all that have events, to prevent starvation. */
@@ -3518,9 +3512,6 @@ linux_nat_wait_1 (struct target_ops *ops,
ourstatus->kind = TARGET_WAITKIND_NO_RESUMED;
- if (!target_is_async_p ())
- clear_sigint_trap ();
-
restore_child_signals_mask (&prev_mask);
return minus_one_ptid;
}
@@ -3546,9 +3537,6 @@ linux_nat_wait_1 (struct target_ops *ops,
sigsuspend (&suspend_mask);
}
- if (!target_is_async_p ())
- clear_sigint_trap ();
-
gdb_assert (lp);
status = lp->status;
@@ -4627,17 +4615,6 @@ static int async_terminal_is_ours = 1;
static void
linux_nat_terminal_inferior (struct target_ops *self)
{
- /* Like target_terminal_inferior, use target_can_async_p, not
- target_is_async_p, since at this point the target is not async
- yet. If it can async, then we know it will become async prior to
- resume. */
- if (!target_can_async_p ())
- {
- /* Async mode is disabled. */
- child_terminal_inferior (self);
- return;
- }
-
child_terminal_inferior (self);
/* Calls to target_terminal_*() are meant to be idempotent. */
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 99ce385..bdd8e21 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -309,16 +309,8 @@ mi_execute_command_wrapper (const char *cmd)
static void
mi_on_sync_execution_done (void)
{
- /* MI generally prints a prompt after a command, indicating it's
- ready for further input. However, due to an historical wart, if
- MI async, and a (CLI) synchronous command was issued, then we
- will print the prompt right after printing "^running", even if we
- cannot actually accept any input until the target stops. See
- mi_on_resume. However, if the target is async but MI is sync,
- then we need to output the MI prompt now, to replicate gdb's
- behavior when neither the target nor MI are async. (Note this
- observer is only called by the asynchronous target event handling
- code.) */
+ /* If we MI is sync, then output the MI prompt now, indicating we're
+ ready for further input. */
if (!mi_async_p ())
{
fputs_unfiltered ("(gdb) \n", raw_stdout);
@@ -333,20 +325,12 @@ mi_execute_command_input_handler (char *cmd)
{
mi_execute_command_wrapper (cmd);
- /* MI generally prints a prompt after a command, indicating it's
- ready for further input. However, due to an historical wart, if
- MI is async, and a synchronous command was issued, then we will
- print the prompt right after printing "^running", even if we
- cannot actually accept any input until the target stops. See
- mi_on_resume.
-
- If MI is not async, then we print the prompt when the command
- finishes. If the target is sync, that means output the prompt
- now, as in that case executing a command doesn't return until the
- command is done. However, if the target is async, we go back to
- the event loop and output the prompt in the
- 'synchronous_command_done' observer. */
- if (!target_is_async_p () || !sync_execution)
+ /* Print a prompt, indicating we're ready for further input, unless
+ we just started a synchronous command. In that case, we're about
+ to go back to the event loop and will output the prompt in the
+ 'synchronous_command_done' observer when the target next
+ stops. */
+ if (!sync_execution)
{
fputs_unfiltered ("(gdb) \n", raw_stdout);
gdb_flush (raw_stdout);
diff --git a/gdb/target.c b/gdb/target.c
index e41a338..210cc70 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2229,6 +2229,17 @@ target_wait (ptid_t ptid, struct target_waitstatus *status, int options)
return (current_target.to_wait) (¤t_target, ptid, status, options);
}
+/* See target.h. */
+
+ptid_t
+default_target_wait (ptid_t ptid,
+ struct target_waitstatus *status,
+ int options)
+{
+ status->kind = TARGET_WAITKIND_NO_RESUMED;
+ return minus_one_ptid;
+}
+
char *
target_pid_to_str (ptid_t ptid)
{
diff --git a/gdb/target.h b/gdb/target.h
index e283c86..48e6857 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -471,7 +471,7 @@ struct target_ops
ptid_t (*to_wait) (struct target_ops *,
ptid_t, struct target_waitstatus *,
int TARGET_DEBUG_PRINTER (target_debug_print_options))
- TARGET_DEFAULT_NORETURN (noprocess ());
+ TARGET_DEFAULT_FUNC (default_to_wait);
void (*to_fetch_registers) (struct target_ops *, struct regcache *, int)
TARGET_DEFAULT_IGNORE ();
void (*to_store_registers) (struct target_ops *, struct regcache *, int)
@@ -1322,6 +1322,12 @@ extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal);
extern ptid_t target_wait (ptid_t ptid, struct target_waitstatus *status,
int options);
+/* The default target_ops::to_wait implementation. */
+
+extern ptid_t default_target_wait (ptid_t ptid,
+ struct target_waitstatus *status,
+ int options);
+
/* Fetch at least register REGNO, or all regs if regno == -1. No result. */
extern void target_fetch_registers (struct regcache *regcache, int regno);
diff --git a/gdb/top.c b/gdb/top.c
index 061b52f..dc4e357 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -368,6 +368,16 @@ check_frame_language_change (void)
/* See top.h. */
void
+wait_sync_command_done (void)
+{
+ while (gdb_do_one_event () >= 0)
+ if (!sync_execution)
+ break;
+}
+
+/* See top.h. */
+
+void
maybe_wait_sync_command_done (int was_sync)
{
/* If the interpreter is in sync mode (we're running a user
@@ -375,11 +385,7 @@ maybe_wait_sync_command_done (int was_sync)
just ran a synchronous command that started the target, wait
for that command to end. */
if (!interpreter_async && !was_sync && sync_execution)
- {
- while (gdb_do_one_event () >= 0)
- if (!sync_execution)
- break;
- }
+ wait_sync_command_done ();
}
/* Execute the line P as a command, in the current user context.
diff --git a/gdb/top.h b/gdb/top.h
index 987279b..c95c319 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -50,6 +50,9 @@ extern void execute_command (char *, int);
extern void maybe_wait_sync_command_done (int was_sync);
+/* Wait for a synchronous execution command to end. */
+extern void wait_sync_command_done (void);
+
extern void check_frame_language_change (void);
/* Prepare for execution of a command.
--
1.9.3
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/7] Merge async and sync code paths some more
2015-08-12 17:02 ` [PATCH 1/7] Merge async and sync code paths some more Pedro Alves
@ 2015-08-12 19:48 ` Simon Marchi
2015-08-17 17:54 ` Pedro Alves
2015-08-18 10:48 ` Yao Qi
2015-10-16 0:35 ` Joel Brobecker
2 siblings, 1 reply; 26+ messages in thread
From: Simon Marchi @ 2015-08-12 19:48 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 15-08-12 01:01 PM, Pedro Alves wrote:
> diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
> index 99ce385..bdd8e21 100644
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -309,16 +309,8 @@ mi_execute_command_wrapper (const char *cmd)
> static void
> mi_on_sync_execution_done (void)
> {
> - /* MI generally prints a prompt after a command, indicating it's
> - ready for further input. However, due to an historical wart, if
> - MI async, and a (CLI) synchronous command was issued, then we
> - will print the prompt right after printing "^running", even if we
> - cannot actually accept any input until the target stops. See
> - mi_on_resume. However, if the target is async but MI is sync,
> - then we need to output the MI prompt now, to replicate gdb's
> - behavior when neither the target nor MI are async. (Note this
> - observer is only called by the asynchronous target event handling
> - code.) */
> + /* If we MI is sync, then output the MI prompt now, indicating we're
> + ready for further input. */
"If we MI is sync"
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/7] Merge async and sync code paths some more
2015-08-12 19:48 ` Simon Marchi
@ 2015-08-17 17:54 ` Pedro Alves
2015-08-17 19:28 ` Simon Marchi
0 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2015-08-17 17:54 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 08/12/2015 08:48 PM, Simon Marchi wrote:
>> + /* If we MI is sync, then output the MI prompt now, indicating we're
>> + ready for further input. */
>
> "If we MI is sync"
Whoops. I removed the stale "we" locally.
Do you intend to comment on the rest of the series?
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/7] Merge async and sync code paths some more
2015-08-17 17:54 ` Pedro Alves
@ 2015-08-17 19:28 ` Simon Marchi
0 siblings, 0 replies; 26+ messages in thread
From: Simon Marchi @ 2015-08-17 19:28 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 15-08-17 01:54 PM, Pedro Alves wrote:
> On 08/12/2015 08:48 PM, Simon Marchi wrote:
>
>>> + /* If we MI is sync, then output the MI prompt now, indicating we're
>>> + ready for further input. */
>>
>> "If we MI is sync"
>
> Whoops. I removed the stale "we" locally.
>
> Do you intend to comment on the rest of the series?
I tried to read and understand a bit from all the patches, but my brain
melted. So, no I don't have any more comments.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/7] Merge async and sync code paths some more
2015-08-12 17:02 ` [PATCH 1/7] Merge async and sync code paths some more Pedro Alves
2015-08-12 19:48 ` Simon Marchi
@ 2015-08-18 10:48 ` Yao Qi
2015-08-19 14:11 ` Pedro Alves
2015-10-16 0:35 ` Joel Brobecker
2 siblings, 1 reply; 26+ messages in thread
From: Yao Qi @ 2015-08-18 10:48 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> + /* do only one step for now, before returning control to the event
s/do/Do
> + loop. Let the continuation figure out how many other steps we
> + need to do, and handle them one at the time, through
> + step_once. */
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index be7a915..fa8e5e3 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -3442,12 +3442,6 @@ linux_nat_wait_1 (struct target_ops *ops,
> target_pid_to_str (lp->ptid));
> }
>
> - if (!target_is_async_p ())
> - {
> - /* Causes SIGINT to be passed on to the attached process. */
> - set_sigint_trap ();
> - }
> -
so SIGINT is handled by event-top.c:handle_sigint in event loop, right?
> /* But if we don't find a pending event, we'll have to wait. Always
> pull all events out of the kernel. We'll randomly select an
> event LWP out of all that have events, to prevent starvation. */
> @@ -3518,9 +3512,6 @@ linux_nat_wait_1 (struct target_ops *ops,
>
> ourstatus->kind = TARGET_WAITKIND_NO_RESUMED;
>
> - if (!target_is_async_p ())
> - clear_sigint_trap ();
> -
> restore_child_signals_mask (&prev_mask);
> return minus_one_ptid;
> }
> diff --git a/gdb/target.c b/gdb/target.c
> index e41a338..210cc70 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -2229,6 +2229,17 @@ target_wait (ptid_t ptid, struct target_waitstatus *status, int options)
> return (current_target.to_wait) (¤t_target, ptid, status, options);
> }
>
> +/* See target.h. */
> +
> +ptid_t
> +default_target_wait (ptid_t ptid,
> + struct target_waitstatus *status,
> + int options)
> +{
> + status->kind = TARGET_WAITKIND_NO_RESUMED;
> + return minus_one_ptid;
> +}
> +
> char *
> target_pid_to_str (ptid_t ptid)
> {
> diff --git a/gdb/target.h b/gdb/target.h
> index e283c86..48e6857 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -471,7 +471,7 @@ struct target_ops
> ptid_t (*to_wait) (struct target_ops *,
> ptid_t, struct target_waitstatus *,
> int TARGET_DEBUG_PRINTER (target_debug_print_options))
> - TARGET_DEFAULT_NORETURN (noprocess ());
> + TARGET_DEFAULT_FUNC (default_to_wait);
s/default_to_wait/default_target_wait/ ?
Otherwise, the patch looks good to me.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/7] Merge async and sync code paths some more
2015-08-18 10:48 ` Yao Qi
@ 2015-08-19 14:11 ` Pedro Alves
2015-08-27 13:26 ` Yao Qi
0 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2015-08-19 14:11 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 08/18/2015 11:48 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> + /* do only one step for now, before returning control to the event
>
> s/do/Do
Fixed.
>
>> + loop. Let the continuation figure out how many other steps we
>> + need to do, and handle them one at the time, through
>> + step_once. */
>
>> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
>> index be7a915..fa8e5e3 100644
>> --- a/gdb/linux-nat.c
>> +++ b/gdb/linux-nat.c
>> @@ -3442,12 +3442,6 @@ linux_nat_wait_1 (struct target_ops *ops,
>> target_pid_to_str (lp->ptid));
>> }
>>
>> - if (!target_is_async_p ())
>> - {
>> - /* Causes SIGINT to be passed on to the attached process. */
>> - set_sigint_trap ();
>> - }
>> -
>
> so SIGINT is handled by event-top.c:handle_sigint in event loop, right?
Nope, not yet at least, though centralizing sigint handling in the event
loop would probably be a good goal.
It's actually still handled by set_sigint_trap -> inflow.c:pass_signal.
The difference is that after this patch, linux_nat_terminal_inferior
also calls set_sigint_trap itself in sync mode, so these call in
linux_nat_wait_1 are no longer necessary. They actually become harmful,
because set_sigint_trap/clear_sigint_trap are not idempotent.
A sequence of calls like set_sigint_trap -> set_sigint_trap -> clear_sigint_trap
ends up with the wrong SIGINT handler set...
>> --- a/gdb/target.h
>> +++ b/gdb/target.h
>> @@ -471,7 +471,7 @@ struct target_ops
>> ptid_t (*to_wait) (struct target_ops *,
>> ptid_t, struct target_waitstatus *,
>> int TARGET_DEBUG_PRINTER (target_debug_print_options))
>> - TARGET_DEFAULT_NORETURN (noprocess ());
>> + TARGET_DEFAULT_FUNC (default_to_wait);
>
> s/default_to_wait/default_target_wait/ ?
>
Urgh yes, and obviously I forgot to regenerate target-delegates.c
as well... Thanks.
> Otherwise, the patch looks good to me.
Thanks, here's the patch that I plan to push, unless you have
further comments.
From 1e1bd67ec4deb4440ff1c3afc2d4c8b1f6eefaa4 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 17 Aug 2015 18:58:03 +0100
Subject: [PATCH] Merge async and sync code paths some more
This patch makes the execution control code use largely the same
mechanisms in both sync- and async-capable targets. This means using
continuations and use the event loop to react to target events on sync
targets as well. The trick is to immediately mark infrun's event loop
source after resume instead of calling wait_for_inferior. Then
fetch_inferior_event is adjusted to do a blocking wait on sync
targets.
Tested on x86_64 Fedora 20, native and gdbserver, with and without
"maint set target-async off".
gdb/ChangeLog:
2015-08-19 Pedro Alves <palves@redhat.com>
* breakpoint.c (bpstat_do_actions_1, until_break_command): Don't
check whether the target can async.
* inf-loop.c (inferior_event_handler): Only call target_async if
the target can async.
* infcall.c: Include top.h and interps.h.
(run_inferior_call): For the interpreter to sync mode while
running the infcall. Call wait_sync_command_done instead of
wait_for_inferior plus normal_stop.
* infcmd.c (prepare_execution_command): Don't check whether the
target can async when running in the foreground.
(step_1): Delete synchronous case handling.
(step_once): Always install a continuation, even in sync mode.
(until_next_command, finish_forward): Don't check whether the
target can async.
(attach_command_post_wait, notice_new_inferior): Always install a
continuation, even in sync mode.
* infrun.c (mark_infrun_async_event_handler): New function.
(proceed): In sync mode, mark infrun's event source instead of
waiting for events here.
(fetch_inferior_event): If the target can't async, do a blocking
wait.
(prepare_to_wait): In sync mode, mark infrun's event source.
(infrun_async_inferior_event_handler): No longer bail out if the
target can't async.
* infrun.h (mark_infrun_async_event_handler): New declaration.
* linux-nat.c (linux_nat_wait_1): Remove calls to
set_sigint_trap/clear_sigint_trap.
(linux_nat_terminal_inferior): No longer check whether the target
can async.
* mi/mi-interp.c (mi_on_sync_execution_done): Update and simplify
comment.
(mi_execute_command_input_handler): No longer check whether the
target is async. Update and simplify comment.
* target.c (default_target_wait): New function.
* target.h (struct target_ops) <to_wait>: Now defaults to
default_target_wait.
(default_target_wait): Declare.
* top.c (wait_sync_command_done): New function, factored out from
...
(maybe_wait_sync_command_done): ... this.
* top.h (wait_sync_command_done): Declare.
* target-delegates.c: Regenerate.
---
gdb/breakpoint.c | 4 +-
gdb/inf-loop.c | 2 +-
gdb/infcall.c | 36 ++++++-------
gdb/infcmd.c | 142 ++++++++++++++++---------------------------------
gdb/infrun.c | 37 ++++++-------
gdb/infrun.h | 4 ++
gdb/linux-nat.c | 23 --------
gdb/mi/mi-interp.c | 32 +++--------
gdb/target-delegates.c | 8 +--
gdb/target.c | 11 ++++
gdb/target.h | 9 +++-
gdb/top.c | 16 ++++--
gdb/top.h | 3 ++
13 files changed, 128 insertions(+), 199 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 052aeb9..87bd244 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4662,7 +4662,7 @@ bpstat_do_actions_1 (bpstat *bsp)
if (breakpoint_proceeded)
{
- if (interpreter_async && target_can_async_p ())
+ if (interpreter_async)
/* If we are in async mode, then the target might be still
running, not stopped at any breakpoint, so nothing for
us to do here -- just return to the event loop. */
@@ -11590,7 +11590,7 @@ until_break_command (char *arg, int from_tty, int anywhere)
be deleted when the target stops. Otherwise, we're already
stopped and delete breakpoints via cleanup chain. */
- if (target_can_async_p () && is_running (inferior_ptid))
+ if (is_running (inferior_ptid))
{
struct until_break_command_continuation_args *args;
args = xmalloc (sizeof (*args));
diff --git a/gdb/inf-loop.c b/gdb/inf-loop.c
index eed881d..7b1f724 100644
--- a/gdb/inf-loop.c
+++ b/gdb/inf-loop.c
@@ -73,7 +73,7 @@ inferior_event_handler (enum inferior_event_type event_type,
/* Unregister the inferior from the event loop. This is done
so that when the inferior is not running we don't get
distracted by spurious inferior output. */
- if (target_has_execution)
+ if (target_has_execution && target_can_async_p ())
target_async (0);
}
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 139c361..b19028d 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -36,6 +36,8 @@
#include "gdbthread.h"
#include "event-top.h"
#include "observer.h"
+#include "top.h"
+#include "interps.h"
/* If we can't find a function's name from its address,
we print this instead. */
@@ -388,10 +390,13 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
ptid_t call_thread_ptid = call_thread->ptid;
int saved_sync_execution = sync_execution;
int was_running = call_thread->state == THREAD_RUNNING;
+ int saved_interpreter_async = interpreter_async;
/* Infcalls run synchronously, in the foreground. */
- if (target_can_async_p ())
- sync_execution = 1;
+ sync_execution = 1;
+ /* So that we don't print the prompt prematurely in
+ fetch_inferior_event. */
+ interpreter_async = 0;
call_thread->control.in_infcall = 1;
@@ -404,25 +409,11 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
TRY
{
- int was_sync = sync_execution;
-
proceed (real_pc, GDB_SIGNAL_0);
/* Inferior function calls are always synchronous, even if the
- target supports asynchronous execution. Do here what
- `proceed' itself does in sync mode. */
- if (target_can_async_p ())
- {
- wait_for_inferior ();
- normal_stop ();
- /* If GDB was previously in sync execution mode, then ensure
- that it remains so. normal_stop calls
- async_enable_stdin, so reset it again here. In other
- cases, stdin will be re-enabled by
- inferior_event_handler, when an exception is thrown. */
- if (was_sync)
- async_disable_stdin ();
- }
+ target supports asynchronous execution. */
+ wait_sync_command_done ();
}
CATCH (e, RETURN_MASK_ALL)
{
@@ -430,6 +421,13 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
}
END_CATCH
+ /* If GDB was previously in sync execution mode, then ensure that it
+ remains so. normal_stop calls async_enable_stdin, so reset it
+ again here. In other cases, stdin will be re-enabled by
+ inferior_event_handler, when an exception is thrown. */
+ sync_execution = saved_sync_execution;
+ interpreter_async = saved_interpreter_async;
+
/* At this point the current thread may have changed. Refresh
CALL_THREAD as it could be invalid if its thread has exited. */
call_thread = find_thread_ptid (call_thread_ptid);
@@ -470,8 +468,6 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
if (call_thread != NULL)
call_thread->control.in_infcall = saved_in_infcall;
- sync_execution = saved_sync_execution;
-
return caught_error;
}
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 82399a4..49620ac 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -504,13 +504,12 @@ prepare_execution_command (struct target_ops *target, int background)
if (background && !target->to_can_async_p (target))
error (_("Asynchronous execution not supported on this target."));
- /* If we don't get a request of running in the bg, then we need
- to simulate synchronous (fg) execution. */
- if (!background && target->to_can_async_p (target))
+ if (!background)
{
- /* Simulate synchronous execution. Note no cleanup is necessary
- for this. stdin is re-enabled whenever an error reaches the
- top level. */
+ /* If we get a request for running in the fg, then we need to
+ simulate synchronous (fg) execution. Note no cleanup is
+ necessary for this. stdin is re-enabled whenever an error
+ reaches the top level. */
async_disable_stdin ();
}
}
@@ -937,44 +936,15 @@ step_1 (int skip_subroutines, int single_inst, char *count_string)
make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
}
- /* In synchronous case, all is well; each step_once call will step once. */
- if (!target_can_async_p ())
- {
- for (; count > 0; count--)
- {
- step_once (skip_subroutines, single_inst, count, thread);
-
- if (!target_has_execution)
- break;
- else
- {
- struct thread_info *tp = inferior_thread ();
+ /* Do only one step for now, before returning control to the event
+ loop. Let the continuation figure out how many other steps we
+ need to do, and handle them one at the time, through
+ step_once. */
+ step_once (skip_subroutines, single_inst, count, thread);
- if (!tp->control.stop_step || !tp->step_multi)
- {
- /* If we stopped for some reason that is not stepping
- there are no further steps to make. */
- tp->step_multi = 0;
- break;
- }
- }
- }
-
- do_cleanups (cleanups);
- }
- else
- {
- /* In the case of an asynchronous target things get complicated;
- do only one step for now, before returning control to the
- event loop. Let the continuation figure out how many other
- steps we need to do, and handle them one at the time, through
- step_once. */
- step_once (skip_subroutines, single_inst, count, thread);
-
- /* We are running, and the continuation is installed. It will
- disable the longjmp breakpoint as appropriate. */
- discard_cleanups (cleanups);
- }
+ /* We are running, and the continuation is installed. It will
+ disable the longjmp breakpoint as appropriate. */
+ discard_cleanups (cleanups);
}
struct step_1_continuation_args
@@ -1033,6 +1003,7 @@ step_once (int skip_subroutines, int single_inst, int count, int thread)
if (count > 0)
{
+ struct step_1_continuation_args *args;
/* Don't assume THREAD is a valid thread id. It is set to -1 if
the longjmp breakpoint was not required. Use the
INFERIOR_PTID thread instead, which is the same thread when
@@ -1116,21 +1087,14 @@ step_once (int skip_subroutines, int single_inst, int count, int thread)
tp->control.stepping_command = 1;
proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
- /* For async targets, register a continuation to do any
- additional steps. For sync targets, the caller will handle
- further stepping. */
- if (target_can_async_p ())
- {
- struct step_1_continuation_args *args;
-
- args = xmalloc (sizeof (*args));
- args->skip_subroutines = skip_subroutines;
- args->single_inst = single_inst;
- args->count = count;
- args->thread = thread;
+ /* Register a continuation to do any additional steps. */
+ args = xmalloc (sizeof (*args));
+ args->skip_subroutines = skip_subroutines;
+ args->single_inst = single_inst;
+ args->count = count;
+ args->thread = thread;
- add_intermediate_continuation (tp, step_1_continuation, args, xfree);
- }
+ add_intermediate_continuation (tp, step_1_continuation, args, xfree);
}
}
@@ -1442,7 +1406,7 @@ until_next_command (int from_tty)
proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
- if (target_can_async_p () && is_running (inferior_ptid))
+ if (is_running (inferior_ptid))
{
struct until_next_continuation_args *cont_args;
@@ -1801,8 +1765,6 @@ finish_forward (struct symbol *function, struct frame_info *frame)
proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
discard_cleanups (old_chain);
- if (!target_can_async_p ())
- do_all_continuations (0);
}
/* "finish": Set a temporary breakpoint at the place the selected
@@ -2532,8 +2494,7 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
/* The user requested a plain `attach', so be sure to leave
the inferior stopped. */
- if (target_can_async_p ())
- async_enable_stdin ();
+ async_enable_stdin ();
/* At least the current thread is already stopped. */
@@ -2663,6 +2624,7 @@ attach_command (char *args, int from_tty)
E.g. Mach 3 or GNU hurd. */
if (!target_attach_no_wait)
{
+ struct attach_command_continuation_args *a;
struct inferior *inferior = current_inferior ();
/* Careful here. See comments in inferior.h. Basically some
@@ -2672,25 +2634,19 @@ attach_command (char *args, int from_tty)
STOP_QUIETLY_NO_SIGSTOP is for. */
inferior->control.stop_soon = STOP_QUIETLY_NO_SIGSTOP;
- if (target_can_async_p ())
- {
- /* sync_execution mode. Wait for stop. */
- struct attach_command_continuation_args *a;
-
- a = xmalloc (sizeof (*a));
- a->args = xstrdup (args);
- a->from_tty = from_tty;
- a->async_exec = async_exec;
- add_inferior_continuation (attach_command_continuation, a,
- attach_command_continuation_free_args);
-
- /* Done with ARGS. */
- do_cleanups (args_chain);
-
- return;
- }
-
- wait_for_inferior ();
+ /* sync_execution mode. Wait for stop. */
+ a = xmalloc (sizeof (*a));
+ a->args = xstrdup (args);
+ a->from_tty = from_tty;
+ a->async_exec = async_exec;
+ add_inferior_continuation (attach_command_continuation, a,
+ attach_command_continuation_free_args);
+ /* Done with ARGS. */
+ do_cleanups (args_chain);
+
+ if (!target_is_async_p ())
+ mark_infrun_async_event_handler ();
+ return;
}
/* Done with ARGS. */
@@ -2731,6 +2687,7 @@ notice_new_inferior (ptid_t ptid, int leave_running, int from_tty)
if (is_executing (inferior_ptid))
{
+ struct attach_command_continuation_args *a;
struct inferior *inferior = current_inferior ();
/* We're going to install breakpoints, and poke at memory,
@@ -2741,22 +2698,15 @@ notice_new_inferior (ptid_t ptid, int leave_running, int from_tty)
inferior->control.stop_soon = STOP_QUIETLY_REMOTE;
/* Wait for stop before proceeding. */
- if (target_can_async_p ())
- {
- struct attach_command_continuation_args *a;
+ a = xmalloc (sizeof (*a));
+ a->args = xstrdup ("");
+ a->from_tty = from_tty;
+ a->async_exec = async_exec;
+ add_inferior_continuation (attach_command_continuation, a,
+ attach_command_continuation_free_args);
- a = xmalloc (sizeof (*a));
- a->args = xstrdup ("");
- a->from_tty = from_tty;
- a->async_exec = async_exec;
- add_inferior_continuation (attach_command_continuation, a,
- attach_command_continuation_free_args);
-
- do_cleanups (old_chain);
- return;
- }
- else
- wait_for_inferior ();
+ do_cleanups (old_chain);
+ return;
}
async_exec = leave_running;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index a695f2e..195ee81 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -131,6 +131,14 @@ infrun_async (int enable)
}
}
+/* See infrun.h. */
+
+void
+mark_infrun_async_event_handler (void)
+{
+ mark_async_event_handler (infrun_async_inferior_event_token);
+}
+
/* When set, stop the 'step' command if we enter a function which has
no line number information. The normal behavior is that we step
over such function. */
@@ -3090,15 +3098,11 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
discard_cleanups (old_chain);
- /* Wait for it to stop (if not standalone)
- and in any case decode why it stopped, and act accordingly. */
- /* Do this only if we are not using the event loop, or if the target
- does not support asynchronous execution. */
+ /* Tell the event loop to wait for it to stop. If the target
+ supports asynchronous execution, it'll do this from within
+ target_resume. */
if (!target_can_async_p ())
- {
- wait_for_inferior ();
- normal_stop ();
- }
+ mark_async_event_handler (infrun_async_inferior_event_token);
}
\f
@@ -3762,7 +3766,8 @@ fetch_inferior_event (void *client_data)
make_cleanup_restore_integer (&execution_direction);
execution_direction = target_execution_direction ();
- ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws, TARGET_WNOHANG);
+ ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws,
+ target_can_async_p () ? TARGET_WNOHANG : 0);
if (debug_infrun)
print_target_wait_results (waiton_ptid, ecs->ptid, &ecs->ws);
@@ -7512,10 +7517,10 @@ prepare_to_wait (struct execution_control_state *ecs)
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: prepare_to_wait\n");
- /* This is the old end of the while loop. Let everybody know we
- want to wait for the inferior some more and get called again
- soon. */
ecs->wait_some_more = 1;
+
+ if (!target_is_async_p ())
+ mark_infrun_async_event_handler ();
}
/* We are done with the step range of a step/next/si/ni command.
@@ -8808,14 +8813,6 @@ static const struct internalvar_funcs siginfo_funcs =
static void
infrun_async_inferior_event_handler (gdb_client_data data)
{
- /* If the target is closed while this event source is marked, we
- will reach here without execution, or a target to call
- target_wait on, which is an error. Instead of tracking whether
- the target has been popped already, or whether we do have threads
- with pending statutes, simply ignore the event. */
- if (!target_is_async_p ())
- return;
-
inferior_event_handler (INF_REG_EVENT, NULL);
}
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 6108648..1af64a6 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -192,6 +192,10 @@ enum gdb_signal gdb_signal_from_command (int num);
/* Enables/disables infrun's async event source in the event loop. */
extern void infrun_async (int enable);
+/* Call infrun's event handler the next time through the event
+ loop. */
+extern void mark_infrun_async_event_handler (void);
+
/* The global queue of threads that need to do a step-over operation
to get past e.g., a breakpoint. */
extern struct thread_info *step_over_queue_head;
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index be7a915..fa8e5e3 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3442,12 +3442,6 @@ linux_nat_wait_1 (struct target_ops *ops,
target_pid_to_str (lp->ptid));
}
- if (!target_is_async_p ())
- {
- /* Causes SIGINT to be passed on to the attached process. */
- set_sigint_trap ();
- }
-
/* But if we don't find a pending event, we'll have to wait. Always
pull all events out of the kernel. We'll randomly select an
event LWP out of all that have events, to prevent starvation. */
@@ -3518,9 +3512,6 @@ linux_nat_wait_1 (struct target_ops *ops,
ourstatus->kind = TARGET_WAITKIND_NO_RESUMED;
- if (!target_is_async_p ())
- clear_sigint_trap ();
-
restore_child_signals_mask (&prev_mask);
return minus_one_ptid;
}
@@ -3546,9 +3537,6 @@ linux_nat_wait_1 (struct target_ops *ops,
sigsuspend (&suspend_mask);
}
- if (!target_is_async_p ())
- clear_sigint_trap ();
-
gdb_assert (lp);
status = lp->status;
@@ -4627,17 +4615,6 @@ static int async_terminal_is_ours = 1;
static void
linux_nat_terminal_inferior (struct target_ops *self)
{
- /* Like target_terminal_inferior, use target_can_async_p, not
- target_is_async_p, since at this point the target is not async
- yet. If it can async, then we know it will become async prior to
- resume. */
- if (!target_can_async_p ())
- {
- /* Async mode is disabled. */
- child_terminal_inferior (self);
- return;
- }
-
child_terminal_inferior (self);
/* Calls to target_terminal_*() are meant to be idempotent. */
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 99ce385..0935c8f 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -309,16 +309,8 @@ mi_execute_command_wrapper (const char *cmd)
static void
mi_on_sync_execution_done (void)
{
- /* MI generally prints a prompt after a command, indicating it's
- ready for further input. However, due to an historical wart, if
- MI async, and a (CLI) synchronous command was issued, then we
- will print the prompt right after printing "^running", even if we
- cannot actually accept any input until the target stops. See
- mi_on_resume. However, if the target is async but MI is sync,
- then we need to output the MI prompt now, to replicate gdb's
- behavior when neither the target nor MI are async. (Note this
- observer is only called by the asynchronous target event handling
- code.) */
+ /* If MI is sync, then output the MI prompt now, indicating we're
+ ready for further input. */
if (!mi_async_p ())
{
fputs_unfiltered ("(gdb) \n", raw_stdout);
@@ -333,20 +325,12 @@ mi_execute_command_input_handler (char *cmd)
{
mi_execute_command_wrapper (cmd);
- /* MI generally prints a prompt after a command, indicating it's
- ready for further input. However, due to an historical wart, if
- MI is async, and a synchronous command was issued, then we will
- print the prompt right after printing "^running", even if we
- cannot actually accept any input until the target stops. See
- mi_on_resume.
-
- If MI is not async, then we print the prompt when the command
- finishes. If the target is sync, that means output the prompt
- now, as in that case executing a command doesn't return until the
- command is done. However, if the target is async, we go back to
- the event loop and output the prompt in the
- 'synchronous_command_done' observer. */
- if (!target_is_async_p () || !sync_execution)
+ /* Print a prompt, indicating we're ready for further input, unless
+ we just started a synchronous command. In that case, we're about
+ to go back to the event loop and will output the prompt in the
+ 'synchronous_command_done' observer when the target next
+ stops. */
+ if (!sync_execution)
{
fputs_unfiltered ("(gdb) \n", raw_stdout);
gdb_flush (raw_stdout);
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 892cf9d..04d9088 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -117,12 +117,6 @@ delegate_wait (struct target_ops *self, ptid_t arg1, struct target_waitstatus *a
}
static ptid_t
-tdefault_wait (struct target_ops *self, ptid_t arg1, struct target_waitstatus *arg2, int arg3)
-{
- noprocess ();
-}
-
-static ptid_t
debug_wait (struct target_ops *self, ptid_t arg1, struct target_waitstatus *arg2, int arg3)
{
ptid_t result;
@@ -4227,7 +4221,7 @@ install_dummy_methods (struct target_ops *ops)
ops->to_detach = tdefault_detach;
ops->to_disconnect = tdefault_disconnect;
ops->to_resume = tdefault_resume;
- ops->to_wait = tdefault_wait;
+ ops->to_wait = default_target_wait;
ops->to_fetch_registers = tdefault_fetch_registers;
ops->to_store_registers = tdefault_store_registers;
ops->to_prepare_to_store = tdefault_prepare_to_store;
diff --git a/gdb/target.c b/gdb/target.c
index e41a338..9ec7a6b 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2229,6 +2229,17 @@ target_wait (ptid_t ptid, struct target_waitstatus *status, int options)
return (current_target.to_wait) (¤t_target, ptid, status, options);
}
+/* See target.h. */
+
+ptid_t
+default_target_wait (struct target_ops *ops,
+ ptid_t ptid, struct target_waitstatus *status,
+ int options)
+{
+ status->kind = TARGET_WAITKIND_NO_RESUMED;
+ return minus_one_ptid;
+}
+
char *
target_pid_to_str (ptid_t ptid)
{
diff --git a/gdb/target.h b/gdb/target.h
index e283c86..43609cf 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -471,7 +471,7 @@ struct target_ops
ptid_t (*to_wait) (struct target_ops *,
ptid_t, struct target_waitstatus *,
int TARGET_DEBUG_PRINTER (target_debug_print_options))
- TARGET_DEFAULT_NORETURN (noprocess ());
+ TARGET_DEFAULT_FUNC (default_target_wait);
void (*to_fetch_registers) (struct target_ops *, struct regcache *, int)
TARGET_DEFAULT_IGNORE ();
void (*to_store_registers) (struct target_ops *, struct regcache *, int)
@@ -1322,6 +1322,13 @@ extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal);
extern ptid_t target_wait (ptid_t ptid, struct target_waitstatus *status,
int options);
+/* The default target_ops::to_wait implementation. */
+
+extern ptid_t default_target_wait (struct target_ops *ops,
+ ptid_t ptid,
+ struct target_waitstatus *status,
+ int options);
+
/* Fetch at least register REGNO, or all regs if regno == -1. No result. */
extern void target_fetch_registers (struct regcache *regcache, int regno);
diff --git a/gdb/top.c b/gdb/top.c
index 061b52f..dc4e357 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -368,6 +368,16 @@ check_frame_language_change (void)
/* See top.h. */
void
+wait_sync_command_done (void)
+{
+ while (gdb_do_one_event () >= 0)
+ if (!sync_execution)
+ break;
+}
+
+/* See top.h. */
+
+void
maybe_wait_sync_command_done (int was_sync)
{
/* If the interpreter is in sync mode (we're running a user
@@ -375,11 +385,7 @@ maybe_wait_sync_command_done (int was_sync)
just ran a synchronous command that started the target, wait
for that command to end. */
if (!interpreter_async && !was_sync && sync_execution)
- {
- while (gdb_do_one_event () >= 0)
- if (!sync_execution)
- break;
- }
+ wait_sync_command_done ();
}
/* Execute the line P as a command, in the current user context.
diff --git a/gdb/top.h b/gdb/top.h
index 987279b..c95c319 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -50,6 +50,9 @@ extern void execute_command (char *, int);
extern void maybe_wait_sync_command_done (int was_sync);
+/* Wait for a synchronous execution command to end. */
+extern void wait_sync_command_done (void);
+
extern void check_frame_language_change (void);
/* Prepare for execution of a command.
--
1.9.3
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/7] Merge async and sync code paths some more
2015-08-19 14:11 ` Pedro Alves
@ 2015-08-27 13:26 ` Yao Qi
0 siblings, 0 replies; 26+ messages in thread
From: Yao Qi @ 2015-08-27 13:26 UTC (permalink / raw)
To: Pedro Alves; +Cc: Yao Qi, gdb-patches
Pedro Alves <palves@redhat.com> writes:
> Nope, not yet at least, though centralizing sigint handling in the event
> loop would probably be a good goal.
>
> It's actually still handled by set_sigint_trap -> inflow.c:pass_signal.
> The difference is that after this patch, linux_nat_terminal_inferior
> also calls set_sigint_trap itself in sync mode, so these call in
> linux_nat_wait_1 are no longer necessary. They actually become harmful,
> because set_sigint_trap/clear_sigint_trap are not idempotent.
> A sequence of calls like set_sigint_trap -> set_sigint_trap -> clear_sigint_trap
> ends up with the wrong SIGINT handler set...
OK, I see.
>
> Thanks, here's the patch that I plan to push, unless you have
> further comments.
I don't have comments, please go ahead.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/7] Merge async and sync code paths some more
2015-08-12 17:02 ` [PATCH 1/7] Merge async and sync code paths some more Pedro Alves
2015-08-12 19:48 ` Simon Marchi
2015-08-18 10:48 ` Yao Qi
@ 2015-10-16 0:35 ` Joel Brobecker
2015-10-16 12:24 ` Pedro Alves
2 siblings, 1 reply; 26+ messages in thread
From: Joel Brobecker @ 2015-10-16 0:35 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 6587 bytes --]
Hi Pedro,
On Wed, Aug 12, 2015 at 06:01:51PM +0100, Pedro Alves wrote:
> This patch makes the execution control code use largely the same
> mechanisms in both sync- and async-capable targets. This means using
> continuations and use the event loop to react to target events on sync
> targets as well. The trick is to immediately mark infrun's event loop
> source after resume instead of calling wait_for_inferior. Then
> fetch_inferior_event is adjusted to do a blocking wait on sync
> targets.
>
> gdb/ChangeLog:
> 2015-08-12 Pedro Alves <palves@redhat.com>
>
> * breakpoint.c (bpstat_do_actions_1, until_break_command): Don't
> check whether the target can async.
> * inf-loop.c (inferior_event_handler): Only call target_async if
> the target can async.
This patch unfortunately breaks attaching on Windows. I suspect this
might be related to the fact that target_attach_no_wait is True on
this platform (meaning, once we have attached to the inferior, we do not
have to wait for an extra event, unlike on Linux).
This is what we observe, after attaching to any process. At first,
it seems like everything worked fine, since the process stops, and
we get the prompt back:
(gdb) att 3156
Attaching to program `C:\[...]\foo.exe', process 3156
[New Thread 3156.0xcd8]
[New Thread 3156.0xfe4]
0x7770000d in ntdll!DbgBreakPoint () from C:\Windows\SysWOW64\ntdll.dll
(gdb)
However, enter any commands at all, and GDB appears to be hanging.
For instance:
(gdb) set lang ada
[nothing happens]
What happens, in fact, is that despite appearances, GDB is not reading
from the prompt. It is waiting for an event from the inferior. And
since our inferior has stopped, there aren't going to be any event
to read.
In chronological order, what happens is that windows_attach calls
do_initial_windows_stuff, which performs the inferior creation,
and repeatedly waits until we get the first SIGTRAP:
while (1)
{
stop_after_trap = 1;
wait_for_inferior ();
tp = inferior_thread ();
if (tp->suspend.stop_signal != GDB_SIGNAL_TRAP)
resume (tp->suspend.stop_signal);
else
break;
}
The call to wait_for_inferior triggers a call to do_target_wait
to get the event, followed by handle_inferior_event to process it.
However, because the first couple of events are "spurious" events,
GDB resumes the execution, and prepares the inferior to wait again:
case TARGET_WAITKIND_SPURIOUS:
[...]
resume (GDB_SIGNAL_0);
prepare_to_wait (ecs);
And prepare_to_wait just does...
ecs->wait_some_more = 1;
if (!target_is_async_p ())
mark_infrun_async_event_handler ();
... which as a result sets the infrun_async_event_handler "ready"
flag to 1.
We get a couple of spurious events before we get our first SIGTRAP,
at which point we exit the "while (1)" loop above, after which
we reach the end of the attach_command, followed by the normal
end-of-command processing (normal_stop, bp handling, printing the
GDB prompt), back finally to the root of the event loop.
Notice that, at this point, nothing has unset the "ready" flag
for the infrun_async_event_handler. So, when another cycle of
gdb_do_one_event from the event loop, we eventually call
check_async_event_handlers, which finds that the infrun async
event handler is "ready", and therefore calls it's associated
"proc" callback, which does...
inferior_event_handler (INF_REG_EVENT, NULL);
... triggering a call to the target's to_wait method, thus hanging
forever.
Comparing what happens on GNU/Linux, the difference is that we do
expect events from the inferior after attaching. And fetching
those events cause the infrun async event handler's "ready" flag
to be unset before we return to the mainloop.
It seems to me that, for target such as Windows where there is
no need to wait for an event after the we've attached to the inferior,
the flag should be reset somewhere, and the the best place seemed
to be at the end of attach_command, when target_attach_no_wait is
true. That's the infrun_async (0) call.
Although, now that I think of it, shouldn't it we do it like in
the mainloop where we clear the "ready" flag before fetching
the inferior event?
Anyways, adding the "infrun_async (0)" did fix the problem, except
it only fixed it for the first attach. It turns out that the same
problem would resurface if you use the attach command a second time.
For instance:
(gdb) attach 1234
[all OK]
(gdb) detach
[so far, so good]
(gdb) attach 1234
[seems OK, and we get the prompt back, but...]
(gdb)
... at this point we realize that GDB is no longer responsive.
This is because the "ready" flag is set via
mark_infrun_async_event_handler, which has no guard, so always
changes the "ready" flag. However, because there was no public
API for unsetting the infrun async event handler other than
infrun_async, I used that. But I missed the fact that the behavior
of that function is guarded by a global:
/* Stores whether infrun_async was previously enabled or disabled.
Starts off as -1, indicating "never enabled/disabled". */
static int infrun_is_async = -1;
void
infrun_async (int enable)
{
if (infrun_is_async != enable)
{
[...]
}
}
That global got set to zero at the end of the first attach.
But no one else changed the value of that global since then.
So, on the second attach, the added "infrun_async (0)" has
no effect, thus leading us to the same perpetual wait for
inferior events.
I couldn't figure out why we needed both infrun_async and
mark_infrun_async_event_handler, and in particular, I didn't
see the need for the infrun_is_async != enable guard.
So, this patch just makes infrun_async always call the
relevant {mark,clear}_infrun_async_event_handler routine.
If you agree that's the right thing to do, I propose we delete
{mark,clear}_infrun_async_event_handler, or replace them by
the associated infrun_async routine. Although, from someone
not very familiar with all this async stuff, the function
names {mark,clear}_infrun_async_event_handler speak a little
more to me. On the other hand, I like infrun_async because of
the debug trace. So we could also eliminate infrun_async and
move the debug trace into {mark,clear}_infrun_async_event_handler.
We'd have to make clear_infrun_async_event_handler non-static.
The attached patch is a prototype tested on x86-windows.
If you agree, I'll make a proper patch submission, after
having tested it on GNU/Linux as well.
WDYT?
Thanks!
--
Joel
[-- Attachment #2: windows-attach.diff --]
[-- Type: text/x-diff, Size: 1886 bytes --]
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index fc27904..8fb133c 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2821,6 +2821,15 @@ attach_command (char *args, int from_tty)
mark_infrun_async_event_handler ();
return;
}
+ else
+ {
+ /* We don't expect any additional attach event from the target.
+ So make sure that the infrun_async_event_handler is disabled.
+ Otherwise, the main event loop might believe that we have
+ inferior events ready, causing us to wait for those event
+ that will never come, since our inferior is now stopped. */
+ infrun_async (0);
+ }
/* Done with ARGS. */
do_cleanups (args_chain);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index cf91370..2c189f9 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -107,29 +107,20 @@ static int maybe_software_singlestep (struct gdbarch *gdbarch, CORE_ADDR pc);
when we have pending events ready to be passed to the core. */
static struct async_event_handler *infrun_async_inferior_event_token;
-/* Stores whether infrun_async was previously enabled or disabled.
- Starts off as -1, indicating "never enabled/disabled". */
-static int infrun_is_async = -1;
-
/* See infrun.h. */
void
infrun_async (int enable)
{
- if (infrun_is_async != enable)
- {
- infrun_is_async = enable;
-
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog,
- "infrun: infrun_async(%d)\n",
- enable);
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: infrun_async(%d)\n",
+ enable);
- if (enable)
- mark_async_event_handler (infrun_async_inferior_event_token);
- else
- clear_async_event_handler (infrun_async_inferior_event_token);
- }
+ if (enable)
+ mark_async_event_handler (infrun_async_inferior_event_token);
+ else
+ clear_async_event_handler (infrun_async_inferior_event_token);
}
/* See infrun.h. */
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/7] Merge async and sync code paths some more
2015-10-16 0:35 ` Joel Brobecker
@ 2015-10-16 12:24 ` Pedro Alves
2015-10-16 16:22 ` Joel Brobecker
0 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2015-10-16 12:24 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
Hi Joel,
I'm trying to build a Windows gdb to see this in
action. Many thanks for the detailed analysis!
Thanks,
Pedro Alves
On 10/16/2015 01:35 AM, Joel Brobecker wrote:
> Hi Pedro,
>
> On Wed, Aug 12, 2015 at 06:01:51PM +0100, Pedro Alves wrote:
>> This patch makes the execution control code use largely the same
>> mechanisms in both sync- and async-capable targets. This means using
>> continuations and use the event loop to react to target events on sync
>> targets as well. The trick is to immediately mark infrun's event loop
>> source after resume instead of calling wait_for_inferior. Then
>> fetch_inferior_event is adjusted to do a blocking wait on sync
>> targets.
>>
>> gdb/ChangeLog:
>> 2015-08-12 Pedro Alves <palves@redhat.com>
>>
>> * breakpoint.c (bpstat_do_actions_1, until_break_command): Don't
>> check whether the target can async.
>> * inf-loop.c (inferior_event_handler): Only call target_async if
>> the target can async.
>
> This patch unfortunately breaks attaching on Windows. I suspect this
> might be related to the fact that target_attach_no_wait is True on
> this platform (meaning, once we have attached to the inferior, we do not
> have to wait for an extra event, unlike on Linux).
>
> This is what we observe, after attaching to any process. At first,
> it seems like everything worked fine, since the process stops, and
> we get the prompt back:
>
> (gdb) att 3156
> Attaching to program `C:\[...]\foo.exe', process 3156
> [New Thread 3156.0xcd8]
> [New Thread 3156.0xfe4]
> 0x7770000d in ntdll!DbgBreakPoint () from C:\Windows\SysWOW64\ntdll.dll
> (gdb)
>
> However, enter any commands at all, and GDB appears to be hanging.
> For instance:
>
> (gdb) set lang ada
> [nothing happens]
>
> What happens, in fact, is that despite appearances, GDB is not reading
> from the prompt. It is waiting for an event from the inferior. And
> since our inferior has stopped, there aren't going to be any event
> to read.
>
> In chronological order, what happens is that windows_attach calls
> do_initial_windows_stuff, which performs the inferior creation,
> and repeatedly waits until we get the first SIGTRAP:
>
> while (1)
> {
> stop_after_trap = 1;
> wait_for_inferior ();
> tp = inferior_thread ();
> if (tp->suspend.stop_signal != GDB_SIGNAL_TRAP)
> resume (tp->suspend.stop_signal);
> else
> break;
> }
>
> The call to wait_for_inferior triggers a call to do_target_wait
> to get the event, followed by handle_inferior_event to process it.
> However, because the first couple of events are "spurious" events,
> GDB resumes the execution, and prepares the inferior to wait again:
>
> case TARGET_WAITKIND_SPURIOUS:
> [...]
> resume (GDB_SIGNAL_0);
> prepare_to_wait (ecs);
>
> And prepare_to_wait just does...
>
> ecs->wait_some_more = 1;
> if (!target_is_async_p ())
> mark_infrun_async_event_handler ();
>
> ... which as a result sets the infrun_async_event_handler "ready"
> flag to 1.
>
> We get a couple of spurious events before we get our first SIGTRAP,
> at which point we exit the "while (1)" loop above, after which
> we reach the end of the attach_command, followed by the normal
> end-of-command processing (normal_stop, bp handling, printing the
> GDB prompt), back finally to the root of the event loop.
>
> Notice that, at this point, nothing has unset the "ready" flag
> for the infrun_async_event_handler. So, when another cycle of
> gdb_do_one_event from the event loop, we eventually call
> check_async_event_handlers, which finds that the infrun async
> event handler is "ready", and therefore calls it's associated
> "proc" callback, which does...
>
> inferior_event_handler (INF_REG_EVENT, NULL);
>
> ... triggering a call to the target's to_wait method, thus hanging
> forever.
>
> Comparing what happens on GNU/Linux, the difference is that we do
> expect events from the inferior after attaching. And fetching
> those events cause the infrun async event handler's "ready" flag
> to be unset before we return to the mainloop.
>
> It seems to me that, for target such as Windows where there is
> no need to wait for an event after the we've attached to the inferior,
> the flag should be reset somewhere, and the the best place seemed
> to be at the end of attach_command, when target_attach_no_wait is
> true. That's the infrun_async (0) call.
>
> Although, now that I think of it, shouldn't it we do it like in
> the mainloop where we clear the "ready" flag before fetching
> the inferior event?
>
> Anyways, adding the "infrun_async (0)" did fix the problem, except
> it only fixed it for the first attach. It turns out that the same
> problem would resurface if you use the attach command a second time.
> For instance:
>
> (gdb) attach 1234
> [all OK]
> (gdb) detach
> [so far, so good]
> (gdb) attach 1234
> [seems OK, and we get the prompt back, but...]
> (gdb)
>
> ... at this point we realize that GDB is no longer responsive.
>
> This is because the "ready" flag is set via
> mark_infrun_async_event_handler, which has no guard, so always
> changes the "ready" flag. However, because there was no public
> API for unsetting the infrun async event handler other than
> infrun_async, I used that. But I missed the fact that the behavior
> of that function is guarded by a global:
>
> /* Stores whether infrun_async was previously enabled or disabled.
> Starts off as -1, indicating "never enabled/disabled". */
> static int infrun_is_async = -1;
>
> void
> infrun_async (int enable)
> {
> if (infrun_is_async != enable)
> {
> [...]
> }
> }
>
> That global got set to zero at the end of the first attach.
> But no one else changed the value of that global since then.
> So, on the second attach, the added "infrun_async (0)" has
> no effect, thus leading us to the same perpetual wait for
> inferior events.
>
> I couldn't figure out why we needed both infrun_async and
> mark_infrun_async_event_handler, and in particular, I didn't
> see the need for the infrun_is_async != enable guard.
> So, this patch just makes infrun_async always call the
> relevant {mark,clear}_infrun_async_event_handler routine.
>
> If you agree that's the right thing to do, I propose we delete
> {mark,clear}_infrun_async_event_handler, or replace them by
> the associated infrun_async routine. Although, from someone
> not very familiar with all this async stuff, the function
> names {mark,clear}_infrun_async_event_handler speak a little
> more to me. On the other hand, I like infrun_async because of
> the debug trace. So we could also eliminate infrun_async and
> move the debug trace into {mark,clear}_infrun_async_event_handler.
> We'd have to make clear_infrun_async_event_handler non-static.
>
> The attached patch is a prototype tested on x86-windows.
> If you agree, I'll make a proper patch submission, after
> having tested it on GNU/Linux as well.
>
> WDYT?
>
> Thanks!
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/7] Merge async and sync code paths some more
2015-10-16 12:24 ` Pedro Alves
@ 2015-10-16 16:22 ` Joel Brobecker
2015-10-16 16:37 ` Pedro Alves
0 siblings, 1 reply; 26+ messages in thread
From: Joel Brobecker @ 2015-10-16 16:22 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> I'm trying to build a Windows gdb to see this in
> action. Many thanks for the detailed analysis!
Thanks Pedro. Let me know if I can help, I don't mind doing the
leg-work! It's just that I needed a bit of guidance because I don't
have the whole picture on how the mainloop is supposed to work,
particulary with the async-vs-non-async, all-stop-vs-non-stop,
etc.
--
Joel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/7] Merge async and sync code paths some more
2015-10-16 16:22 ` Joel Brobecker
@ 2015-10-16 16:37 ` Pedro Alves
2015-10-16 17:05 ` Joel Brobecker
0 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2015-10-16 16:37 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1539 bytes --]
On 10/16/2015 05:22 PM, Joel Brobecker wrote:
>> I'm trying to build a Windows gdb to see this in
>> action. Many thanks for the detailed analysis!
>
> Thanks Pedro. Let me know if I can help, I don't mind doing the
> leg-work! It's just that I needed a bit of guidance because I don't
> have the whole picture on how the mainloop is supposed to work,
> particulary with the async-vs-non-async, all-stop-vs-non-stop,
> etc.
>
I had forgotten how building on Cygwin takes ages! :-P
So my main gripe with the patch is this:
@@ -2821,6 +2821,15 @@ attach_command (char *args, int from_tty)
mark_infrun_async_event_handler ();
return;
}
+ else
+ {
+ /* We don't expect any additional attach event from the target.
+ So make sure that the infrun_async_event_handler is disabled.
+ Otherwise, the main event loop might believe that we have
+ inferior events ready, causing us to wait for those event
+ that will never come, since our inferior is now stopped. */
+ infrun_async (0);
+ }
If debugging more than one inferior (granted, Windows doesn't
support this, but, alas), this prevents gdb from reacting to pending events
for threads of the other inferior.
How about we make do_initial_windows_stuff call windows_wait/windows_resume
directly, like we did for gdbserver here:
https://sourceware.org/ml/gdb-patches/2013-12/msg00371.html
As mentioned in that thread, I've wanted to do this before
in order to get rid of stop_after_trap.
The patch below seems to work for me.
Thanks,
Pedro Alves
[-- Attachment #2: 0001-no-wfi.patch --]
[-- Type: text/x-patch, Size: 1766 bytes --]
From b77696d634a1ff7e41fe75af24c9d6efcc7c46d2 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 16 Oct 2015 16:12:45 +0100
Subject: [PATCH] no wfi
---
gdb/windows-nat.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 4ab74b4..e6c396b 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1642,7 +1642,6 @@ windows_add_all_dlls (void)
static void
do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
{
- extern int stop_after_trap;
int i;
struct inferior *inf;
struct thread_info *tp;
@@ -1681,16 +1680,20 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
target_terminal_inferior ();
windows_initialization_done = 0;
- inf->control.stop_soon = STOP_QUIETLY;
+
while (1)
{
- stop_after_trap = 1;
- wait_for_inferior ();
- tp = inferior_thread ();
- if (tp->suspend.stop_signal != GDB_SIGNAL_TRAP)
- resume (tp->suspend.stop_signal);
- else
+ struct target_waitstatus status;
+
+ windows_wait (ops, minus_one_ptid, &status, 0);
+
+ /* Note windows_wait returns TARGET_WAITKIND_SPURIOUS for thread
+ events. */
+ if (status.kind != TARGET_WAITKIND_LOADED
+ && status.kind != TARGET_WAITKIND_SPURIOUS)
break;
+
+ windows_resume (ops, minus_one_ptid, 0, GDB_SIGNAL_0);
}
/* Now that the inferior has been started and all DLLs have been mapped,
@@ -1711,8 +1714,6 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
windows_add_all_dlls ();
windows_initialization_done = 1;
- inf->control.stop_soon = NO_STOP_QUIETLY;
- stop_after_trap = 0;
return;
}
--
1.9.3
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/7] Merge async and sync code paths some more
2015-10-16 16:37 ` Pedro Alves
@ 2015-10-16 17:05 ` Joel Brobecker
2015-10-22 16:18 ` Pedro Alves
0 siblings, 1 reply; 26+ messages in thread
From: Joel Brobecker @ 2015-10-16 17:05 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> I had forgotten how building on Cygwin takes ages! :-P
:-)
> So my main gripe with the patch is this:
>
> @@ -2821,6 +2821,15 @@ attach_command (char *args, int from_tty)
> mark_infrun_async_event_handler ();
> return;
> }
> + else
> + {
> + /* We don't expect any additional attach event from the target.
> + So make sure that the infrun_async_event_handler is disabled.
> + Otherwise, the main event loop might believe that we have
> + inferior events ready, causing us to wait for those event
> + that will never come, since our inferior is now stopped. */
> + infrun_async (0);
> + }
>
> If debugging more than one inferior (granted, Windows doesn't
> support this, but, alas), this prevents gdb from reacting to pending events
> for threads of the other inferior.
Ah, yes indeed. And besides, the patch is not Windows-specific
either, so presumably could negatively affect another target
that does support multi-inferior debugging.
> How about we make do_initial_windows_stuff call windows_wait/windows_resume
> directly, like we did for gdbserver here:
>
> https://sourceware.org/ml/gdb-patches/2013-12/msg00371.html
>
> As mentioned in that thread, I've wanted to do this before
> in order to get rid of stop_after_trap.
>
> The patch below seems to work for me.
Of course! Why take the roundabout way when you can take the direct
route? I don't see a reason in this case, and it completely avoids
the issue of the async even handler. And it looks like a nice
simplification to boot, at least to me.
Patch looks good, and I tested it against AdaCore's testsuite, showing
that it fixes the "attach" regressions without introducing any new
failure.
Slightly unrelated to your patch, now, but would you mind sharing
your thoughts on:
> I couldn't figure out why we needed both infrun_async and
> mark_infrun_async_event_handler, and in particular, I didn't
> see the need for the infrun_is_async != enable guard.
> So, this patch just makes infrun_async always call the
> relevant {mark,clear}_infrun_async_event_handler routine.
>
> If you agree that's the right thing to do, I propose we delete
> {mark,clear}_infrun_async_event_handler, or replace them by
> the associated infrun_async routine. Although, from someone
> not very familiar with all this async stuff, the function
> names {mark,clear}_infrun_async_event_handler speak a little
> more to me. On the other hand, I like infrun_async because of
> the debug trace. So we could also eliminate infrun_async and
> move the debug trace into {mark,clear}_infrun_async_event_handler.
> We'd have to make clear_infrun_async_event_handler non-static.
If there is an improvement to be had, be it just adding more comments,
I can take care of that.
Thanks a lot!
--
Joel
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/7] Merge async and sync code paths some more
2015-10-16 17:05 ` Joel Brobecker
@ 2015-10-22 16:18 ` Pedro Alves
0 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-10-22 16:18 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On 10/16/2015 06:05 PM, Joel Brobecker wrote:
> Of course! Why take the roundabout way when you can take the direct
> route? I don't see a reason in this case, and it completely avoids
> the issue of the async even handler. And it looks like a nice
> simplification to boot, at least to me.
>
> Patch looks good, and I tested it against AdaCore's testsuite, showing
> that it fixes the "attach" regressions without introducing any new
> failure.
Alright, now pushed, as below.
From c72f45d16c16954478dcd87531df146f68acd87c Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 22 Oct 2015 16:40:45 +0100
Subject: [PATCH] gdb/Windows: use windows_wait/windows_resume directly in
initial startup
Explanation below based on what Joel wrote at:
https://sourceware.org/ml/gdb-patches/2015-10/msg00274.html
The merge async/sync code paths patch broke attaching on Windows.
This is what we observe, after attaching to any process. At first, it
seems like everything worked fine, since the process stops, and we get
the prompt back:
(gdb) att 3156
Attaching to program `C:\[...]\foo.exe', process 3156
[New Thread 3156.0xcd8]
[New Thread 3156.0xfe4]
0x7770000d in ntdll!DbgBreakPoint () from C:\Windows\SysWOW64\ntdll.dll
(gdb)
However, enter any commands at all, and GDB appears to be hanging.
For instance:
(gdb) set lang ada
[nothing happens]
Despite appearances, GDB is not reading from the prompt. It is
blocked waiting for an event from the inferior. And since our
inferior is stopped, there aren't going to be any events to read.
In chronological order, what happens is that windows_attach calls
do_initial_windows_stuff, which performs the inferior creation,
and repeatedly waits until we get the first SIGTRAP:
while (1)
{
stop_after_trap = 1;
wait_for_inferior ();
tp = inferior_thread ();
if (tp->suspend.stop_signal != GDB_SIGNAL_TRAP)
resume (tp->suspend.stop_signal);
else
break;
}
The call to wait_for_inferior triggers a call to do_target_wait to get
the event, followed by handle_inferior_event to process it. However,
because the first couple of events are "spurious" events, GDB resumes
the execution, and prepares the inferior to wait again:
case TARGET_WAITKIND_SPURIOUS:
[...]
resume (GDB_SIGNAL_0);
prepare_to_wait (ecs);
And prepare_to_wait just does...
ecs->wait_some_more = 1;
if (!target_is_async_p ())
mark_infrun_async_event_handler ();
... which as a result sets the infrun_async_event_handler "ready"
flag to 1.
We get a couple of spurious events before we get the initial SIGTRAP,
at which point we exit the "while (1)" loop above, after which we
reach the end of the attach_command, followed by the normal
end-of-command processing (normal_stop, bp handling, printing the GDB
prompt), back finally to the root of the event loop.
Notice that, at this point, nothing has unset the "ready" flag for the
infrun_async_event_handler. So, when another cycle of
gdb_do_one_event from the event loop, we eventually call
check_async_event_handlers, which finds that the infrun async event
handler is "ready", and therefore calls it's associated "proc"
callback, which does...
inferior_event_handler (INF_REG_EVENT, NULL);
... triggering a blocking call to target_wait, thus hanging forever.
The fix is to use windows_wait and windows_resume directly, similarly
to gdbserver. This will also allow getting rid of 'stop_after_trap'.
gdb/ChangeLog:
2015-10-22 Pedro Alves <palves@redhat.com>
* windows-nat.c (do_initial_windows_stuff): Rewrite loop using
windows_wait and windows_resume directly instead of
wait_for_inferior and resume.
---
gdb/ChangeLog | 6 ++++++
gdb/windows-nat.c | 21 +++++++++++----------
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0df8246..65d056b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2015-10-22 Pedro Alves <palves@redhat.com>
+
+ * windows-nat.c (do_initial_windows_stuff): Rewrite loop using
+ windows_wait and windows_resume directly instead of
+ wait_for_inferior and resume.
+
2015-10-22 Simon Marchi <simon.marchi@polymtl.ca>
* xtensa-tdep.h (XTREG): Add casts.
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 4ab74b4..e6c396b 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1642,7 +1642,6 @@ windows_add_all_dlls (void)
static void
do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
{
- extern int stop_after_trap;
int i;
struct inferior *inf;
struct thread_info *tp;
@@ -1681,16 +1680,20 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
target_terminal_inferior ();
windows_initialization_done = 0;
- inf->control.stop_soon = STOP_QUIETLY;
+
while (1)
{
- stop_after_trap = 1;
- wait_for_inferior ();
- tp = inferior_thread ();
- if (tp->suspend.stop_signal != GDB_SIGNAL_TRAP)
- resume (tp->suspend.stop_signal);
- else
+ struct target_waitstatus status;
+
+ windows_wait (ops, minus_one_ptid, &status, 0);
+
+ /* Note windows_wait returns TARGET_WAITKIND_SPURIOUS for thread
+ events. */
+ if (status.kind != TARGET_WAITKIND_LOADED
+ && status.kind != TARGET_WAITKIND_SPURIOUS)
break;
+
+ windows_resume (ops, minus_one_ptid, 0, GDB_SIGNAL_0);
}
/* Now that the inferior has been started and all DLLs have been mapped,
@@ -1711,8 +1714,6 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
windows_add_all_dlls ();
windows_initialization_done = 1;
- inf->control.stop_soon = NO_STOP_QUIETLY;
- stop_after_trap = 0;
return;
}
--
1.9.3
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 2/7] Replace "struct continuation" mechanism by something more extensible
2015-08-12 17:02 [PATCH 0/7] Replace continuations with an extendable "class" Pedro Alves
2015-08-12 17:02 ` [PATCH 1/7] Merge async and sync code paths some more Pedro Alves
@ 2015-08-12 17:02 ` Pedro Alves
2015-08-18 12:50 ` Yao Qi
2015-08-12 17:02 ` [PATCH 4/7] Convert the until/advance commands to thread_fsm mechanism Pedro Alves
` (5 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2015-08-12 17:02 UTC (permalink / raw)
To: gdb-patches
This adds an object oriented replacement for the "struct continuation"
mechanism, and converts the stepping commands (step, next, stepi,
nexti) and the "finish" commands to use it.
It adds a new thread "class" (struct thread_fsm) that contains the
necessary info and callbacks to manage the state machine of a thread's
execution command.
This allows getting rid of some hacks. E.g., in fetch_inferior_event
and normal_stop we no longer need to know whether a thread is doing a
multi-step (e.g., step N). This effectively makes the
intermediate_continuations unused -- they'll be garbage collected in a
separate patch. (They were never a proper abstraction, IMO. See how
fetch_inferior_event needs to check step_multi before knowing whether
to call INF_EXEC_CONTINUE or INF_EXEC_COMPLETE.)
The target async vs !async uiout hacks in mi_on_normal_stop go away
too.
print_stop_event is no longer called from normal_stop. Instead it is
now called from within each interpreter's normal_stop observer. This
clears the path to make each interpreter print a stop event the way it
sees fit. Currently we have some hacks in common code to
differenciate CLI vs TUI vs MI around this area.
The "finish" command's FSM class stores the return value plus that
value's position in the value history, so that those can be printed to
both MI and CLI's streams. This fixes the CLI "finish" command when
run from MI -- it now also includes the function's return value in the
CLI stream:
(gdb)
~"callee3 (strarg=0x400730 \"A string argument.\") at src/gdb/testsuite/gdb.mi/basics.c:35\n"
~"35\t}\n"
+~"Value returned is $1 = 0\n"
*stopped,reason="function-finished",frame=...,gdb-result-var="$1",return-value="0",thread-id="1",stopped-threads="all",core="0"
-FAIL: gdb.mi/mi-cli.exp: CLI finish: check CLI output
+PASS: gdb.mi/mi-cli.exp: CLI finish: check CLI output
gdb/ChangeLog:
2015-08-12 Pedro Alves <palves@redhat.com>
* Makefile.in (COMMON_OBS): Add thread-fsm.o.
* breakpoint.c (handle_jit_event): Print debug output.
(bpstat_what): Split event callback handling to ...
(bpstat_run_callbacks): ... this new function.
(momentary_bkpt_print_it): No longer handle bp_finish here.
* breakpoint.h (bpstat_run_callbacks): Declare.
* gdbthread.h (struct thread_info) <step_multi>: Delete field.
<thread_fsm>: New field.
(thread_cancel_execution_command): Declare.
* infcmd.c: Include thread-fsm.h.
(struct step_command_fsm): New.
(step_command_fsm_ops): New global.
(new_step_command_fsm, step_command_fsm_prepare): New functions.
(step_1): Adjust to use step_command_fsm_prepare and
prepare_one_step.
(struct step_1_continuation_args): Delete.
(step_1_continuation): Delete.
(step_command_fsm_should_stop): New function.
(step_once): Delete.
(step_command_fsm_clean_up, step_command_fsm_async_reply_reason)
(prepare_one_step): New function, based on step_once.
(until_next_command): Remove step_multi reference.
(struct return_value_info): New.
(print_return_value): Rename to ...
(print_return_value_1): ... this. New struct return_value_info
parameter. Adjust.
(print_return_value): Reimplement as wrapper around
print_return_value_1.
(struct finish_command_fsm): New.
(finish_command_continuation): Delete.
(finish_command_fsm_ops): New global.
(new_finish_command_fsm, finish_command_fsm_should_stop): New
functions.
(finish_command_fsm_clean_up, finish_command_fsm_return_value):
New.
(finish_command_continuation_free_arg): Delete.
(finish_command_fsm_async_reply_reason): New.
(finish_backward, finish_forward): Change symbol parameter to a
finish_command_fsm. Adjust.
(finish_command): Create a finish_command_fsm. Adjust.
* infrun.c: Include "thread-fsm.h".
(clear_proceed_status_thread): Delete the thread's FSM.
(infrun_thread_stop_requested_callback): Cancel the thread's
execution command.
(clean_up_just_stopped_threads_fsms): New function.
(fetch_inferior_event): Handle the event_thread's should_stop
method saying the command isn't done yet.
(process_event_stop_test): Run breakpoint callbacks here.
(print_stop_event): Rename to ...
(print_stop_location): ... this.
(restore_current_uiout_cleanup): New function.
(print_stop_event): Reimplement.
(normal_stop): No longer notify the end_stepping_range observers
here handle "step N" nor "finish" here. No longer call
print_stop_event here.
* infrun.h (struct return_value_info): Forward declare.
(print_return_value): Declare.
(print_stop_event): Change prototype.
* thread-fsm.c: New file.
* thread-fsm.h: New file.
* thread.c: Include "thread-fsm.h".
(thread_cancel_execution_command): New function.
(clear_thread_inferior_resources): Call it.
* cli/cli-interp.c (cli_on_normal_stop): New function.
(cli_interpreter_init): Install cli_on_normal_stop as normal_stop
observer.
* mi/mi-interp.c: Include "thread-fsm.h".
(restore_current_uiout_cleanup): Delete.
(mi_on_normal_stop): If the thread has an FSM associated, and it
finished, ask it for the async-reply-reason to print. Always call
print_stop_event here, regardless of the top-level interpreter.
Check bpstat_what to tell whether an asynchronous breakpoint hit
triggered.
* tui/tui-interp.c (tui_on_normal_stop): New function.
(tui_init): Install tui_on_normal_stop as normal_stop
* observer.
gdb/testsuite/ChangeLog:
2015-08-09 Pedro Alves <palves@redhat.com>
* gdb.mi/mi-cli.exp: Add CLI finish tests.
---
gdb/Makefile.in | 2 +-
gdb/breakpoint.c | 29 +--
gdb/breakpoint.h | 6 +-
gdb/cli/cli-interp.c | 13 +
gdb/gdbthread.h | 11 +-
gdb/infcmd.c | 543 ++++++++++++++++++++++++----------------
gdb/infrun.c | 157 ++++++++----
gdb/infrun.h | 14 +-
gdb/mi/mi-interp.c | 108 +++-----
gdb/testsuite/gdb.mi/mi-cli.exp | 18 ++
gdb/thread-fsm.c | 97 +++++++
gdb/thread-fsm.h | 98 ++++++++
gdb/thread.c | 16 ++
gdb/tui/tui-interp.c | 13 +
14 files changed, 773 insertions(+), 352 deletions(-)
create mode 100644 gdb/thread-fsm.c
create mode 100644 gdb/thread-fsm.h
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 596ddeb..8559d2b 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1028,7 +1028,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
linespec.o dictionary.o \
location.o infcall.o \
infcmd.o infrun.o \
- expprint.o environ.o stack.o thread.o \
+ expprint.o environ.o stack.o thread.o thread-fsm.o \
exceptions.o \
extension.o \
filesystem.o \
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index dbf69da..ef04bde 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5694,6 +5694,9 @@ handle_jit_event (void)
struct frame_info *frame;
struct gdbarch *gdbarch;
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog, "handling bp_jit_event\n");
+
/* Switch terminal for any messages produced by
breakpoint_re_set. */
target_terminal_ours_for_output ();
@@ -5885,16 +5888,13 @@ bpstat_what (bpstat bs_head)
retval.main_action = max (retval.main_action, this_action);
}
- /* These operations may affect the bs->breakpoint_at state so they are
- delayed after MAIN_ACTION is decided above. */
-
- if (jit_event)
- {
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "bpstat_what: bp_jit_event\n");
+ return retval;
+}
- handle_jit_event ();
- }
+void
+bpstat_run_callbacks (bpstat bs_head)
+{
+ bpstat bs;
for (bs = bs_head; bs != NULL; bs = bs->next)
{
@@ -5904,6 +5904,9 @@ bpstat_what (bpstat bs_head)
continue;
switch (b->type)
{
+ case bp_jit_event:
+ handle_jit_event ();
+ break;
case bp_gnu_ifunc_resolver:
gnu_ifunc_resolver_stop (b);
break;
@@ -5912,8 +5915,6 @@ bpstat_what (bpstat bs_head)
break;
}
}
-
- return retval;
}
/* Nonzero if we should step constantly (e.g. watchpoints on machines
@@ -13205,12 +13206,6 @@ momentary_bkpt_print_it (bpstat bs)
switch (b->type)
{
- case bp_finish:
- ui_out_field_string
- (uiout, "reason",
- async_reason_lookup (EXEC_ASYNC_FUNCTION_FINISHED));
- break;
-
case bp_until:
ui_out_field_string
(uiout, "reason",
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 877766a..896d3eb 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1005,7 +1005,11 @@ struct bpstat_what
/* Tell what to do about this bpstat. */
struct bpstat_what bpstat_what (bpstat);
-\f
+
+/* Run breakpoint event callbacks associated with the breakpoints that
+ triggered. */
+extern void bpstat_run_callbacks (bpstat bs_head);
+
/* Find the bpstat associated with a breakpoint. NULL otherwise. */
bpstat bpstat_find_breakpoint (bpstat, struct breakpoint *);
diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
index ce43a4a..174a10b 100644
--- a/gdb/cli/cli-interp.c
+++ b/gdb/cli/cli-interp.c
@@ -40,6 +40,18 @@ static struct gdb_exception safe_execute_command (struct ui_out *uiout,
quiet (i.e., another interpreter is being run with
interpreter-exec), print nothing. */
+/* Observer for the normal_stop notification. */
+
+static void
+cli_on_normal_stop (struct bpstats *bs, int print_frame)
+{
+ if (!interp_quiet_p (cli_interp))
+ {
+ if (print_frame)
+ print_stop_event (cli_uiout);
+ }
+}
+
/* Observer for the signal_received notification. */
static void
@@ -109,6 +121,7 @@ static void *
cli_interpreter_init (struct interp *self, int top_level)
{
/* If changing this, remember to update tui-interp.c as well. */
+ observer_attach_normal_stop (cli_on_normal_stop);
observer_attach_end_stepping_range (cli_on_end_stepping_range);
observer_attach_signal_received (cli_on_signal_received);
observer_attach_signal_exited (cli_on_signal_exited);
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 83b2616..0c99782 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -277,9 +277,10 @@ struct thread_info
command. */
struct continuation *intermediate_continuations;
- /* If stepping, nonzero means step count is > 1 so don't print frame
- next time inferior stops if it stops due to stepping. */
- int step_multi;
+ /* 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;
/* 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
@@ -557,6 +558,10 @@ extern struct thread_info *thread_step_over_chain_next (struct thread_info *tp);
extern int thread_is_in_step_over_chain (struct thread_info *tp);
+/* Cancel any ongoing execution command. */
+
+extern void thread_cancel_execution_command (struct thread_info *thr);
+
extern struct thread_info *thread_list;
#endif /* GDBTHREAD_H */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 3faff72..67b5df0 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -55,6 +55,7 @@
#include "linespec.h"
#include "cli/cli-utils.h"
#include "infcall.h"
+#include "thread-fsm.h"
/* Local functions: */
@@ -89,8 +90,6 @@ static void signal_command (char *, int);
static void jump_command (char *, int);
static void step_1 (int, int, char *);
-static void step_once (int skip_subroutines, int single_inst,
- int count, int thread);
static void next_command (char *, int);
@@ -900,14 +899,88 @@ delete_longjmp_breakpoint_cleanup (void *arg)
delete_longjmp_breakpoint (thread);
}
+/* Data for the FSM that manages the step/next/stepi/nexti
+ commands. */
+
+struct step_command_fsm
+{
+ /* The base class. */
+ struct thread_fsm thread_fsm;
+
+ /* How many steps left in a "step N"-like command. */
+ int count;
+
+ /* If true, this is a next/nexti, otherwise a step/stepi. */
+ int skip_subroutines;
+
+ /* If true, this is a stepi/nexti, otherwise a step/step. */
+ int single_inst;
+
+ /* The thread that the command was run on. */
+ int thread;
+};
+
+static void step_command_fsm_clean_up (struct thread_fsm *self);
+static int step_command_fsm_should_stop (struct thread_fsm *self);
+static enum async_reply_reason
+ step_command_fsm_async_reply_reason (struct thread_fsm *self);
+
+/* step_command_fsm's vtable. */
+
+static struct thread_fsm_ops step_command_fsm_ops =
+{
+ NULL,
+ step_command_fsm_clean_up,
+ step_command_fsm_should_stop,
+ NULL, /* return_value */
+ step_command_fsm_async_reply_reason,
+};
+
+/* Allocate a new step_command_fsm. */
+
+static struct step_command_fsm *
+new_step_command_fsm (void)
+{
+ struct step_command_fsm *sm;
+
+ sm = XCNEW (struct step_command_fsm);
+ thread_fsm_ctor (&sm->thread_fsm, &step_command_fsm_ops);
+
+ return sm;
+}
+
+/* Prepare for a step/next/etc. command. Any target resource
+ allocated here is undone in the FSM's clean_up method. */
+
+static void
+step_command_fsm_prepare (struct step_command_fsm *sm,
+ int skip_subroutines, int single_inst,
+ int count, int thread)
+{
+ struct thread_info *tp = inferior_thread ();
+
+ sm->skip_subroutines = skip_subroutines;
+ sm->single_inst = single_inst;
+ sm->count = count;
+ sm->thread = thread;
+
+ /* Leave the si command alone. */
+ if (!sm->single_inst || sm->skip_subroutines)
+ set_longjmp_breakpoint (tp, get_frame_id (get_current_frame ()));
+
+ tp->control.stepping_command = 1;
+}
+
+static int prepare_one_step (struct step_command_fsm *sm);
+
static void
step_1 (int skip_subroutines, int single_inst, char *count_string)
{
- int count = 1;
- struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
+ int count;
int async_exec;
- int thread = -1;
struct cleanup *args_chain;
+ struct thread_info *thr;
+ struct step_command_fsm *step_sm;
ERROR_NO_INFERIOR;
ensure_not_tfind_mode ();
@@ -924,101 +997,105 @@ step_1 (int skip_subroutines, int single_inst, char *count_string)
/* Done with ARGS. */
do_cleanups (args_chain);
- if (!single_inst || skip_subroutines) /* Leave si command alone. */
- {
- struct thread_info *tp = inferior_thread ();
-
- if (in_thread_list (inferior_ptid))
- thread = pid_to_thread_id (inferior_ptid);
+ clear_proceed_status (1);
- set_longjmp_breakpoint (tp, get_frame_id (get_current_frame ()));
+ /* Setup the execution command state machine to handle all the COUNT
+ steps. */
+ thr = inferior_thread ();
+ step_sm = new_step_command_fsm ();
+ thr->thread_fsm = &step_sm->thread_fsm;
- make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
- }
+ step_command_fsm_prepare (step_sm, skip_subroutines,
+ single_inst, count, thr->num);
- /* do only one step for now, before returning control to the event
+ /* Do only one step for now, before returning control to the event
loop. Let the continuation figure out how many other steps we
need to do, and handle them one at the time, through
step_once. */
- step_once (skip_subroutines, single_inst, count, thread);
-
- /* We are running, and the continuation is installed. It will
- disable the longjmp breakpoint as appropriate. */
- discard_cleanups (cleanups);
+ if (!prepare_one_step (step_sm))
+ proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
+ else
+ {
+ /* Stepped into an inline frame. Pretend that we've
+ stopped. */
+ thread_fsm_clean_up (thr->thread_fsm);
+ normal_stop ();
+ inferior_event_handler (INF_EXEC_COMPLETE, NULL);
+ }
}
-struct step_1_continuation_args
-{
- int count;
- int skip_subroutines;
- int single_inst;
- int thread;
-};
-
-/* Called after we are done with one step operation, to check whether
- we need to step again, before we print the prompt and return control
- to the user. If count is > 1, we will need to do one more call to
- proceed(), via step_once(). Basically it is like step_once and
- step_1_continuation are co-recursive. */
+/* Implementation of the 'should_stop' FSM method for stepping
+ commands. Called after we are done with one step operation, to
+ check whether we need to step again, before we print the prompt and
+ return control to the user. If count is > 1, returns false, as we
+ will need to keep going. */
-static void
-step_1_continuation (void *args, int err)
+static int
+step_command_fsm_should_stop (struct thread_fsm *self)
{
- struct step_1_continuation_args *a = args;
+ struct step_command_fsm *sm = (struct step_command_fsm *) self;
+ struct thread_info *tp;
- if (target_has_execution)
+ tp = inferior_thread ();
+ if (tp->control.stop_step)
{
- struct thread_info *tp;
+ /* There are more steps to make, and we did stop due to
+ ending a stepping range. Do another step. */
+ if (--sm->count > 0)
+ return prepare_one_step (sm);
- tp = inferior_thread ();
- if (!err
- && tp->step_multi && tp->control.stop_step)
- {
- /* There are more steps to make, and we did stop due to
- ending a stepping range. Do another step. */
- step_once (a->skip_subroutines, a->single_inst,
- a->count - 1, a->thread);
- return;
- }
- tp->step_multi = 0;
+ thread_fsm_set_finished (self);
}
- /* We either hit an error, or stopped for some reason that is
- not stepping, or there are no further steps to make.
- Cleanup. */
- if (!a->single_inst || a->skip_subroutines)
- delete_longjmp_breakpoint (a->thread);
+ return 1;
}
-/* Do just one step operation. This is useful to implement the 'step
- n' kind of commands. In case of asynchronous targets, we will have
- to set up a continuation to be done after the target stops (after
- this one step). For synch targets, the caller handles further
- stepping. */
+/* Implementation of the 'clean_up' FSM method for stepping commands. */
static void
-step_once (int skip_subroutines, int single_inst, int count, int thread)
+step_command_fsm_clean_up (struct thread_fsm *self)
{
- struct frame_info *frame = get_current_frame ();
+ struct step_command_fsm *sm = (struct step_command_fsm *) self;
+
+ if (!sm->single_inst || sm->skip_subroutines)
+ delete_longjmp_breakpoint (sm->thread);
+}
- if (count > 0)
+/* Implementation of the 'async_reply_reason' FSM method for stepping
+ commands. */
+
+static enum async_reply_reason
+step_command_fsm_async_reply_reason (struct thread_fsm *self)
+{
+ return EXEC_ASYNC_END_STEPPING_RANGE;
+}
+
+/* Prepare for one step in "step N". The actual target resumption is
+ done by the caller. Return true if we're done and should thus
+ report a stop to the user. Returns false if the target needs to be
+ resumed. */
+
+static int
+prepare_one_step (struct step_command_fsm *sm)
+{
+ if (sm->count > 0)
{
- struct step_1_continuation_args *args;
+ struct frame_info *frame = get_current_frame ();
+
/* Don't assume THREAD is a valid thread id. It is set to -1 if
the longjmp breakpoint was not required. Use the
INFERIOR_PTID thread instead, which is the same thread when
THREAD is set. */
struct thread_info *tp = inferior_thread ();
- clear_proceed_status (1);
set_step_frame ();
- if (!single_inst)
+ if (!sm->single_inst)
{
CORE_ADDR pc;
/* Step at an inlined function behaves like "down". */
- if (!skip_subroutines
+ if (!sm->skip_subroutines
&& inline_skipped_frames (inferior_ptid))
{
ptid_t resume_ptid;
@@ -1028,17 +1105,8 @@ step_once (int skip_subroutines, int single_inst, int count, int thread)
set_running (resume_ptid, 1);
step_into_inline_frame (inferior_ptid);
- if (count > 1)
- step_once (skip_subroutines, single_inst, count - 1, thread);
- else
- {
- /* Pretend that we've stopped. */
- normal_stop ();
-
- if (target_can_async_p ())
- inferior_event_handler (INF_EXEC_COMPLETE, NULL);
- }
- return;
+ sm->count--;
+ return prepare_one_step (sm);
}
pc = get_frame_pc (frame);
@@ -1073,29 +1141,22 @@ step_once (int skip_subroutines, int single_inst, int count, int thread)
{
/* Say we are stepping, but stop after one insn whatever it does. */
tp->control.step_range_start = tp->control.step_range_end = 1;
- if (!skip_subroutines)
+ if (!sm->skip_subroutines)
/* It is stepi.
Don't step over function calls, not even to functions lacking
line numbers. */
tp->control.step_over_calls = STEP_OVER_NONE;
}
- if (skip_subroutines)
+ if (sm->skip_subroutines)
tp->control.step_over_calls = STEP_OVER_ALL;
- tp->step_multi = (count > 1);
- tp->control.stepping_command = 1;
- proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
-
- /* Register a continuation to do any additional steps. */
- args = xmalloc (sizeof (*args));
- args->skip_subroutines = skip_subroutines;
- args->single_inst = single_inst;
- args->count = count;
- args->thread = thread;
-
- add_intermediate_continuation (tp, step_1_continuation, args, xfree);
+ return 0;
}
+
+ /* Done. */
+ thread_fsm_set_finished (&sm->thread_fsm);
+ return 1;
}
\f
@@ -1399,8 +1460,6 @@ until_next_command (int from_tty)
tp->control.step_over_calls = STEP_OVER_ALL;
- tp->step_multi = 0; /* Only one call to proceed */
-
set_longjmp_breakpoint (tp, get_frame_id (frame));
old_chain = make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
@@ -1528,18 +1587,29 @@ get_return_value (struct value *function, struct type *value_type,
return value;
}
-/* Print the result of a function at the end of a 'finish' command.
- DTOR_DATA (if not NULL) can represent inferior registers right after
- an inferior call has finished. */
+/* The captured function return value/type and its position in the
+ value history. */
-static void
-print_return_value (struct value *function, struct type *value_type,
- struct dummy_frame_context_saver *ctx_saver)
+struct return_value_info
{
- struct value *value = get_return_value (function, value_type, ctx_saver);
- struct ui_out *uiout = current_uiout;
+ /* The captured return value. May be NULL if we weren't able to
+ retrieve it. See get_return_value. */
+ struct value *value;
- if (value)
+ /* The return type. In some cases, we'll not be able extract the
+ return value, but we always know the type. */
+ struct type *type;
+
+ /* If we captured a value, this is the value history index. */
+ int value_history_index;
+};
+
+/* Helper for print_return_value. */
+
+static void
+print_return_value_1 (struct ui_out *uiout, struct return_value_info *rv)
+{
+ if (rv->value != NULL)
{
struct value_print_options opts;
struct ui_file *stb;
@@ -1550,10 +1620,10 @@ print_return_value (struct value *function, struct type *value_type,
old_chain = make_cleanup_ui_file_delete (stb);
ui_out_text (uiout, "Value returned is ");
ui_out_field_fmt (uiout, "gdb-result-var", "$%d",
- record_latest_value (value));
+ rv->value_history_index);
ui_out_text (uiout, " = ");
get_no_prettyformat_print_options (&opts);
- value_print (value, stb, &opts);
+ value_print (rv->value, stb, &opts);
ui_out_field_stream (uiout, "return-value", stb);
ui_out_text (uiout, "\n");
do_cleanups (old_chain);
@@ -1563,7 +1633,7 @@ print_return_value (struct value *function, struct type *value_type,
struct cleanup *oldchain;
char *type_name;
- type_name = type_to_string (value_type);
+ type_name = type_to_string (rv->type);
oldchain = make_cleanup (xfree, type_name);
ui_out_text (uiout, "Value returned has type: ");
ui_out_field_string (uiout, "return-type", type_name);
@@ -1573,100 +1643,173 @@ print_return_value (struct value *function, struct type *value_type,
}
}
-/* Stuff that needs to be done by the finish command after the target
- has stopped. In asynchronous mode, we wait for the target to stop
- in the call to poll or select in the event loop, so it is
- impossible to do all the stuff as part of the finish_command
- function itself. The only chance we have to complete this command
- is in fetch_inferior_event, which is called by the event loop as
- soon as it detects that the target has stopped. */
+/* Print the result of a function at the end of a 'finish' command.
+ RV points at an object representing the captured return value/type
+ and its position in the value history. */
-struct finish_command_continuation_args
+void
+print_return_value (struct ui_out *uiout, struct return_value_info *rv)
{
- /* The thread that as current when the command was executed. */
+ if (rv->type == NULL || TYPE_CODE (rv->type) == TYPE_CODE_VOID)
+ return;
+
+ TRY
+ {
+ /* print_return_value_1 can throw an exception in some
+ circumstances. We need to catch this so that we still
+ delete the breakpoint. */
+ print_return_value_1 (uiout, rv);
+ }
+ CATCH (ex, RETURN_MASK_ALL)
+ {
+ exception_print (gdb_stdout, ex);
+ }
+ END_CATCH
+}
+
+/* Data for the FSM that manages the finish command. */
+
+struct finish_command_fsm
+{
+ /* The base class. */
+ struct thread_fsm thread_fsm;
+
+ /* The thread that was current when the command was executed. */
int thread;
+
+ /* The momentary breakpoint set at the function's return address in
+ the caller. */
struct breakpoint *breakpoint;
+
+ /* The function that we're stepping out of. */
struct symbol *function;
- /* Inferior registers stored right before dummy_frame has been freed
- after an inferior call. It can be NULL if no inferior call was
- involved, GDB will then use current inferior registers. */
- struct dummy_frame_context_saver *ctx_saver;
+ /* If the FSM finishes successfully, this stores the function's
+ return value. */
+ struct return_value_info return_value;
};
-static void
-finish_command_continuation (void *arg, int err)
+static int finish_command_fsm_should_stop (struct thread_fsm *self);
+static void finish_command_fsm_clean_up (struct thread_fsm *self);
+static struct return_value_info *
+ finish_command_fsm_return_value (struct thread_fsm *self);
+static enum async_reply_reason
+ finish_command_fsm_async_reply_reason (struct thread_fsm *self);
+
+/* finish_command_fsm's vtable. */
+
+static struct thread_fsm_ops finish_command_fsm_ops =
+{
+ NULL, /* dtor */
+ finish_command_fsm_clean_up,
+ finish_command_fsm_should_stop,
+ finish_command_fsm_return_value,
+ finish_command_fsm_async_reply_reason,
+};
+
+/* Allocate a new finish_command_fsm. */
+
+static struct finish_command_fsm *
+new_finish_command_fsm (int thread)
+{
+ struct finish_command_fsm *sm;
+
+ sm = XCNEW (struct finish_command_fsm);
+ thread_fsm_ctor (&sm->thread_fsm, &finish_command_fsm_ops);
+
+ sm->thread = thread;
+
+ return sm;
+}
+
+/* Implementation of the 'should_stop' FSM method for the finish
+ commands. Detects whether the thread stepped out of the function
+ successfully, and if so, captures the function's return value and
+ marks the FSM finished. */
+
+static int
+finish_command_fsm_should_stop (struct thread_fsm *self)
{
- struct finish_command_continuation_args *a = arg;
+ struct finish_command_fsm *f = (struct finish_command_fsm *) self;
+ struct return_value_info *rv = &f->return_value;
+ struct thread_info *tp = inferior_thread ();
- if (!err)
+ if (f->function != NULL
+ && bpstat_find_breakpoint (tp->control.stop_bpstat,
+ f->breakpoint) != NULL)
{
- struct thread_info *tp = NULL;
- bpstat bs = NULL;
+ /* We're done. */
+ thread_fsm_set_finished (self);
- if (!ptid_equal (inferior_ptid, null_ptid)
- && target_has_execution
- && is_stopped (inferior_ptid))
- {
- tp = inferior_thread ();
- bs = tp->control.stop_bpstat;
- }
+ rv->type = TYPE_TARGET_TYPE (SYMBOL_TYPE (f->function));
+ if (rv->type == NULL)
+ internal_error (__FILE__, __LINE__,
+ _("finish_command: function has no target type"));
- if (bpstat_find_breakpoint (bs, a->breakpoint) != NULL
- && a->function != NULL)
+ if (TYPE_CODE (rv->type) != TYPE_CODE_VOID)
{
- struct type *value_type;
+ struct value *func;
- value_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (a->function));
- if (!value_type)
- internal_error (__FILE__, __LINE__,
- _("finish_command: function has no target type"));
+ func = read_var_value (f->function, get_current_frame ());
+ rv->value = get_return_value (func, rv->type, NULL);
+ rv->value_history_index = record_latest_value (rv->value);
+ }
+ }
+ else if (tp->control.stop_step)
+ {
+ /* Finishing from an inline frame, or reverse finishing. In
+ either case, there's no way to retrieve the return value. */
+ thread_fsm_set_finished (self);
+ }
- if (TYPE_CODE (value_type) != TYPE_CODE_VOID)
- {
- struct value *func;
+ return 1;
+}
- func = read_var_value (a->function, get_current_frame ());
- TRY
- {
- /* print_return_value can throw an exception in some
- circumstances. We need to catch this so that we still
- delete the breakpoint. */
- print_return_value (func, value_type, a->ctx_saver);
- }
- CATCH (ex, RETURN_MASK_ALL)
- {
- exception_print (gdb_stdout, ex);
- }
- END_CATCH
- }
- }
+/* Implementation of the 'clean_up' FSM method for the finish
+ commands. */
- /* We suppress normal call of normal_stop observer and do it
- here so that the *stopped notification includes the return
- value. */
- if (bs != NULL && tp->control.proceed_to_finish)
- observer_notify_normal_stop (bs, 1 /* print frame */);
+static void
+finish_command_fsm_clean_up (struct thread_fsm *self)
+{
+ struct finish_command_fsm *f = (struct finish_command_fsm *) self;
+
+ if (f->breakpoint != NULL)
+ {
+ delete_breakpoint (f->breakpoint);
+ f->breakpoint = NULL;
}
+ delete_longjmp_breakpoint (f->thread);
+}
- delete_breakpoint (a->breakpoint);
- delete_longjmp_breakpoint (a->thread);
+/* Implementation of the 'return_value' FSM method for the finish
+ commands. */
+
+static struct return_value_info *
+finish_command_fsm_return_value (struct thread_fsm *self)
+{
+ struct finish_command_fsm *f = (struct finish_command_fsm *) self;
+
+ return &f->return_value;
}
-static void
-finish_command_continuation_free_arg (void *arg)
+/* Implementation of the 'async_reply_reason' FSM method for the
+ finish commands. */
+
+static enum async_reply_reason
+finish_command_fsm_async_reply_reason (struct thread_fsm *self)
{
- struct finish_command_continuation_args *cargs = arg;
+ struct finish_command_fsm *f = (struct finish_command_fsm *) self;
- if (cargs->ctx_saver != NULL)
- dummy_frame_context_saver_drop (cargs->ctx_saver);
- xfree (cargs);
+ if (execution_direction == EXEC_REVERSE)
+ return EXEC_ASYNC_END_STEPPING_RANGE;
+ else
+ return EXEC_ASYNC_FUNCTION_FINISHED;
}
/* finish_backward -- helper function for finish_command. */
static void
-finish_backward (struct symbol *function)
+finish_backward (struct finish_command_fsm *sm)
{
struct symtab_and_line sal;
struct thread_info *tp = inferior_thread ();
@@ -1715,56 +1858,33 @@ finish_backward (struct symbol *function)
}
}
-/* finish_forward -- helper function for finish_command. */
+/* finish_forward -- helper function for finish_command. FRAME is the
+ frame that called the function we're about to step out of. */
static void
-finish_forward (struct symbol *function, struct frame_info *frame)
+finish_forward (struct finish_command_fsm *sm, struct frame_info *frame)
{
struct frame_id frame_id = get_frame_id (frame);
struct gdbarch *gdbarch = get_frame_arch (frame);
struct symtab_and_line sal;
struct thread_info *tp = inferior_thread ();
- struct breakpoint *breakpoint;
- struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
- struct finish_command_continuation_args *cargs;
- int thread = tp->num;
- struct dummy_frame_context_saver *saver = NULL;
sal = find_pc_line (get_frame_pc (frame), 0);
sal.pc = get_frame_pc (frame);
- if (get_frame_type (frame) == DUMMY_FRAME)
- {
- saver = dummy_frame_context_saver_setup (get_stack_frame_id (frame),
- inferior_ptid);
- make_cleanup (dummy_frame_context_saver_cleanup, saver);
- }
-
- breakpoint = set_momentary_breakpoint (gdbarch, sal,
- get_stack_frame_id (frame),
- bp_finish);
+ sm->breakpoint = set_momentary_breakpoint (gdbarch, sal,
+ get_stack_frame_id (frame),
+ bp_finish);
/* set_momentary_breakpoint invalidates FRAME. */
frame = NULL;
- make_cleanup_delete_breakpoint (breakpoint);
-
set_longjmp_breakpoint (tp, frame_id);
- make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
/* We want to print return value, please... */
tp->control.proceed_to_finish = 1;
- cargs = xmalloc (sizeof (*cargs));
-
- cargs->thread = thread;
- cargs->breakpoint = breakpoint;
- cargs->function = function;
- cargs->ctx_saver = saver;
- add_continuation (tp, finish_command_continuation, cargs,
- finish_command_continuation_free_arg);
- proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
- discard_cleanups (old_chain);
+ proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
}
/* "finish": Set a temporary breakpoint at the place the selected
@@ -1774,9 +1894,10 @@ static void
finish_command (char *arg, int from_tty)
{
struct frame_info *frame;
- struct symbol *function;
int async_exec;
struct cleanup *args_chain;
+ struct finish_command_fsm *sm;
+ struct thread_info *tp;
ERROR_NO_INFERIOR;
ensure_not_tfind_mode ();
@@ -1801,9 +1922,14 @@ finish_command (char *arg, int from_tty)
clear_proceed_status (0);
+ tp = inferior_thread ();
+
+ sm = new_finish_command_fsm (tp->num);
+
+ tp->thread_fsm = &sm->thread_fsm;
+
/* Finishing from an inline frame is completely different. We don't
- try to show the "return value" - no way to locate it. So we do
- not need a completion. */
+ try to show the "return value" - no way to locate it. */
if (get_frame_type (get_selected_frame (_("No selected frame.")))
== INLINE_FRAME)
{
@@ -1812,7 +1938,6 @@ finish_command (char *arg, int from_tty)
called by that frame. We don't use the magic "1" value for
step_range_end, because then infrun will think this is nexti,
and not step over the rest of this inlined function call. */
- struct thread_info *tp = inferior_thread ();
struct symtab_and_line empty_sal;
init_sal (&empty_sal);
@@ -1840,7 +1965,7 @@ finish_command (char *arg, int from_tty)
/* Find the function we will return from. */
- function = find_pc_function (get_frame_pc (get_selected_frame (NULL)));
+ sm->function = find_pc_function (get_frame_pc (get_selected_frame (NULL)));
/* Print info on the selected frame, including level number but not
source. */
@@ -1850,10 +1975,10 @@ finish_command (char *arg, int from_tty)
printf_filtered (_("Run back to call of "));
else
{
- if (function != NULL && TYPE_NO_RETURN (function->type)
+ if (sm->function != NULL && TYPE_NO_RETURN (sm->function->type)
&& !query (_("warning: Function %s does not return normally.\n"
"Try to finish anyway? "),
- SYMBOL_PRINT_NAME (function)))
+ SYMBOL_PRINT_NAME (sm->function)))
error (_("Not confirmed."));
printf_filtered (_("Run till exit from "));
}
@@ -1862,9 +1987,9 @@ finish_command (char *arg, int from_tty)
}
if (execution_direction == EXEC_REVERSE)
- finish_backward (function);
+ finish_backward (sm);
else
- finish_forward (function, frame);
+ finish_forward (sm, frame);
}
\f
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 195ee81..06526f0 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -62,6 +62,7 @@
#include "terminal.h"
#include "solist.h"
#include "event-loop.h"
+#include "thread-fsm.h"
/* Prototypes for local functions */
@@ -2745,6 +2746,9 @@ clear_proceed_status_thread (struct thread_info *tp)
if (!signal_pass_state (tp->suspend.stop_signal))
tp->suspend.stop_signal = GDB_SIGNAL_0;
+ thread_fsm_delete (tp->thread_fsm);
+ tp->thread_fsm = NULL;
+
tp->control.trap_expected = 0;
tp->control.step_range_start = 0;
tp->control.step_range_end = 0;
@@ -3216,7 +3220,7 @@ infrun_thread_stop_requested_callback (struct thread_info *info, void *arg)
have consistent output as if the stop event had been
reported. */
ecs->ptid = info->ptid;
- ecs->event_thread = find_thread_ptid (info->ptid);
+ ecs->event_thread = info;
ecs->ws.kind = TARGET_WAITKIND_STOPPED;
ecs->ws.value.sig = GDB_SIGNAL_0;
@@ -3226,6 +3230,9 @@ infrun_thread_stop_requested_callback (struct thread_info *info, void *arg)
{
struct thread_info *tp;
+ /* Cancel any running execution command. */
+ thread_cancel_execution_command (info);
+
normal_stop ();
/* Finish off the continuations. */
@@ -3714,6 +3721,35 @@ reinstall_readline_callback_handler_cleanup (void *arg)
gdb_rl_callback_handler_reinstall ();
}
+/* Clean up the FSMs of threads that are now stopped. In non-stop,
+ that's just the event thread. In all-stop, that's all threads. */
+
+static void
+clean_up_just_stopped_threads_fsms (struct execution_control_state *ecs)
+{
+ struct thread_info *thr = ecs->event_thread;
+
+ if (thr != NULL && thr->thread_fsm != NULL)
+ thread_fsm_clean_up (thr->thread_fsm);
+
+ if (!non_stop)
+ {
+ ALL_NON_EXITED_THREADS (thr)
+ {
+ if (thr->thread_fsm == NULL)
+ continue;
+ if (thr == ecs->event_thread)
+ continue;
+
+ switch_to_thread (thr->ptid);
+ thread_fsm_clean_up (thr->thread_fsm);
+ }
+
+ if (ecs->event_thread != NULL)
+ switch_to_thread (ecs->event_thread->ptid);
+ }
+}
+
/* Asynchronous version of wait_for_inferior. It is called by the
event loop whenever a change of state is detected on the file
descriptor corresponding to the target. It can be called more than
@@ -3792,22 +3828,31 @@ fetch_inferior_event (void *client_data)
if (!ecs->wait_some_more)
{
struct inferior *inf = find_inferior_ptid (ecs->ptid);
+ int should_stop = 1;
+ struct thread_info *thr = ecs->event_thread;
delete_just_stopped_threads_infrun_breakpoints ();
- /* We may not find an inferior if this was a process exit. */
- if (inf == NULL || inf->control.stop_soon == NO_STOP_QUIETLY)
- normal_stop ();
-
- if (target_has_execution
- && ecs->ws.kind != TARGET_WAITKIND_NO_RESUMED
- && ecs->ws.kind != TARGET_WAITKIND_EXITED
- && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED
- && ecs->event_thread->step_multi
- && ecs->event_thread->control.stop_step)
- inferior_event_handler (INF_EXEC_CONTINUE, NULL);
+ if (thr != NULL)
+ {
+ struct thread_fsm *thread_fsm = thr->thread_fsm;
+
+ if (thread_fsm != NULL)
+ should_stop = thread_fsm_should_stop (thread_fsm);
+ }
+
+ if (!should_stop)
+ {
+ keep_going (ecs);
+ }
else
{
+ clean_up_just_stopped_threads_fsms (ecs);
+
+ /* We may not find an inferior if this was a process exit. */
+ if (inf == NULL || inf->control.stop_soon == NO_STOP_QUIETLY)
+ normal_stop ();
+
inferior_event_handler (INF_EXEC_COMPLETE, NULL);
cmd_done = 1;
}
@@ -5889,6 +5934,10 @@ process_event_stop_test (struct execution_control_state *ecs)
stop_stack_dummy = what.call_dummy;
}
+ /* A few breakpoint types have callbacks associated (e.g.,
+ bp_jit_event). Run them now. */
+ bpstat_run_callbacks (ecs->event_thread->control.stop_bpstat);
+
/* If we hit an internal event that triggers symbol changes, the
current frame will be invalidated within bpstat_what (e.g., if we
hit an internal solib event). Re-fetch it. */
@@ -7655,8 +7704,8 @@ print_no_history_reason (struct ui_out *uiout)
bpstat_print contains the logic deciding in detail what to print,
based on the event(s) that just occurred. */
-void
-print_stop_event (struct target_waitstatus *ws)
+static void
+print_stop_location (struct target_waitstatus *ws)
{
int bpstat_ret;
enum print_what source_flag;
@@ -7707,9 +7756,50 @@ print_stop_event (struct target_waitstatus *ws)
SRC_AND_LOC: Print location and source line. */
if (do_frame_printing)
print_stack_frame (get_selected_frame (NULL), 0, source_flag, 1);
+}
+
+/* Cleanup that restores a previous current uiout. */
+
+static void
+restore_current_uiout_cleanup (void *arg)
+{
+ struct ui_out *saved_uiout = arg;
+
+ current_uiout = saved_uiout;
+}
+
+/* See infrun.h. */
+
+void
+print_stop_event (struct ui_out *uiout)
+{
+ struct cleanup *old_chain;
+ struct target_waitstatus last;
+ ptid_t last_ptid;
+ struct thread_info *tp;
+
+ get_last_target_status (&last_ptid, &last);
+
+ old_chain = make_cleanup (restore_current_uiout_cleanup, current_uiout);
+ current_uiout = uiout;
+
+ print_stop_location (&last);
/* Display the auto-display expressions. */
do_displays ();
+
+ do_cleanups (old_chain);
+
+ tp = inferior_thread ();
+ if (tp->thread_fsm != NULL
+ && thread_fsm_finished_p (tp->thread_fsm))
+ {
+ struct return_value_info *rv;
+
+ rv = thread_fsm_return_value (tp->thread_fsm);
+ if (rv != NULL)
+ print_return_value (uiout, rv);
+ }
}
/* Here to return control to GDB when the inferior stops for real.
@@ -7822,20 +7912,6 @@ normal_stop (void)
if (stopped_by_random_signal)
disable_current_display ();
- /* Notify observers if we finished a "step"-like command, etc. */
- if (target_has_execution
- && last.kind != TARGET_WAITKIND_SIGNALLED
- && last.kind != TARGET_WAITKIND_EXITED
- && inferior_thread ()->control.stop_step)
- {
- /* But not if in the middle of doing a "step n" operation for
- n > 1 */
- if (inferior_thread ()->step_multi)
- goto done;
-
- observer_notify_end_stepping_range ();
- }
-
target_terminal_ours ();
async_enable_stdin ();
@@ -7877,15 +7953,7 @@ normal_stop (void)
or if the program has exited. */
if (!stop_stack_dummy)
- {
- select_frame (get_current_frame ());
-
- /* If --batch-silent is enabled then there's no need to print the current
- source location, and to try risks causing an error message about
- missing source files. */
- if (stop_print_frame && !batch_silent)
- print_stop_event (&last);
- }
+ select_frame (get_current_frame ());
if (stop_stack_dummy == STOP_STACK_DUMMY)
{
@@ -7909,16 +7977,9 @@ normal_stop (void)
}
done:
- annotate_stopped ();
/* Suppress the stop observer if we're in the middle of:
- - a step n (n > 1), as there still more steps to be done.
-
- - a "finish" command, as the observer will be called in
- finish_command_continuation, so it can include the inferior
- function's return value.
-
- calling an inferior function, as we pretend we inferior didn't
run at all. The return value of the call is handled by the
expression evaluator, through call_function_by_hand. */
@@ -7927,11 +7988,7 @@ done:
|| last.kind == TARGET_WAITKIND_SIGNALLED
|| last.kind == TARGET_WAITKIND_EXITED
|| last.kind == TARGET_WAITKIND_NO_RESUMED
- || (!(inferior_thread ()->step_multi
- && inferior_thread ()->control.stop_step)
- && !(inferior_thread ()->control.stop_bpstat
- && inferior_thread ()->control.proceed_to_finish)
- && !inferior_thread ()->control.in_infcall))
+ || !inferior_thread ()->control.in_infcall)
{
if (!ptid_equal (inferior_ptid, null_ptid))
observer_notify_normal_stop (inferior_thread ()->control.stop_bpstat,
@@ -7940,6 +7997,8 @@ done:
observer_notify_normal_stop (NULL, stop_print_frame);
}
+ annotate_stopped ();
+
if (target_has_execution)
{
if (last.kind != TARGET_WAITKIND_SIGNALLED
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 1af64a6..34a8d4a 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -23,6 +23,7 @@
struct target_waitstatus;
struct frame_info;
struct address_space;
+struct return_value_info;
/* True if we are debugging run control. */
extern unsigned int debug_infrun;
@@ -148,7 +149,18 @@ extern void print_exited_reason (struct ui_out *uiout, int exitstatus);
inferior has stopped. */
extern void print_no_history_reason (struct ui_out *uiout);
-extern void print_stop_event (struct target_waitstatus *ws);
+/* Print the result of a function at the end of a 'finish' command.
+ RV points at an object representing the captured return value/type
+ and its position in the value history. */
+
+extern void print_return_value (struct ui_out *uiout,
+ struct return_value_info *rv);
+
+/* Print current location without a level number, if we have changed
+ functions or hit a breakpoint. Print source line if we have one.
+ If the execution command captured a return value, print it. */
+
+extern void print_stop_event (struct ui_out *uiout);
extern int signal_stop_state (int);
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index bdd8e21..4e1cf31 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -37,6 +37,7 @@
#include "objfiles.h"
#include "tracepoint.h"
#include "cli-out.h"
+#include "thread-fsm.h"
/* These are the interpreter setup, etc. functions for the MI
interpreter. */
@@ -454,16 +455,6 @@ mi_inferior_removed (struct inferior *inf)
gdb_flush (mi->event_channel);
}
-/* Cleanup that restores a previous current uiout. */
-
-static void
-restore_current_uiout_cleanup (void *arg)
-{
- struct ui_out *saved_uiout = arg;
-
- current_uiout = saved_uiout;
-}
-
/* Return the MI interpreter, if it is active -- either because it's
the top-level interpreter or the interpreter executing the current
command. Returns NULL if the MI interpreter is not being used. */
@@ -581,73 +572,48 @@ mi_on_normal_stop (struct bpstats *bs, int print_frame)
if (print_frame)
{
+ struct thread_info *tp;
int core;
- if (current_uiout != mi_uiout)
- {
- /* The normal_stop function has printed frame information
- into CLI uiout, or some other non-MI uiout. There's no
- way we can extract proper fields from random uiout
- object, so we print the frame again. In practice, this
- can only happen when running a CLI command in MI. */
- struct ui_out *saved_uiout = current_uiout;
- struct target_waitstatus last;
- ptid_t last_ptid;
-
- current_uiout = mi_uiout;
+ tp = inferior_thread ();
- get_last_target_status (&last_ptid, &last);
- print_stop_event (&last);
+ if (tp->thread_fsm != NULL
+ && thread_fsm_finished_p (tp->thread_fsm))
+ {
+ enum async_reply_reason reason;
- current_uiout = saved_uiout;
+ reason = thread_fsm_async_reply_reason (tp->thread_fsm);
+ ui_out_field_string (mi_uiout, "reason",
+ async_reason_lookup (reason));
}
- /* Otherwise, frame information has already been printed by
- normal_stop. */
- else
+ print_stop_event (mi_uiout);
+
+ /* Breakpoint hits should always be mirrored to the console.
+ Deciding what to mirror to the console wrt to breakpoints and
+ random stops gets messy real fast. E.g., say "s" trips on a
+ breakpoint. We'd clearly want to mirror the event to the
+ console in this case. But what about more complicated cases
+ like "s&; thread n; s&", and one of those steps spawning a
+ new thread, and that thread hitting a breakpoint? It's
+ impossible in general to track whether the thread had any
+ relation to the commands that had been executed. So we just
+ simplify and always mirror breakpoints and random events to
+ the console.
+
+ OTOH, we should print the source line to the console when
+ stepping or other similar commands, iff the step was started
+ by a console command, but not if it was started with
+ -exec-step or similar. */
+ if ((bpstat_what (tp->control.stop_bpstat).main_action
+ == BPSTAT_WHAT_STOP_NOISY)
+ || !(tp->thread_fsm != NULL
+ && thread_fsm_finished_p (tp->thread_fsm))
+ || (tp->control.command_interp != NULL
+ && tp->control.command_interp != top_level_interpreter ()))
{
- /* Breakpoint hits should always be mirrored to the console.
- Deciding what to mirror to the console wrt to breakpoints
- and random stops gets messy real fast. E.g., say "s"
- trips on a breakpoint. We'd clearly want to mirror the
- event to the console in this case. But what about more
- complicated cases like "s&; thread n; s&", and one of
- those steps spawning a new thread, and that thread
- hitting a breakpoint? It's impossible in general to
- track whether the thread had any relation to the commands
- that had been executed. So we just simplify and always
- mirror breakpoints and random events to the console.
-
- Also, CLI execution commands (-interpreter-exec console
- "next", for example) in async mode have the opposite
- issue as described in the "then" branch above --
- normal_stop has already printed frame information to MI
- uiout, but nothing has printed the same information to
- the CLI channel. We should print the source line to the
- console when stepping or other similar commands, iff the
- step was started by a console command (but not if it was
- started with -exec-step or similar). */
- struct thread_info *tp = inferior_thread ();
-
- if ((!tp->control.stop_step
- && !tp->control.proceed_to_finish)
- || (tp->control.command_interp != NULL
- && tp->control.command_interp != top_level_interpreter ()))
- {
- struct mi_interp *mi = top_level_interpreter_data ();
- struct target_waitstatus last;
- ptid_t last_ptid;
- struct cleanup *old_chain;
-
- /* Set the current uiout to CLI uiout temporarily. */
- old_chain = make_cleanup (restore_current_uiout_cleanup,
- current_uiout);
- current_uiout = mi->cli_uiout;
-
- get_last_target_status (&last_ptid, &last);
- print_stop_event (&last);
-
- do_cleanups (old_chain);
- }
+ struct mi_interp *mi = top_level_interpreter_data ();
+
+ print_stop_event (mi->cli_uiout);
}
ui_out_field_int (mi_uiout, "thread-id",
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index 08c8f02..5f75bef 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -156,6 +156,24 @@ if {[regexp "A + B" "$output"]} {
mi_expect_stop "end-stepping-range" "callee4" "" ".*basics.c" $line_callee4_next_step \
"" "check *stopped from CLI command 2"
+# Test that CLI's "finish" command prints the function's return value
+# to both the CLI and MI streams, and that the same result variable is
+# printed to both streams.
+with_test_prefix "CLI finish" {
+ mi_send_resuming_command "interpreter-exec console finish" "send CLI command"
+
+ set output [mi_gdb_expect_cli_output "\\*stopped" "collect CLI output"]
+ gdb_assert {[regexp \
+ "callee3 .* at .*basics.c:.*.*Value returned is .1 = 0\\\\n" \
+ $output]} \
+ "check CLI output"
+
+ mi_expect_stop "function-finished" "callee3" ".*" \
+ ".*basics.c" ".*" \
+ ",gdb-result-var=\".1\",return-value=\"0\"" \
+ "check MI output"
+}
+
mi_gdb_test "600-break-insert -t basics.c:$line_main_hello" \
{600\^done,bkpt=.number="3",type="breakpoint".*\}} \
"-break-insert -t basics.c:\$line_main_hello"
diff --git a/gdb/thread-fsm.c b/gdb/thread-fsm.c
new file mode 100644
index 0000000..761ad1c
--- /dev/null
+++ b/gdb/thread-fsm.c
@@ -0,0 +1,97 @@
+/* Thread command's finish-state machine, for GDB, the GNU debugger.
+ Copyright (C) 2015 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "thread-fsm.h"
+
+/* See thread-fsm.h. */
+
+void
+thread_fsm_ctor (struct thread_fsm *self, struct thread_fsm_ops *ops)
+{
+ self->finished = 0;
+ self->ops = ops;
+}
+
+/* See thread-fsm.h. */
+
+void
+thread_fsm_delete (struct thread_fsm *self)
+{
+ if (self != NULL)
+ {
+ if (self->ops->dtor != NULL)
+ self->ops->dtor (self);
+ xfree (self);
+ }
+}
+
+/* See thread-fsm.h. */
+
+void
+thread_fsm_clean_up (struct thread_fsm *self)
+{
+ if (self->ops->clean_up != NULL)
+ self->ops->clean_up (self);
+}
+
+/* See thread-fsm.h. */
+
+int
+thread_fsm_should_stop (struct thread_fsm *self)
+{
+ return self->ops->should_stop (self);
+}
+
+/* See thread-fsm.h. */
+
+struct return_value_info *
+thread_fsm_return_value (struct thread_fsm *self)
+{
+ if (self->ops->return_value != NULL)
+ return self->ops->return_value (self);
+ return NULL;
+}
+
+/* See thread-fsm.h. */
+
+void
+thread_fsm_set_finished (struct thread_fsm *self)
+{
+ self->finished = 1;
+}
+
+/* See thread-fsm.h. */
+
+int
+thread_fsm_finished_p (struct thread_fsm *self)
+{
+ return self->finished;
+}
+
+/* See thread-fsm.h. */
+
+enum async_reply_reason
+thread_fsm_async_reply_reason (struct thread_fsm *self)
+{
+ /* If we didn't finish, then the stop reason must come from
+ elsewhere. E.g., a breakpoint hit or a signal intercepted. */
+ gdb_assert (thread_fsm_finished_p (self));
+
+ return self->ops->async_reply_reason (self);
+}
diff --git a/gdb/thread-fsm.h b/gdb/thread-fsm.h
new file mode 100644
index 0000000..031684b
--- /dev/null
+++ b/gdb/thread-fsm.h
@@ -0,0 +1,98 @@
+/* Thread command's finish-state machine, for GDB, the GNU debugger.
+ Copyright (C) 2015 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef THREAD_FSM_H
+#define THREAD_FSM_H
+
+#include "mi/mi-common.h" /* For enum async_reply_reason. */
+
+struct return_value_info;
+struct thread_fsm_ops;
+
+/* A thread finite-state machine structure contains the necessary info
+ and callbacks to manage the state machine protocol of a thread's
+ execution command. */
+
+struct thread_fsm
+{
+ /* Pointer of the virtual table of methods. */
+ struct thread_fsm_ops *ops;
+
+ /* Whether the FSM is done successfully. */
+ int finished;
+};
+
+/* The virtual table of a thread_fsm. */
+
+struct thread_fsm_ops
+{
+ /* The destructor. This should simply free heap allocated data
+ structures. Cleaning up target resources (like, e.g.,
+ breakpoints) should be done in the clean_up method. */
+ void (*dtor) (struct thread_fsm *self);
+
+ /* Called to clean up target resources after the FSM. E.g., if the
+ FSM created internal breakpoints, this is where they should be
+ deleted. */
+ void (*clean_up) (struct thread_fsm *self);
+
+ /* Called after handle_inferior_event decides the target is done
+ (that is, after stop_waiting). The FSM is given a chance to
+ decide whether the command is done and thus the target should
+ stop, or whether there's still more to do and thus the thread
+ should be re-resumed. This is a good place to cache target data
+ too. For example, the "finish" command saves the just-finished
+ function's return value here. */
+ int (*should_stop) (struct thread_fsm *self);
+
+ /* If this FSM saved a function's return value, you can use this
+ method to retrieve it. Otherwise, this returns NULL. */
+ struct return_value_info *(*return_value) (struct thread_fsm *self);
+
+ /* The async_reply_reason that is broadcast to MI clients if this
+ FSM finishes successfully. */
+ enum async_reply_reason (*async_reply_reason) (struct thread_fsm *self);
+};
+/* Initialize FSM. */
+extern void thread_fsm_ctor (struct thread_fsm *fsm,
+ struct thread_fsm_ops *ops);
+
+/* Calls the FSM's dtor method, and then frees FSM. */
+extern void thread_fsm_delete (struct thread_fsm *fsm);
+
+/* Calls the FSM's clean_up method. */
+extern void thread_fsm_clean_up (struct thread_fsm *fsm);
+
+/* Calls the FSM's should_stop method. */
+extern int thread_fsm_should_stop (struct thread_fsm *fsm);
+
+/* Calls the FSM's return_value method. */
+extern struct return_value_info *
+ thread_fsm_return_value (struct thread_fsm *fsm);
+
+/* Marks the FSM as completed successfully. */
+extern void thread_fsm_set_finished (struct thread_fsm *fsm);
+
+/* Returns true if the FSM completed successfully. */
+extern int thread_fsm_finished_p (struct thread_fsm *fsm);
+
+/* Calls the FSM's reply_reason method. */
+extern enum async_reply_reason
+ thread_fsm_async_reply_reason (struct thread_fsm *fsm);
+
+#endif /* THREAD_FSM_H */
diff --git a/gdb/thread.c b/gdb/thread.c
index 4dde722..87a5950 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -43,6 +43,7 @@
#include "gdb_regex.h"
#include "cli/cli-utils.h"
#include "continuations.h"
+#include "thread-fsm.h"
/* Definition of struct thread_info exported to gdbthread.h. */
@@ -158,6 +159,19 @@ thread_has_single_step_breakpoint_here (struct thread_info *tp,
&& breakpoint_has_location_inserted_here (ss_bps, aspace, addr));
}
+/* See gdbthread.h. */
+
+void
+thread_cancel_execution_command (struct thread_info *thr)
+{
+ if (thr->thread_fsm != NULL)
+ {
+ thread_fsm_clean_up (thr->thread_fsm);
+ thread_fsm_delete (thr->thread_fsm);
+ thr->thread_fsm = NULL;
+ }
+}
+
static void
clear_thread_inferior_resources (struct thread_info *tp)
{
@@ -175,6 +189,8 @@ clear_thread_inferior_resources (struct thread_info *tp)
btrace_teardown (tp);
+ thread_cancel_execution_command (tp);
+
do_all_intermediate_continuations_thread (tp, 1);
do_all_continuations_thread (tp, 1);
}
diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c
index 1a5639d..dbb6976 100644
--- a/gdb/tui/tui-interp.c
+++ b/gdb/tui/tui-interp.c
@@ -55,6 +55,18 @@ tui_exit (void)
quiet (i.e., another interpreter is being run with
interpreter-exec), print nothing. */
+/* Observer for the normal_stop notification. */
+
+static void
+tui_on_normal_stop (struct bpstats *bs, int print_frame)
+{
+ if (!interp_quiet_p (tui_interp))
+ {
+ if (print_frame)
+ print_stop_event (tui_ui_out (tui_interp));
+ }
+}
+
/* Observer for the signal_received notification. */
static void
@@ -134,6 +146,7 @@ tui_init (struct interp *self, int top_level)
tui_initialize_readline ();
/* If changing this, remember to update cli-interp.c as well. */
+ observer_attach_normal_stop (tui_on_normal_stop);
observer_attach_signal_received (tui_on_signal_received);
observer_attach_end_stepping_range (tui_on_end_stepping_range);
observer_attach_signal_exited (tui_on_signal_exited);
--
1.9.3
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 2/7] Replace "struct continuation" mechanism by something more extensible
2015-08-12 17:02 ` [PATCH 2/7] Replace "struct continuation" mechanism by something more extensible Pedro Alves
@ 2015-08-18 12:50 ` Yao Qi
2015-08-19 14:55 ` Pedro Alves
0 siblings, 1 reply; 26+ messages in thread
From: Yao Qi @ 2015-08-18 12:50 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> (tui_init): Install tui_on_normal_stop as normal_stop
> * observer.
>
Remove *.
> +
> +/* Prepare for a step/next/etc. command. Any target resource
> + allocated here is undone in the FSM's clean_up method. */
> +
> +static void
> +step_command_fsm_prepare (struct step_command_fsm *sm,
> + int skip_subroutines, int single_inst,
> + int count, int thread)
> +{
> + struct thread_info *tp = inferior_thread ();
Nit: get thread_info by THREAD rather than inferior_thread ()?
> +
> +/* Implementation of the 'should_stop' FSM method for the finish
> + commands. Detects whether the thread stepped out of the function
> + successfully, and if so, captures the function's return value and
> + marks the FSM finished. */
> +
> +static int
> +finish_command_fsm_should_stop (struct thread_fsm *self)
> {
> - struct finish_command_continuation_args *a = arg;
> + struct finish_command_fsm *f = (struct finish_command_fsm *) self;
> + struct return_value_info *rv = &f->return_value;
> + struct thread_info *tp = inferior_thread ();
Nit: Get TP from f->thread rather than inferior_thread ()?
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 2/7] Replace "struct continuation" mechanism by something more extensible
2015-08-18 12:50 ` Yao Qi
@ 2015-08-19 14:55 ` Pedro Alves
0 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-08-19 14:55 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 08/18/2015 01:50 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> (tui_init): Install tui_on_normal_stop as normal_stop
>> * observer.
>>
>
> Remove *.
Fixed.
>
>> +
>> +/* Prepare for a step/next/etc. command. Any target resource
>> + allocated here is undone in the FSM's clean_up method. */
>> +
>> +static void
>> +step_command_fsm_prepare (struct step_command_fsm *sm,
>> + int skip_subroutines, int single_inst,
>> + int count, int thread)
>> +{
>> + struct thread_info *tp = inferior_thread ();
>
> Nit: get thread_info by THREAD rather than inferior_thread ()?
>
Done, both cases.
Here's the updated patch.
---
From b1163194157fa78258ff5f61b9aee1473de803eb Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Sun, 16 Aug 2015 16:24:14 +0100
Subject: [PATCH 2/7] Replace "struct continuation" mechanism by something more
extensible
This adds an object oriented replacement for the "struct continuation"
mechanism, and converts the stepping commands (step, next, stepi,
nexti) and the "finish" commands to use it.
It adds a new thread "class" (struct thread_fsm) that contains the
necessary info and callbacks to manage the state machine of a thread's
execution command.
This allows getting rid of some hacks. E.g., in fetch_inferior_event
and normal_stop we no longer need to know whether a thread is doing a
multi-step (e.g., step N). This effectively makes the
intermediate_continuations unused -- they'll be garbage collected in a
separate patch. (They were never a proper abstraction, IMO. See how
fetch_inferior_event needs to check step_multi before knowing whether
to call INF_EXEC_CONTINUE or INF_EXEC_COMPLETE.)
The target async vs !async uiout hacks in mi_on_normal_stop go away
too.
print_stop_event is no longer called from normal_stop. Instead it is
now called from within each interpreter's normal_stop observer. This
clears the path to make each interpreter print a stop event the way it
sees fit. Currently we have some hacks in common code to
differenciate CLI vs TUI vs MI around this area.
The "finish" command's FSM class stores the return value plus that
value's position in the value history, so that those can be printed to
both MI and CLI's streams. This fixes the CLI "finish" command when
run from MI -- it now also includes the function's return value in the
CLI stream:
(gdb)
~"callee3 (strarg=0x400730 \"A string argument.\") at src/gdb/testsuite/gdb.mi/basics.c:35\n"
~"35\t}\n"
+~"Value returned is $1 = 0\n"
*stopped,reason="function-finished",frame=...,gdb-result-var="$1",return-value="0",thread-id="1",stopped-threads="all",core="0"
-FAIL: gdb.mi/mi-cli.exp: CLI finish: check CLI output
+PASS: gdb.mi/mi-cli.exp: CLI finish: check CLI output
gdb/ChangeLog:
2015-08-19 Pedro Alves <palves@redhat.com>
* Makefile.in (COMMON_OBS): Add thread-fsm.o.
* breakpoint.c (handle_jit_event): Print debug output.
(bpstat_what): Split event callback handling to ...
(bpstat_run_callbacks): ... this new function.
(momentary_bkpt_print_it): No longer handle bp_finish here.
* breakpoint.h (bpstat_run_callbacks): Declare.
* gdbthread.h (struct thread_info) <step_multi>: Delete field.
<thread_fsm>: New field.
(thread_cancel_execution_command): Declare.
* infcmd.c: Include thread-fsm.h.
(struct step_command_fsm): New.
(step_command_fsm_ops): New global.
(new_step_command_fsm, step_command_fsm_prepare): New functions.
(step_1): Adjust to use step_command_fsm_prepare and
prepare_one_step.
(struct step_1_continuation_args): Delete.
(step_1_continuation): Delete.
(step_command_fsm_should_stop): New function.
(step_once): Delete.
(step_command_fsm_clean_up, step_command_fsm_async_reply_reason)
(prepare_one_step): New function, based on step_once.
(until_next_command): Remove step_multi reference.
(struct return_value_info): New.
(print_return_value): Rename to ...
(print_return_value_1): ... this. New struct return_value_info
parameter. Adjust.
(print_return_value): Reimplement as wrapper around
print_return_value_1.
(struct finish_command_fsm): New.
(finish_command_continuation): Delete.
(finish_command_fsm_ops): New global.
(new_finish_command_fsm, finish_command_fsm_should_stop): New
functions.
(finish_command_fsm_clean_up, finish_command_fsm_return_value):
New.
(finish_command_continuation_free_arg): Delete.
(finish_command_fsm_async_reply_reason): New.
(finish_backward, finish_forward): Change symbol parameter to a
finish_command_fsm. Adjust.
(finish_command): Create a finish_command_fsm. Adjust.
* infrun.c: Include "thread-fsm.h".
(clear_proceed_status_thread): Delete the thread's FSM.
(infrun_thread_stop_requested_callback): Cancel the thread's
execution command.
(clean_up_just_stopped_threads_fsms): New function.
(fetch_inferior_event): Handle the event_thread's should_stop
method saying the command isn't done yet.
(process_event_stop_test): Run breakpoint callbacks here.
(print_stop_event): Rename to ...
(print_stop_location): ... this.
(restore_current_uiout_cleanup): New function.
(print_stop_event): Reimplement.
(normal_stop): No longer notify the end_stepping_range observers
here handle "step N" nor "finish" here. No longer call
print_stop_event here.
* infrun.h (struct return_value_info): Forward declare.
(print_return_value): Declare.
(print_stop_event): Change prototype.
* thread-fsm.c: New file.
* thread-fsm.h: New file.
* thread.c: Include "thread-fsm.h".
(thread_cancel_execution_command): New function.
(clear_thread_inferior_resources): Call it.
* cli/cli-interp.c (cli_on_normal_stop): New function.
(cli_interpreter_init): Install cli_on_normal_stop as normal_stop
observer.
* mi/mi-interp.c: Include "thread-fsm.h".
(restore_current_uiout_cleanup): Delete.
(mi_on_normal_stop): If the thread has an FSM associated, and it
finished, ask it for the async-reply-reason to print. Always call
print_stop_event here, regardless of the top-level interpreter.
Check bpstat_what to tell whether an asynchronous breakpoint hit
triggered.
* tui/tui-interp.c (tui_on_normal_stop): New function.
(tui_init): Install tui_on_normal_stop as normal_stop observer.
gdb/testsuite/ChangeLog:
2015-08-09 Pedro Alves <palves@redhat.com>
* gdb.mi/mi-cli.exp: Add CLI finish tests.
---
gdb/Makefile.in | 2 +-
gdb/breakpoint.c | 29 +--
gdb/breakpoint.h | 6 +-
gdb/cli/cli-interp.c | 13 +
gdb/gdbthread.h | 11 +-
gdb/infcmd.c | 538 ++++++++++++++++++++++++----------------
gdb/infrun.c | 157 ++++++++----
gdb/infrun.h | 14 +-
gdb/mi/mi-interp.c | 108 +++-----
gdb/testsuite/gdb.mi/mi-cli.exp | 18 ++
gdb/thread-fsm.c | 97 ++++++++
gdb/thread-fsm.h | 98 ++++++++
gdb/thread.c | 16 ++
gdb/tui/tui-interp.c | 13 +
14 files changed, 769 insertions(+), 351 deletions(-)
create mode 100644 gdb/thread-fsm.c
create mode 100644 gdb/thread-fsm.h
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 13b9041..af6bbe6 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1029,7 +1029,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
linespec.o dictionary.o namespace.o \
location.o infcall.o \
infcmd.o infrun.o \
- expprint.o environ.o stack.o thread.o \
+ expprint.o environ.o stack.o thread.o thread-fsm.o \
exceptions.o \
extension.o \
filesystem.o \
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 87bd244..2302eb3 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5694,6 +5694,9 @@ handle_jit_event (void)
struct frame_info *frame;
struct gdbarch *gdbarch;
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog, "handling bp_jit_event\n");
+
/* Switch terminal for any messages produced by
breakpoint_re_set. */
target_terminal_ours_for_output ();
@@ -5885,16 +5888,13 @@ bpstat_what (bpstat bs_head)
retval.main_action = max (retval.main_action, this_action);
}
- /* These operations may affect the bs->breakpoint_at state so they are
- delayed after MAIN_ACTION is decided above. */
-
- if (jit_event)
- {
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "bpstat_what: bp_jit_event\n");
+ return retval;
+}
- handle_jit_event ();
- }
+void
+bpstat_run_callbacks (bpstat bs_head)
+{
+ bpstat bs;
for (bs = bs_head; bs != NULL; bs = bs->next)
{
@@ -5904,6 +5904,9 @@ bpstat_what (bpstat bs_head)
continue;
switch (b->type)
{
+ case bp_jit_event:
+ handle_jit_event ();
+ break;
case bp_gnu_ifunc_resolver:
gnu_ifunc_resolver_stop (b);
break;
@@ -5912,8 +5915,6 @@ bpstat_what (bpstat bs_head)
break;
}
}
-
- return retval;
}
/* Nonzero if we should step constantly (e.g. watchpoints on machines
@@ -13205,12 +13206,6 @@ momentary_bkpt_print_it (bpstat bs)
switch (b->type)
{
- case bp_finish:
- ui_out_field_string
- (uiout, "reason",
- async_reason_lookup (EXEC_ASYNC_FUNCTION_FINISHED));
- break;
-
case bp_until:
ui_out_field_string
(uiout, "reason",
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 877766a..896d3eb 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1005,7 +1005,11 @@ struct bpstat_what
/* Tell what to do about this bpstat. */
struct bpstat_what bpstat_what (bpstat);
-\f
+
+/* Run breakpoint event callbacks associated with the breakpoints that
+ triggered. */
+extern void bpstat_run_callbacks (bpstat bs_head);
+
/* Find the bpstat associated with a breakpoint. NULL otherwise. */
bpstat bpstat_find_breakpoint (bpstat, struct breakpoint *);
diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
index ce43a4a..174a10b 100644
--- a/gdb/cli/cli-interp.c
+++ b/gdb/cli/cli-interp.c
@@ -40,6 +40,18 @@ static struct gdb_exception safe_execute_command (struct ui_out *uiout,
quiet (i.e., another interpreter is being run with
interpreter-exec), print nothing. */
+/* Observer for the normal_stop notification. */
+
+static void
+cli_on_normal_stop (struct bpstats *bs, int print_frame)
+{
+ if (!interp_quiet_p (cli_interp))
+ {
+ if (print_frame)
+ print_stop_event (cli_uiout);
+ }
+}
+
/* Observer for the signal_received notification. */
static void
@@ -109,6 +121,7 @@ static void *
cli_interpreter_init (struct interp *self, int top_level)
{
/* If changing this, remember to update tui-interp.c as well. */
+ observer_attach_normal_stop (cli_on_normal_stop);
observer_attach_end_stepping_range (cli_on_end_stepping_range);
observer_attach_signal_received (cli_on_signal_received);
observer_attach_signal_exited (cli_on_signal_exited);
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 83b2616..0c99782 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -277,9 +277,10 @@ struct thread_info
command. */
struct continuation *intermediate_continuations;
- /* If stepping, nonzero means step count is > 1 so don't print frame
- next time inferior stops if it stops due to stepping. */
- int step_multi;
+ /* 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;
/* 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
@@ -557,6 +558,10 @@ extern struct thread_info *thread_step_over_chain_next (struct thread_info *tp);
extern int thread_is_in_step_over_chain (struct thread_info *tp);
+/* Cancel any ongoing execution command. */
+
+extern void thread_cancel_execution_command (struct thread_info *thr);
+
extern struct thread_info *thread_list;
#endif /* GDBTHREAD_H */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 49620ac..6b4529b 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -55,6 +55,7 @@
#include "linespec.h"
#include "cli/cli-utils.h"
#include "infcall.h"
+#include "thread-fsm.h"
/* Local functions: */
@@ -89,8 +90,6 @@ static void signal_command (char *, int);
static void jump_command (char *, int);
static void step_1 (int, int, char *);
-static void step_once (int skip_subroutines, int single_inst,
- int count, int thread);
static void next_command (char *, int);
@@ -900,14 +899,86 @@ delete_longjmp_breakpoint_cleanup (void *arg)
delete_longjmp_breakpoint (thread);
}
+/* Data for the FSM that manages the step/next/stepi/nexti
+ commands. */
+
+struct step_command_fsm
+{
+ /* The base class. */
+ struct thread_fsm thread_fsm;
+
+ /* How many steps left in a "step N"-like command. */
+ int count;
+
+ /* If true, this is a next/nexti, otherwise a step/stepi. */
+ int skip_subroutines;
+
+ /* If true, this is a stepi/nexti, otherwise a step/step. */
+ int single_inst;
+
+ /* The thread that the command was run on. */
+ int thread;
+};
+
+static void step_command_fsm_clean_up (struct thread_fsm *self);
+static int step_command_fsm_should_stop (struct thread_fsm *self);
+static enum async_reply_reason
+ step_command_fsm_async_reply_reason (struct thread_fsm *self);
+
+/* step_command_fsm's vtable. */
+
+static struct thread_fsm_ops step_command_fsm_ops =
+{
+ NULL,
+ step_command_fsm_clean_up,
+ step_command_fsm_should_stop,
+ NULL, /* return_value */
+ step_command_fsm_async_reply_reason,
+};
+
+/* Allocate a new step_command_fsm. */
+
+static struct step_command_fsm *
+new_step_command_fsm (void)
+{
+ struct step_command_fsm *sm;
+
+ sm = XCNEW (struct step_command_fsm);
+ thread_fsm_ctor (&sm->thread_fsm, &step_command_fsm_ops);
+
+ return sm;
+}
+
+/* Prepare for a step/next/etc. command. Any target resource
+ allocated here is undone in the FSM's clean_up method. */
+
+static void
+step_command_fsm_prepare (struct step_command_fsm *sm,
+ int skip_subroutines, int single_inst,
+ int count, struct thread_info *thread)
+{
+ sm->skip_subroutines = skip_subroutines;
+ sm->single_inst = single_inst;
+ sm->count = count;
+ sm->thread = thread->num;
+
+ /* Leave the si command alone. */
+ if (!sm->single_inst || sm->skip_subroutines)
+ set_longjmp_breakpoint (thread, get_frame_id (get_current_frame ()));
+
+ thread->control.stepping_command = 1;
+}
+
+static int prepare_one_step (struct step_command_fsm *sm);
+
static void
step_1 (int skip_subroutines, int single_inst, char *count_string)
{
- int count = 1;
- struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
+ int count;
int async_exec;
- int thread = -1;
struct cleanup *args_chain;
+ struct thread_info *thr;
+ struct step_command_fsm *step_sm;
ERROR_NO_INFERIOR;
ensure_not_tfind_mode ();
@@ -924,101 +995,104 @@ step_1 (int skip_subroutines, int single_inst, char *count_string)
/* Done with ARGS. */
do_cleanups (args_chain);
- if (!single_inst || skip_subroutines) /* Leave si command alone. */
- {
- struct thread_info *tp = inferior_thread ();
-
- if (in_thread_list (inferior_ptid))
- thread = pid_to_thread_id (inferior_ptid);
+ clear_proceed_status (1);
- set_longjmp_breakpoint (tp, get_frame_id (get_current_frame ()));
+ /* Setup the execution command state machine to handle all the COUNT
+ steps. */
+ thr = inferior_thread ();
+ step_sm = new_step_command_fsm ();
+ thr->thread_fsm = &step_sm->thread_fsm;
- make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
- }
+ step_command_fsm_prepare (step_sm, skip_subroutines,
+ single_inst, count, thr);
/* Do only one step for now, before returning control to the event
loop. Let the continuation figure out how many other steps we
need to do, and handle them one at the time, through
step_once. */
- step_once (skip_subroutines, single_inst, count, thread);
-
- /* We are running, and the continuation is installed. It will
- disable the longjmp breakpoint as appropriate. */
- discard_cleanups (cleanups);
+ if (!prepare_one_step (step_sm))
+ proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
+ else
+ {
+ /* Stepped into an inline frame. Pretend that we've
+ stopped. */
+ thread_fsm_clean_up (thr->thread_fsm);
+ normal_stop ();
+ inferior_event_handler (INF_EXEC_COMPLETE, NULL);
+ }
}
-struct step_1_continuation_args
-{
- int count;
- int skip_subroutines;
- int single_inst;
- int thread;
-};
-
-/* Called after we are done with one step operation, to check whether
- we need to step again, before we print the prompt and return control
- to the user. If count is > 1, we will need to do one more call to
- proceed(), via step_once(). Basically it is like step_once and
- step_1_continuation are co-recursive. */
+/* Implementation of the 'should_stop' FSM method for stepping
+ commands. Called after we are done with one step operation, to
+ check whether we need to step again, before we print the prompt and
+ return control to the user. If count is > 1, returns false, as we
+ will need to keep going. */
-static void
-step_1_continuation (void *args, int err)
+static int
+step_command_fsm_should_stop (struct thread_fsm *self)
{
- struct step_1_continuation_args *a = args;
+ struct step_command_fsm *sm = (struct step_command_fsm *) self;
+ struct thread_info *tp = find_thread_id (sm->thread);
- if (target_has_execution)
+ if (tp->control.stop_step)
{
- struct thread_info *tp;
+ /* There are more steps to make, and we did stop due to
+ ending a stepping range. Do another step. */
+ if (--sm->count > 0)
+ return prepare_one_step (sm);
- tp = inferior_thread ();
- if (!err
- && tp->step_multi && tp->control.stop_step)
- {
- /* There are more steps to make, and we did stop due to
- ending a stepping range. Do another step. */
- step_once (a->skip_subroutines, a->single_inst,
- a->count - 1, a->thread);
- return;
- }
- tp->step_multi = 0;
+ thread_fsm_set_finished (self);
}
- /* We either hit an error, or stopped for some reason that is
- not stepping, or there are no further steps to make.
- Cleanup. */
- if (!a->single_inst || a->skip_subroutines)
- delete_longjmp_breakpoint (a->thread);
+ return 1;
}
-/* Do just one step operation. This is useful to implement the 'step
- n' kind of commands. In case of asynchronous targets, we will have
- to set up a continuation to be done after the target stops (after
- this one step). For synch targets, the caller handles further
- stepping. */
+/* Implementation of the 'clean_up' FSM method for stepping commands. */
static void
-step_once (int skip_subroutines, int single_inst, int count, int thread)
+step_command_fsm_clean_up (struct thread_fsm *self)
{
- struct frame_info *frame = get_current_frame ();
+ struct step_command_fsm *sm = (struct step_command_fsm *) self;
+
+ if (!sm->single_inst || sm->skip_subroutines)
+ delete_longjmp_breakpoint (sm->thread);
+}
+
+/* Implementation of the 'async_reply_reason' FSM method for stepping
+ commands. */
+
+static enum async_reply_reason
+step_command_fsm_async_reply_reason (struct thread_fsm *self)
+{
+ return EXEC_ASYNC_END_STEPPING_RANGE;
+}
+
+/* Prepare for one step in "step N". The actual target resumption is
+ done by the caller. Return true if we're done and should thus
+ report a stop to the user. Returns false if the target needs to be
+ resumed. */
- if (count > 0)
+static int
+prepare_one_step (struct step_command_fsm *sm)
+{
+ if (sm->count > 0)
{
- struct step_1_continuation_args *args;
+ struct frame_info *frame = get_current_frame ();
+
/* Don't assume THREAD is a valid thread id. It is set to -1 if
the longjmp breakpoint was not required. Use the
INFERIOR_PTID thread instead, which is the same thread when
THREAD is set. */
struct thread_info *tp = inferior_thread ();
- clear_proceed_status (1);
set_step_frame ();
- if (!single_inst)
+ if (!sm->single_inst)
{
CORE_ADDR pc;
/* Step at an inlined function behaves like "down". */
- if (!skip_subroutines
+ if (!sm->skip_subroutines
&& inline_skipped_frames (inferior_ptid))
{
ptid_t resume_ptid;
@@ -1028,17 +1102,8 @@ step_once (int skip_subroutines, int single_inst, int count, int thread)
set_running (resume_ptid, 1);
step_into_inline_frame (inferior_ptid);
- if (count > 1)
- step_once (skip_subroutines, single_inst, count - 1, thread);
- else
- {
- /* Pretend that we've stopped. */
- normal_stop ();
-
- if (target_can_async_p ())
- inferior_event_handler (INF_EXEC_COMPLETE, NULL);
- }
- return;
+ sm->count--;
+ return prepare_one_step (sm);
}
pc = get_frame_pc (frame);
@@ -1073,29 +1138,22 @@ step_once (int skip_subroutines, int single_inst, int count, int thread)
{
/* Say we are stepping, but stop after one insn whatever it does. */
tp->control.step_range_start = tp->control.step_range_end = 1;
- if (!skip_subroutines)
+ if (!sm->skip_subroutines)
/* It is stepi.
Don't step over function calls, not even to functions lacking
line numbers. */
tp->control.step_over_calls = STEP_OVER_NONE;
}
- if (skip_subroutines)
+ if (sm->skip_subroutines)
tp->control.step_over_calls = STEP_OVER_ALL;
- tp->step_multi = (count > 1);
- tp->control.stepping_command = 1;
- proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
-
- /* Register a continuation to do any additional steps. */
- args = xmalloc (sizeof (*args));
- args->skip_subroutines = skip_subroutines;
- args->single_inst = single_inst;
- args->count = count;
- args->thread = thread;
-
- add_intermediate_continuation (tp, step_1_continuation, args, xfree);
+ return 0;
}
+
+ /* Done. */
+ thread_fsm_set_finished (&sm->thread_fsm);
+ return 1;
}
\f
@@ -1399,8 +1457,6 @@ until_next_command (int from_tty)
tp->control.step_over_calls = STEP_OVER_ALL;
- tp->step_multi = 0; /* Only one call to proceed */
-
set_longjmp_breakpoint (tp, get_frame_id (frame));
old_chain = make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
@@ -1528,18 +1584,29 @@ get_return_value (struct value *function, struct type *value_type,
return value;
}
-/* Print the result of a function at the end of a 'finish' command.
- DTOR_DATA (if not NULL) can represent inferior registers right after
- an inferior call has finished. */
+/* The captured function return value/type and its position in the
+ value history. */
-static void
-print_return_value (struct value *function, struct type *value_type,
- struct dummy_frame_context_saver *ctx_saver)
+struct return_value_info
{
- struct value *value = get_return_value (function, value_type, ctx_saver);
- struct ui_out *uiout = current_uiout;
+ /* The captured return value. May be NULL if we weren't able to
+ retrieve it. See get_return_value. */
+ struct value *value;
+
+ /* The return type. In some cases, we'll not be able extract the
+ return value, but we always know the type. */
+ struct type *type;
+
+ /* If we captured a value, this is the value history index. */
+ int value_history_index;
+};
- if (value)
+/* Helper for print_return_value. */
+
+static void
+print_return_value_1 (struct ui_out *uiout, struct return_value_info *rv)
+{
+ if (rv->value != NULL)
{
struct value_print_options opts;
struct ui_file *stb;
@@ -1550,10 +1617,10 @@ print_return_value (struct value *function, struct type *value_type,
old_chain = make_cleanup_ui_file_delete (stb);
ui_out_text (uiout, "Value returned is ");
ui_out_field_fmt (uiout, "gdb-result-var", "$%d",
- record_latest_value (value));
+ rv->value_history_index);
ui_out_text (uiout, " = ");
get_no_prettyformat_print_options (&opts);
- value_print (value, stb, &opts);
+ value_print (rv->value, stb, &opts);
ui_out_field_stream (uiout, "return-value", stb);
ui_out_text (uiout, "\n");
do_cleanups (old_chain);
@@ -1563,7 +1630,7 @@ print_return_value (struct value *function, struct type *value_type,
struct cleanup *oldchain;
char *type_name;
- type_name = type_to_string (value_type);
+ type_name = type_to_string (rv->type);
oldchain = make_cleanup (xfree, type_name);
ui_out_text (uiout, "Value returned has type: ");
ui_out_field_string (uiout, "return-type", type_name);
@@ -1573,100 +1640,173 @@ print_return_value (struct value *function, struct type *value_type,
}
}
-/* Stuff that needs to be done by the finish command after the target
- has stopped. In asynchronous mode, we wait for the target to stop
- in the call to poll or select in the event loop, so it is
- impossible to do all the stuff as part of the finish_command
- function itself. The only chance we have to complete this command
- is in fetch_inferior_event, which is called by the event loop as
- soon as it detects that the target has stopped. */
+/* Print the result of a function at the end of a 'finish' command.
+ RV points at an object representing the captured return value/type
+ and its position in the value history. */
+
+void
+print_return_value (struct ui_out *uiout, struct return_value_info *rv)
+{
+ if (rv->type == NULL || TYPE_CODE (rv->type) == TYPE_CODE_VOID)
+ return;
+
+ TRY
+ {
+ /* print_return_value_1 can throw an exception in some
+ circumstances. We need to catch this so that we still
+ delete the breakpoint. */
+ print_return_value_1 (uiout, rv);
+ }
+ CATCH (ex, RETURN_MASK_ALL)
+ {
+ exception_print (gdb_stdout, ex);
+ }
+ END_CATCH
+}
+
+/* Data for the FSM that manages the finish command. */
-struct finish_command_continuation_args
+struct finish_command_fsm
{
- /* The thread that as current when the command was executed. */
+ /* The base class. */
+ struct thread_fsm thread_fsm;
+
+ /* The thread that was current when the command was executed. */
int thread;
+
+ /* The momentary breakpoint set at the function's return address in
+ the caller. */
struct breakpoint *breakpoint;
+
+ /* The function that we're stepping out of. */
struct symbol *function;
- /* Inferior registers stored right before dummy_frame has been freed
- after an inferior call. It can be NULL if no inferior call was
- involved, GDB will then use current inferior registers. */
- struct dummy_frame_context_saver *ctx_saver;
+ /* If the FSM finishes successfully, this stores the function's
+ return value. */
+ struct return_value_info return_value;
};
-static void
-finish_command_continuation (void *arg, int err)
+static int finish_command_fsm_should_stop (struct thread_fsm *self);
+static void finish_command_fsm_clean_up (struct thread_fsm *self);
+static struct return_value_info *
+ finish_command_fsm_return_value (struct thread_fsm *self);
+static enum async_reply_reason
+ finish_command_fsm_async_reply_reason (struct thread_fsm *self);
+
+/* finish_command_fsm's vtable. */
+
+static struct thread_fsm_ops finish_command_fsm_ops =
+{
+ NULL, /* dtor */
+ finish_command_fsm_clean_up,
+ finish_command_fsm_should_stop,
+ finish_command_fsm_return_value,
+ finish_command_fsm_async_reply_reason,
+};
+
+/* Allocate a new finish_command_fsm. */
+
+static struct finish_command_fsm *
+new_finish_command_fsm (int thread)
+{
+ struct finish_command_fsm *sm;
+
+ sm = XCNEW (struct finish_command_fsm);
+ thread_fsm_ctor (&sm->thread_fsm, &finish_command_fsm_ops);
+
+ sm->thread = thread;
+
+ return sm;
+}
+
+/* Implementation of the 'should_stop' FSM method for the finish
+ commands. Detects whether the thread stepped out of the function
+ successfully, and if so, captures the function's return value and
+ marks the FSM finished. */
+
+static int
+finish_command_fsm_should_stop (struct thread_fsm *self)
{
- struct finish_command_continuation_args *a = arg;
+ struct finish_command_fsm *f = (struct finish_command_fsm *) self;
+ struct return_value_info *rv = &f->return_value;
+ struct thread_info *tp = find_thread_id (f->thread);
- if (!err)
+ if (f->function != NULL
+ && bpstat_find_breakpoint (tp->control.stop_bpstat,
+ f->breakpoint) != NULL)
{
- struct thread_info *tp = NULL;
- bpstat bs = NULL;
+ /* We're done. */
+ thread_fsm_set_finished (self);
- if (!ptid_equal (inferior_ptid, null_ptid)
- && target_has_execution
- && is_stopped (inferior_ptid))
- {
- tp = inferior_thread ();
- bs = tp->control.stop_bpstat;
- }
+ rv->type = TYPE_TARGET_TYPE (SYMBOL_TYPE (f->function));
+ if (rv->type == NULL)
+ internal_error (__FILE__, __LINE__,
+ _("finish_command: function has no target type"));
- if (bpstat_find_breakpoint (bs, a->breakpoint) != NULL
- && a->function != NULL)
+ if (TYPE_CODE (rv->type) != TYPE_CODE_VOID)
{
- struct type *value_type;
+ struct value *func;
+
+ func = read_var_value (f->function, get_current_frame ());
+ rv->value = get_return_value (func, rv->type, NULL);
+ rv->value_history_index = record_latest_value (rv->value);
+ }
+ }
+ else if (tp->control.stop_step)
+ {
+ /* Finishing from an inline frame, or reverse finishing. In
+ either case, there's no way to retrieve the return value. */
+ thread_fsm_set_finished (self);
+ }
- value_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (a->function));
- if (!value_type)
- internal_error (__FILE__, __LINE__,
- _("finish_command: function has no target type"));
+ return 1;
+}
- if (TYPE_CODE (value_type) != TYPE_CODE_VOID)
- {
- struct value *func;
+/* Implementation of the 'clean_up' FSM method for the finish
+ commands. */
- func = read_var_value (a->function, get_current_frame ());
- TRY
- {
- /* print_return_value can throw an exception in some
- circumstances. We need to catch this so that we still
- delete the breakpoint. */
- print_return_value (func, value_type, a->ctx_saver);
- }
- CATCH (ex, RETURN_MASK_ALL)
- {
- exception_print (gdb_stdout, ex);
- }
- END_CATCH
- }
- }
+static void
+finish_command_fsm_clean_up (struct thread_fsm *self)
+{
+ struct finish_command_fsm *f = (struct finish_command_fsm *) self;
- /* We suppress normal call of normal_stop observer and do it
- here so that the *stopped notification includes the return
- value. */
- if (bs != NULL && tp->control.proceed_to_finish)
- observer_notify_normal_stop (bs, 1 /* print frame */);
+ if (f->breakpoint != NULL)
+ {
+ delete_breakpoint (f->breakpoint);
+ f->breakpoint = NULL;
}
+ delete_longjmp_breakpoint (f->thread);
+}
- delete_breakpoint (a->breakpoint);
- delete_longjmp_breakpoint (a->thread);
+/* Implementation of the 'return_value' FSM method for the finish
+ commands. */
+
+static struct return_value_info *
+finish_command_fsm_return_value (struct thread_fsm *self)
+{
+ struct finish_command_fsm *f = (struct finish_command_fsm *) self;
+
+ return &f->return_value;
}
-static void
-finish_command_continuation_free_arg (void *arg)
+/* Implementation of the 'async_reply_reason' FSM method for the
+ finish commands. */
+
+static enum async_reply_reason
+finish_command_fsm_async_reply_reason (struct thread_fsm *self)
{
- struct finish_command_continuation_args *cargs = arg;
+ struct finish_command_fsm *f = (struct finish_command_fsm *) self;
- if (cargs->ctx_saver != NULL)
- dummy_frame_context_saver_drop (cargs->ctx_saver);
- xfree (cargs);
+ if (execution_direction == EXEC_REVERSE)
+ return EXEC_ASYNC_END_STEPPING_RANGE;
+ else
+ return EXEC_ASYNC_FUNCTION_FINISHED;
}
/* finish_backward -- helper function for finish_command. */
static void
-finish_backward (struct symbol *function)
+finish_backward (struct finish_command_fsm *sm)
{
struct symtab_and_line sal;
struct thread_info *tp = inferior_thread ();
@@ -1715,56 +1855,33 @@ finish_backward (struct symbol *function)
}
}
-/* finish_forward -- helper function for finish_command. */
+/* finish_forward -- helper function for finish_command. FRAME is the
+ frame that called the function we're about to step out of. */
static void
-finish_forward (struct symbol *function, struct frame_info *frame)
+finish_forward (struct finish_command_fsm *sm, struct frame_info *frame)
{
struct frame_id frame_id = get_frame_id (frame);
struct gdbarch *gdbarch = get_frame_arch (frame);
struct symtab_and_line sal;
struct thread_info *tp = inferior_thread ();
- struct breakpoint *breakpoint;
- struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
- struct finish_command_continuation_args *cargs;
- int thread = tp->num;
- struct dummy_frame_context_saver *saver = NULL;
sal = find_pc_line (get_frame_pc (frame), 0);
sal.pc = get_frame_pc (frame);
- if (get_frame_type (frame) == DUMMY_FRAME)
- {
- saver = dummy_frame_context_saver_setup (get_stack_frame_id (frame),
- inferior_ptid);
- make_cleanup (dummy_frame_context_saver_cleanup, saver);
- }
-
- breakpoint = set_momentary_breakpoint (gdbarch, sal,
- get_stack_frame_id (frame),
- bp_finish);
+ sm->breakpoint = set_momentary_breakpoint (gdbarch, sal,
+ get_stack_frame_id (frame),
+ bp_finish);
/* set_momentary_breakpoint invalidates FRAME. */
frame = NULL;
- make_cleanup_delete_breakpoint (breakpoint);
-
set_longjmp_breakpoint (tp, frame_id);
- make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
/* We want to print return value, please... */
tp->control.proceed_to_finish = 1;
- cargs = xmalloc (sizeof (*cargs));
-
- cargs->thread = thread;
- cargs->breakpoint = breakpoint;
- cargs->function = function;
- cargs->ctx_saver = saver;
- add_continuation (tp, finish_command_continuation, cargs,
- finish_command_continuation_free_arg);
- proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
- discard_cleanups (old_chain);
+ proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
}
/* "finish": Set a temporary breakpoint at the place the selected
@@ -1774,9 +1891,10 @@ static void
finish_command (char *arg, int from_tty)
{
struct frame_info *frame;
- struct symbol *function;
int async_exec;
struct cleanup *args_chain;
+ struct finish_command_fsm *sm;
+ struct thread_info *tp;
ERROR_NO_INFERIOR;
ensure_not_tfind_mode ();
@@ -1801,9 +1919,14 @@ finish_command (char *arg, int from_tty)
clear_proceed_status (0);
+ tp = inferior_thread ();
+
+ sm = new_finish_command_fsm (tp->num);
+
+ tp->thread_fsm = &sm->thread_fsm;
+
/* Finishing from an inline frame is completely different. We don't
- try to show the "return value" - no way to locate it. So we do
- not need a completion. */
+ try to show the "return value" - no way to locate it. */
if (get_frame_type (get_selected_frame (_("No selected frame.")))
== INLINE_FRAME)
{
@@ -1812,7 +1935,6 @@ finish_command (char *arg, int from_tty)
called by that frame. We don't use the magic "1" value for
step_range_end, because then infrun will think this is nexti,
and not step over the rest of this inlined function call. */
- struct thread_info *tp = inferior_thread ();
struct symtab_and_line empty_sal;
init_sal (&empty_sal);
@@ -1840,7 +1962,7 @@ finish_command (char *arg, int from_tty)
/* Find the function we will return from. */
- function = find_pc_function (get_frame_pc (get_selected_frame (NULL)));
+ sm->function = find_pc_function (get_frame_pc (get_selected_frame (NULL)));
/* Print info on the selected frame, including level number but not
source. */
@@ -1850,10 +1972,10 @@ finish_command (char *arg, int from_tty)
printf_filtered (_("Run back to call of "));
else
{
- if (function != NULL && TYPE_NO_RETURN (function->type)
+ if (sm->function != NULL && TYPE_NO_RETURN (sm->function->type)
&& !query (_("warning: Function %s does not return normally.\n"
"Try to finish anyway? "),
- SYMBOL_PRINT_NAME (function)))
+ SYMBOL_PRINT_NAME (sm->function)))
error (_("Not confirmed."));
printf_filtered (_("Run till exit from "));
}
@@ -1862,9 +1984,9 @@ finish_command (char *arg, int from_tty)
}
if (execution_direction == EXEC_REVERSE)
- finish_backward (function);
+ finish_backward (sm);
else
- finish_forward (function, frame);
+ finish_forward (sm, frame);
}
\f
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 195ee81..06526f0 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -62,6 +62,7 @@
#include "terminal.h"
#include "solist.h"
#include "event-loop.h"
+#include "thread-fsm.h"
/* Prototypes for local functions */
@@ -2745,6 +2746,9 @@ clear_proceed_status_thread (struct thread_info *tp)
if (!signal_pass_state (tp->suspend.stop_signal))
tp->suspend.stop_signal = GDB_SIGNAL_0;
+ thread_fsm_delete (tp->thread_fsm);
+ tp->thread_fsm = NULL;
+
tp->control.trap_expected = 0;
tp->control.step_range_start = 0;
tp->control.step_range_end = 0;
@@ -3216,7 +3220,7 @@ infrun_thread_stop_requested_callback (struct thread_info *info, void *arg)
have consistent output as if the stop event had been
reported. */
ecs->ptid = info->ptid;
- ecs->event_thread = find_thread_ptid (info->ptid);
+ ecs->event_thread = info;
ecs->ws.kind = TARGET_WAITKIND_STOPPED;
ecs->ws.value.sig = GDB_SIGNAL_0;
@@ -3226,6 +3230,9 @@ infrun_thread_stop_requested_callback (struct thread_info *info, void *arg)
{
struct thread_info *tp;
+ /* Cancel any running execution command. */
+ thread_cancel_execution_command (info);
+
normal_stop ();
/* Finish off the continuations. */
@@ -3714,6 +3721,35 @@ reinstall_readline_callback_handler_cleanup (void *arg)
gdb_rl_callback_handler_reinstall ();
}
+/* Clean up the FSMs of threads that are now stopped. In non-stop,
+ that's just the event thread. In all-stop, that's all threads. */
+
+static void
+clean_up_just_stopped_threads_fsms (struct execution_control_state *ecs)
+{
+ struct thread_info *thr = ecs->event_thread;
+
+ if (thr != NULL && thr->thread_fsm != NULL)
+ thread_fsm_clean_up (thr->thread_fsm);
+
+ if (!non_stop)
+ {
+ ALL_NON_EXITED_THREADS (thr)
+ {
+ if (thr->thread_fsm == NULL)
+ continue;
+ if (thr == ecs->event_thread)
+ continue;
+
+ switch_to_thread (thr->ptid);
+ thread_fsm_clean_up (thr->thread_fsm);
+ }
+
+ if (ecs->event_thread != NULL)
+ switch_to_thread (ecs->event_thread->ptid);
+ }
+}
+
/* Asynchronous version of wait_for_inferior. It is called by the
event loop whenever a change of state is detected on the file
descriptor corresponding to the target. It can be called more than
@@ -3792,22 +3828,31 @@ fetch_inferior_event (void *client_data)
if (!ecs->wait_some_more)
{
struct inferior *inf = find_inferior_ptid (ecs->ptid);
+ int should_stop = 1;
+ struct thread_info *thr = ecs->event_thread;
delete_just_stopped_threads_infrun_breakpoints ();
- /* We may not find an inferior if this was a process exit. */
- if (inf == NULL || inf->control.stop_soon == NO_STOP_QUIETLY)
- normal_stop ();
-
- if (target_has_execution
- && ecs->ws.kind != TARGET_WAITKIND_NO_RESUMED
- && ecs->ws.kind != TARGET_WAITKIND_EXITED
- && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED
- && ecs->event_thread->step_multi
- && ecs->event_thread->control.stop_step)
- inferior_event_handler (INF_EXEC_CONTINUE, NULL);
+ if (thr != NULL)
+ {
+ struct thread_fsm *thread_fsm = thr->thread_fsm;
+
+ if (thread_fsm != NULL)
+ should_stop = thread_fsm_should_stop (thread_fsm);
+ }
+
+ if (!should_stop)
+ {
+ keep_going (ecs);
+ }
else
{
+ clean_up_just_stopped_threads_fsms (ecs);
+
+ /* We may not find an inferior if this was a process exit. */
+ if (inf == NULL || inf->control.stop_soon == NO_STOP_QUIETLY)
+ normal_stop ();
+
inferior_event_handler (INF_EXEC_COMPLETE, NULL);
cmd_done = 1;
}
@@ -5889,6 +5934,10 @@ process_event_stop_test (struct execution_control_state *ecs)
stop_stack_dummy = what.call_dummy;
}
+ /* A few breakpoint types have callbacks associated (e.g.,
+ bp_jit_event). Run them now. */
+ bpstat_run_callbacks (ecs->event_thread->control.stop_bpstat);
+
/* If we hit an internal event that triggers symbol changes, the
current frame will be invalidated within bpstat_what (e.g., if we
hit an internal solib event). Re-fetch it. */
@@ -7655,8 +7704,8 @@ print_no_history_reason (struct ui_out *uiout)
bpstat_print contains the logic deciding in detail what to print,
based on the event(s) that just occurred. */
-void
-print_stop_event (struct target_waitstatus *ws)
+static void
+print_stop_location (struct target_waitstatus *ws)
{
int bpstat_ret;
enum print_what source_flag;
@@ -7707,9 +7756,50 @@ print_stop_event (struct target_waitstatus *ws)
SRC_AND_LOC: Print location and source line. */
if (do_frame_printing)
print_stack_frame (get_selected_frame (NULL), 0, source_flag, 1);
+}
+
+/* Cleanup that restores a previous current uiout. */
+
+static void
+restore_current_uiout_cleanup (void *arg)
+{
+ struct ui_out *saved_uiout = arg;
+
+ current_uiout = saved_uiout;
+}
+
+/* See infrun.h. */
+
+void
+print_stop_event (struct ui_out *uiout)
+{
+ struct cleanup *old_chain;
+ struct target_waitstatus last;
+ ptid_t last_ptid;
+ struct thread_info *tp;
+
+ get_last_target_status (&last_ptid, &last);
+
+ old_chain = make_cleanup (restore_current_uiout_cleanup, current_uiout);
+ current_uiout = uiout;
+
+ print_stop_location (&last);
/* Display the auto-display expressions. */
do_displays ();
+
+ do_cleanups (old_chain);
+
+ tp = inferior_thread ();
+ if (tp->thread_fsm != NULL
+ && thread_fsm_finished_p (tp->thread_fsm))
+ {
+ struct return_value_info *rv;
+
+ rv = thread_fsm_return_value (tp->thread_fsm);
+ if (rv != NULL)
+ print_return_value (uiout, rv);
+ }
}
/* Here to return control to GDB when the inferior stops for real.
@@ -7822,20 +7912,6 @@ normal_stop (void)
if (stopped_by_random_signal)
disable_current_display ();
- /* Notify observers if we finished a "step"-like command, etc. */
- if (target_has_execution
- && last.kind != TARGET_WAITKIND_SIGNALLED
- && last.kind != TARGET_WAITKIND_EXITED
- && inferior_thread ()->control.stop_step)
- {
- /* But not if in the middle of doing a "step n" operation for
- n > 1 */
- if (inferior_thread ()->step_multi)
- goto done;
-
- observer_notify_end_stepping_range ();
- }
-
target_terminal_ours ();
async_enable_stdin ();
@@ -7877,15 +7953,7 @@ normal_stop (void)
or if the program has exited. */
if (!stop_stack_dummy)
- {
- select_frame (get_current_frame ());
-
- /* If --batch-silent is enabled then there's no need to print the current
- source location, and to try risks causing an error message about
- missing source files. */
- if (stop_print_frame && !batch_silent)
- print_stop_event (&last);
- }
+ select_frame (get_current_frame ());
if (stop_stack_dummy == STOP_STACK_DUMMY)
{
@@ -7909,16 +7977,9 @@ normal_stop (void)
}
done:
- annotate_stopped ();
/* Suppress the stop observer if we're in the middle of:
- - a step n (n > 1), as there still more steps to be done.
-
- - a "finish" command, as the observer will be called in
- finish_command_continuation, so it can include the inferior
- function's return value.
-
- calling an inferior function, as we pretend we inferior didn't
run at all. The return value of the call is handled by the
expression evaluator, through call_function_by_hand. */
@@ -7927,11 +7988,7 @@ done:
|| last.kind == TARGET_WAITKIND_SIGNALLED
|| last.kind == TARGET_WAITKIND_EXITED
|| last.kind == TARGET_WAITKIND_NO_RESUMED
- || (!(inferior_thread ()->step_multi
- && inferior_thread ()->control.stop_step)
- && !(inferior_thread ()->control.stop_bpstat
- && inferior_thread ()->control.proceed_to_finish)
- && !inferior_thread ()->control.in_infcall))
+ || !inferior_thread ()->control.in_infcall)
{
if (!ptid_equal (inferior_ptid, null_ptid))
observer_notify_normal_stop (inferior_thread ()->control.stop_bpstat,
@@ -7940,6 +7997,8 @@ done:
observer_notify_normal_stop (NULL, stop_print_frame);
}
+ annotate_stopped ();
+
if (target_has_execution)
{
if (last.kind != TARGET_WAITKIND_SIGNALLED
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 1af64a6..34a8d4a 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -23,6 +23,7 @@
struct target_waitstatus;
struct frame_info;
struct address_space;
+struct return_value_info;
/* True if we are debugging run control. */
extern unsigned int debug_infrun;
@@ -148,7 +149,18 @@ extern void print_exited_reason (struct ui_out *uiout, int exitstatus);
inferior has stopped. */
extern void print_no_history_reason (struct ui_out *uiout);
-extern void print_stop_event (struct target_waitstatus *ws);
+/* Print the result of a function at the end of a 'finish' command.
+ RV points at an object representing the captured return value/type
+ and its position in the value history. */
+
+extern void print_return_value (struct ui_out *uiout,
+ struct return_value_info *rv);
+
+/* Print current location without a level number, if we have changed
+ functions or hit a breakpoint. Print source line if we have one.
+ If the execution command captured a return value, print it. */
+
+extern void print_stop_event (struct ui_out *uiout);
extern int signal_stop_state (int);
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 0935c8f..385b975 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -37,6 +37,7 @@
#include "objfiles.h"
#include "tracepoint.h"
#include "cli-out.h"
+#include "thread-fsm.h"
/* These are the interpreter setup, etc. functions for the MI
interpreter. */
@@ -454,16 +455,6 @@ mi_inferior_removed (struct inferior *inf)
gdb_flush (mi->event_channel);
}
-/* Cleanup that restores a previous current uiout. */
-
-static void
-restore_current_uiout_cleanup (void *arg)
-{
- struct ui_out *saved_uiout = arg;
-
- current_uiout = saved_uiout;
-}
-
/* Return the MI interpreter, if it is active -- either because it's
the top-level interpreter or the interpreter executing the current
command. Returns NULL if the MI interpreter is not being used. */
@@ -581,73 +572,48 @@ mi_on_normal_stop (struct bpstats *bs, int print_frame)
if (print_frame)
{
+ struct thread_info *tp;
int core;
- if (current_uiout != mi_uiout)
- {
- /* The normal_stop function has printed frame information
- into CLI uiout, or some other non-MI uiout. There's no
- way we can extract proper fields from random uiout
- object, so we print the frame again. In practice, this
- can only happen when running a CLI command in MI. */
- struct ui_out *saved_uiout = current_uiout;
- struct target_waitstatus last;
- ptid_t last_ptid;
-
- current_uiout = mi_uiout;
+ tp = inferior_thread ();
- get_last_target_status (&last_ptid, &last);
- print_stop_event (&last);
+ if (tp->thread_fsm != NULL
+ && thread_fsm_finished_p (tp->thread_fsm))
+ {
+ enum async_reply_reason reason;
- current_uiout = saved_uiout;
+ reason = thread_fsm_async_reply_reason (tp->thread_fsm);
+ ui_out_field_string (mi_uiout, "reason",
+ async_reason_lookup (reason));
}
- /* Otherwise, frame information has already been printed by
- normal_stop. */
- else
+ print_stop_event (mi_uiout);
+
+ /* Breakpoint hits should always be mirrored to the console.
+ Deciding what to mirror to the console wrt to breakpoints and
+ random stops gets messy real fast. E.g., say "s" trips on a
+ breakpoint. We'd clearly want to mirror the event to the
+ console in this case. But what about more complicated cases
+ like "s&; thread n; s&", and one of those steps spawning a
+ new thread, and that thread hitting a breakpoint? It's
+ impossible in general to track whether the thread had any
+ relation to the commands that had been executed. So we just
+ simplify and always mirror breakpoints and random events to
+ the console.
+
+ OTOH, we should print the source line to the console when
+ stepping or other similar commands, iff the step was started
+ by a console command, but not if it was started with
+ -exec-step or similar. */
+ if ((bpstat_what (tp->control.stop_bpstat).main_action
+ == BPSTAT_WHAT_STOP_NOISY)
+ || !(tp->thread_fsm != NULL
+ && thread_fsm_finished_p (tp->thread_fsm))
+ || (tp->control.command_interp != NULL
+ && tp->control.command_interp != top_level_interpreter ()))
{
- /* Breakpoint hits should always be mirrored to the console.
- Deciding what to mirror to the console wrt to breakpoints
- and random stops gets messy real fast. E.g., say "s"
- trips on a breakpoint. We'd clearly want to mirror the
- event to the console in this case. But what about more
- complicated cases like "s&; thread n; s&", and one of
- those steps spawning a new thread, and that thread
- hitting a breakpoint? It's impossible in general to
- track whether the thread had any relation to the commands
- that had been executed. So we just simplify and always
- mirror breakpoints and random events to the console.
-
- Also, CLI execution commands (-interpreter-exec console
- "next", for example) in async mode have the opposite
- issue as described in the "then" branch above --
- normal_stop has already printed frame information to MI
- uiout, but nothing has printed the same information to
- the CLI channel. We should print the source line to the
- console when stepping or other similar commands, iff the
- step was started by a console command (but not if it was
- started with -exec-step or similar). */
- struct thread_info *tp = inferior_thread ();
-
- if ((!tp->control.stop_step
- && !tp->control.proceed_to_finish)
- || (tp->control.command_interp != NULL
- && tp->control.command_interp != top_level_interpreter ()))
- {
- struct mi_interp *mi = top_level_interpreter_data ();
- struct target_waitstatus last;
- ptid_t last_ptid;
- struct cleanup *old_chain;
-
- /* Set the current uiout to CLI uiout temporarily. */
- old_chain = make_cleanup (restore_current_uiout_cleanup,
- current_uiout);
- current_uiout = mi->cli_uiout;
-
- get_last_target_status (&last_ptid, &last);
- print_stop_event (&last);
-
- do_cleanups (old_chain);
- }
+ struct mi_interp *mi = top_level_interpreter_data ();
+
+ print_stop_event (mi->cli_uiout);
}
ui_out_field_int (mi_uiout, "thread-id",
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index 08c8f02..5f75bef 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -156,6 +156,24 @@ if {[regexp "A + B" "$output"]} {
mi_expect_stop "end-stepping-range" "callee4" "" ".*basics.c" $line_callee4_next_step \
"" "check *stopped from CLI command 2"
+# Test that CLI's "finish" command prints the function's return value
+# to both the CLI and MI streams, and that the same result variable is
+# printed to both streams.
+with_test_prefix "CLI finish" {
+ mi_send_resuming_command "interpreter-exec console finish" "send CLI command"
+
+ set output [mi_gdb_expect_cli_output "\\*stopped" "collect CLI output"]
+ gdb_assert {[regexp \
+ "callee3 .* at .*basics.c:.*.*Value returned is .1 = 0\\\\n" \
+ $output]} \
+ "check CLI output"
+
+ mi_expect_stop "function-finished" "callee3" ".*" \
+ ".*basics.c" ".*" \
+ ",gdb-result-var=\".1\",return-value=\"0\"" \
+ "check MI output"
+}
+
mi_gdb_test "600-break-insert -t basics.c:$line_main_hello" \
{600\^done,bkpt=.number="3",type="breakpoint".*\}} \
"-break-insert -t basics.c:\$line_main_hello"
diff --git a/gdb/thread-fsm.c b/gdb/thread-fsm.c
new file mode 100644
index 0000000..761ad1c
--- /dev/null
+++ b/gdb/thread-fsm.c
@@ -0,0 +1,97 @@
+/* Thread command's finish-state machine, for GDB, the GNU debugger.
+ Copyright (C) 2015 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "thread-fsm.h"
+
+/* See thread-fsm.h. */
+
+void
+thread_fsm_ctor (struct thread_fsm *self, struct thread_fsm_ops *ops)
+{
+ self->finished = 0;
+ self->ops = ops;
+}
+
+/* See thread-fsm.h. */
+
+void
+thread_fsm_delete (struct thread_fsm *self)
+{
+ if (self != NULL)
+ {
+ if (self->ops->dtor != NULL)
+ self->ops->dtor (self);
+ xfree (self);
+ }
+}
+
+/* See thread-fsm.h. */
+
+void
+thread_fsm_clean_up (struct thread_fsm *self)
+{
+ if (self->ops->clean_up != NULL)
+ self->ops->clean_up (self);
+}
+
+/* See thread-fsm.h. */
+
+int
+thread_fsm_should_stop (struct thread_fsm *self)
+{
+ return self->ops->should_stop (self);
+}
+
+/* See thread-fsm.h. */
+
+struct return_value_info *
+thread_fsm_return_value (struct thread_fsm *self)
+{
+ if (self->ops->return_value != NULL)
+ return self->ops->return_value (self);
+ return NULL;
+}
+
+/* See thread-fsm.h. */
+
+void
+thread_fsm_set_finished (struct thread_fsm *self)
+{
+ self->finished = 1;
+}
+
+/* See thread-fsm.h. */
+
+int
+thread_fsm_finished_p (struct thread_fsm *self)
+{
+ return self->finished;
+}
+
+/* See thread-fsm.h. */
+
+enum async_reply_reason
+thread_fsm_async_reply_reason (struct thread_fsm *self)
+{
+ /* If we didn't finish, then the stop reason must come from
+ elsewhere. E.g., a breakpoint hit or a signal intercepted. */
+ gdb_assert (thread_fsm_finished_p (self));
+
+ return self->ops->async_reply_reason (self);
+}
diff --git a/gdb/thread-fsm.h b/gdb/thread-fsm.h
new file mode 100644
index 0000000..031684b
--- /dev/null
+++ b/gdb/thread-fsm.h
@@ -0,0 +1,98 @@
+/* Thread command's finish-state machine, for GDB, the GNU debugger.
+ Copyright (C) 2015 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef THREAD_FSM_H
+#define THREAD_FSM_H
+
+#include "mi/mi-common.h" /* For enum async_reply_reason. */
+
+struct return_value_info;
+struct thread_fsm_ops;
+
+/* A thread finite-state machine structure contains the necessary info
+ and callbacks to manage the state machine protocol of a thread's
+ execution command. */
+
+struct thread_fsm
+{
+ /* Pointer of the virtual table of methods. */
+ struct thread_fsm_ops *ops;
+
+ /* Whether the FSM is done successfully. */
+ int finished;
+};
+
+/* The virtual table of a thread_fsm. */
+
+struct thread_fsm_ops
+{
+ /* The destructor. This should simply free heap allocated data
+ structures. Cleaning up target resources (like, e.g.,
+ breakpoints) should be done in the clean_up method. */
+ void (*dtor) (struct thread_fsm *self);
+
+ /* Called to clean up target resources after the FSM. E.g., if the
+ FSM created internal breakpoints, this is where they should be
+ deleted. */
+ void (*clean_up) (struct thread_fsm *self);
+
+ /* Called after handle_inferior_event decides the target is done
+ (that is, after stop_waiting). The FSM is given a chance to
+ decide whether the command is done and thus the target should
+ stop, or whether there's still more to do and thus the thread
+ should be re-resumed. This is a good place to cache target data
+ too. For example, the "finish" command saves the just-finished
+ function's return value here. */
+ int (*should_stop) (struct thread_fsm *self);
+
+ /* If this FSM saved a function's return value, you can use this
+ method to retrieve it. Otherwise, this returns NULL. */
+ struct return_value_info *(*return_value) (struct thread_fsm *self);
+
+ /* The async_reply_reason that is broadcast to MI clients if this
+ FSM finishes successfully. */
+ enum async_reply_reason (*async_reply_reason) (struct thread_fsm *self);
+};
+/* Initialize FSM. */
+extern void thread_fsm_ctor (struct thread_fsm *fsm,
+ struct thread_fsm_ops *ops);
+
+/* Calls the FSM's dtor method, and then frees FSM. */
+extern void thread_fsm_delete (struct thread_fsm *fsm);
+
+/* Calls the FSM's clean_up method. */
+extern void thread_fsm_clean_up (struct thread_fsm *fsm);
+
+/* Calls the FSM's should_stop method. */
+extern int thread_fsm_should_stop (struct thread_fsm *fsm);
+
+/* Calls the FSM's return_value method. */
+extern struct return_value_info *
+ thread_fsm_return_value (struct thread_fsm *fsm);
+
+/* Marks the FSM as completed successfully. */
+extern void thread_fsm_set_finished (struct thread_fsm *fsm);
+
+/* Returns true if the FSM completed successfully. */
+extern int thread_fsm_finished_p (struct thread_fsm *fsm);
+
+/* Calls the FSM's reply_reason method. */
+extern enum async_reply_reason
+ thread_fsm_async_reply_reason (struct thread_fsm *fsm);
+
+#endif /* THREAD_FSM_H */
diff --git a/gdb/thread.c b/gdb/thread.c
index 4dde722..87a5950 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -43,6 +43,7 @@
#include "gdb_regex.h"
#include "cli/cli-utils.h"
#include "continuations.h"
+#include "thread-fsm.h"
/* Definition of struct thread_info exported to gdbthread.h. */
@@ -158,6 +159,19 @@ thread_has_single_step_breakpoint_here (struct thread_info *tp,
&& breakpoint_has_location_inserted_here (ss_bps, aspace, addr));
}
+/* See gdbthread.h. */
+
+void
+thread_cancel_execution_command (struct thread_info *thr)
+{
+ if (thr->thread_fsm != NULL)
+ {
+ thread_fsm_clean_up (thr->thread_fsm);
+ thread_fsm_delete (thr->thread_fsm);
+ thr->thread_fsm = NULL;
+ }
+}
+
static void
clear_thread_inferior_resources (struct thread_info *tp)
{
@@ -175,6 +189,8 @@ clear_thread_inferior_resources (struct thread_info *tp)
btrace_teardown (tp);
+ thread_cancel_execution_command (tp);
+
do_all_intermediate_continuations_thread (tp, 1);
do_all_continuations_thread (tp, 1);
}
diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c
index 1a5639d..dbb6976 100644
--- a/gdb/tui/tui-interp.c
+++ b/gdb/tui/tui-interp.c
@@ -55,6 +55,18 @@ tui_exit (void)
quiet (i.e., another interpreter is being run with
interpreter-exec), print nothing. */
+/* Observer for the normal_stop notification. */
+
+static void
+tui_on_normal_stop (struct bpstats *bs, int print_frame)
+{
+ if (!interp_quiet_p (tui_interp))
+ {
+ if (print_frame)
+ print_stop_event (tui_ui_out (tui_interp));
+ }
+}
+
/* Observer for the signal_received notification. */
static void
@@ -134,6 +146,7 @@ tui_init (struct interp *self, int top_level)
tui_initialize_readline ();
/* If changing this, remember to update cli-interp.c as well. */
+ observer_attach_normal_stop (tui_on_normal_stop);
observer_attach_signal_received (tui_on_signal_received);
observer_attach_end_stepping_range (tui_on_end_stepping_range);
observer_attach_signal_exited (tui_on_signal_exited);
--
1.9.3
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 4/7] Convert the until/advance commands to thread_fsm mechanism
2015-08-12 17:02 [PATCH 0/7] Replace continuations with an extendable "class" Pedro Alves
2015-08-12 17:02 ` [PATCH 1/7] Merge async and sync code paths some more Pedro Alves
2015-08-12 17:02 ` [PATCH 2/7] Replace "struct continuation" mechanism by something more extensible Pedro Alves
@ 2015-08-12 17:02 ` Pedro Alves
2015-08-12 17:02 ` [PATCH 3/7] Convert infcalls " Pedro Alves
` (4 subsequent siblings)
7 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-08-12 17:02 UTC (permalink / raw)
To: gdb-patches
gdb/ChangeLog:
2015-08-12 Pedro Alves <palves@redhat.com>
* breakpoint.c: Include "thread-fsm.h".
(struct until_break_command_continuation_args): Delete.
(struct until_break_fsm): New.
(until_break_fsm_ops): New global.
(new_until_break_fsm, until_break_fsm_should_stop): New functions.
(until_break_command_continuation): Delete.
(until_break_fsm_clean_up): New function.
(until_break_fsm_async_reply_reason): New function.
(until_break_command): Adjust to create an until_break_fsm instead
of a continuation.
(momentary_bkpt_print_it): No longer print MI's async-stop-reason
here.
* infcmd.c (struct until_next_fsm): New.
(until_next_fsm_ops): New global.
(new_until_next_fsm, until_next_fsm_should_stop): New function.
(until_next_continuation): Delete.
(until_next_fsm_clean_up, until_next_fsm_async_reply_reason): New
functions.
(until_next_command): Adjust to create a new until_next_fsm
instead of a continuation.
---
gdb/breakpoint.c | 181 +++++++++++++++++++++++++++++++++++--------------------
gdb/infcmd.c | 92 ++++++++++++++++++++++------
2 files changed, 188 insertions(+), 85 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index ef04bde..eb15e58 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -68,6 +68,7 @@
#include "interps.h"
#include "format.h"
#include "location.h"
+#include "thread-fsm.h"
/* readline include files */
#include "readline/readline.h"
@@ -11467,29 +11468,109 @@ awatch_command (char *arg, int from_tty)
}
\f
-/* Helper routines for the until_command routine in infcmd.c. Here
- because it uses the mechanisms of breakpoints. */
+/* Data for the FSM that manages the until(location)/advance commands
+ in infcmd.c. Here because it uses the mechanisms of
+ breakpoints. */
-struct until_break_command_continuation_args
+struct until_break_fsm
{
- struct breakpoint *breakpoint;
- struct breakpoint *breakpoint2;
- int thread_num;
+ /* The base class. */
+ struct thread_fsm thread_fsm;
+
+ /* The thread that as current when the command was executed. */
+ int thread;
+
+ /* The breakpoint set at the destination location. */
+ struct breakpoint *location_breakpoint;
+
+ /* Breakpoint set at the return address in the caller frame. May be
+ NULL. */
+ struct breakpoint *caller_breakpoint;
};
-/* This function is called by fetch_inferior_event via the
- cmd_continuation pointer, to complete the until command. It takes
- care of cleaning up the temporary breakpoints set up by the until
- command. */
+static void until_break_fsm_clean_up (struct thread_fsm *self);
+static int until_break_fsm_should_stop (struct thread_fsm *self);
+static enum async_reply_reason
+ until_break_fsm_async_reply_reason (struct thread_fsm *self);
+
+/* until_break_fsm's vtable. */
+
+static struct thread_fsm_ops until_break_fsm_ops =
+{
+ NULL, /* dtor */
+ until_break_fsm_clean_up,
+ until_break_fsm_should_stop,
+ NULL, /* return_value */
+ until_break_fsm_async_reply_reason,
+};
+
+/* Allocate a new until_break_command_fsm. */
+
+static struct until_break_fsm *
+new_until_break_fsm (int thread,
+ struct breakpoint *location_breakpoint,
+ struct breakpoint *caller_breakpoint)
+{
+ struct until_break_fsm *sm;
+
+ sm = XCNEW (struct until_break_fsm);
+ thread_fsm_ctor (&sm->thread_fsm, &until_break_fsm_ops);
+
+ sm->thread = thread;
+ sm->location_breakpoint = location_breakpoint;
+ sm->caller_breakpoint = caller_breakpoint;
+
+ return sm;
+}
+
+/* Implementation of the 'should_stop' FSM method for the
+ until(location)/advance commands. */
+
+static int
+until_break_fsm_should_stop (struct thread_fsm *self)
+{
+ struct until_break_fsm *sm = (struct until_break_fsm *) self;
+ struct thread_info *tp = inferior_thread ();
+
+ if (bpstat_find_breakpoint (tp->control.stop_bpstat,
+ sm->location_breakpoint) != NULL
+ || (sm->caller_breakpoint != NULL
+ && bpstat_find_breakpoint (tp->control.stop_bpstat,
+ sm->caller_breakpoint) != NULL))
+ thread_fsm_set_finished (self);
+
+ return 1;
+}
+
+/* Implementation of the 'clean_up' FSM method for the
+ until(location)/advance commands. */
+
static void
-until_break_command_continuation (void *arg, int err)
+until_break_fsm_clean_up (struct thread_fsm *self)
{
- struct until_break_command_continuation_args *a = arg;
+ struct until_break_fsm *sm = (struct until_break_fsm *) self;
- delete_breakpoint (a->breakpoint);
- if (a->breakpoint2)
- delete_breakpoint (a->breakpoint2);
- delete_longjmp_breakpoint (a->thread_num);
+ /* Clean up our temporary breakpoints. */
+ if (sm->location_breakpoint != NULL)
+ {
+ delete_breakpoint (sm->location_breakpoint);
+ sm->location_breakpoint = NULL;
+ }
+ if (sm->caller_breakpoint != NULL)
+ {
+ delete_breakpoint (sm->caller_breakpoint);
+ sm->caller_breakpoint = NULL;
+ }
+ delete_longjmp_breakpoint (sm->thread);
+}
+
+/* Implementation of the 'async_reply_reason' FSM method for the
+ until(location)/advance commands. */
+
+static enum async_reply_reason
+until_break_fsm_async_reply_reason (struct thread_fsm *self)
+{
+ return EXEC_ASYNC_LOCATION_REACHED;
}
void
@@ -11501,12 +11582,13 @@ until_break_command (char *arg, int from_tty, int anywhere)
struct gdbarch *frame_gdbarch;
struct frame_id stack_frame_id;
struct frame_id caller_frame_id;
- struct breakpoint *breakpoint;
- struct breakpoint *breakpoint2 = NULL;
+ struct breakpoint *location_breakpoint;
+ struct breakpoint *caller_breakpoint = NULL;
struct cleanup *old_chain, *cleanup;
int thread;
struct thread_info *tp;
struct event_location *location;
+ struct until_break_fsm *sm;
clear_proceed_status (0);
@@ -11556,14 +11638,16 @@ until_break_command (char *arg, int from_tty, int anywhere)
if (frame_id_p (caller_frame_id))
{
struct symtab_and_line sal2;
+ struct gdbarch *caller_gdbarch;
sal2 = find_pc_line (frame_unwind_caller_pc (frame), 0);
sal2.pc = frame_unwind_caller_pc (frame);
- breakpoint2 = set_momentary_breakpoint (frame_unwind_caller_arch (frame),
- sal2,
- caller_frame_id,
- bp_until);
- make_cleanup_delete_breakpoint (breakpoint2);
+ caller_gdbarch = frame_unwind_caller_arch (frame);
+ caller_breakpoint = set_momentary_breakpoint (caller_gdbarch,
+ sal2,
+ caller_frame_id,
+ bp_until);
+ make_cleanup_delete_breakpoint (caller_breakpoint);
set_longjmp_breakpoint (tp, caller_frame_id);
make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
@@ -11575,38 +11659,21 @@ until_break_command (char *arg, int from_tty, int anywhere)
if (anywhere)
/* If the user told us to continue until a specified location,
we don't specify a frame at which we need to stop. */
- breakpoint = set_momentary_breakpoint (frame_gdbarch, sal,
- null_frame_id, bp_until);
+ location_breakpoint = set_momentary_breakpoint (frame_gdbarch, sal,
+ null_frame_id, bp_until);
else
/* Otherwise, specify the selected frame, because we want to stop
only at the very same frame. */
- breakpoint = set_momentary_breakpoint (frame_gdbarch, sal,
- stack_frame_id, bp_until);
- make_cleanup_delete_breakpoint (breakpoint);
+ location_breakpoint = set_momentary_breakpoint (frame_gdbarch, sal,
+ stack_frame_id, bp_until);
+ make_cleanup_delete_breakpoint (location_breakpoint);
- proceed (-1, GDB_SIGNAL_DEFAULT);
+ sm = new_until_break_fsm (tp->num, location_breakpoint, caller_breakpoint);
+ tp->thread_fsm = &sm->thread_fsm;
- /* If we are running asynchronously, and proceed call above has
- actually managed to start the target, arrange for breakpoints to
- be deleted when the target stops. Otherwise, we're already
- stopped and delete breakpoints via cleanup chain. */
-
- if (is_running (inferior_ptid))
- {
- struct until_break_command_continuation_args *args;
- args = xmalloc (sizeof (*args));
-
- args->breakpoint = breakpoint;
- args->breakpoint2 = breakpoint2;
- args->thread_num = thread;
+ discard_cleanups (old_chain);
- discard_cleanups (old_chain);
- add_continuation (inferior_thread (),
- until_break_command_continuation, args,
- xfree);
- }
- else
- do_cleanups (old_chain);
+ proceed (-1, GDB_SIGNAL_DEFAULT);
do_cleanups (cleanup);
}
@@ -13198,22 +13265,6 @@ momentary_bkpt_check_status (bpstat bs)
static enum print_stop_action
momentary_bkpt_print_it (bpstat bs)
{
- struct ui_out *uiout = current_uiout;
-
- if (ui_out_is_mi_like_p (uiout))
- {
- struct breakpoint *b = bs->breakpoint_at;
-
- switch (b->type)
- {
- case bp_until:
- ui_out_field_string
- (uiout, "reason",
- async_reason_lookup (EXEC_ASYNC_LOCATION_REACHED));
- break;
- }
- }
-
return PRINT_UNKNOWN;
}
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 67b5df0..e5d01ff 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1388,22 +1388,81 @@ queue_signal_command (char *signum_exp, int from_tty)
tp->suspend.stop_signal = oursig;
}
-/* Continuation args to be passed to the "until" command
- continuation. */
-struct until_next_continuation_args
+/* Data for the FSM that manages the until (with no argument)
+ command. */
+
+struct until_next_fsm
{
- /* The thread that was current when the command was executed. */
+ /* The base class. */
+ struct thread_fsm thread_fsm;
+
+ /* The thread that as current when the command was executed. */
int thread;
};
-/* A continuation callback for until_next_command. */
+static int until_next_fsm_should_stop (struct thread_fsm *self);
+static void until_next_fsm_clean_up (struct thread_fsm *self);
+static enum async_reply_reason
+ until_next_fsm_async_reply_reason (struct thread_fsm *self);
+
+/* until_next_fsm's vtable. */
+
+static struct thread_fsm_ops until_next_fsm_ops =
+{
+ NULL, /* dtor */
+ until_next_fsm_clean_up,
+ until_next_fsm_should_stop,
+ NULL, /* return_value */
+ until_next_fsm_async_reply_reason,
+};
+
+/* Allocate a new until_next_fsm. */
+
+static struct until_next_fsm *
+new_until_next_fsm (int thread)
+{
+ struct until_next_fsm *sm;
+
+ sm = XCNEW (struct until_next_fsm);
+ thread_fsm_ctor (&sm->thread_fsm, &until_next_fsm_ops);
+
+ sm->thread = thread;
+
+ return sm;
+}
+
+/* Implementation of the 'should_stop' FSM method for the until (with
+ no arg) command. */
+
+static int
+until_next_fsm_should_stop (struct thread_fsm *self)
+{
+ struct thread_info *tp = inferior_thread ();
+
+ if (tp->control.stop_step)
+ thread_fsm_set_finished (self);
+
+ return 1;
+}
+
+/* Implementation of the 'clean_up' FSM method for the until (with no
+ arg) command. */
static void
-until_next_continuation (void *arg, int err)
+until_next_fsm_clean_up (struct thread_fsm *self)
{
- struct until_next_continuation_args *a = arg;
+ struct until_next_fsm *sm = (struct until_next_fsm *) self;
- delete_longjmp_breakpoint (a->thread);
+ delete_longjmp_breakpoint (sm->thread);
+}
+
+/* Implementation of the 'async_reply_reason' FSM method for the until
+ (with no arg) command. */
+
+static enum async_reply_reason
+until_next_fsm_async_reply_reason (struct thread_fsm *self)
+{
+ return EXEC_ASYNC_END_STEPPING_RANGE;
}
/* Proceed until we reach a different source line with pc greater than
@@ -1424,6 +1483,7 @@ until_next_command (int from_tty)
struct thread_info *tp = inferior_thread ();
int thread = tp->num;
struct cleanup *old_chain;
+ struct until_next_fsm *sm;
clear_proceed_status (0);
set_step_frame ();
@@ -1463,20 +1523,12 @@ until_next_command (int from_tty)
set_longjmp_breakpoint (tp, get_frame_id (frame));
old_chain = make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
- proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
-
- if (is_running (inferior_ptid))
- {
- struct until_next_continuation_args *cont_args;
+ sm = new_until_next_fsm (tp->num);
+ tp->thread_fsm = &sm->thread_fsm;
+ discard_cleanups (old_chain);
- discard_cleanups (old_chain);
- cont_args = XNEW (struct until_next_continuation_args);
- cont_args->thread = inferior_thread ()->num;
+ proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
- add_continuation (tp, until_next_continuation, cont_args, xfree);
- }
- else
- do_cleanups (old_chain);
}
static void
--
1.9.3
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 3/7] Convert infcalls to thread_fsm mechanism
2015-08-12 17:02 [PATCH 0/7] Replace continuations with an extendable "class" Pedro Alves
` (2 preceding siblings ...)
2015-08-12 17:02 ` [PATCH 4/7] Convert the until/advance commands to thread_fsm mechanism Pedro Alves
@ 2015-08-12 17:02 ` Pedro Alves
2015-08-12 17:11 ` [PATCH 7/7] Delete enum inferior_event_handler::INF_TIMER Pedro Alves
` (3 subsequent siblings)
7 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-08-12 17:02 UTC (permalink / raw)
To: gdb-patches
This removes infcall-specific special casing from normal_stop,
simplifying it.
Like the "finish" command's, the FSM is responsible for storing the
function's return value.
gdb/ChangeLog:
2015-08-12 Pedro Alves <palves@redhat.com>
* infcall.c: Include thread_fsm.h.
(struct call_return_meta_info): New.
(get_call_return_value): New function, factored out from
call_function_by_hand_dummy.
(struct call_thread_fsm): New.
(call_thread_fsm_ops): New global.
(new_call_thread_fsm, call_thread_fsm_should_stop)
(call_thread_fsm_should_notify_stop): New functions.
(run_inferior_call): Add 'sm' parameter. Associate the FSM with
the thread.
(call_function_by_hand_dummy): Create a new call_thread_fsm
instance, associate it with the thread, and wait for the FSM to
finish. If finished successfully, fetch the function's result
value out of the FSM.
* infrun.c (fetch_inferior_event): If the FSM says the stop
shouldn't be notified, don't call normal_stop.
(maybe_remove_breakpoints): New function, factored out from ...
(normal_stop): ... here. Simplify.
* infrun.h (maybe_remove_breakpoints): Declare.
* thread-fsm.c (thread_fsm_should_notify_stop): New function.
(thread-fsm.h) <struct thread_fsm_ops>: New field.
(thread_fsm_should_notify_stop): Declare.
---
gdb/infcall.c | 305 ++++++++++++++++++++++++++++++++++++++++++-------------
gdb/infrun.c | 159 +++++++++++++----------------
gdb/infrun.h | 4 +
gdb/thread-fsm.c | 10 ++
gdb/thread-fsm.h | 6 ++
5 files changed, 324 insertions(+), 160 deletions(-)
diff --git a/gdb/infcall.c b/gdb/infcall.c
index b19028d..5acdff0 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -38,6 +38,7 @@
#include "observer.h"
#include "top.h"
#include "interps.h"
+#include "thread-fsm.h"
/* If we can't find a function's name from its address,
we print this instead. */
@@ -374,6 +375,174 @@ get_function_name (CORE_ADDR funaddr, char *buf, int buf_size)
}
}
+/* All the meta data necessary to extract the call's return value. */
+
+struct call_return_meta_info
+{
+ /* The caller frame's architecture. */
+ struct gdbarch *gdbarch;
+
+ /* The called function. */
+ struct value *function;
+
+ /* The return value's type. */
+ struct type *value_type;
+
+ /* Are we returning a value using a structure return or a normal
+ value return? */
+ int struct_return_p;
+
+ /* If using a structure return, this is the structure's address. */
+ CORE_ADDR struct_addr;
+
+ /* Whether stack temporaries are enabled. */
+ int stack_temporaries_enabled;
+};
+
+/* Extract the called function's return value. */
+
+static struct value *
+get_call_return_value (struct call_return_meta_info *ri)
+{
+ struct value *retval = NULL;
+ int stack_temporaries = thread_stack_temporaries_enabled_p (inferior_ptid);
+
+ if (TYPE_CODE (ri->value_type) == TYPE_CODE_VOID)
+ retval = allocate_value (ri->value_type);
+ else if (ri->struct_return_p)
+ {
+ if (stack_temporaries)
+ {
+ retval = value_from_contents_and_address (ri->value_type, NULL,
+ ri->struct_addr);
+ push_thread_stack_temporary (inferior_ptid, retval);
+ }
+ else
+ {
+ retval = allocate_value (ri->value_type);
+ read_value_memory (retval, 0, 1, ri->struct_addr,
+ value_contents_raw (retval),
+ TYPE_LENGTH (ri->value_type));
+ }
+ }
+ else
+ {
+ retval = allocate_value (ri->value_type);
+ gdbarch_return_value (ri->gdbarch, ri->function, ri->value_type,
+ get_current_regcache (),
+ value_contents_raw (retval), NULL);
+ if (stack_temporaries && class_or_union_p (ri->value_type))
+ {
+ /* Values of class type returned in registers are copied onto
+ the stack and their lval_type set to lval_memory. This is
+ required because further evaluation of the expression
+ could potentially invoke methods on the return value
+ requiring GDB to evaluate the "this" pointer. To evaluate
+ the this pointer, GDB needs the memory address of the
+ value. */
+ value_force_lval (retval, ri->struct_addr);
+ push_thread_stack_temporary (inferior_ptid, retval);
+ }
+ }
+
+ gdb_assert (retval != NULL);
+ return retval;
+}
+
+/* Data for the FSM that manages an infcall. It's main job is to
+ record the called function's return value. */
+
+struct call_thread_fsm
+{
+ /* The base class. */
+ struct thread_fsm thread_fsm;
+
+ /* All the info necessary to be able to extract the return
+ value. */
+ struct call_return_meta_info return_meta_info;
+
+ /* The called function's return value. This is extracted from the
+ target before the dummy frame is popped. */
+ struct value *return_value;
+};
+
+static int call_thread_fsm_should_stop (struct thread_fsm *self);
+static int call_thread_fsm_should_notify_stop (struct thread_fsm *self);
+
+/* call_thread_fsm's vtable. */
+
+static struct thread_fsm_ops call_thread_fsm_ops =
+{
+ NULL, /*dtor */
+ NULL, /* clean_up */
+ call_thread_fsm_should_stop,
+ NULL, /* return_value */
+ NULL, /* async_reply_reason*/
+ call_thread_fsm_should_notify_stop,
+};
+
+/* Allocate a new call_thread_fsm object. */
+
+static struct call_thread_fsm *
+new_call_thread_fsm (struct gdbarch *gdbarch, struct value *function,
+ struct type *value_type,
+ int struct_return_p, CORE_ADDR struct_addr)
+{
+ struct call_thread_fsm *sm;
+
+ sm = XCNEW (struct call_thread_fsm);
+ thread_fsm_ctor (&sm->thread_fsm, &call_thread_fsm_ops);
+
+ sm->return_meta_info.gdbarch = gdbarch;
+ sm->return_meta_info.function = function;
+ sm->return_meta_info.value_type = value_type;
+ sm->return_meta_info.struct_return_p = struct_return_p;
+ sm->return_meta_info.struct_addr = struct_addr;
+
+ return sm;
+}
+
+/* Implementation of should_stop method for infcalls. */
+
+static int
+call_thread_fsm_should_stop (struct thread_fsm *self)
+{
+ struct call_thread_fsm *f = (struct call_thread_fsm *) self;
+
+ if (stop_stack_dummy == STOP_STACK_DUMMY)
+ {
+ /* Done. */
+ thread_fsm_set_finished (self);
+
+ /* Stash the return value before the dummy frame is popped and
+ registers are restored to what they were before the
+ call.. */
+ f->return_value = get_call_return_value (&f->return_meta_info);
+
+ /* Break out of wait_sync_command_done. */
+ async_enable_stdin ();
+ }
+
+ return 1;
+}
+
+/* Implementation of should_notify_stop method for infcalls. */
+
+static int
+call_thread_fsm_should_notify_stop (struct thread_fsm *self)
+{
+ if (thread_fsm_finished_p (self))
+ {
+ /* Infcall succeeded. Be silent and proceed with evaluating the
+ expression. */
+ return 0;
+ }
+
+ /* Something wrong happened. E.g., an unexpected breakpoint
+ triggered, or a signal was intercepted. Notify the stop. */
+ return 1;
+}
+
/* Subroutine of call_function_by_hand to simplify it.
Start up the inferior and wait for it to stop.
Return the exception if there's an error, or an exception with
@@ -383,7 +552,8 @@ get_function_name (CORE_ADDR funaddr, char *buf, int buf_size)
thrown errors. The caller should rethrow if there's an error. */
static struct gdb_exception
-run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
+run_inferior_call (struct call_thread_fsm *sm,
+ struct thread_info *call_thread, CORE_ADDR real_pc)
{
struct gdb_exception caught_error = exception_none;
int saved_in_infcall = call_thread->control.in_infcall;
@@ -402,6 +572,11 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
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->thread_fsm;
+
disable_watchpoints_before_interactive_call_start ();
/* We want to print return value, please... */
@@ -620,8 +795,6 @@ call_function_by_hand_dummy (struct value *function,
struct gdb_exception e;
char name_buf[RAW_FUNCTION_ADDRESS_SIZE];
int stack_temporaries = thread_stack_temporaries_enabled_p (inferior_ptid);
- struct dummy_frame_context_saver *context_saver;
- struct cleanup *context_saver_cleanup;
if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
ftype = check_typedef (TYPE_TARGET_TYPE (ftype));
@@ -1008,13 +1181,6 @@ call_function_by_hand_dummy (struct value *function,
register_dummy_frame_dtor (dummy_id, inferior_ptid,
dummy_dtor, dummy_dtor_data);
- /* dummy_frame_context_saver_setup must be called last so that its
- saving of inferior registers gets called first (before possible
- DUMMY_DTOR destructor). */
- context_saver = dummy_frame_context_saver_setup (dummy_id, inferior_ptid);
- context_saver_cleanup = make_cleanup (dummy_frame_context_saver_cleanup,
- context_saver);
-
/* Register a clean-up for unwind_on_terminating_exception_breakpoint. */
terminate_bp_cleanup = make_cleanup (cleanup_delete_std_terminate_breakpoint,
NULL);
@@ -1027,6 +1193,12 @@ call_function_by_hand_dummy (struct value *function,
in a block so that it's only in scope during the time it's valid. */
{
struct thread_info *tp = inferior_thread ();
+ struct thread_fsm *saved_sm;
+ struct call_thread_fsm *sm;
+
+ /* Save the current FSM. We'll override it. */
+ saved_sm = tp->thread_fsm;
+ tp->thread_fsm = NULL;
/* Save this thread's ptid, we need it later but the thread
may have exited. */
@@ -1034,10 +1206,57 @@ call_function_by_hand_dummy (struct value *function,
/* Run the inferior until it stops. */
- e = run_inferior_call (tp, real_pc);
- }
+ /* Create the FSM used to manage the infcall. It tells infrun to
+ not report the stop to the user, and captures the return value
+ before the dummy frame is popped. run_inferior_call registers
+ it with the thread ASAP. */
+ sm = new_call_thread_fsm (gdbarch, function,
+ values_type,
+ struct_return || hidden_first_param_p,
+ struct_addr);
+
+ e = run_inferior_call (sm, tp, real_pc);
+
+ observer_notify_inferior_call_post (call_thread_ptid, funaddr);
+
+ tp = find_thread_ptid (call_thread_ptid);
+ if (tp != NULL)
+ {
+ /* The FSM should still be the same. */
+ gdb_assert (tp->thread_fsm == &sm->thread_fsm);
+
+ if (thread_fsm_finished_p (tp->thread_fsm))
+ {
+ struct value *retval;
+
+ /* The inferior call is successful. Pop the dummy frame,
+ which runs its destructors and restores the inferior's
+ suspend state, and restore the inferior control
+ state. */
+ dummy_frame_pop (dummy_id, call_thread_ptid);
+ restore_infcall_control_state (inf_status);
+
+ /* Get the return value. */
+ retval = sm->return_value;
+
+ /* Clean up / destroy the call FSM, and restore the
+ original one. */
+ thread_fsm_clean_up (tp->thread_fsm);
+ thread_fsm_delete (tp->thread_fsm);
+ tp->thread_fsm = saved_sm;
- observer_notify_inferior_call_post (call_thread_ptid, funaddr);
+ maybe_remove_breakpoints ();
+
+ do_cleanups (terminate_bp_cleanup);
+ gdb_assert (retval != NULL);
+ return retval;
+ }
+
+ /* Didn't complete. Restore previous state machine, and
+ handle the error. */
+ tp->thread_fsm = saved_sm;
+ }
+ }
/* Rethrow an error if we got one trying to run the inferior. */
@@ -1119,7 +1338,6 @@ When the function is done executing, GDB will silently stop."),
name);
}
- if (stopped_by_random_signal || stop_stack_dummy != STOP_STACK_DUMMY)
{
/* Make a copy as NAME may be in an objfile freed by dummy_frame_pop. */
char *name = xstrdup (get_function_name (funaddr,
@@ -1221,65 +1439,10 @@ When the function is done executing, GDB will silently stop."),
name);
}
- /* The above code errors out, so ... */
- internal_error (__FILE__, __LINE__, _("... should not be here"));
}
- do_cleanups (terminate_bp_cleanup);
-
- /* If we get here the called FUNCTION ran to completion,
- and the dummy frame has already been popped. */
-
- {
- struct value *retval = NULL;
-
- /* Inferior call is successful. Restore the inferior status.
- At this stage, leave the RETBUF alone. */
- restore_infcall_control_state (inf_status);
-
- if (TYPE_CODE (values_type) == TYPE_CODE_VOID)
- retval = allocate_value (values_type);
- else if (struct_return || hidden_first_param_p)
- {
- if (stack_temporaries)
- {
- retval = value_from_contents_and_address (values_type, NULL,
- struct_addr);
- push_thread_stack_temporary (inferior_ptid, retval);
- }
- else
- {
- retval = allocate_value (values_type);
- read_value_memory (retval, 0, 1, struct_addr,
- value_contents_raw (retval),
- TYPE_LENGTH (values_type));
- }
- }
- else
- {
- retval = allocate_value (values_type);
- gdbarch_return_value (gdbarch, function, values_type,
- dummy_frame_context_saver_get_regs (context_saver),
- value_contents_raw (retval), NULL);
- if (stack_temporaries && class_or_union_p (values_type))
- {
- /* Values of class type returned in registers are copied onto
- the stack and their lval_type set to lval_memory. This is
- required because further evaluation of the expression
- could potentially invoke methods on the return value
- requiring GDB to evaluate the "this" pointer. To evaluate
- the this pointer, GDB needs the memory address of the
- value. */
- value_force_lval (retval, struct_addr);
- push_thread_stack_temporary (inferior_ptid, retval);
- }
- }
-
- do_cleanups (context_saver_cleanup);
-
- gdb_assert (retval);
- return retval;
- }
+ /* The above code errors out, so ... */
+ gdb_assert_not_reached ("... should not be here");
}
\f
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 06526f0..56cd267 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3830,6 +3830,7 @@ fetch_inferior_event (void *client_data)
struct inferior *inf = find_inferior_ptid (ecs->ptid);
int should_stop = 1;
struct thread_info *thr = ecs->event_thread;
+ int should_notify_stop = 1;
delete_just_stopped_threads_infrun_breakpoints ();
@@ -3849,12 +3850,21 @@ fetch_inferior_event (void *client_data)
{
clean_up_just_stopped_threads_fsms (ecs);
- /* We may not find an inferior if this was a process exit. */
- if (inf == NULL || inf->control.stop_soon == NO_STOP_QUIETLY)
- normal_stop ();
+ if (thr != NULL && thr->thread_fsm != NULL)
+ {
+ should_notify_stop
+ = thread_fsm_should_notify_stop (thr->thread_fsm);
+ }
+
+ if (should_notify_stop)
+ {
+ /* We may not find an inferior if this was a process exit. */
+ if (inf == NULL || inf->control.stop_soon == NO_STOP_QUIETLY)
+ normal_stop ();
- inferior_event_handler (INF_EXEC_COMPLETE, NULL);
- cmd_done = 1;
+ inferior_event_handler (INF_EXEC_COMPLETE, NULL);
+ cmd_done = 1;
+ }
}
}
@@ -7802,6 +7812,23 @@ print_stop_event (struct ui_out *uiout)
}
}
+/* See infrun.h. */
+
+void
+maybe_remove_breakpoints (void)
+{
+ if (!breakpoints_should_be_inserted_now () && target_has_execution)
+ {
+ if (remove_breakpoints ())
+ {
+ target_terminal_ours_for_output ();
+ printf_filtered (_("Cannot remove breakpoints because "
+ "program is no longer writable.\nFurther "
+ "execution is probably impossible.\n"));
+ }
+ }
+}
+
/* Here to return control to GDB when the inferior stops for real.
Print appropriate messages, remove breakpoints, give terminal our modes.
@@ -7895,16 +7922,7 @@ normal_stop (void)
}
/* Note: this depends on the update_thread_list call above. */
- if (!breakpoints_should_be_inserted_now () && target_has_execution)
- {
- if (remove_breakpoints ())
- {
- target_terminal_ours_for_output ();
- printf_filtered (_("Cannot remove breakpoints because "
- "program is no longer writable.\nFurther "
- "execution is probably impossible.\n"));
- }
- }
+ maybe_remove_breakpoints ();
/* If an auto-display called a function and that got a signal,
delete that auto-display to avoid an infinite recursion. */
@@ -7915,87 +7933,50 @@ normal_stop (void)
target_terminal_ours ();
async_enable_stdin ();
- /* Set the current source location. This will also happen if we
- display the frame below, but the current SAL will be incorrect
- during a user hook-stop function. */
- if (has_stack_frames () && !stop_stack_dummy)
- set_current_sal_from_frame (get_current_frame ());
-
- /* Let the user/frontend see the threads as stopped, but defer to
- call_function_by_hand if the thread finished an infcall
- successfully. We may be e.g., evaluating a breakpoint condition.
- In that case, the thread had state THREAD_RUNNING before the
- infcall, and shall remain marked running, all without informing
- the user/frontend about state transition changes. */
- if (target_has_execution
- && inferior_thread ()->control.in_infcall
- && stop_stack_dummy == STOP_STACK_DUMMY)
- discard_cleanups (old_chain);
- else
- do_cleanups (old_chain);
-
- /* Look up the hook_stop and run it (CLI internally handles problem
- of stop_command's pre-hook not existing). */
- if (stop_command)
- catch_errors (hook_stop_stub, stop_command,
- "Error while running hook_stop:\n", RETURN_MASK_ALL);
+ /* Let the user/frontend see the threads as stopped. */
+ do_cleanups (old_chain);
- if (!has_stack_frames ())
- goto done;
+ /* Select innermost stack frame - i.e., current frame is frame 0,
+ and current location is based on that. Handle the case where the
+ dummy call is returning after being stopped. E.g. the dummy call
+ previously hit a breakpoint. (If the dummy call returns
+ normally, we won't reach here.) Do this before the stop hook is
+ run, so that it doesn't get to see the temporary dummy frame,
+ which is not where we'll present the stop. */
+ if (has_stack_frames ())
+ {
+ if (stop_stack_dummy == STOP_STACK_DUMMY)
+ {
+ /* Pop the empty frame that contains the stack dummy. This
+ also restores inferior state prior to the call (struct
+ infcall_suspend_state). */
+ struct frame_info *frame = get_current_frame ();
- if (last.kind == TARGET_WAITKIND_SIGNALLED
- || last.kind == TARGET_WAITKIND_EXITED)
- goto done;
+ gdb_assert (get_frame_type (frame) == DUMMY_FRAME);
+ frame_pop (frame);
+ /* frame_pop calls reinit_frame_cache as the last thing it
+ does which means there's now no selected frame. */
+ }
- /* Select innermost stack frame - i.e., current frame is frame 0,
- and current location is based on that.
- Don't do this on return from a stack dummy routine,
- or if the program has exited. */
-
- if (!stop_stack_dummy)
- select_frame (get_current_frame ());
-
- if (stop_stack_dummy == STOP_STACK_DUMMY)
- {
- /* Pop the empty frame that contains the stack dummy.
- This also restores inferior state prior to the call
- (struct infcall_suspend_state). */
- struct frame_info *frame = get_current_frame ();
-
- gdb_assert (get_frame_type (frame) == DUMMY_FRAME);
- frame_pop (frame);
- /* frame_pop() calls reinit_frame_cache as the last thing it
- does which means there's currently no selected frame. We
- don't need to re-establish a selected frame if the dummy call
- returns normally, that will be done by
- restore_infcall_control_state. However, we do have to handle
- the case where the dummy call is returning after being
- stopped (e.g. the dummy call previously hit a breakpoint).
- We can't know which case we have so just always re-establish
- a selected frame here. */
select_frame (get_current_frame ());
- }
-
-done:
- /* Suppress the stop observer if we're in the middle of:
+ /* Set the current source location. */
+ set_current_sal_from_frame (get_current_frame ());
+ }
- - calling an inferior function, as we pretend we inferior didn't
- run at all. The return value of the call is handled by the
- expression evaluator, through call_function_by_hand. */
+ /* Look up the hook_stop and run it (CLI internally handles problem
+ of stop_command's pre-hook not existing). */
+ if (stop_command)
+ catch_errors (hook_stop_stub, stop_command,
+ "Error while running hook_stop:\n", RETURN_MASK_ALL);
- if (!target_has_execution
- || last.kind == TARGET_WAITKIND_SIGNALLED
- || last.kind == TARGET_WAITKIND_EXITED
- || last.kind == TARGET_WAITKIND_NO_RESUMED
- || !inferior_thread ()->control.in_infcall)
- {
- if (!ptid_equal (inferior_ptid, null_ptid))
- observer_notify_normal_stop (inferior_thread ()->control.stop_bpstat,
- stop_print_frame);
- else
- observer_notify_normal_stop (NULL, stop_print_frame);
- }
+ /* Notify observers about the stop. This is where the interpreters
+ print the stop event. */
+ if (!ptid_equal (inferior_ptid, null_ptid))
+ observer_notify_normal_stop (inferior_thread ()->control.stop_bpstat,
+ stop_print_frame);
+ else
+ observer_notify_normal_stop (NULL, stop_print_frame);
annotate_stopped ();
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 34a8d4a..a5d6814 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -212,4 +212,8 @@ extern void mark_infrun_async_event_handler (void);
to get past e.g., a breakpoint. */
extern struct thread_info *step_over_queue_head;
+/* Remove breakpoints if possible (usually that means, if everything
+ is stopped). On failure, print a message. */
+extern void maybe_remove_breakpoints (void);
+
#endif /* INFRUN_H */
diff --git a/gdb/thread-fsm.c b/gdb/thread-fsm.c
index 761ad1c..4a27d02 100644
--- a/gdb/thread-fsm.c
+++ b/gdb/thread-fsm.c
@@ -95,3 +95,13 @@ thread_fsm_async_reply_reason (struct thread_fsm *self)
return self->ops->async_reply_reason (self);
}
+
+/* See thread-fsm.h. */
+
+int
+thread_fsm_should_notify_stop (struct thread_fsm *self)
+{
+ if (self->ops->should_notify_stop != NULL)
+ return self->ops->should_notify_stop (self);
+ return 1;
+}
diff --git a/gdb/thread-fsm.h b/gdb/thread-fsm.h
index 031684b..c22c51f 100644
--- a/gdb/thread-fsm.h
+++ b/gdb/thread-fsm.h
@@ -67,6 +67,9 @@ struct thread_fsm_ops
/* The async_reply_reason that is broadcast to MI clients if this
FSM finishes successfully. */
enum async_reply_reason (*async_reply_reason) (struct thread_fsm *self);
+
+ /* Whether the stop should be notified to the user/frontend. */
+ int (*should_notify_stop) (struct thread_fsm *self);
};
/* Initialize FSM. */
extern void thread_fsm_ctor (struct thread_fsm *fsm,
@@ -95,4 +98,7 @@ extern int thread_fsm_finished_p (struct thread_fsm *fsm);
extern enum async_reply_reason
thread_fsm_async_reply_reason (struct thread_fsm *fsm);
+/* Calls the FSM's should_notify_stop method. */
+extern int thread_fsm_should_notify_stop (struct thread_fsm *self);
+
#endif /* THREAD_FSM_H */
--
1.9.3
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 7/7] Delete enum inferior_event_handler::INF_TIMER
2015-08-12 17:02 [PATCH 0/7] Replace continuations with an extendable "class" Pedro Alves
` (3 preceding siblings ...)
2015-08-12 17:02 ` [PATCH 3/7] Convert infcalls " Pedro Alves
@ 2015-08-12 17:11 ` Pedro Alves
2015-08-18 11:22 ` Yao Qi
2015-08-12 17:11 ` [PATCH 5/7] Garbage collect dummy_frame_ctx_saver Pedro Alves
` (2 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2015-08-12 17:11 UTC (permalink / raw)
To: gdb-patches
Nothing ever uses this.
gdb/ChangeLog:
2015-08-12 Pedro Alves <palves@redhat.com>
* inf-loop.c (inferior_event_handler): Delete INF_TIMER case.
* target.h (enum inferior_event_type) <INF_TIMER>: Delete.
---
gdb/inf-loop.c | 1 -
gdb/target.h | 2 --
2 files changed, 3 deletions(-)
diff --git a/gdb/inf-loop.c b/gdb/inf-loop.c
index 8aecfe6..aac936f 100644
--- a/gdb/inf-loop.c
+++ b/gdb/inf-loop.c
@@ -80,7 +80,6 @@ inferior_event_handler (enum inferior_event_type event_type,
}
break;
- case INF_TIMER:
default:
printf_unfiltered (_("Event type not recognized.\n"));
break;
diff --git a/gdb/target.h b/gdb/target.h
index dc3a6d6..bc23f9e 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -122,8 +122,6 @@ enum inferior_event_type
/* Process a normal inferior event which will result in target_wait
being called. */
INF_REG_EVENT,
- /* We are called because a timer went off. */
- INF_TIMER,
/* We are called to do stuff after the inferior stops. */
INF_EXEC_COMPLETE,
};
--
1.9.3
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 5/7] Garbage collect dummy_frame_ctx_saver
2015-08-12 17:02 [PATCH 0/7] Replace continuations with an extendable "class" Pedro Alves
` (4 preceding siblings ...)
2015-08-12 17:11 ` [PATCH 7/7] Delete enum inferior_event_handler::INF_TIMER Pedro Alves
@ 2015-08-12 17:11 ` Pedro Alves
2015-08-12 17:11 ` [PATCH 6/7] Garbage collect thread continuations Pedro Alves
2015-08-18 12:52 ` [PATCH 0/7] Replace continuations with an extendable "class" Yao Qi
7 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-08-12 17:11 UTC (permalink / raw)
To: gdb-patches
Since the "finish" command and infcall's FSMs are now responsible for
saving the return value, the dummy_frame_ctx_saver is no longer needed
anywhere.
gdb/ChangeLog:
2015-08-12 Pedro Alves <palves@redhat.com>
* infcall.c (struct dummy_frame_context_saver): Delete.
(dummy_frame_context_saver_free, dummy_frame_context_saver_dtor)
(dummy_frame_context_saver_drop)
(dummy_frame_context_saver_cleanup)
(dummy_frame_context_saver_get_regs)
(dummy_frame_context_saver_setup): Delete.
* infcall.h (dummy_frame_context_saver_drop)
(dummy_frame_context_saver_cleanup)
(dummy_frame_context_saver_get_regs, dummy_frame_context_saver):
Delete.
(get_return_value): Remove 'ctx_saver' paremeter. Adjust.
* inferior.h (get_return_value): Remove 'ctx_saver' paremeter.
* python/py-finishbreakpoint.c (bpfinishpy_pre_stop_hook): Adjust.
---
gdb/infcall.c | 92 +---------------------------------------
gdb/infcall.h | 9 ----
gdb/infcmd.c | 19 +++------
gdb/inferior.h | 6 +--
gdb/python/py-finishbreakpoint.c | 5 +--
5 files changed, 10 insertions(+), 121 deletions(-)
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 5acdff0..e3df9f7 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -18,6 +18,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "defs.h"
+#include "infcall.h"
#include "breakpoint.h"
#include "tracepoint.h"
#include "target.h"
@@ -30,7 +31,6 @@
#include "objfiles.h"
#include "gdbcmd.h"
#include "command.h"
-#include "infcall.h"
#include "dummy-frame.h"
#include "ada-lang.h"
#include "gdbthread.h"
@@ -661,96 +661,6 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
return call_function_by_hand_dummy (function, nargs, args, NULL, NULL);
}
-/* Data for dummy_frame_context_saver. Structure can be freed only
- after both dummy_frame_context_saver_dtor and
- dummy_frame_context_saver_drop have been called for it. */
-
-struct dummy_frame_context_saver
-{
- /* Inferior registers fetched before associated dummy_frame got freed
- and before any other destructors of associated dummy_frame got called.
- It is initialized to NULL. */
- struct regcache *retbuf;
-
- /* It is 1 if this dummy_frame_context_saver_drop has been already
- called. */
- int drop_done;
-};
-
-/* Free struct dummy_frame_context_saver. */
-
-static void
-dummy_frame_context_saver_free (struct dummy_frame_context_saver *saver)
-{
- regcache_xfree (saver->retbuf);
- xfree (saver);
-}
-
-/* Destructor for associated dummy_frame. */
-
-static void
-dummy_frame_context_saver_dtor (void *data_voidp, int registers_valid)
-{
- struct dummy_frame_context_saver *data = data_voidp;
-
- gdb_assert (data->retbuf == NULL);
-
- if (data->drop_done)
- dummy_frame_context_saver_free (data);
- else if (registers_valid)
- data->retbuf = regcache_dup (get_current_regcache ());
-}
-
-/* Caller is no longer interested in this
- struct dummy_frame_context_saver. After its associated dummy_frame
- gets freed struct dummy_frame_context_saver can be also freed. */
-
-void
-dummy_frame_context_saver_drop (struct dummy_frame_context_saver *saver)
-{
- saver->drop_done = 1;
-
- if (!find_dummy_frame_dtor (dummy_frame_context_saver_dtor, saver))
- dummy_frame_context_saver_free (saver);
-}
-
-/* Stub dummy_frame_context_saver_drop compatible with make_cleanup. */
-
-void
-dummy_frame_context_saver_cleanup (void *data)
-{
- struct dummy_frame_context_saver *saver = data;
-
- dummy_frame_context_saver_drop (saver);
-}
-
-/* Fetch RETBUF field of possibly opaque DTOR_DATA.
- RETBUF must not be NULL. */
-
-struct regcache *
-dummy_frame_context_saver_get_regs (struct dummy_frame_context_saver *saver)
-{
- gdb_assert (saver->retbuf != NULL);
- return saver->retbuf;
-}
-
-/* Register provider of inferior registers at the time DUMMY_ID frame of
- PTID gets freed (before inferior registers get restored to those
- before dummy_frame). */
-
-struct dummy_frame_context_saver *
-dummy_frame_context_saver_setup (struct frame_id dummy_id, ptid_t ptid)
-{
- struct dummy_frame_context_saver *saver;
-
- saver = xmalloc (sizeof (*saver));
- saver->retbuf = NULL;
- saver->drop_done = 0;
- register_dummy_frame_dtor (dummy_id, inferior_ptid,
- dummy_frame_context_saver_dtor, saver);
- return saver;
-}
-
/* All this stuff with a dummy frame may seem unnecessarily complicated
(why not just save registers in GDB?). The purpose of pushing a dummy
frame which looks just like a real frame is so that if you call a
diff --git a/gdb/infcall.h b/gdb/infcall.h
index 43b5f66..77c5101 100644
--- a/gdb/infcall.h
+++ b/gdb/infcall.h
@@ -50,13 +50,4 @@ extern struct value *
dummy_frame_dtor_ftype *dummy_dtor,
void *dummy_dtor_data);
-struct dummy_frame_context_saver;
-extern void dummy_frame_context_saver_drop
- (struct dummy_frame_context_saver *data);
-extern void dummy_frame_context_saver_cleanup (void *data_voidp);
-extern struct regcache *dummy_frame_context_saver_get_regs
- (struct dummy_frame_context_saver *saver);
-extern struct dummy_frame_context_saver *dummy_frame_context_saver_setup
- (struct frame_id dummy_id, ptid_t ptid);
-
#endif
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index e5d01ff..87b28a5 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1588,22 +1588,15 @@ advance_command (char *arg, int from_tty)
right after an inferior call has finished. */
struct value *
-get_return_value (struct value *function, struct type *value_type,
- struct dummy_frame_context_saver *ctx_saver)
+get_return_value (struct value *function, struct type *value_type)
{
- struct regcache *stop_regs = NULL;
+ struct regcache *stop_regs;
struct gdbarch *gdbarch;
struct value *value;
- struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
+ struct cleanup *cleanup;
- /* If registers were not saved, use the current registers. */
- if (ctx_saver != NULL)
- stop_regs = dummy_frame_context_saver_get_regs (ctx_saver);
- else
- {
- stop_regs = regcache_dup (get_current_regcache ());
- make_cleanup_regcache_xfree (stop_regs);
- }
+ stop_regs = regcache_dup (get_current_regcache ());
+ cleanup = make_cleanup_regcache_xfree (stop_regs);
gdbarch = get_regcache_arch (stop_regs);
@@ -1803,7 +1796,7 @@ finish_command_fsm_should_stop (struct thread_fsm *self)
struct value *func;
func = read_var_value (f->function, get_current_frame ());
- rv->value = get_return_value (func, rv->type, NULL);
+ rv->value = get_return_value (func, rv->type);
rv->value_history_index = record_latest_value (rv->value);
}
}
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 48cba45..e09cb00 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -165,10 +165,8 @@ extern void detach_command (char *, int);
extern void notice_new_inferior (ptid_t, int, int);
-struct dummy_frame_context_saver;
-extern struct value *get_return_value
- (struct value *function, struct type *value_type,
- struct dummy_frame_context_saver *ctx_saver);
+extern struct value *get_return_value (struct value *function,
+ struct type *value_type);
/* Prepare for execution command. TARGET is the target that will run
the command. BACKGROUND determines whether this is a foreground
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index e543bb3..e1629b0 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -107,10 +107,7 @@ bpfinishpy_pre_stop_hook (struct gdbpy_breakpoint_object *bp_obj)
value_object_to_value (self_finishbp->function_value);
struct type *value_type =
type_object_to_type (self_finishbp->return_type);
-
- /* bpfinishpy_init cannot finish into DUMMY_FRAME (throws an error
- in such case) so it is OK to always pass CTX_SAVER as NULL. */
- struct value *ret = get_return_value (function, value_type, NULL);
+ struct value *ret = get_return_value (function, value_type);
if (ret)
{
--
1.9.3
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 6/7] Garbage collect thread continuations
2015-08-12 17:02 [PATCH 0/7] Replace continuations with an extendable "class" Pedro Alves
` (5 preceding siblings ...)
2015-08-12 17:11 ` [PATCH 5/7] Garbage collect dummy_frame_ctx_saver Pedro Alves
@ 2015-08-12 17:11 ` Pedro Alves
2015-08-18 12:52 ` [PATCH 0/7] Replace continuations with an extendable "class" Yao Qi
7 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-08-12 17:11 UTC (permalink / raw)
To: gdb-patches
Nothing uses thread continuations anymore.
(inferior continuations are still used by the attach command.)
gdb/ChangeLog:
2015-08-12 Pedro Alves <palves@redhat.com>
* continuations.c (add_continuation, restore_thread_cleanup)
(do_all_continuations_ptid, do_all_continuations_thread_callback)
(do_all_continuations_thread, do_all_continuations)
(discard_all_continuations_thread_callback)
(discard_all_continuations_thread, discard_all_continuations)
(add_intermediate_continuation)
(do_all_intermediate_continuations_thread_callback)
(do_all_intermediate_continuations_thread)
(do_all_intermediate_continuations)
(discard_all_intermediate_continuations_thread_callback)
(discard_all_intermediate_continuations_thread)
(discard_all_intermediate_continuations): Delete.
* continuations.h (add_continuation, do_all_continuations)
(do_all_continuations_thread, discard_all_continuations)
(discard_all_continuations_thread, add_intermediate_continuation)
(do_all_intermediate_continuations)
(do_all_intermediate_continuations_thread)
(discard_all_intermediate_continuations)
(discard_all_intermediate_continuations_thread): Delete
declarations.
* event-top.c (stdin_event_handler): Delete references to
continuations.
* gdbthread.h (struct thread_info): Delete continuations and
intermediate_continuations fields.
* inf-loop.c (inferior_event_handler): Remove references to
continuations.
* infrun.c (infrun_thread_stop_requested_callback): Remove
references to continuations.
* target.h (enum inferior_event_type) <INF_EXEC_CONTINUE>: Delete.
* thread.c: Don't include "continuations.h".
(clear_thread_inferior_resources): Remove references to
continuations.
---
gdb/continuations.c | 190 ----------------------------------------------------
gdb/continuations.h | 19 ------
gdb/event-top.c | 2 -
gdb/gdbthread.h | 12 ----
gdb/inf-loop.c | 67 +-----------------
gdb/infrun.c | 7 --
gdb/target.h | 5 --
gdb/thread.c | 4 --
8 files changed, 3 insertions(+), 303 deletions(-)
diff --git a/gdb/continuations.c b/gdb/continuations.c
index e753dc1..c6e14bc 100644
--- a/gdb/continuations.c
+++ b/gdb/continuations.c
@@ -132,193 +132,3 @@ discard_all_inferior_continuations (struct inferior *inf)
{
discard_my_continuations (&inf->continuations);
}
-
-/* Add a continuation to the continuation list of THREAD. The new
- continuation will be added at the front. */
-
-void
-add_continuation (struct thread_info *thread,
- continuation_ftype *hook, void *args,
- continuation_free_arg_ftype *free_arg)
-{
- make_continuation (&thread->continuations, hook, args, free_arg);
-}
-
-static void
-restore_thread_cleanup (void *arg)
-{
- ptid_t *ptid_p = arg;
-
- switch_to_thread (*ptid_p);
-}
-
-/* Walk down the continuation list of PTID, and execute all the
- continuations. There is a problem though. In some cases new
- continuations may be added while we are in the middle of this loop.
- If this happens they will be added in the front, and done before we
- have a chance of exhausting those that were already there. We need
- to then save the beginning of the list in a pointer and do the
- continuations from there on, instead of using the global beginning
- of list as our iteration pointer. */
-
-static void
-do_all_continuations_ptid (ptid_t ptid,
- struct continuation **continuations_p,
- int err)
-{
- struct cleanup *old_chain;
- ptid_t current_thread;
-
- if (*continuations_p == NULL)
- return;
-
- current_thread = inferior_ptid;
-
- /* Restore selected thread on exit. Don't try to restore the frame
- as well, because:
-
- - When running continuations, the selected frame is always #0.
-
- - The continuations may trigger symbol file loads, which may
- change the frame layout (frame ids change), which would trigger
- a warning if we used make_cleanup_restore_current_thread. */
-
- old_chain = make_cleanup (restore_thread_cleanup, ¤t_thread);
-
- /* Let the continuation see this thread as selected. */
- switch_to_thread (ptid);
-
- do_my_continuations (continuations_p, err);
-
- do_cleanups (old_chain);
-}
-
-/* Callback for iterate over threads. */
-
-static int
-do_all_continuations_thread_callback (struct thread_info *thread, void *data)
-{
- int err = * (int *) data;
- do_all_continuations_ptid (thread->ptid, &thread->continuations, err);
- return 0;
-}
-
-/* Do all continuations of thread THREAD. */
-
-void
-do_all_continuations_thread (struct thread_info *thread, int err)
-{
- do_all_continuations_thread_callback (thread, &err);
-}
-
-/* Do all continuations of all threads. */
-
-void
-do_all_continuations (int err)
-{
- iterate_over_threads (do_all_continuations_thread_callback, &err);
-}
-
-/* Callback for iterate over threads. */
-
-static int
-discard_all_continuations_thread_callback (struct thread_info *thread,
- void *data)
-{
- discard_my_continuations (&thread->continuations);
- return 0;
-}
-
-/* Get rid of all the continuations of THREAD. */
-
-void
-discard_all_continuations_thread (struct thread_info *thread)
-{
- discard_all_continuations_thread_callback (thread, NULL);
-}
-
-/* Get rid of all the continuations of all threads. */
-
-void
-discard_all_continuations (void)
-{
- iterate_over_threads (discard_all_continuations_thread_callback, NULL);
-}
-
-
-/* Add a continuation to the intermediate continuation list of THREAD.
- The new continuation will be added at the front. */
-
-void
-add_intermediate_continuation (struct thread_info *thread,
- continuation_ftype *hook,
- void *args,
- continuation_free_arg_ftype *free_arg)
-{
- make_continuation (&thread->intermediate_continuations, hook,
- args, free_arg);
-}
-
-/* Walk down the cmd_continuation list, and execute all the
- continuations. There is a problem though. In some cases new
- continuations may be added while we are in the middle of this
- loop. If this happens they will be added in the front, and done
- before we have a chance of exhausting those that were already
- there. We need to then save the beginning of the list in a pointer
- and do the continuations from there on, instead of using the
- global beginning of list as our iteration pointer. */
-
-static int
-do_all_intermediate_continuations_thread_callback (struct thread_info *thread,
- void *data)
-{
- int err = * (int *) data;
-
- do_all_continuations_ptid (thread->ptid,
- &thread->intermediate_continuations, err);
- return 0;
-}
-
-/* Do all intermediate continuations of thread THREAD. */
-
-void
-do_all_intermediate_continuations_thread (struct thread_info *thread, int err)
-{
- do_all_intermediate_continuations_thread_callback (thread, &err);
-}
-
-/* Do all intermediate continuations of all threads. */
-
-void
-do_all_intermediate_continuations (int err)
-{
- iterate_over_threads (do_all_intermediate_continuations_thread_callback,
- &err);
-}
-
-/* Callback for iterate over threads. */
-
-static int
-discard_all_intermediate_continuations_thread_callback (struct thread_info *thread,
- void *data)
-{
- discard_my_continuations (&thread->intermediate_continuations);
- return 0;
-}
-
-/* Get rid of all the intermediate continuations of THREAD. */
-
-void
-discard_all_intermediate_continuations_thread (struct thread_info *thread)
-{
- discard_all_intermediate_continuations_thread_callback (thread, NULL);
-}
-
-/* Get rid of all the intermediate continuations of all threads. */
-
-void
-discard_all_intermediate_continuations (void)
-{
- iterate_over_threads (discard_all_intermediate_continuations_thread_callback,
- NULL);
-}
diff --git a/gdb/continuations.h b/gdb/continuations.h
index c043a4e..b8c6e3e 100644
--- a/gdb/continuations.h
+++ b/gdb/continuations.h
@@ -20,7 +20,6 @@
#ifndef CONTINUATIONS_H
#define CONTINUATIONS_H
-struct thread_info;
struct inferior;
/* To continue the execution commands when running gdb asynchronously.
@@ -45,24 +44,6 @@ typedef void (continuation_ftype) (void *arg, int err);
continuation is called, or discarded. */
typedef void (continuation_free_arg_ftype) (void *);
-/* Thread specific continuations. */
-
-extern void add_continuation (struct thread_info *,
- continuation_ftype *, void *,
- continuation_free_arg_ftype *);
-extern void do_all_continuations (int err);
-extern void do_all_continuations_thread (struct thread_info *, int err);
-extern void discard_all_continuations (void);
-extern void discard_all_continuations_thread (struct thread_info *);
-
-extern void add_intermediate_continuation (struct thread_info *,
- continuation_ftype *, void *,
- continuation_free_arg_ftype *);
-extern void do_all_intermediate_continuations (int err);
-extern void do_all_intermediate_continuations_thread (struct thread_info *, int err);
-extern void discard_all_intermediate_continuations (void);
-extern void discard_all_intermediate_continuations_thread (struct thread_info *);
-
/* Inferior specific (any thread) continuations. */
extern void add_inferior_continuation (continuation_ftype *,
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 1762e3b..84650ca 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -419,8 +419,6 @@ stdin_event_handler (int error, gdb_client_data client_data)
{
printf_unfiltered (_("error detected on stdin\n"));
delete_file_handler (input_fd);
- discard_all_continuations ();
- discard_all_intermediate_continuations ();
/* If stdin died, we may as well kill gdb. */
quit_command ((char *) 0, stdin == instream);
}
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 0c99782..0f9734d 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -265,18 +265,6 @@ struct thread_info
when GDB gets back SIGTRAP from step_resume_breakpoint. */
int step_after_step_resume_breakpoint;
- /* Per-thread command support. */
-
- /* Pointer to what is left to do for an execution command after the
- target stops. Used only in asynchronous mode, by targets that
- support async execution. Several execution commands use it. */
- struct continuation *continuations;
-
- /* Similar to the above, but used when a single execution command
- requires several resume/stop iterations. Used by the step
- command. */
- struct continuation *intermediate_continuations;
-
/* 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. */
diff --git a/gdb/inf-loop.c b/gdb/inf-loop.c
index 7b1f724..8aecfe6 100644
--- a/gdb/inf-loop.c
+++ b/gdb/inf-loop.c
@@ -32,39 +32,16 @@
#include "top.h"
#include "observer.h"
-/* General function to handle events in the inferior. So far it just
- takes care of detecting errors reported by select() or poll(),
- otherwise it assumes that all is OK, and goes on reading data from
- the fd. This however may not always be what we want to do. */
+/* General function to handle events in the inferior. */
+
void
inferior_event_handler (enum inferior_event_type event_type,
gdb_client_data client_data)
{
- struct cleanup *cleanup_if_error = make_bpstat_clear_actions_cleanup ();
-
switch (event_type)
{
case INF_REG_EVENT:
- /* Catch errors for now, until the inner layers of
- fetch_inferior_event (i.e. readchar) can return meaningful
- error status. If an error occurs while getting an event from
- the target, just cancel the current command. */
- {
-
- TRY
- {
- fetch_inferior_event (client_data);
- }
- CATCH (ex, RETURN_MASK_ALL)
- {
- bpstat_clear_actions ();
- do_all_intermediate_continuations (1);
- do_all_continuations (1);
-
- throw_exception (ex);
- }
- END_CATCH
- }
+ fetch_inferior_event (client_data);
break;
case INF_EXEC_COMPLETE:
@@ -82,36 +59,10 @@ inferior_event_handler (enum inferior_event_type event_type,
if (!ptid_equal (inferior_ptid, null_ptid))
do_all_inferior_continuations (0);
- /* If we were doing a multi-step (eg: step n, next n), but it
- got interrupted by a breakpoint, still do the pending
- continuations. The continuation itself is responsible for
- distinguishing the cases. The continuations are allowed to
- touch the inferior memory, e.g. to remove breakpoints, so run
- them before running breakpoint commands, which may resume the
- target. */
- if (non_stop
- && target_has_execution
- && !ptid_equal (inferior_ptid, null_ptid))
- do_all_intermediate_continuations_thread (inferior_thread (), 0);
- else
- do_all_intermediate_continuations (0);
-
- /* Always finish the previous command before running any
- breakpoint commands. Any stop cancels the previous command.
- E.g. a "finish" or "step-n" command interrupted by an
- unrelated breakpoint is canceled. */
- if (non_stop
- && target_has_execution
- && !ptid_equal (inferior_ptid, null_ptid))
- do_all_continuations_thread (inferior_thread (), 0);
- else
- do_all_continuations (0);
-
/* When running a command list (from a user command, say), these
are only run when the command list is all done. */
if (interpreter_async)
{
-
check_frame_language_change ();
/* Don't propagate breakpoint commands errors. Either we're
@@ -129,21 +80,9 @@ inferior_event_handler (enum inferior_event_type event_type,
}
break;
- case INF_EXEC_CONTINUE:
- /* Is there anything left to do for the command issued to
- complete? */
-
- if (non_stop)
- do_all_intermediate_continuations_thread (inferior_thread (), 0);
- else
- do_all_intermediate_continuations (0);
- break;
-
case INF_TIMER:
default:
printf_unfiltered (_("Event type not recognized.\n"));
break;
}
-
- discard_cleanups (cleanup_if_error);
}
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 56cd267..6206c8a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3228,17 +3228,10 @@ infrun_thread_stop_requested_callback (struct thread_info *info, void *arg)
if (!ecs->wait_some_more)
{
- struct thread_info *tp;
-
/* Cancel any running execution command. */
thread_cancel_execution_command (info);
normal_stop ();
-
- /* Finish off the continuations. */
- tp = inferior_thread ();
- do_all_intermediate_continuations_thread (tp, 1);
- do_all_continuations_thread (tp, 1);
}
do_cleanups (old_chain);
diff --git a/gdb/target.h b/gdb/target.h
index 48e6857..dc3a6d6 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -126,11 +126,6 @@ enum inferior_event_type
INF_TIMER,
/* We are called to do stuff after the inferior stops. */
INF_EXEC_COMPLETE,
- /* We are called to do some stuff after the inferior stops, but we
- are expected to reenter the proceed() and
- handle_inferior_event() functions. This is used only in case of
- 'step n' like commands. */
- INF_EXEC_CONTINUE
};
\f
/* Target objects which can be transfered using target_read,
diff --git a/gdb/thread.c b/gdb/thread.c
index 87a5950..2fd90b5 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -42,7 +42,6 @@
#include "cli/cli-decode.h"
#include "gdb_regex.h"
#include "cli/cli-utils.h"
-#include "continuations.h"
#include "thread-fsm.h"
/* Definition of struct thread_info exported to gdbthread.h. */
@@ -190,9 +189,6 @@ clear_thread_inferior_resources (struct thread_info *tp)
btrace_teardown (tp);
thread_cancel_execution_command (tp);
-
- do_all_intermediate_continuations_thread (tp, 1);
- do_all_continuations_thread (tp, 1);
}
static void
--
1.9.3
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 0/7] Replace continuations with an extendable "class"
2015-08-12 17:02 [PATCH 0/7] Replace continuations with an extendable "class" Pedro Alves
` (6 preceding siblings ...)
2015-08-12 17:11 ` [PATCH 6/7] Garbage collect thread continuations Pedro Alves
@ 2015-08-18 12:52 ` Yao Qi
2015-08-19 14:56 ` Pedro Alves
2015-09-09 17:33 ` Pedro Alves
7 siblings, 2 replies; 26+ messages in thread
From: Yao Qi @ 2015-08-18 12:52 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> Mainly, this series replaces the continuations mechanism with a
> "class" that manages a thread's execution-command-specific state.
> This fixes bugs, and cleans up core run control and interpreter code
> in the process, as well as clearing the path for further
> fixes/cleanups.
Hi Pedro,
I go through this series, and sent my comments to some patches.
Overall, they look good to me.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 0/7] Replace continuations with an extendable "class"
2015-08-18 12:52 ` [PATCH 0/7] Replace continuations with an extendable "class" Yao Qi
@ 2015-08-19 14:56 ` Pedro Alves
2015-09-09 17:33 ` Pedro Alves
1 sibling, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-08-19 14:56 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 08/18/2015 01:52 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> Mainly, this series replaces the continuations mechanism with a
>> "class" that manages a thread's execution-command-specific state.
>> This fixes bugs, and cleans up core run control and interpreter code
>> in the process, as well as clearing the path for further
>> fixes/cleanups.
>
> Hi Pedro,
> I go through this series, and sent my comments to some patches.
> Overall, they look good to me.
Thanks much for reviewing Yao. I'll push this in if you're happy
with the responses I sent.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/7] Replace continuations with an extendable "class"
2015-08-18 12:52 ` [PATCH 0/7] Replace continuations with an extendable "class" Yao Qi
2015-08-19 14:56 ` Pedro Alves
@ 2015-09-09 17:33 ` Pedro Alves
1 sibling, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-09-09 17:33 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 08/18/2015 01:52 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> Mainly, this series replaces the continuations mechanism with a
>> "class" that manages a thread's execution-command-specific state.
>> This fixes bugs, and cleans up core run control and interpreter code
>> in the process, as well as clearing the path for further
>> fixes/cleanups.
>
> Hi Pedro,
> I go through this series, and sent my comments to some patches.
> Overall, they look good to me.
Thanks for the review Yao. I pushed this in now.
--
Pedro Alves
^ permalink raw reply [flat|nested] 26+ messages in thread