* [PATCH 01/30] Don't rely on immediate_quit in command_line_input
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
@ 2016-03-18 19:18 ` Pedro Alves
2016-03-18 19:18 ` [PATCH 18/30] Fix inconsistent handling of EINTR in ser-*.c backends Pedro Alves
` (29 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:18 UTC (permalink / raw)
To: gdb-patches
AFAICS, immediate_quit was only needed here nowdays to be able to
interrupt gdb_readline_no_editing.
command_line_input can also take the gdb_readline_wrapper path, but
since that is built on top of the event loop (gdb_select / poll and
asynchronous signal handlers), it can be interrupted.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* top.c: Include "gdb_select.h".
(gdb_readline_no_editing): Wait for input with gdb_select instead
of blocking in fgetc.
(command_line_input): Don't set immediate_quit.
---
gdb/top.c | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/gdb/top.c b/gdb/top.c
index 89fe832..90a3f48 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -51,6 +51,7 @@
#include "filenames.h"
#include "frame.h"
#include "buffer.h"
+#include "gdb_select.h"
/* readline include files. */
#include "readline/readline.h"
@@ -592,6 +593,10 @@ static char *
gdb_readline_no_editing (const char *prompt)
{
struct buffer line_buffer;
+ /* Read from stdin if we are executing a user defined command. This
+ is the right thing for prompt_for_continue, at least. */
+ FILE *stream = instream != NULL ? instream : stdin;
+ int fd = fileno (stream);
buffer_init (&line_buffer);
@@ -607,10 +612,26 @@ gdb_readline_no_editing (const char *prompt)
while (1)
{
int c;
+ int numfds;
+ fd_set readfds;
- /* Read from stdin if we are executing a user defined command.
- This is the right thing for prompt_for_continue, at least. */
- c = fgetc (instream ? instream : stdin);
+ QUIT;
+
+ /* Wait until at least one byte of data is available. Control-C
+ can interrupt gdb_select, but not fgetc. */
+ FD_ZERO (&readfds);
+ FD_SET (fd, &readfds);
+ if (gdb_select (fd + 1, &readfds, NULL, NULL, NULL) == -1)
+ {
+ if (errno == EINTR)
+ {
+ /* If this was ctrl-c, the QUIT above handles it. */
+ continue;
+ }
+ perror_with_name (("select"));
+ }
+
+ c = fgetc (stream);
if (c == EOF)
{
@@ -1048,10 +1069,6 @@ command_line_input (const char *prompt_arg, int repeat, char *annotation_suffix)
/* Starting a new command line. */
cmd_line_buffer.used_size = 0;
- /* Control-C quits instantly if typed while in this loop
- since it should not wait until the user types a newline. */
- immediate_quit++;
- QUIT;
#ifdef STOP_SIGNAL
if (job_control)
signal (STOP_SIGNAL, handle_stop_sig);
@@ -1109,7 +1126,6 @@ command_line_input (const char *prompt_arg, int repeat, char *annotation_suffix)
if (job_control)
signal (STOP_SIGNAL, SIG_DFL);
#endif
- immediate_quit--;
return cmd;
}
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 18/30] Fix inconsistent handling of EINTR in ser-*.c backends
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
2016-03-18 19:18 ` [PATCH 01/30] Don't rely on immediate_quit in command_line_input Pedro Alves
@ 2016-03-18 19:18 ` Pedro Alves
2016-03-18 19:19 ` [PATCH 28/30] target remote: Don't rely on immediate_quit (introduce quit handlers) Pedro Alves
` (28 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:18 UTC (permalink / raw)
To: gdb-patches
- If serial->write_prim returns EINTR, ser_bas_write returns it to the
caller. This just looks wrong to me -- part of the output may have
already been sent, and there's no way for the caller to know that,
and thus no way for a caller to handle a partial write correctly.
- While ser-unix.c:ser_unix_read_prim retries on EINTR,
ser-tcp.c:net_read_prim does not.
This commit moves EINTR handling to the ser_base_write and
ser_base_readchar level, so all serial backends (at least those that
use it) end up handling EINTR consistently.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* ser-base.c (fd_event): Retry read_prim on EINTR.
(do_ser_base_readchar): Retry read_prim on EINTR.
(ser_base_write): Retry write_prim on EINTR.
* ser-unix.c (ser_unix_read_prim): Don't retry on EINTR here.
(ser_unix_write_prim): Remove comment.
---
gdb/ser-base.c | 20 +++++++++++++++++---
gdb/ser-unix.c | 12 +-----------
2 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/gdb/ser-base.c b/gdb/ser-base.c
index 21d52cd..25af66a 100644
--- a/gdb/ser-base.c
+++ b/gdb/ser-base.c
@@ -164,7 +164,13 @@ fd_event (int error, void *context)
pull characters out of the buffer. See also
generic_readchar(). */
int nr;
- nr = scb->ops->read_prim (scb, BUFSIZ);
+
+ do
+ {
+ nr = scb->ops->read_prim (scb, BUFSIZ);
+ }
+ while (nr < 0 && errno == EINTR);
+
if (nr == 0)
{
scb->bufcnt = SERIAL_EOF;
@@ -358,7 +364,11 @@ do_ser_base_readchar (struct serial *scb, int timeout)
if (status < 0)
return status;
- status = scb->ops->read_prim (scb, BUFSIZ);
+ do
+ {
+ status = scb->ops->read_prim (scb, BUFSIZ);
+ }
+ while (status < 0 && errno == EINTR);
if (status <= 0)
{
@@ -448,7 +458,11 @@ ser_base_write (struct serial *scb, const void *buf, size_t count)
cc = scb->ops->write_prim (scb, str, count);
if (cc < 0)
- return 1;
+ {
+ if (errno == EINTR)
+ continue;
+ return 1;
+ }
count -= cc;
str += cc;
}
diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c
index c54b2e1..562e98b 100644
--- a/gdb/ser-unix.c
+++ b/gdb/ser-unix.c
@@ -1002,21 +1002,11 @@ when debugging using remote targets."),
int
ser_unix_read_prim (struct serial *scb, size_t count)
{
- int status;
-
- while (1)
- {
- status = read (scb->fd, scb->buf, count);
- if (status != -1 || errno != EINTR)
- break;
- }
- return status;
+ return read (scb->fd, scb->buf, count);
}
int
ser_unix_write_prim (struct serial *scb, const void *buf, size_t len)
{
- /* ??? Historically, GDB has not retried calls to "write" that
- result in EINTR. */
return write (scb->fd, buf, len);
}
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 28/30] target remote: Don't rely on immediate_quit (introduce quit handlers)
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
2016-03-18 19:18 ` [PATCH 01/30] Don't rely on immediate_quit in command_line_input Pedro Alves
2016-03-18 19:18 ` [PATCH 18/30] Fix inconsistent handling of EINTR in ser-*.c backends Pedro Alves
@ 2016-03-18 19:19 ` Pedro Alves
2016-03-18 19:19 ` [PATCH 20/30] Use target_terminal_ours_for_output in cp-support.c Pedro Alves
` (27 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:19 UTC (permalink / raw)
To: gdb-patches
remote.c is the last user of immediate_quit. It's relied on to
immediately break the initial remote connection sync up, if the user
does Ctrl-C, assuming that was because the target isn't responding.
At that stage, since the connection isn't synced yet, disconnecting is
the only safe thing to do. This commit reworks that, to not rely on
throwing from the SIGINT signal handler.
So, this commit:
- Introduces the concept of a "quit handler". This is used to
override what does the QUIT macro do when the quit flag is set.
- Makes the "struct serial" reachar / write code call QUIT in the
partial read/write loops, so the current quit handler is invoked
whenever a serial->read_prim / serial->write_prim returns EINTR.
- Makes the "struct serial" reachar / write code call
interruptible_select instead of gdb_select, so that QUITs are
detected in a race-free manner.
- Stops remote.c from setting immediate_quit during the initial
connection.
- Instead, we install a custom quit handler whenever we're calling
into the serial code. This custom quit handler knows to immediately
throw a quit when we're in the initial connection setup, and
otherwise defer handling the quit/Ctrl-C request to later, when
we're safely out of a packet command/response sequence. This also
is what is now responsible for handling "double Ctrl-C because
target connection is stuck/wedged."
- remote.c no longer installs a specialized SIGINT handlers, and
instead re-uses the quit flag. Since we want to rely on the QUIT
macro, the SIGINT handler must also set the quit. And the easiest
is just to not install custom SIGINT handler in remote.c. Let the
standard SIGINT handler do its job of setting the quit flag.
Centralizing SIGINT handlers seems like a good thing to me, anyway.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* defs.h (quit_handler_ftype, quit_handler)
(make_cleanup_override_quit_handler, default_quit_handler): New.
(QUIT): Adjust comments.
* event-top.c (default_quit_handler): New function.
(quit_handler): New global.
(struct quit_handler_cleanup_data): New.
(restore_quit_handler, restore_quit_handler_dtor)
(make_cleanup_override_quit_handler): New.
(async_request_quit): Call QUIT.
* remote.c (struct remote_state) <got_ctrlc_during_io>: New field.
(async_sigint_remote_twice_token, async_sigint_remote_token):
Delete.
(remote_close): Update comments.
(remote_start_remote): Don't set immediate_quit. Set starting_up
earlier.
(remote_serial_quit_handler, remote_unpush_and_throw): New
functions.
(remote_open_1): Clear got_ctrlc_during_io.
(async_initialize_sigint_signal_handler)
(async_handle_remote_sigint, async_handle_remote_sigint_twice)
(remote_check_pending_interrupt, async_remote_interrupt)
(async_remote_interrupt_twice)
(async_cleanup_sigint_signal_handler, ofunc)
(sync_remote_interrupt, sync_remote_interrupt_twice): Delete.
(remote_terminal_inferior, remote_terminal_ours): Remove async
checks.
(remote_wait_as): Don't install a SIGINT handler in sync mode.
(readchar, remote_serial_write): Override the quit handler with
remote_serial_quit_handler.
(getpkt_or_notif_sane_1): Don't call QUIT.
(initialize_remote_ops): Don't install
remote_check_pending_interrupt.
(_initialize_remote): Don't create async_sigint_remote_token and
async_sigint_remote_twice_token.
* ser-base.c (ser_base_wait_for): Call QUIT and use
interruptible_select.
(ser_base_write): Call QUIT.
* ser-go32.c (dos_readchar, dos_write): Call QUIT.
* ser-unix.c (wait_for): Don't use VTIME. Always take the
gdb_select path, but call QUIT and interruptible_select.
* utils.c (maybe_quit): Call the current quit handler. Don't call
target_check_pending_interrupt.
(defaulted_query, prompt_for_continue): Override the quit handler
with the default quit handler.
---
gdb/defs.h | 34 +++++++-
gdb/event-top.c | 66 +++++++++++++-
gdb/remote.c | 263 ++++++++++++++++++++++++--------------------------------
gdb/ser-base.c | 10 ++-
gdb/ser-go32.c | 4 +
gdb/ser-unix.c | 96 +++------------------
gdb/utils.c | 8 +-
7 files changed, 232 insertions(+), 249 deletions(-)
diff --git a/gdb/defs.h b/gdb/defs.h
index 006f660..83e4e11 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -145,6 +145,34 @@ extern int check_quit_flag (void);
/* * Set the quit flag. */
extern void set_quit_flag (void);
+/* The current quit handler (and its type). This is called from the
+ QUIT macro. See default_quit_handler below for default behavior.
+ Parts of GDB temporarily override this to e.g., completely suppress
+ Ctrl-C because it would not be safe to throw. E.g., normally, you
+ wouldn't want to quit between a RSP command and its response, as
+ that would break the communication with the target, but you may
+ still want to intercept the Ctrl-C and offer to disconnect if the
+ user presses Ctrl-C multiple times while the target is stuck
+ waiting for the wedged remote stub. */
+typedef void (quit_handler_ftype) (void);
+extern quit_handler_ftype *quit_handler;
+
+/* Override the current quit handler. Sets NEW_QUIT_HANDLER as
+ current quit handler, and installs a cleanup that when run restores
+ the previous quit handler. */
+struct cleanup *
+ make_cleanup_override_quit_handler (quit_handler_ftype *new_quit_handler);
+
+/* The default quit handler. Checks whether Ctrl-C was pressed, and
+ if so:
+
+ - If GDB owns the terminal, throws a quit exception.
+
+ - If GDB does not own the terminal, forwards the Ctrl-C to the
+ target.
+*/
+extern void default_quit_handler (void);
+
/* Flag that function quit should call quit_force. */
extern volatile int sync_quit_force_run;
@@ -156,10 +184,8 @@ extern void quit (void);
extern void maybe_quit (void);
-/* Check whether a Ctrl-C was typed, and if so, call quit. The target
- is given a chance to process the Ctrl-C. E.g., it may detect that
- repeated Ctrl-C requests were issued, and choose to close the
- connection. */
+/* Check whether a Ctrl-C was typed, and if so, call the current quit
+ handler. */
#define QUIT maybe_quit ()
/* Set the serial event associated with the quit flag. */
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 7fedb48..386920e 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -827,6 +827,68 @@ quit_serial_event_fd (void)
return serial_event_fd (quit_serial_event);
}
+/* See defs.h. */
+
+void
+default_quit_handler (void)
+{
+ if (check_quit_flag ())
+ {
+ if (target_terminal_is_ours ())
+ quit ();
+ else
+ target_pass_ctrlc ();
+ }
+}
+
+/* See defs.h. */
+quit_handler_ftype *quit_handler = default_quit_handler;
+
+/* Data for make_cleanup_override_quit_handler. Wrap the previous
+ handler pointer in a data struct because it's not portable to cast
+ a function pointer to a data pointer, which is what make_cleanup
+ expects. */
+struct quit_handler_cleanup_data
+{
+ /* The previous quit handler. */
+ quit_handler_ftype *prev_handler;
+};
+
+/* Cleanup call that restores the previous quit handler. */
+
+static void
+restore_quit_handler (void *arg)
+{
+ struct quit_handler_cleanup_data *data
+ = (struct quit_handler_cleanup_data *) arg;
+
+ quit_handler = data->prev_handler;
+}
+
+/* Destructor for the quit handler cleanup. */
+
+static void
+restore_quit_handler_dtor (void *arg)
+{
+ xfree (arg);
+}
+
+/* See defs.h. */
+
+struct cleanup *
+make_cleanup_override_quit_handler (quit_handler_ftype *new_quit_handler)
+{
+ struct cleanup *old_chain;
+ struct quit_handler_cleanup_data *data;
+
+ data = XNEW (struct quit_handler_cleanup_data);
+ data->prev_handler = quit_handler;
+ old_chain = make_cleanup_dtor (restore_quit_handler, data,
+ restore_quit_handler_dtor);
+ quit_handler = new_quit_handler;
+ return old_chain;
+}
+
/* Handle a SIGINT. */
void
@@ -920,9 +982,7 @@ async_request_quit (gdb_client_data arg)
back here, that means that an exception was thrown to unwind the
current command before we got back to the event loop. So there
is no reason to call quit again here. */
-
- if (check_quit_flag ())
- quit ();
+ QUIT;
}
#ifdef SIGQUIT
diff --git a/gdb/remote.c b/gdb/remote.c
index 2aa4e07..2607c9d 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -97,14 +97,10 @@ static char *remote_exec_file_var;
enum { REMOTE_ALIGN_WRITES = 16 };
/* Prototypes for local functions. */
-static void async_cleanup_sigint_signal_handler (void *dummy);
static int getpkt_sane (char **buf, long *sizeof_buf, int forever);
static int getpkt_or_notif_sane (char **buf, long *sizeof_buf,
int forever, int *is_notif);
-static void async_handle_remote_sigint (int);
-static void async_handle_remote_sigint_twice (int);
-
static void remote_files_info (struct target_ops *ignore);
static void remote_prepare_to_store (struct target_ops *self,
@@ -141,8 +137,6 @@ static void remote_async (struct target_ops *ops, int enable);
static void remote_thread_events (struct target_ops *ops, int enable);
-static void sync_remote_interrupt_twice (int signo);
-
static void interrupt_query (void);
static void set_general_thread (struct ptid ptid);
@@ -241,6 +235,8 @@ static int stop_reply_queue_length (void);
static void readahead_cache_invalidate (void);
+static void remote_unpush_and_throw (void);
+
/* For "remote". */
static struct cmd_list_element *remote_cmdlist;
@@ -363,6 +359,14 @@ struct remote_state
responded to that. */
int ctrlc_pending_p;
+ /* True if we saw a Ctrl-C while reading or writing from/to the
+ remote descriptor. At that point it is not safe to send a remote
+ interrupt packet, so we instead remember we saw the Ctrl-C and
+ process it once we're done with sending/receiving the current
+ packet, which should be shortly. If however that takes too long,
+ and the user presses Ctrl-C again, we offer to disconnect. */
+ int got_ctrlc_during_io;
+
/* Descriptor for I/O to remote machine. Initialize it to NULL so that
remote_open knows that we don't have a file open when the program
starts. */
@@ -1689,10 +1693,6 @@ remote_remove_exec_catchpoint (struct target_ops *ops, int pid)
return 0;
}
-/* Tokens for use by the asynchronous signal handlers for SIGINT. */
-static struct async_signal_handler *async_sigint_remote_twice_token;
-static struct async_signal_handler *async_sigint_remote_token;
-
\f
/* Asynchronous signal handle registered as event loop source for
when we have pending events ready to be passed to the core. */
@@ -3497,8 +3497,7 @@ remote_close (struct target_ops *self)
if (rs->remote_desc == NULL)
return; /* already closed */
- /* Make sure we leave stdin registered in the event loop, and we
- don't leave the async SIGINT signal handler installed. */
+ /* Make sure we leave stdin registered in the event loop. */
remote_terminal_ours (self);
serial_close (rs->remote_desc);
@@ -3994,6 +3993,8 @@ process_initial_stop_replies (int from_tty)
set_last_target_status (inferior_ptid, thread->suspend.waitstatus);
}
+/* Start the remote connection and sync state. */
+
static void
remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
{
@@ -4001,18 +4002,21 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
struct packet_config *noack_config;
char *wait_status = NULL;
- immediate_quit++; /* Allow user to interrupt it. */
+ /* Signal other parts that we're going through the initial setup,
+ and so things may not be stable yet. E.g., we don't try to
+ install tracepoints until we've relocated symbols. Also, a
+ Ctrl-C before we're connected and synced up can't interrupt the
+ target. Instead, it offers to drop the (potentially wedged)
+ connection. */
+ rs->starting_up = 1;
+
QUIT;
if (interrupt_on_connect)
send_interrupt_sequence ();
/* Ack any packet which the remote side has already sent. */
- serial_write (rs->remote_desc, "+", 1);
-
- /* Signal other parts that we're going through the initial setup,
- and so things may not be stable yet. */
- rs->starting_up = 1;
+ remote_serial_write ("+", 1);
/* The first packet we send to the target is the optional "supported
packets" request. If the target can answer this, it will tell us
@@ -4197,7 +4201,6 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
strcpy (rs->buf, wait_status);
rs->cached_wait_status = 1;
- immediate_quit--;
start_remote (from_tty); /* Initialize gdb process mechanisms. */
}
else
@@ -4831,6 +4834,58 @@ remote_query_supported (void)
}
}
+/* Serial QUIT handler for the remote serial descriptor.
+
+ Defers handling a Ctrl-C until we're done with the current
+ command/response packet sequence, unless:
+
+ - We're setting up the connection. Don't send a remote interrupt
+ request, as we're not fully synced yet. Quit immediately
+ instead.
+
+ - The target has been resumed in the foreground
+ (target_terminal_is_ours is false) with a synchronous resume
+ packet, and we're blocked waiting for the stop reply, thus a
+ Ctrl-C should be immediately sent to the target.
+
+ - We get a second Ctrl-C while still within the same serial read or
+ write. In that case the serial is seemingly wedged --- offer to
+ quit/disconnect.
+
+ - We see a second Ctrl-C without target response, after having
+ previously interrupted the target. In that case the target/stub
+ is probably wedged --- offer to quit/disconnect.
+*/
+
+static void
+remote_serial_quit_handler (void)
+{
+ struct remote_state *rs = get_remote_state ();
+
+ if (check_quit_flag ())
+ {
+ /* If we're starting up, we're not fully synced yet. Quit
+ immediately. */
+ if (rs->starting_up)
+ quit ();
+ else if (rs->got_ctrlc_during_io)
+ {
+ if (query (_("The target is not responding to GDB commands.\n"
+ "Stop debugging it? ")))
+ remote_unpush_and_throw ();
+ }
+ /* If ^C has already been sent once, offer to disconnect. */
+ else if (!target_terminal_is_ours () && rs->ctrlc_pending_p)
+ interrupt_query ();
+ /* All-stop protocol, and blocked waiting for stop reply. Send
+ an interrupt request. */
+ else if (!target_terminal_is_ours () && rs->waiting_for_stop_reply)
+ target_interrupt (inferior_ptid);
+ else
+ rs->got_ctrlc_during_io = 1;
+ }
+}
+
/* Remove any of the remote.c targets from target stack. Upper targets depend
on it so remove them first. */
@@ -4841,6 +4896,13 @@ remote_unpush_target (void)
}
static void
+remote_unpush_and_throw (void)
+{
+ remote_unpush_target ();
+ throw_error (TARGET_CLOSE_ERROR, _("Disconnected from target."));
+}
+
+static void
remote_open_1 (const char *name, int from_tty,
struct target_ops *target, int extended_p)
{
@@ -4929,6 +4991,7 @@ remote_open_1 (const char *name, int from_tty,
rs->extended = extended_p;
rs->waiting_for_stop_reply = 0;
rs->ctrlc_pending_p = 0;
+ rs->got_ctrlc_during_io = 0;
rs->general_thread = not_sent_ptid;
rs->continue_thread = not_sent_ptid;
@@ -5635,108 +5698,6 @@ remote_resume (struct target_ops *ops,
}
\f
-/* Set up the signal handler for SIGINT, while the target is
- executing, ovewriting the 'regular' SIGINT signal handler. */
-static void
-async_initialize_sigint_signal_handler (void)
-{
- signal (SIGINT, async_handle_remote_sigint);
-}
-
-/* Signal handler for SIGINT, while the target is executing. */
-static void
-async_handle_remote_sigint (int sig)
-{
- signal (sig, async_handle_remote_sigint_twice);
- /* Note we need to go through gdb_call_async_signal_handler in order
- to wake up the event loop on Windows. */
- gdb_call_async_signal_handler (async_sigint_remote_token, 0);
-}
-
-/* Signal handler for SIGINT, installed after SIGINT has already been
- sent once. It will take effect the second time that the user sends
- a ^C. */
-static void
-async_handle_remote_sigint_twice (int sig)
-{
- signal (sig, async_handle_remote_sigint);
- /* See note in async_handle_remote_sigint. */
- gdb_call_async_signal_handler (async_sigint_remote_twice_token, 0);
-}
-
-/* Implementation of to_check_pending_interrupt. */
-
-static void
-remote_check_pending_interrupt (struct target_ops *self)
-{
- struct async_signal_handler *token = async_sigint_remote_twice_token;
-
- if (async_signal_handler_is_marked (token))
- {
- clear_async_signal_handler (token);
- call_async_signal_handler (token);
- }
-}
-
-/* Perform the real interruption of the target execution, in response
- to a ^C. */
-static void
-async_remote_interrupt (gdb_client_data arg)
-{
- if (remote_debug)
- fprintf_unfiltered (gdb_stdlog, "async_remote_interrupt called\n");
-
- target_interrupt (inferior_ptid);
-}
-
-/* Perform interrupt, if the first attempt did not succeed. Just give
- up on the target alltogether. */
-static void
-async_remote_interrupt_twice (gdb_client_data arg)
-{
- if (remote_debug)
- fprintf_unfiltered (gdb_stdlog, "async_remote_interrupt_twice called\n");
-
- interrupt_query ();
-}
-
-/* Reinstall the usual SIGINT handlers, after the target has
- stopped. */
-static void
-async_cleanup_sigint_signal_handler (void *dummy)
-{
- signal (SIGINT, handle_sigint);
-}
-
-/* Send ^C to target to halt it. Target will respond, and send us a
- packet. */
-static void (*ofunc) (int);
-
-/* The command line interface's interrupt routine. This function is installed
- as a signal handler for SIGINT. The first time a user requests an
- interrupt, we call remote_interrupt to send a break or ^C. If there is no
- response from the target (it didn't stop when the user requested it),
- we ask the user if he'd like to detach from the target. */
-
-static void
-sync_remote_interrupt (int signo)
-{
- /* If this doesn't work, try more severe steps. */
- signal (signo, sync_remote_interrupt_twice);
-
- gdb_call_async_signal_handler (async_sigint_remote_token, 1);
-}
-
-/* The user typed ^C twice. */
-
-static void
-sync_remote_interrupt_twice (int signo)
-{
- signal (signo, ofunc);
- gdb_call_async_signal_handler (async_sigint_remote_twice_token, 1);
- signal (signo, sync_remote_interrupt);
-}
-
/* Non-stop version of target_stop. Uses `vCont;t' to stop a remote
thread, all threads of a remote process, or all threads of all
processes. */
@@ -5926,10 +5887,6 @@ interrupt_query (void)
static void
remote_terminal_inferior (struct target_ops *self)
{
- if (!target_async_permitted)
- /* Nothing to do. */
- return;
-
/* FIXME: cagney/1999-09-27: Make calls to target_terminal_*()
idempotent. The event-loop GDB talking to an asynchronous target
with a synchronous command calls this function from both
@@ -5940,7 +5897,6 @@ remote_terminal_inferior (struct target_ops *self)
return;
delete_file_handler (input_fd);
remote_async_terminal_ours_p = 0;
- async_initialize_sigint_signal_handler ();
/* NOTE: At this point we could also register our selves as the
recipient of all input. Any characters typed could then be
passed on down to the target. */
@@ -5949,14 +5905,9 @@ remote_terminal_inferior (struct target_ops *self)
static void
remote_terminal_ours (struct target_ops *self)
{
- if (!target_async_permitted)
- /* Nothing to do. */
- return;
-
/* See FIXME in remote_terminal_inferior. */
if (remote_async_terminal_ours_p)
return;
- async_cleanup_sigint_signal_handler (NULL);
add_file_handler (input_fd, stdin_event_handler, 0);
remote_async_terminal_ours_p = 1;
}
@@ -6927,15 +6878,6 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
return minus_one_ptid;
}
- if (!target_is_async_p ())
- {
- ofunc = signal (SIGINT, sync_remote_interrupt);
- /* If the user hit C-c before this packet, or between packets,
- pretend that it was hit right here. */
- if (check_quit_flag ())
- sync_remote_interrupt (SIGINT);
- }
-
/* FIXME: cagney/1999-09-27: If we're in async mode we should
_never_ wait for ever -> test on target_is_async_p().
However, before we do that we need to ensure that the caller
@@ -6943,9 +6885,6 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
ret = getpkt_or_notif_sane (&rs->buf, &rs->buf_size,
forever, &is_notif);
- if (!target_is_async_p ())
- signal (SIGINT, ofunc);
-
/* GDB gets a notification. Return to core as this event is
not interesting. */
if (ret != -1 && is_notif)
@@ -8163,16 +8102,29 @@ unpush_and_perror (const char *string)
safe_strerror (saved_errno));
}
-/* Read a single character from the remote end. */
+/* Read a single character from the remote end. The current quit
+ handler is overridden to avoid quitting in the middle of packet
+ sequence, as that would break communication with the remote server.
+ See remote_serial_quit_handler for more detail. */
static int
readchar (int timeout)
{
int ch;
struct remote_state *rs = get_remote_state ();
+ struct cleanup *old_chain;
+
+ old_chain = make_cleanup_override_quit_handler (remote_serial_quit_handler);
+
+ rs->got_ctrlc_during_io = 0;
ch = serial_readchar (rs->remote_desc, timeout);
+ if (rs->got_ctrlc_during_io)
+ set_quit_flag ();
+
+ do_cleanups (old_chain);
+
if (ch >= 0)
return ch;
@@ -8193,18 +8145,31 @@ readchar (int timeout)
}
/* Wrapper for serial_write that closes the target and throws if
- writing fails. */
+ writing fails. The current quit handler is overridden to avoid
+ quitting in the middle of packet sequence, as that would break
+ communication with the remote server. See
+ remote_serial_quit_handler for more detail. */
static void
remote_serial_write (const char *str, int len)
{
struct remote_state *rs = get_remote_state ();
+ struct cleanup *old_chain;
+
+ old_chain = make_cleanup_override_quit_handler (remote_serial_quit_handler);
+
+ rs->got_ctrlc_during_io = 0;
if (serial_write (rs->remote_desc, str, len))
{
unpush_and_perror (_("Remote communication error. "
"Target disconnected."));
}
+
+ if (rs->got_ctrlc_during_io)
+ set_quit_flag ();
+
+ do_cleanups (old_chain);
}
/* Send the command in *BUF to the remote machine, and read the reply
@@ -8729,7 +8694,6 @@ getpkt_or_notif_sane_1 (char **buf, long *sizeof_buf, int forever,
if (forever) /* Watchdog went off? Kill the target. */
{
- QUIT;
remote_unpush_target ();
throw_error (TARGET_CLOSE_ERROR,
_("Watchdog timeout has expired. "
@@ -13070,7 +13034,6 @@ Specify the serial device it is connected to\n\
remote_ops.to_stop = remote_stop;
remote_ops.to_interrupt = remote_interrupt;
remote_ops.to_pass_ctrlc = remote_pass_ctrlc;
- remote_ops.to_check_pending_interrupt = remote_check_pending_interrupt;
remote_ops.to_xfer_partial = remote_xfer_partial;
remote_ops.to_rcmd = remote_rcmd;
remote_ops.to_pid_to_exec_file = remote_pid_to_exec_file;
@@ -13471,12 +13434,6 @@ _initialize_remote (void)
when it exits. */
observer_attach_inferior_exit (discard_pending_stop_replies);
- /* Set up signal handlers. */
- async_sigint_remote_token =
- create_async_signal_handler (async_remote_interrupt, NULL);
- async_sigint_remote_twice_token =
- create_async_signal_handler (async_remote_interrupt_twice, NULL);
-
#if 0
init_remote_threadtests ();
#endif
diff --git a/gdb/ser-base.c b/gdb/ser-base.c
index 25af66a..c51be07 100644
--- a/gdb/ser-base.c
+++ b/gdb/ser-base.c
@@ -213,6 +213,7 @@ ser_base_wait_for (struct serial *scb, int timeout)
int numfds;
struct timeval tv;
fd_set readfds, exceptfds;
+ int nfds;
/* NOTE: Some OS's can scramble the READFDS when the select()
call fails (ex the kernel with Red Hat 5.2). Initialize all
@@ -226,10 +227,13 @@ ser_base_wait_for (struct serial *scb, int timeout)
FD_SET (scb->fd, &readfds);
FD_SET (scb->fd, &exceptfds);
+ QUIT;
+
+ nfds = scb->fd + 1;
if (timeout >= 0)
- numfds = gdb_select (scb->fd + 1, &readfds, 0, &exceptfds, &tv);
+ numfds = interruptible_select (nfds, &readfds, 0, &exceptfds, &tv);
else
- numfds = gdb_select (scb->fd + 1, &readfds, 0, &exceptfds, 0);
+ numfds = interruptible_select (nfds, &readfds, 0, &exceptfds, 0);
if (numfds <= 0)
{
@@ -455,6 +459,8 @@ ser_base_write (struct serial *scb, const void *buf, size_t count)
while (count > 0)
{
+ QUIT;
+
cc = scb->ops->write_prim (scb, str, count);
if (cc < 0)
diff --git a/gdb/ser-go32.c b/gdb/ser-go32.c
index 72bdc13..10c79ba 100644
--- a/gdb/ser-go32.c
+++ b/gdb/ser-go32.c
@@ -616,6 +616,8 @@ dos_readchar (struct serial *scb, int timeout)
then = rawclock () + (timeout * RAWHZ);
while ((c = dos_getc (port)) < 0)
{
+ QUIT;
+
if (timeout >= 0 && (rawclock () - then) >= 0)
return SERIAL_TIMEOUT;
}
@@ -794,6 +796,8 @@ dos_write (struct serial *scb, const void *buf, size_t count)
while (count > 0)
{
+ QUIT;
+
/* Send the data, fifosize bytes at a time. */
cnt = fifosize > count ? count : fifosize;
port->txbusy = 1;
diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c
index 562e98b..2e7b1b4 100644
--- a/gdb/ser-unix.c
+++ b/gdb/ser-unix.c
@@ -449,10 +449,7 @@ hardwire_raw (struct serial *scb)
}
/* Wait for input on scb, with timeout seconds. Returns 0 on success,
- otherwise SERIAL_TIMEOUT or SERIAL_ERROR.
-
- For termio{s}, we actually just setup VTIME if necessary, and let the
- timeout occur in the read() in hardwire_read(). */
+ otherwise SERIAL_TIMEOUT or SERIAL_ERROR. */
/* FIXME: cagney/1999-09-16: Don't replace this with the equivalent
ser_base*() until the old TERMIOS/SGTTY/... timer code has been
@@ -466,7 +463,6 @@ hardwire_raw (struct serial *scb)
static int
wait_for (struct serial *scb, int timeout)
{
-#ifdef HAVE_SGTTY
while (1)
{
struct timeval tv;
@@ -483,92 +479,22 @@ wait_for (struct serial *scb, int timeout)
FD_ZERO (&readfds);
FD_SET (scb->fd, &readfds);
+ QUIT;
+
if (timeout >= 0)
- numfds = gdb_select (scb->fd + 1, &readfds, 0, 0, &tv);
+ numfds = interruptible_select (scb->fd + 1, &readfds, 0, 0, &tv);
else
- numfds = gdb_select (scb->fd + 1, &readfds, 0, 0, 0);
+ numfds = interruptible_select (scb->fd + 1, &readfds, 0, 0, 0);
- if (numfds <= 0)
- if (numfds == 0)
- return SERIAL_TIMEOUT;
- else if (errno == EINTR)
- continue;
- else
- return SERIAL_ERROR; /* Got an error from select or poll. */
+ if (numfds == -1 && errno == EINTR)
+ continue;
+ else if (numfds == -1)
+ return SERIAL_ERROR;
+ else if (numfds == 0)
+ return SERIAL_TIMEOUT;
return 0;
}
-#endif /* HAVE_SGTTY */
-
-#if defined HAVE_TERMIO || defined HAVE_TERMIOS
- if (timeout == scb->current_timeout)
- return 0;
-
- scb->current_timeout = timeout;
-
- {
- struct hardwire_ttystate state;
-
- if (get_tty_state (scb, &state))
- fprintf_unfiltered (gdb_stderr, "get_tty_state failed: %s\n",
- safe_strerror (errno));
-
-#ifdef HAVE_TERMIOS
- if (timeout < 0)
- {
- /* No timeout. */
- state.termios.c_cc[VTIME] = 0;
- state.termios.c_cc[VMIN] = 1;
- }
- else
- {
- state.termios.c_cc[VMIN] = 0;
- state.termios.c_cc[VTIME] = timeout * 10;
- if (state.termios.c_cc[VTIME] != timeout * 10)
- {
-
- /* If c_cc is an 8-bit signed character, we can't go
- bigger than this. If it is always unsigned, we could use
- 25. */
-
- scb->current_timeout = 12;
- state.termios.c_cc[VTIME] = scb->current_timeout * 10;
- scb->timeout_remaining = timeout - scb->current_timeout;
- }
- }
-#endif
-
-#ifdef HAVE_TERMIO
- if (timeout < 0)
- {
- /* No timeout. */
- state.termio.c_cc[VTIME] = 0;
- state.termio.c_cc[VMIN] = 1;
- }
- else
- {
- state.termio.c_cc[VMIN] = 0;
- state.termio.c_cc[VTIME] = timeout * 10;
- if (state.termio.c_cc[VTIME] != timeout * 10)
- {
- /* If c_cc is an 8-bit signed character, we can't go
- bigger than this. If it is always unsigned, we could use
- 25. */
-
- scb->current_timeout = 12;
- state.termio.c_cc[VTIME] = scb->current_timeout * 10;
- scb->timeout_remaining = timeout - scb->current_timeout;
- }
- }
-#endif
-
- if (set_tty_state (scb, &state))
- fprintf_unfiltered (gdb_stderr, "set_tty_state failed: %s\n",
- safe_strerror (errno));
-
- return 0;
- }
-#endif /* HAVE_TERMIO || HAVE_TERMIOS */
}
/* Read a character with user-specified timeout. TIMEOUT is number of
diff --git a/gdb/utils.c b/gdb/utils.c
index 46765c5..5d778f6 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1057,11 +1057,13 @@ quit (void)
void
maybe_quit (void)
{
- if (check_quit_flag () || sync_quit_force_run)
+ if (sync_quit_force_run)
quit ();
+
+ quit_handler ();
+
if (deprecated_interactive_hook)
deprecated_interactive_hook ();
- target_check_pending_interrupt ();
}
\f
@@ -1289,6 +1291,7 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
/* We'll need to handle input. */
target_terminal_ours ();
+ make_cleanup_override_quit_handler (default_quit_handler);
while (1)
{
@@ -1863,6 +1866,7 @@ prompt_for_continue (void)
/* We'll need to handle input. */
make_cleanup_restore_target_terminal ();
target_terminal_ours ();
+ make_cleanup_override_quit_handler (default_quit_handler);
/* Call gdb_readline_wrapper, not readline, in order to keep an
event loop running. */
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 20/30] Use target_terminal_ours_for_output in cp-support.c
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (2 preceding siblings ...)
2016-03-18 19:19 ` [PATCH 28/30] target remote: Don't rely on immediate_quit (introduce quit handlers) Pedro Alves
@ 2016-03-18 19:19 ` Pedro Alves
2016-03-18 19:19 ` [PATCH 03/30] TUI: check whether in secondary prompt instead of immediate_quit Pedro Alves
` (26 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:19 UTC (permalink / raw)
To: gdb-patches
We're only doing output here, so leave raw/cooked mode alone, as well
as the SIGINT handler.
Restore terminal settings after output, while at it.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* cp-support.c (gdb_demangle): Use target_terminal_ours_for_output
instead of target_terminal_ours, and restore target terminal with
a cleanup.
---
gdb/cp-support.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index a71c6ad..c7f5074 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1601,7 +1601,9 @@ gdb_demangle (const char *name, int options)
"demangler-warning", short_msg);
make_cleanup (xfree, long_msg);
- target_terminal_ours ();
+ make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
+
begin_line ();
if (core_dump_allowed)
fprintf_unfiltered (gdb_stderr,
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 03/30] TUI: check whether in secondary prompt instead of immediate_quit
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (3 preceding siblings ...)
2016-03-18 19:19 ` [PATCH 20/30] Use target_terminal_ours_for_output in cp-support.c Pedro Alves
@ 2016-03-18 19:19 ` Pedro Alves
2016-03-18 19:19 ` [PATCH 26/30] Use target_terminal_ours_for_output in MI Pedro Alves
` (25 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:19 UTC (permalink / raw)
To: gdb-patches
As can be seen in the tui_redisplay_readline comment:
"The command could call prompt_for_continue and we must not restore
SingleKey so that the prompt and normal keymap are used."
immediate_quit is being used as proxy for "secondary prompt".
We have a better predicate nowadays, so use it.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* tui/tui-io.c (tui_redisplay_readline): Check
gdb_in_secondary_prompt_p instead of immediate_quit.
* tui/tui.c: Include top.h.
(tui_rl_startup_hook): Check gdb_in_secondary_prompt_p instead of
immediate_quit.
---
gdb/tui/tui-io.c | 2 +-
gdb/tui/tui.c | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 18c648c..3fa32db 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -212,7 +212,7 @@ tui_redisplay_readline (void)
The command could call prompt_for_continue and we must not
restore SingleKey so that the prompt and normal keymap are used. */
if (tui_current_key_mode == TUI_ONE_COMMAND_MODE && rl_end == 0
- && immediate_quit == 0)
+ && !gdb_in_secondary_prompt_p ())
tui_set_key_mode (TUI_SINGLE_KEY_MODE);
if (tui_current_key_mode == TUI_SINGLE_KEY_MODE)
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 273a0d9..96f8df7 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -38,6 +38,7 @@
#include "symtab.h"
#include "source.h"
#include "terminal.h"
+#include "top.h"
#include <ctype.h>
#include <signal.h>
@@ -302,7 +303,8 @@ static int
tui_rl_startup_hook (void)
{
rl_already_prompted = 1;
- if (tui_current_key_mode != TUI_COMMAND_MODE && immediate_quit == 0)
+ if (tui_current_key_mode != TUI_COMMAND_MODE
+ && !gdb_in_secondary_prompt_p ())
tui_set_key_mode (TUI_SINGLE_KEY_MODE);
tui_redisplay_readline ();
return 0;
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 26/30] Use target_terminal_ours_for_output in MI
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (4 preceding siblings ...)
2016-03-18 19:19 ` [PATCH 03/30] TUI: check whether in secondary prompt instead of immediate_quit Pedro Alves
@ 2016-03-18 19:19 ` Pedro Alves
2016-03-18 19:19 ` [PATCH 30/30] Eliminate target_check_pending_interrupt Pedro Alves
` (24 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:19 UTC (permalink / raw)
To: gdb-patches
The MI code only does output, so leave raw/cooked mode alone, as well
as the SIGINT handler. Restore terminal settings after output, while
at it. Also, a couple events missed calling target_terminal_ours
before output, even.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* mi/mi-interp.c (mi_new_thread): Put
target_terminal_ours_for_output in effect while outputting.
(mi_thread_exit): Use target_terminal_ours_for_output instead of
target_terminal_ours.
(mi_record_changed, mi_inferior_added, mi_inferior_appeared)
(mi_inferior_exit, mi_inferior_removed, mi_traceframe_changed)
(mi_tsv_created, mi_tsv_deleted, mi_tsv_modified)
(mi_breakpoint_created, mi_breakpoint_deleted)
(mi_breakpoint_modified, mi_solib_loaded, mi_solib_unloaded)
(mi_command_param_changed, mi_memory_changed)
(report_initial_inferior): Use target_terminal_ours_for_output
instead of target_terminal_ours. Restore terminal settings.
* mi/mi-main.c (mi_execute_command): Use
target_terminal_ours_for_output instead of target_terminal_ours.
Restore terminal settings.
---
gdb/mi/mi-interp.c | 125 ++++++++++++++++++++++++++++++++++++++++++++---------
gdb/mi/mi-main.c | 7 ++-
2 files changed, 111 insertions(+), 21 deletions(-)
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index d02012c..b37dc96 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -357,13 +357,19 @@ mi_new_thread (struct thread_info *t)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
struct inferior *inf = find_inferior_ptid (t->ptid);
+ struct cleanup *old_chain;
gdb_assert (inf);
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
+
fprintf_unfiltered (mi->event_channel,
"thread-created,id=\"%d\",group-id=\"i%d\"",
t->global_num, inf->num);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
static void
@@ -380,7 +386,8 @@ mi_thread_exit (struct thread_info *t, int silent)
mi = (struct mi_interp *) top_level_interpreter_data ();
old_chain = make_cleanup_restore_target_terminal ();
- target_terminal_ours ();
+ target_terminal_ours_for_output ();
+
fprintf_unfiltered (mi->event_channel,
"thread-exited,id=\"%d\",group-id=\"i%d\"",
t->global_num, inf->num);
@@ -395,43 +402,62 @@ static void
mi_record_changed (struct inferior *inferior, int started)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
+ struct cleanup *old_chain;
+
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
fprintf_unfiltered (mi->event_channel, "record-%s,thread-group=\"i%d\"",
started ? "started" : "stopped", inferior->num);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
static void
mi_inferior_added (struct inferior *inf)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
+ struct cleanup *old_chain;
+
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
- target_terminal_ours ();
fprintf_unfiltered (mi->event_channel,
"thread-group-added,id=\"i%d\"",
inf->num);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
static void
mi_inferior_appeared (struct inferior *inf)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
+ struct cleanup *old_chain;
+
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
- target_terminal_ours ();
fprintf_unfiltered (mi->event_channel,
"thread-group-started,id=\"i%d\",pid=\"%d\"",
inf->num, inf->pid);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
static void
mi_inferior_exit (struct inferior *inf)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
+ struct cleanup *old_chain;
+
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
- target_terminal_ours ();
if (inf->has_exit_code)
fprintf_unfiltered (mi->event_channel,
"thread-group-exited,id=\"i%d\",exit-code=\"%s\"",
@@ -439,20 +465,26 @@ mi_inferior_exit (struct inferior *inf)
else
fprintf_unfiltered (mi->event_channel,
"thread-group-exited,id=\"i%d\"", inf->num);
+ gdb_flush (mi->event_channel);
- gdb_flush (mi->event_channel);
+ do_cleanups (old_chain);
}
static void
mi_inferior_removed (struct inferior *inf)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
+ struct cleanup *old_chain;
+
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
- target_terminal_ours ();
fprintf_unfiltered (mi->event_channel,
"thread-group-removed,id=\"i%d\"",
inf->num);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
/* Return the MI interpreter, if it is active -- either because it's
@@ -675,11 +707,13 @@ static void
mi_traceframe_changed (int tfnum, int tpnum)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
+ struct cleanup *old_chain;
if (mi_suppress_notification.traceframe)
return;
- target_terminal_ours ();
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
if (tfnum >= 0)
fprintf_unfiltered (mi->event_channel, "traceframe-changed,"
@@ -689,6 +723,8 @@ mi_traceframe_changed (int tfnum, int tpnum)
fprintf_unfiltered (mi->event_channel, "traceframe-changed,end");
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
/* Emit notification on creating a trace state variable. */
@@ -697,14 +733,18 @@ static void
mi_tsv_created (const struct trace_state_variable *tsv)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
+ struct cleanup *old_chain;
- target_terminal_ours ();
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
fprintf_unfiltered (mi->event_channel, "tsv-created,"
"name=\"%s\",initial=\"%s\"\n",
tsv->name, plongest (tsv->initial_value));
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
/* Emit notification on deleting a trace state variable. */
@@ -713,8 +753,10 @@ static void
mi_tsv_deleted (const struct trace_state_variable *tsv)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
+ struct cleanup *old_chain;
- target_terminal_ours ();
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
if (tsv != NULL)
fprintf_unfiltered (mi->event_channel, "tsv-deleted,"
@@ -723,6 +765,8 @@ mi_tsv_deleted (const struct trace_state_variable *tsv)
fprintf_unfiltered (mi->event_channel, "tsv-deleted\n");
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
/* Emit notification on modifying a trace state variable. */
@@ -732,8 +776,10 @@ mi_tsv_modified (const struct trace_state_variable *tsv)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
+ struct cleanup *old_chain;
- target_terminal_ours ();
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
fprintf_unfiltered (mi->event_channel,
"tsv-modified");
@@ -749,6 +795,8 @@ mi_tsv_modified (const struct trace_state_variable *tsv)
ui_out_redirect (mi_uiout, NULL);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
/* Emit notification about a created breakpoint. */
@@ -758,6 +806,7 @@ mi_breakpoint_created (struct breakpoint *b)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
+ struct cleanup *old_chain;
if (mi_suppress_notification.breakpoint)
return;
@@ -765,7 +814,9 @@ mi_breakpoint_created (struct breakpoint *b)
if (b->number <= 0)
return;
- target_terminal_ours ();
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
+
fprintf_unfiltered (mi->event_channel,
"breakpoint-created");
/* We want the output from gdb_breakpoint_query to go to
@@ -788,6 +839,8 @@ mi_breakpoint_created (struct breakpoint *b)
ui_out_redirect (mi_uiout, NULL);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
/* Emit notification about deleted breakpoint. */
@@ -796,6 +849,7 @@ static void
mi_breakpoint_deleted (struct breakpoint *b)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
+ struct cleanup *old_chain;
if (mi_suppress_notification.breakpoint)
return;
@@ -803,12 +857,15 @@ mi_breakpoint_deleted (struct breakpoint *b)
if (b->number <= 0)
return;
- target_terminal_ours ();
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
fprintf_unfiltered (mi->event_channel, "breakpoint-deleted,id=\"%d\"",
b->number);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
/* Emit notification about modified breakpoint. */
@@ -818,6 +875,7 @@ mi_breakpoint_modified (struct breakpoint *b)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
+ struct cleanup *old_chain;
if (mi_suppress_notification.breakpoint)
return;
@@ -825,7 +883,9 @@ mi_breakpoint_modified (struct breakpoint *b)
if (b->number <= 0)
return;
- target_terminal_ours ();
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
+
fprintf_unfiltered (mi->event_channel,
"breakpoint-modified");
/* We want the output from gdb_breakpoint_query to go to
@@ -848,6 +908,8 @@ mi_breakpoint_modified (struct breakpoint *b)
ui_out_redirect (mi_uiout, NULL);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
static int
@@ -947,8 +1009,10 @@ mi_solib_loaded (struct so_list *solib)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
struct ui_out *uiout = interp_ui_out (top_level_interpreter ());
+ struct cleanup *old_chain;
- target_terminal_ours ();
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
fprintf_unfiltered (mi->event_channel, "library-loaded");
@@ -960,12 +1024,15 @@ mi_solib_loaded (struct so_list *solib)
ui_out_field_int (uiout, "symbols-loaded", solib->symbols_loaded);
if (!gdbarch_has_global_solist (target_gdbarch ()))
{
- ui_out_field_fmt (uiout, "thread-group", "i%d", current_inferior ()->num);
+ ui_out_field_fmt (uiout, "thread-group", "i%d",
+ current_inferior ()->num);
}
ui_out_redirect (uiout, NULL);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
static void
@@ -973,8 +1040,10 @@ mi_solib_unloaded (struct so_list *solib)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
struct ui_out *uiout = interp_ui_out (top_level_interpreter ());
+ struct cleanup *old_chain;
- target_terminal_ours ();
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
fprintf_unfiltered (mi->event_channel, "library-unloaded");
@@ -985,12 +1054,15 @@ mi_solib_unloaded (struct so_list *solib)
ui_out_field_string (uiout, "host-name", solib->so_name);
if (!gdbarch_has_global_solist (target_gdbarch ()))
{
- ui_out_field_fmt (uiout, "thread-group", "i%d", current_inferior ()->num);
+ ui_out_field_fmt (uiout, "thread-group", "i%d",
+ current_inferior ()->num);
}
ui_out_redirect (uiout, NULL);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
/* Emit notification about the command parameter change. */
@@ -1000,11 +1072,13 @@ mi_command_param_changed (const char *param, const char *value)
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
+ struct cleanup *old_chain;
if (mi_suppress_notification.cmd_param_changed)
return;
- target_terminal_ours ();
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
fprintf_unfiltered (mi->event_channel,
"cmd-param-changed");
@@ -1017,6 +1091,8 @@ mi_command_param_changed (const char *param, const char *value)
ui_out_redirect (mi_uiout, NULL);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
/* Emit notification about the target memory change. */
@@ -1028,11 +1104,13 @@ mi_memory_changed (struct inferior *inferior, CORE_ADDR memaddr,
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter_data ();
struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
struct obj_section *sec;
+ struct cleanup *old_chain;
if (mi_suppress_notification.memory)
return;
- target_terminal_ours ();
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
fprintf_unfiltered (mi->event_channel,
"memory-changed");
@@ -1058,6 +1136,8 @@ mi_memory_changed (struct inferior *inferior, CORE_ADDR memaddr,
ui_out_redirect (mi_uiout, NULL);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
static int
@@ -1068,12 +1148,17 @@ report_initial_inferior (struct inferior *inf, void *closure)
and top_level_interpreter_data is set, we cannot call
it here. */
struct mi_interp *mi = (struct mi_interp *) closure;
+ struct cleanup *old_chain;
+
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
- target_terminal_ours ();
fprintf_unfiltered (mi->event_channel,
"thread-group-added,id=\"i%d\"",
inf->num);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
return 0;
}
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index e25eedf..7cb7bf5 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2171,12 +2171,17 @@ mi_execute_command (const char *cmd, int from_tty)
if (report_change)
{
struct thread_info *ti = inferior_thread ();
+ struct cleanup *old_chain;
+
+ old_chain = make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
- target_terminal_ours ();
fprintf_unfiltered (mi->event_channel,
"thread-selected,id=\"%d\"",
ti->global_num);
gdb_flush (mi->event_channel);
+
+ do_cleanups (old_chain);
}
}
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 30/30] Eliminate target_check_pending_interrupt
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (5 preceding siblings ...)
2016-03-18 19:19 ` [PATCH 26/30] Use target_terminal_ours_for_output in MI Pedro Alves
@ 2016-03-18 19:19 ` Pedro Alves
2016-03-18 19:19 ` [PATCH 27/30] TUI: GC tui_target_has_run Pedro Alves
` (23 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:19 UTC (permalink / raw)
To: gdb-patches
This is no longer called anywhere.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* target.c (target_check_pending_interrupt): Delete.
* target.h (struct target_ops) <to_check_pending_interrupt>:
Remove method.
(target_check_pending_interrupt): Remove declaration.
* target-delegates.c: Regenerate.
---
gdb/target-delegates.c | 26 --------------------------
gdb/target.c | 8 --------
gdb/target.h | 10 ----------
3 files changed, 44 deletions(-)
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 640803a..03aa2cc 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -1625,28 +1625,6 @@ debug_pass_ctrlc (struct target_ops *self)
}
static void
-delegate_check_pending_interrupt (struct target_ops *self)
-{
- self = self->beneath;
- self->to_check_pending_interrupt (self);
-}
-
-static void
-tdefault_check_pending_interrupt (struct target_ops *self)
-{
-}
-
-static void
-debug_check_pending_interrupt (struct target_ops *self)
-{
- fprintf_unfiltered (gdb_stdlog, "-> %s->to_check_pending_interrupt (...)\n", debug_target.to_shortname);
- debug_target.to_check_pending_interrupt (&debug_target);
- fprintf_unfiltered (gdb_stdlog, "<- %s->to_check_pending_interrupt (", debug_target.to_shortname);
- target_debug_print_struct_target_ops_p (&debug_target);
- fputs_unfiltered (")\n", gdb_stdlog);
-}
-
-static void
delegate_rcmd (struct target_ops *self, const char *arg1, struct ui_file *arg2)
{
self = self->beneath;
@@ -4213,8 +4191,6 @@ install_delegators (struct target_ops *ops)
ops->to_interrupt = delegate_interrupt;
if (ops->to_pass_ctrlc == NULL)
ops->to_pass_ctrlc = delegate_pass_ctrlc;
- if (ops->to_check_pending_interrupt == NULL)
- ops->to_check_pending_interrupt = delegate_check_pending_interrupt;
if (ops->to_rcmd == NULL)
ops->to_rcmd = delegate_rcmd;
if (ops->to_pid_to_exec_file == NULL)
@@ -4462,7 +4438,6 @@ install_dummy_methods (struct target_ops *ops)
ops->to_stop = tdefault_stop;
ops->to_interrupt = tdefault_interrupt;
ops->to_pass_ctrlc = default_target_pass_ctrlc;
- ops->to_check_pending_interrupt = tdefault_check_pending_interrupt;
ops->to_rcmd = default_rcmd;
ops->to_pid_to_exec_file = tdefault_pid_to_exec_file;
ops->to_log_command = tdefault_log_command;
@@ -4619,7 +4594,6 @@ init_debug_target (struct target_ops *ops)
ops->to_stop = debug_stop;
ops->to_interrupt = debug_interrupt;
ops->to_pass_ctrlc = debug_pass_ctrlc;
- ops->to_check_pending_interrupt = debug_check_pending_interrupt;
ops->to_rcmd = debug_rcmd;
ops->to_pid_to_exec_file = debug_pid_to_exec_file;
ops->to_log_command = debug_log_command;
diff --git a/gdb/target.c b/gdb/target.c
index d580983..a9744c4 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3378,14 +3378,6 @@ default_target_pass_ctrlc (struct target_ops *ops)
target_interrupt (inferior_ptid);
}
-/* See target.h. */
-
-void
-target_check_pending_interrupt (void)
-{
- (*current_target.to_check_pending_interrupt) (¤t_target);
-}
-
/* See target/target.h. */
void
diff --git a/gdb/target.h b/gdb/target.h
index 00625fec..6b5b6e0 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -647,8 +647,6 @@ struct target_ops
TARGET_DEFAULT_IGNORE ();
void (*to_pass_ctrlc) (struct target_ops *)
TARGET_DEFAULT_FUNC (default_target_pass_ctrlc);
- void (*to_check_pending_interrupt) (struct target_ops *)
- TARGET_DEFAULT_IGNORE ();
void (*to_rcmd) (struct target_ops *,
const char *command, struct ui_file *output)
TARGET_DEFAULT_FUNC (default_rcmd);
@@ -1729,14 +1727,6 @@ extern void target_pass_ctrlc (void);
target_interrupt. */
extern void default_target_pass_ctrlc (struct target_ops *ops);
-/* Some targets install their own SIGINT handler while the target is
- running. This method is called from the QUIT macro to give such
- targets a chance to process a Ctrl-C. The target may e.g., choose
- to interrupt the (potentially) long running operation, or give up
- waiting and disconnect. */
-
-extern void target_check_pending_interrupt (void);
-
/* Send the specified COMMAND to the target's monitor
(shell,interpreter) for execution. The result of the query is
placed in OUTBUF. */
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 27/30] TUI: GC tui_target_has_run
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (6 preceding siblings ...)
2016-03-18 19:19 ` [PATCH 30/30] Eliminate target_check_pending_interrupt Pedro Alves
@ 2016-03-18 19:19 ` Pedro Alves
2016-03-18 19:19 ` [PATCH 19/30] ada-lang.c: Introduce type_as_string and use it Pedro Alves
` (22 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:19 UTC (permalink / raw)
To: gdb-patches
Nothing actually uses this global.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* tui/tui-hooks.c (tui_target_has_run): Delete.
(tui_about_to_proceed): Delete.
(tui_about_to_proceed_observer): Delete.
(tui_install_hooks, tui_remove_hooks): Don't install/remove an
about_to_proceed observer.
---
gdb/tui/tui-hooks.c | 24 ------------------------
1 file changed, 24 deletions(-)
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index bfe7aeb..5a03b61 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -54,8 +54,6 @@
"gdb_curses.h". */
#include "readline/readline.h"
-int tui_target_has_run = 0;
-
static void
tui_new_objfile_hook (struct objfile* objfile)
{
@@ -109,23 +107,6 @@ tui_event_modify_breakpoint (struct breakpoint *b)
tui_update_all_breakpoint_info ();
}
-/* Called when a command is about to proceed the inferior. */
-
-static void
-tui_about_to_proceed (void)
-{
- /* Leave tui mode (optional). */
-#if 0
- if (tui_active)
- {
- target_terminal_ours ();
- endwin ();
- target_terminal_inferior ();
- }
-#endif
- tui_target_has_run = 1;
-}
-
/* Refresh TUI's frame and register information. This is a hook intended to be
used to update the screen after potential frame and register changes.
@@ -230,7 +211,6 @@ static struct observer *tui_bp_created_observer;
static struct observer *tui_bp_deleted_observer;
static struct observer *tui_bp_modified_observer;
static struct observer *tui_inferior_exit_observer;
-static struct observer *tui_about_to_proceed_observer;
static struct observer *tui_before_prompt_observer;
static struct observer *tui_normal_stop_observer;
static struct observer *tui_register_changed_observer;
@@ -255,8 +235,6 @@ tui_install_hooks (void)
= observer_attach_breakpoint_modified (tui_event_modify_breakpoint);
tui_inferior_exit_observer
= observer_attach_inferior_exit (tui_inferior_exit);
- tui_about_to_proceed_observer
- = observer_attach_about_to_proceed (tui_about_to_proceed);
tui_before_prompt_observer
= observer_attach_before_prompt (tui_before_prompt);
tui_normal_stop_observer
@@ -280,8 +258,6 @@ tui_remove_hooks (void)
tui_bp_modified_observer = NULL;
observer_detach_inferior_exit (tui_inferior_exit_observer);
tui_inferior_exit_observer = NULL;
- observer_detach_about_to_proceed (tui_about_to_proceed_observer);
- tui_about_to_proceed_observer = NULL;
observer_detach_before_prompt (tui_before_prompt_observer);
tui_before_prompt_observer = NULL;
observer_detach_normal_stop (tui_normal_stop_observer);
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 19/30] ada-lang.c: Introduce type_as_string and use it
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (7 preceding siblings ...)
2016-03-18 19:19 ` [PATCH 27/30] TUI: GC tui_target_has_run Pedro Alves
@ 2016-03-18 19:19 ` Pedro Alves
2016-03-18 19:19 ` [PATCH 14/30] Don't call clear_quit_flag in captured_main Pedro Alves
` (21 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:19 UTC (permalink / raw)
To: gdb-patches
A couple wrong things here
- We should not use target_terminal_ours when all we want is output.
We should use target_terminal_ours_for_output instead, which
preserves raw/cooked terminal modes, and SIGINT forwarding.
- Most importantly, relying on stderr output immediately preceding
the error/exception print isn't correct. The exception could be
caught and handled, for example; MI frontends won't display the
stderr part in an error dialog box. Etc.
This commit introduces a type_as_string helper that allows building a
full error string including type info.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* ada-lang.c (type_as_string, type_as_string_and_cleanup): New
functions.
(ada_lookup_struct_elt_type): Use type_as_string_and_cleanup.
---
gdb/ada-lang.c | 74 ++++++++++++++++++++++++++++++++++------------------------
1 file changed, 44 insertions(+), 30 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index d874129..7cdb693 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -7576,6 +7576,39 @@ ada_value_struct_elt (struct value *arg, char *name, int no_err)
"a value that is not a record."));
}
+/* Return a string representation of type TYPE. Caller must free
+ result. */
+
+static char *
+type_as_string (struct type *type)
+{
+ struct ui_file *tmp_stream = mem_fileopen ();
+ struct cleanup *old_chain;
+ char *str;
+
+ tmp_stream = mem_fileopen ();
+ old_chain = make_cleanup_ui_file_delete (tmp_stream);
+
+ type_print (type, "", tmp_stream, -1);
+ str = ui_file_xstrdup (tmp_stream, NULL);
+
+ do_cleanups (old_chain);
+ return str;
+}
+
+/* Return a string representation of type TYPE, and install a cleanup
+ that releases it. */
+
+static char *
+type_as_string_and_cleanup (struct type *type)
+{
+ char *str;
+
+ str = type_as_string (type);
+ make_cleanup (xfree, str);
+ return str;
+}
+
/* Given a type TYPE, look up the type of the component of type named NAME.
If DISPP is non-null, add its byte displacement from the beginning of a
structure (pointed to by a value) of type TYPE to *DISPP (does not
@@ -7616,22 +7649,15 @@ ada_lookup_struct_elt_type (struct type *type, char *name, int refok,
|| (TYPE_CODE (type) != TYPE_CODE_STRUCT
&& TYPE_CODE (type) != TYPE_CODE_UNION))
{
+ char *type_str;
+
if (noerr)
return NULL;
- else
- {
- target_terminal_ours ();
- gdb_flush (gdb_stdout);
- if (type == NULL)
- error (_("Type (null) is not a structure or union type"));
- else
- {
- /* XXX: type_sprint */
- fprintf_unfiltered (gdb_stderr, _("Type "));
- type_print (type, "", gdb_stderr, -1);
- error (_(" is not a structure or union type"));
- }
- }
+
+ type_str = (type != NULL
+ ? type_as_string_and_cleanup (type)
+ : _("(null)"));
+ error (_("Type %s is not a structure or union type"), type_str);
}
type = to_static_fixed_type (type);
@@ -7701,22 +7727,10 @@ ada_lookup_struct_elt_type (struct type *type, char *name, int refok,
BadName:
if (!noerr)
{
- target_terminal_ours ();
- gdb_flush (gdb_stdout);
- if (name == NULL)
- {
- /* XXX: type_sprint */
- fprintf_unfiltered (gdb_stderr, _("Type "));
- type_print (type, "", gdb_stderr, -1);
- error (_(" has no component named <null>"));
- }
- else
- {
- /* XXX: type_sprint */
- fprintf_unfiltered (gdb_stderr, _("Type "));
- type_print (type, "", gdb_stderr, -1);
- error (_(" has no component named %s"), name);
- }
+ char *name_str = name != NULL ? name : _("<null>");
+
+ error (_("Type %s has no component named %s"),
+ type_as_string_and_cleanup (type), name_str);
}
return NULL;
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 14/30] Don't call clear_quit_flag in captured_main
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (8 preceding siblings ...)
2016-03-18 19:19 ` [PATCH 19/30] ada-lang.c: Introduce type_as_string and use it Pedro Alves
@ 2016-03-18 19:19 ` Pedro Alves
2016-03-18 19:19 ` [PATCH 29/30] Eliminate immediate_quit Pedro Alves
` (20 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:19 UTC (permalink / raw)
To: gdb-patches
This call seems pointless. For instance, a SIGINT handler is only
installed later on. And if wasn't, I can't see why we'd want to lose
a Ctrl-C request.
Getting rid of this allows getting rid of clear_quit_flag.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* main.c (captured_main): Don't clear the quit flag.
---
gdb/main.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/gdb/main.c b/gdb/main.c
index 93ed98f..c149b70 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -505,7 +505,6 @@ captured_main (void *data)
dirarg = (char **) xmalloc (dirsize * sizeof (*dirarg));
ndir = 0;
- clear_quit_flag ();
saved_command_line = (char *) xstrdup ("");
instream = stdin;
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 29/30] Eliminate immediate_quit
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (9 preceding siblings ...)
2016-03-18 19:19 ` [PATCH 14/30] Don't call clear_quit_flag in captured_main Pedro Alves
@ 2016-03-18 19:19 ` Pedro Alves
2016-03-18 19:19 ` [PATCH 22/30] Use target_terminal_ours_for_output in infcmd.c Pedro Alves
` (19 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:19 UTC (permalink / raw)
To: gdb-patches
This finally gets rid of immediate_quit (and surrounding
infrustruture), as nothing sets it anymore.
gdb_call_async_signal_handler was only necessary in order to handle
immediate_quit. We can just call mark_async_signal_handler directly
on all hosts now.
In turn, we can clean up mingw-hdep.c's gdb_select a bit, as
sigint_event / sigint_handler is no longer needed.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* defs.h: Update comments on SIGINT handling.
(immediate_quit): Delete declaration.
* event-loop.c (call_async_signal_handler): Delete.
* event-loop.h (call_async_signal_handler): Delete declaration.
(mark_async_signal_handler): Update comments.
(gdb_call_async_signal_handler): Delete declaration.
* event-top.c (handle_sigint): Call mark_async_signal_handler
instead of gdb_call_async_signal_handler.
* exceptions.c (prepare_to_throw_exception): Remove reference to
immediate_quit.
(exception_fprintf): Remove comments about immediate_quit.
* mingw-hdep.c (sigint_event, sigint_handler): Delete.
(gdb_select): Don't wait on sigint_event.
(gdb_call_async_signal_handler): Delete.
(_initialize_mingw_hdep): Delete.
* posix-hdep.c (gdb_call_async_signal_handler): Delete.
* utils.c (immediate_quit): Delete.
---
gdb/defs.h | 11 ++++-------
gdb/event-loop.c | 9 ---------
gdb/event-loop.h | 24 +++---------------------
gdb/event-top.c | 13 +++----------
gdb/exceptions.c | 8 +-------
gdb/mingw-hdep.c | 52 +---------------------------------------------------
gdb/posix-hdep.c | 13 -------------
gdb/utils.c | 13 -------------
8 files changed, 12 insertions(+), 131 deletions(-)
diff --git a/gdb/defs.h b/gdb/defs.h
index 83e4e11..482ef1c 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -125,11 +125,10 @@ extern char *python_libdir;
/* * Search path for separate debug files. */
extern char *debug_file_directory;
-/* GDB has two methods for handling SIGINT. When immediate_quit is
- nonzero, a SIGINT results in an immediate longjmp out of the signal
- handler. Otherwise, SIGINT simply sets a flag; code that might
- take a long time, and which ought to be interruptible, checks this
- flag using the QUIT macro.
+/* GDB's SIGINT handler basically sets a flag; code that might take a
+ long time before it gets back to the event loop, and which ought to
+ be interruptible, checks this flag using the QUIT macro, which, if
+ GDB has the terminal, throws a quit exception.
In addition to setting a flag, the SIGINT handler also marks a
select/poll-able file descriptor as read-ready. That is used by
@@ -176,8 +175,6 @@ extern void default_quit_handler (void);
/* Flag that function quit should call quit_force. */
extern volatile int sync_quit_force_run;
-extern int immediate_quit;
-
extern void quit (void);
/* Helper for the QUIT macro. */
diff --git a/gdb/event-loop.c b/gdb/event-loop.c
index 052d535..60ef2a5 100644
--- a/gdb/event-loop.c
+++ b/gdb/event-loop.c
@@ -911,15 +911,6 @@ create_async_signal_handler (sig_handler_func * proc,
return async_handler_ptr;
}
-/* Call the handler from HANDLER immediately. This function runs
- signal handlers when returning to the event loop would be too
- slow. */
-void
-call_async_signal_handler (struct async_signal_handler *handler)
-{
- (*handler->proc) (handler->client_data);
-}
-
/* Mark the handler (ASYNC_HANDLER_PTR) as ready. This information
will be used when the handlers are invoked, after we have waited
for some event. The caller of this function is the interrupt
diff --git a/gdb/event-loop.h b/gdb/event-loop.h
index 155dafa..7159d97 100644
--- a/gdb/event-loop.h
+++ b/gdb/event-loop.h
@@ -91,16 +91,9 @@ extern int create_timer (int milliseconds,
gdb_client_data client_data);
extern void delete_timer (int id);
-/* Call the handler from HANDLER immediately. This function
- runs signal handlers when returning to the event loop would be too
- slow. Do not call this directly; use gdb_call_async_signal_handler,
- below, with IMMEDIATE_P == 1. */
-void call_async_signal_handler (struct async_signal_handler *handler);
-
-/* Call the handler from HANDLER the next time through the event loop.
- Do not call this directly; use gdb_call_async_signal_handler,
- below, with IMMEDIATE_P == 0. */
-void mark_async_signal_handler (struct async_signal_handler *handler);
+/* Call the handler from HANDLER the next time through the event
+ loop. */
+extern void mark_async_signal_handler (struct async_signal_handler *handler);
/* Returns true if HANDLER is marked ready. */
@@ -111,17 +104,6 @@ extern int
extern void clear_async_signal_handler (struct async_signal_handler *handler);
-/* Wrapper for the body of signal handlers. Call this function from
- any SIGINT handler which needs to access GDB data structures or
- escape via longjmp. If IMMEDIATE_P is set, this triggers either
- immediately (for POSIX platforms), or from gdb_select (for
- MinGW). If IMMEDIATE_P is clear, the handler will run the next
- time we return to the event loop and any current select calls
- will be interrupted. */
-
-void gdb_call_async_signal_handler (struct async_signal_handler *handler,
- int immediate_p);
-
/* Create and register an asynchronous event source in the event loop,
and set PROC as its callback. CLIENT_DATA is passed as argument to
PROC upon its invocation. Returns a pointer to an opaque structure
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 386920e..d599d2a 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -900,18 +900,11 @@ handle_sigint (int sig)
it may be quite a while before we get back to the event loop. So
set quit_flag to 1 here. Then if QUIT is called before we get to
the event loop, we will unwind as expected. */
-
set_quit_flag ();
- /* If immediate_quit is set, we go ahead and process the SIGINT right
- away, even if we usually would defer this to the event loop. The
- assumption here is that it is safe to process ^C immediately if
- immediate_quit is set. If we didn't, SIGINT would be really
- processed only the next time through the event loop. To get to
- that point, though, the command that we want to interrupt needs to
- finish first, which is unacceptable. If immediate quit is not set,
- we process SIGINT the next time through the loop, which is fine. */
- gdb_call_async_signal_handler (sigint_token, immediate_quit);
+ /* In case nothing calls QUIT before the event loop is reached, the
+ event loop handles it. */
+ mark_async_signal_handler (sigint_token);
}
/* See gdb_select.h. */
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index ffdd1f37..7f6599f 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -30,7 +30,6 @@
void
prepare_to_throw_exception (void)
{
- immediate_quit = 0;
}
static void
@@ -148,12 +147,7 @@ exception_fprintf (struct ui_file *file, struct gdb_exception e,
returned by catch_exceptions(). It is an internal_error() for
FUNC() to return a negative value.
- See exceptions.h for further usage details.
-
- Must not be called with immediate_quit in effect (bad things might
- happen, say we got a signal in the middle of a memcpy to quit_return).
- This is an OK restriction; with very few exceptions immediate_quit can
- be replaced by judicious use of QUIT. */
+ See exceptions.h for further usage details. */
/* MAYBE: cagney/1999-11-05: catch_errors() in conjunction with
error() et al. could maintain a set of flags that indicate the
diff --git a/gdb/mingw-hdep.c b/gdb/mingw-hdep.c
index 8247a8c..2e010fa 100644
--- a/gdb/mingw-hdep.c
+++ b/gdb/mingw-hdep.c
@@ -27,14 +27,6 @@
#include <windows.h>
-/* This event is signalled whenever an asynchronous SIGINT handler
- needs to perform an action in the main thread. */
-static HANDLE sigint_event;
-
-/* When SIGINT_EVENT is signalled, gdb_select will call this
- function. */
-struct async_signal_handler *sigint_handler;
-
/* Return an absolute file name of the running GDB, if possible, or
ARGV0 if not. The return value is in malloc'ed storage. */
@@ -120,8 +112,7 @@ gdb_select (int n, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
}
}
- gdb_assert (num_handles < MAXIMUM_WAIT_OBJECTS);
- handles[num_handles++] = sigint_event;
+ gdb_assert (num_handles <= MAXIMUM_WAIT_OBJECTS);
event = WaitForMultipleObjects (num_handles,
handles,
@@ -184,46 +175,5 @@ gdb_select (int n, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
while (RL_ISSTATE (RL_STATE_SIGHANDLER))
Sleep (1);
- if (h == sigint_event
- || WaitForSingleObject (sigint_event, 0) == WAIT_OBJECT_0)
- {
- if (sigint_handler != NULL)
- call_async_signal_handler (sigint_handler);
-
- if (num_ready == 0)
- {
- errno = EINTR;
- return -1;
- }
- }
-
return num_ready;
}
-
-/* Wrapper for the body of signal handlers. On Windows systems, a
- SIGINT handler runs in its own thread. We can't longjmp from
- there, and we shouldn't even prompt the user. Delay HANDLER
- until the main thread is next in gdb_select. */
-
-void
-gdb_call_async_signal_handler (struct async_signal_handler *handler,
- int immediate_p)
-{
- if (immediate_p)
- sigint_handler = handler;
- else
- {
- mark_async_signal_handler (handler);
- sigint_handler = NULL;
- }
- SetEvent (sigint_event);
-}
-
-/* -Wmissing-prototypes */
-extern initialize_file_ftype _initialize_mingw_hdep;
-
-void
-_initialize_mingw_hdep (void)
-{
- sigint_event = CreateEvent (0, FALSE, FALSE, 0);
-}
diff --git a/gdb/posix-hdep.c b/gdb/posix-hdep.c
index 90cab8f..e9db576 100644
--- a/gdb/posix-hdep.c
+++ b/gdb/posix-hdep.c
@@ -30,16 +30,3 @@ gdb_select (int n, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
{
return select (n, readfds, writefds, exceptfds, timeout);
}
-
-/* Wrapper for the body of signal handlers. Nothing special needed on
- POSIX platforms. */
-
-void
-gdb_call_async_signal_handler (struct async_signal_handler *handler,
- int immediate_p)
-{
- if (immediate_p)
- call_async_signal_handler (handler);
- else
- mark_async_signal_handler (handler);
-}
diff --git a/gdb/utils.c b/gdb/utils.c
index 5d778f6..d58c4c1 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -109,19 +109,6 @@ static int debug_timestamp = 0;
int job_control;
-/* Nonzero means quit immediately if Control-C is typed now, rather
- than waiting until QUIT is executed. Be careful in setting this;
- code which executes with immediate_quit set has to be very careful
- about being able to deal with being interrupted at any time. It is
- almost always better to use QUIT; the only exception I can think of
- is being able to quit out of a system call (using EINTR loses if
- the SIGINT happens between the previous QUIT and the system call).
- To immediately quit in the case in which a SIGINT happens between
- the previous QUIT and setting immediate_quit (desirable anytime we
- expect to block), call QUIT after setting immediate_quit. */
-
-int immediate_quit;
-
/* Nonzero means that strings with character values >0x7F should be printed
as octal escapes. Zero means just print the value (e.g. it's an
international character, and the terminal or window can cope.) */
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 22/30] Use target_terminal_ours_for_output in infcmd.c
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (10 preceding siblings ...)
2016-03-18 19:19 ` [PATCH 29/30] Eliminate immediate_quit Pedro Alves
@ 2016-03-18 19:19 ` Pedro Alves
2016-03-18 19:19 ` [PATCH 02/30] Inline command_loop in read_command_line Pedro Alves
` (18 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:19 UTC (permalink / raw)
To: gdb-patches
We're only doing output here, so leave raw/cooked mode alone, as well
as the SIGINT handler.
No need to restore terminal settings, we'll set inferior modes on the
following resume.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* infcmd.c (post_create_inferior, prepare_one_step): Use
target_terminal_ours_for_output instead of target_terminal_ours.
---
gdb/infcmd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 3a0265f..a80b4c6 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -406,7 +406,7 @@ post_create_inferior (struct target_ops *target, int from_tty)
{
/* Be sure we own the terminal in case write operations are performed. */
- target_terminal_ours ();
+ target_terminal_ours_for_output ();
/* If the target hasn't taken care of this already, do it now.
Targets which need to access registers during to_open,
@@ -1128,7 +1128,7 @@ prepare_one_step (struct step_command_fsm *sm)
&tp->control.step_range_end) == 0)
error (_("Cannot find bounds of current function"));
- target_terminal_ours ();
+ target_terminal_ours_for_output ();
printf_filtered (_("Single stepping until exit from function %s,"
"\nwhich has no line number information.\n"),
name);
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 02/30] Inline command_loop in read_command_line
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (11 preceding siblings ...)
2016-03-18 19:19 ` [PATCH 22/30] Use target_terminal_ours_for_output in infcmd.c Pedro Alves
@ 2016-03-18 19:19 ` Pedro Alves
2016-03-18 19:24 ` [PATCH 23/30] Use target_terminal_ours_for_output in warning/internal_error Pedro Alves
` (17 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:19 UTC (permalink / raw)
To: gdb-patches
read_command_line is the only caller, and here we can assume we're
reading a regular file, not stdin.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* top.c (read_command_file): Inline command_loop here.
(command_loop): Delete.
---
gdb/top.c | 35 +++++++++++++++--------------------
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/gdb/top.c b/gdb/top.c
index 90a3f48..41ff6b2 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -283,7 +283,21 @@ read_command_file (FILE *stream)
cleanups = make_cleanup (do_restore_instream_cleanup, instream);
instream = stream;
- command_loop ();
+
+ /* Read commands from `instream' and execute them until end of file
+ or error reading instream. */
+
+ while (instream != NULL && !feof (instream))
+ {
+ char *command;
+
+ /* Get a command-line. This calls the readline package. */
+ command = command_line_input (NULL, 0, NULL);
+ if (command == NULL)
+ break;
+ command_handler (command);
+ }
+
do_cleanups (cleanups);
}
\f
@@ -528,25 +542,6 @@ execute_command_to_string (char *p, int from_tty)
return retval;
}
-/* Read commands from `instream' and execute them
- until end of file or error reading instream. */
-
-void
-command_loop (void)
-{
- while (instream && !feof (instream))
- {
- char *command;
-
- /* Get a command-line. This calls the readline package. */
- command = command_line_input (instream == stdin ?
- get_prompt () : (char *) NULL,
- instream == stdin, "prompt");
- if (command == NULL)
- return;
- command_handler (command);
- }
-}
\f
/* When nonzero, cause dont_repeat to do nothing. This should only be
set via prevent_dont_repeat. */
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 23/30] Use target_terminal_ours_for_output in warning/internal_error
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (12 preceding siblings ...)
2016-03-18 19:19 ` [PATCH 02/30] Inline command_loop in read_command_line Pedro Alves
@ 2016-03-18 19:24 ` Pedro Alves
2016-03-18 19:24 ` [PATCH 25/30] Do target_terminal_ours in query & friends instead of in all callers Pedro Alves
` (16 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:24 UTC (permalink / raw)
To: gdb-patches
We're only doing output here, so leave raw/cooked mode alone, as well
as the SIGINT handler.
And restore terminal settings, while at it.
gdb/ChangeLog:
YYYY-MM-DD Pedro Alves <palves@redhat.com>
* utils.c (vwarning, internal_vproblem): Use
make_cleanup_restore_target_terminal and
target_terminal_ours_for_output.
---
gdb/utils.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/gdb/utils.c b/gdb/utils.c
index a909b38..4d81c23 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -503,8 +503,13 @@ vwarning (const char *string, va_list args)
(*deprecated_warning_hook) (string, args);
else
{
+ struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
+
if (target_supports_terminal_ours ())
- target_terminal_ours ();
+ {
+ make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
+ }
if (filtered_printing_initialized ())
wrap_here (""); /* Force out any buffered output. */
gdb_flush (gdb_stdout);
@@ -512,6 +517,8 @@ vwarning (const char *string, va_list args)
fputs_unfiltered (warning_pre_print, gdb_stderr);
vfprintf_unfiltered (gdb_stderr, string, args);
fprintf_unfiltered (gdb_stderr, "\n");
+
+ do_cleanups (old_chain);
}
}
@@ -709,7 +716,10 @@ internal_vproblem (struct internal_problem *problem,
/* Try to get the message out and at the start of a new line. */
if (target_supports_terminal_ours ())
- target_terminal_ours ();
+ {
+ make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
+ }
if (filtered_printing_initialized ())
begin_line ();
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 25/30] Do target_terminal_ours in query & friends instead of in all callers
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (13 preceding siblings ...)
2016-03-18 19:24 ` [PATCH 23/30] Use target_terminal_ours_for_output in warning/internal_error Pedro Alves
@ 2016-03-18 19:24 ` Pedro Alves
2016-03-18 19:24 ` [PATCH 17/30] Pass Ctrl-C to the target in target_terminal_inferior Pedro Alves
` (15 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:24 UTC (permalink / raw)
To: gdb-patches
Any time a caller calls query & friends / prompt_for_continue without
ensuring that gdb owns the terminal for input is a bug. So do that in
defaulted_query / prompt_for_continue directly instead.
An example of a case where we currently miss calling
target_terminal_ours is internal_error. Ever since defaulted_query
was made to use gdb_readline_callback, there's no way to answer the
internal error query if the internal error happens while the target is
has the terminal:
(gdb) c
Continuing.
.../src/gdb/linux-nat.c:1676: internal-error: linux_nat_resume: Assertion `dummy_counter < 10' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) _
Entering 'y' or 'n' does not work, GDB does not respond.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
PR gdb/19828
* gnu-nat.c (inf_validate_task_sc): Don't call
target_terminal_ours / target_terminal_inferior around query.
* i386-tdep.c (i386_record_lea_modrm, i386_process_record): Don't
call target_terminal_ours / target_terminal_inferior around
yquery.
* linux-record.c (record_linux_system_call): Don't call
target_terminal_ours / target_terminal_inferior around yquery.
* nto-procfs.c (interrupt_query): Don't call target_terminal_ours
/ target_terminal_inferior around query.
* record-full.c (record_full_check_insn_num): Remove
'set_terminal' parameter. Don't call target_terminal_ours /
target_terminal_inferior around query.
(record_full_message, record_full_registers_change)
(record_full_xfer_partial): Adjust.
* remote.c (interrupt_query): Don't call target_terminal_ours /
target_terminal_inferior around query.
* utils.c (defaulted_query): Install cleanup to restore target
terminal. Put target_terminal_ours_for_output in effect while
defaulted producing, and target_terminal_ours in in effect while
handling input.
(prompt_for_continue): Install cleanup to restore target terminal.
Put target_terminal_ours in in effect while handling input.
---
gdb/gnu-nat.c | 12 +++---------
gdb/i386-tdep.c | 49 ++++++++++++-------------------------------------
gdb/linux-record.c | 54 ++++++++++++++----------------------------------------
gdb/nto-procfs.c | 4 ----
gdb/record-full.c | 22 +++++++---------------
gdb/remote.c | 6 ------
gdb/utils.c | 16 +++++++++++++---
7 files changed, 49 insertions(+), 114 deletions(-)
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index b0b870c..c268732 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -852,15 +852,9 @@ inf_validate_task_sc (struct inf *inf)
if (inf->task->cur_sc < suspend_count)
{
- int abort;
-
- target_terminal_ours (); /* Allow I/O. */
- abort = !query (_("Pid %d has an additional task suspend count of %d;"
- " clear it? "), inf->pid,
- suspend_count - inf->task->cur_sc);
- target_terminal_inferior (); /* Give it back to the child. */
-
- if (abort)
+ if (!query (_("Pid %d has an additional task suspend count of %d;"
+ " clear it? "), inf->pid,
+ suspend_count - inf->task->cur_sc))
error (_("Additional task suspend count left untouched."));
inf->task->cur_sc = suspend_count;
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 4c66edf..a328c18 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -4914,17 +4914,12 @@ i386_record_lea_modrm (struct i386_record_s *irp)
{
if (record_full_memory_query)
{
- int q;
-
- target_terminal_ours ();
- q = yquery (_("\
+ if (yquery (_("\
Process record ignores the memory change of instruction at address %s\n\
because it can't get the value of the segment register.\n\
Do you want to stop the program?"),
- paddress (gdbarch, irp->orig_addr));
- target_terminal_inferior ();
- if (q)
- return -1;
+ paddress (gdbarch, irp->orig_addr)))
+ return -1;
}
return 0;
@@ -5805,16 +5800,11 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
{
if (record_full_memory_query)
{
- int q;
-
- target_terminal_ours ();
- q = yquery (_("\
+ if (yquery (_("\
Process record ignores the memory change of instruction at address %s\n\
because it can't get the value of the segment register.\n\
Do you want to stop the program?"),
- paddress (gdbarch, ir.orig_addr));
- target_terminal_inferior ();
- if (q)
+ paddress (gdbarch, ir.orig_addr)))
return -1;
}
}
@@ -6479,16 +6469,11 @@ Do you want to stop the program?"),
/* addr += ((uint32_t) read_register (I386_ES_REGNUM)) << 4; */
if (record_full_memory_query)
{
- int q;
-
- target_terminal_ours ();
- q = yquery (_("\
+ if (yquery (_("\
Process record ignores the memory change of instruction at address %s\n\
because it can't get the value of the segment register.\n\
Do you want to stop the program?"),
- paddress (gdbarch, ir.orig_addr));
- target_terminal_inferior ();
- if (q)
+ paddress (gdbarch, ir.orig_addr)))
return -1;
}
}
@@ -7034,17 +7019,12 @@ Do you want to stop the program?"),
{
if (record_full_memory_query)
{
- int q;
-
- target_terminal_ours ();
- q = yquery (_("\
+ if (yquery (_("\
Process record ignores the memory change of instruction at address %s\n\
because it can't get the value of the segment register.\n\
Do you want to stop the program?"),
- paddress (gdbarch, ir.orig_addr));
- target_terminal_inferior ();
- if (q)
- return -1;
+ paddress (gdbarch, ir.orig_addr)))
+ return -1;
}
}
else
@@ -7091,16 +7071,11 @@ Do you want to stop the program?"),
{
if (record_full_memory_query)
{
- int q;
-
- target_terminal_ours ();
- q = yquery (_("\
+ if (yquery (_("\
Process record ignores the memory change of instruction at address %s\n\
because it can't get the value of the segment register.\n\
Do you want to stop the program?"),
- paddress (gdbarch, ir.orig_addr));
- target_terminal_inferior ();
- if (q)
+ paddress (gdbarch, ir.orig_addr)))
return -1;
}
}
diff --git a/gdb/linux-record.c b/gdb/linux-record.c
index bf20419..fda7ada 100644
--- a/gdb/linux-record.c
+++ b/gdb/linux-record.c
@@ -254,17 +254,10 @@ record_linux_system_call (enum gdb_syscall syscall,
break;
case gdb_sys_exit:
- {
- int q;
-
- target_terminal_ours ();
- q = yquery (_("The next instruction is syscall exit. "
- "It will make the program exit. "
- "Do you want to stop the program?"));
- target_terminal_inferior ();
- if (q)
- return 1;
- }
+ if (yquery (_("The next instruction is syscall exit. "
+ "It will make the program exit. "
+ "Do you want to stop the program?")))
+ return 1;
break;
case gdb_sys_fork:
@@ -663,17 +656,10 @@ record_linux_system_call (enum gdb_syscall syscall,
break;
case gdb_sys_reboot:
- {
- int q;
-
- target_terminal_ours ();
- q = yquery (_("The next instruction is syscall reboot. "
- "It will restart the computer. "
- "Do you want to stop the program?"));
- target_terminal_inferior ();
- if (q)
- return 1;
- }
+ if (yquery (_("The next instruction is syscall reboot. "
+ "It will restart the computer. "
+ "Do you want to stop the program?")))
+ return 1;
break;
case gdb_old_readdir:
@@ -693,17 +679,12 @@ record_linux_system_call (enum gdb_syscall syscall,
regcache_raw_read_unsigned (regcache, tdep->arg2, &len);
if (record_full_memory_query)
{
- int q;
-
- target_terminal_ours ();
- q = yquery (_("\
+ if (yquery (_("\
The next instruction is syscall munmap.\n\
It will free the memory addr = 0x%s len = %u.\n\
It will make record target cannot record some memory change.\n\
Do you want to stop the program?"),
- OUTPUT_REG (tmpulongest, tdep->arg1), (int) len);
- target_terminal_inferior ();
- if (q)
+ OUTPUT_REG (tmpulongest, tdep->arg1), (int) len))
return 1;
}
}
@@ -1764,17 +1745,10 @@ Do you want to stop the program?"),
break;
case gdb_sys_exit_group:
- {
- int q;
-
- target_terminal_ours ();
- q = yquery (_("The next instruction is syscall exit_group. "
- "It will make the program exit. "
- "Do you want to stop the program?"));
- target_terminal_inferior ();
- if (q)
- return 1;
- }
+ if (yquery (_("The next instruction is syscall exit_group. "
+ "It will make the program exit. "
+ "Do you want to stop the program?")))
+ return 1;
break;
case gdb_sys_lookup_dcookie:
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 992de29..eb7dcfe 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -723,16 +723,12 @@ do_attach (ptid_t ptid)
static void
interrupt_query (void)
{
- target_terminal_ours ();
-
if (query (_("Interrupted while waiting for the program.\n\
Give up (and stop debugging it)? ")))
{
target_mourn_inferior ();
quit ();
}
-
- target_terminal_inferior ();
}
/* The user typed ^C twice. */
diff --git a/gdb/record-full.c b/gdb/record-full.c
index f6023bf..aec41ad 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -535,26 +535,18 @@ record_full_arch_list_add_end (void)
}
static void
-record_full_check_insn_num (int set_terminal)
+record_full_check_insn_num (void)
{
if (record_full_insn_num == record_full_insn_max_num)
{
/* Ask user what to do. */
if (record_full_stop_at_limit)
{
- int q;
-
- if (set_terminal)
- target_terminal_ours ();
- q = yquery (_("Do you want to auto delete previous execution "
+ if (!yquery (_("Do you want to auto delete previous execution "
"log entries when record/replay buffer becomes "
- "full (record full stop-at-limit)?"));
- if (set_terminal)
- target_terminal_inferior ();
- if (q)
- record_full_stop_at_limit = 0;
- else
+ "full (record full stop-at-limit)?")))
error (_("Process record: stopped by user."));
+ record_full_stop_at_limit = 0;
}
}
}
@@ -583,7 +575,7 @@ record_full_message (struct regcache *regcache, enum gdb_signal signal)
record_full_arch_list_tail = NULL;
/* Check record_full_insn_num. */
- record_full_check_insn_num (1);
+ record_full_check_insn_num ();
/* If gdb sends a signal value to target_resume,
save it in the 'end' field of the previous instruction.
@@ -1420,7 +1412,7 @@ static void
record_full_registers_change (struct regcache *regcache, int regnum)
{
/* Check record_full_insn_num. */
- record_full_check_insn_num (0);
+ record_full_check_insn_num ();
record_full_arch_list_head = NULL;
record_full_arch_list_tail = NULL;
@@ -1546,7 +1538,7 @@ record_full_xfer_partial (struct target_ops *ops, enum target_object object,
}
/* Check record_full_insn_num */
- record_full_check_insn_num (0);
+ record_full_check_insn_num ();
/* Record registers change to list as an instruction. */
record_full_arch_list_head = NULL;
diff --git a/gdb/remote.c b/gdb/remote.c
index 3c8de40..2aa4e07 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5900,10 +5900,6 @@ static void
interrupt_query (void)
{
struct remote_state *rs = get_remote_state ();
- struct cleanup *old_chain;
-
- old_chain = make_cleanup_restore_target_terminal ();
- target_terminal_ours ();
if (rs->waiting_for_stop_reply && rs->ctrlc_pending_p)
{
@@ -5920,8 +5916,6 @@ interrupt_query (void)
"Give up waiting? ")))
quit ();
}
-
- do_cleanups (old_chain);
}
/* Enable/disable target terminal ownership. Most targets can use
diff --git a/gdb/utils.c b/gdb/utils.c
index 46f61c1..46765c5 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1245,12 +1245,15 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
if (!confirm || server_command)
return def_value;
+ old_chain = make_cleanup_restore_target_terminal ();
+
/* If input isn't coming from the user directly, just say what
question we're asking, and then answer the default automatically. This
way, important error messages don't get lost when talking to GDB
over a pipe. */
if (! input_from_terminal_p ())
{
+ target_terminal_ours_for_output ();
wrap_here ("");
vfprintf_filtered (gdb_stdout, ctlstr, args);
@@ -1259,15 +1262,18 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
y_string, n_string, def_answer);
gdb_flush (gdb_stdout);
+ do_cleanups (old_chain);
return def_value;
}
if (deprecated_query_hook)
{
- return deprecated_query_hook (ctlstr, args);
- }
+ int res;
- old_chain = make_cleanup (null_cleanup, NULL);
+ res = deprecated_query_hook (ctlstr, args);
+ do_cleanups (old_chain);
+ return res;
+ }
/* Format the question outside of the loop, to avoid reusing args. */
question = xstrvprintf (ctlstr, args);
@@ -1281,6 +1287,9 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
/* Used for calculating time spend waiting for user. */
gettimeofday (&prompt_started, NULL);
+ /* We'll need to handle input. */
+ target_terminal_ours ();
+
while (1)
{
char *response, answer;
@@ -1852,6 +1861,7 @@ prompt_for_continue (void)
reinitialize_more_filter ();
/* We'll need to handle input. */
+ make_cleanup_restore_target_terminal ();
target_terminal_ours ();
/* Call gdb_readline_wrapper, not readline, in order to keep an
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 17/30] Pass Ctrl-C to the target in target_terminal_inferior
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (14 preceding siblings ...)
2016-03-18 19:24 ` [PATCH 25/30] Do target_terminal_ours in query & friends instead of in all callers Pedro Alves
@ 2016-03-18 19:24 ` Pedro Alves
2016-03-18 19:25 ` [PATCH 12/30] Don't call clear_quit_flag in command_handler Pedro Alves
` (14 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:24 UTC (permalink / raw)
To: gdb-patches
If the user presses Ctrl-C immediately before target_terminal_inferior
is called and the target is resumed, instead of after, the Ctrl-C ends
up pending in the quit flag until the target next stops.
remote.c has this bit to handle this:
if (!target_is_async_p ())
{
ofunc = signal (SIGINT, sync_remote_interrupt);
/* If the user hit C-c before this packet, or between packets,
pretend that it was hit right here. */
if (check_quit_flag ())
sync_remote_interrupt (SIGINT);
}
But that's only reachable if async is off, while async is on by
default nowadays. It's also obviously not reacheable on native
targets.
This patch generalizes that to all targets.
We can't remove that remote.c bit yet, until we get rid of the sync
SIGINT handler though. That'll be done later in the series.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* remote.c (remote_pass_ctrlc): New function.
(init_remote_ops): Install it.
* target.c (target_terminal_inferior): Pass pending Ctrl-C to the
target.
(target_pass_ctrlc, default_target_pass_ctrlc): New functions.
* target.h (struct target_ops) <to_pass_ctrlc>: New method.
(target_pass_ctrlc, default_target_pass_ctrlc): New declarations.
* target-delegates.c: Regenerate.
---
gdb/remote.c | 22 ++++++++++++++++++++++
gdb/target-delegates.c | 21 +++++++++++++++++++++
gdb/target.c | 21 +++++++++++++++++++++
gdb/target.h | 13 +++++++++++++
4 files changed, 77 insertions(+)
diff --git a/gdb/remote.c b/gdb/remote.c
index f932455..3c8de40 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5873,6 +5873,27 @@ remote_interrupt (struct target_ops *self, ptid_t ptid)
remote_interrupt_as ();
}
+/* Implement the to_pass_ctrlc function for the remote targets. */
+
+static void
+remote_pass_ctrlc (struct target_ops *self)
+{
+ struct remote_state *rs = get_remote_state ();
+
+ if (remote_debug)
+ fprintf_unfiltered (gdb_stdlog, "remote_pass_ctrlc called\n");
+
+ /* If we're starting up, we're not fully synced yet. Quit
+ immediately. */
+ if (rs->starting_up)
+ quit ();
+ /* If ^C has already been sent once, offer to disconnect. */
+ else if (rs->ctrlc_pending_p)
+ interrupt_query ();
+ else
+ target_interrupt (inferior_ptid);
+}
+
/* Ask the user what to do when an interrupt is received. */
static void
@@ -13054,6 +13075,7 @@ Specify the serial device it is connected to\n\
remote_ops.to_get_ada_task_ptid = remote_get_ada_task_ptid;
remote_ops.to_stop = remote_stop;
remote_ops.to_interrupt = remote_interrupt;
+ remote_ops.to_pass_ctrlc = remote_pass_ctrlc;
remote_ops.to_check_pending_interrupt = remote_check_pending_interrupt;
remote_ops.to_xfer_partial = remote_xfer_partial;
remote_ops.to_rcmd = remote_rcmd;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index d23bc75..640803a 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -1608,6 +1608,23 @@ debug_interrupt (struct target_ops *self, ptid_t arg1)
}
static void
+delegate_pass_ctrlc (struct target_ops *self)
+{
+ self = self->beneath;
+ self->to_pass_ctrlc (self);
+}
+
+static void
+debug_pass_ctrlc (struct target_ops *self)
+{
+ fprintf_unfiltered (gdb_stdlog, "-> %s->to_pass_ctrlc (...)\n", debug_target.to_shortname);
+ debug_target.to_pass_ctrlc (&debug_target);
+ fprintf_unfiltered (gdb_stdlog, "<- %s->to_pass_ctrlc (", debug_target.to_shortname);
+ target_debug_print_struct_target_ops_p (&debug_target);
+ fputs_unfiltered (")\n", gdb_stdlog);
+}
+
+static void
delegate_check_pending_interrupt (struct target_ops *self)
{
self = self->beneath;
@@ -4194,6 +4211,8 @@ install_delegators (struct target_ops *ops)
ops->to_stop = delegate_stop;
if (ops->to_interrupt == NULL)
ops->to_interrupt = delegate_interrupt;
+ if (ops->to_pass_ctrlc == NULL)
+ ops->to_pass_ctrlc = delegate_pass_ctrlc;
if (ops->to_check_pending_interrupt == NULL)
ops->to_check_pending_interrupt = delegate_check_pending_interrupt;
if (ops->to_rcmd == NULL)
@@ -4442,6 +4461,7 @@ install_dummy_methods (struct target_ops *ops)
ops->to_thread_name = tdefault_thread_name;
ops->to_stop = tdefault_stop;
ops->to_interrupt = tdefault_interrupt;
+ ops->to_pass_ctrlc = default_target_pass_ctrlc;
ops->to_check_pending_interrupt = tdefault_check_pending_interrupt;
ops->to_rcmd = default_rcmd;
ops->to_pid_to_exec_file = tdefault_pid_to_exec_file;
@@ -4598,6 +4618,7 @@ init_debug_target (struct target_ops *ops)
ops->to_thread_name = debug_thread_name;
ops->to_stop = debug_stop;
ops->to_interrupt = debug_interrupt;
+ ops->to_pass_ctrlc = debug_pass_ctrlc;
ops->to_check_pending_interrupt = debug_check_pending_interrupt;
ops->to_rcmd = debug_rcmd;
ops->to_pid_to_exec_file = debug_pid_to_exec_file;
diff --git a/gdb/target.c b/gdb/target.c
index ac66a3a..d580983 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -491,6 +491,11 @@ target_terminal_inferior (void)
inferior's terminal modes. */
(*current_target.to_terminal_inferior) (¤t_target);
terminal_state = terminal_is_inferior;
+
+ /* If the user hit C-c before, pretend that it was hit right
+ here. */
+ if (check_quit_flag ())
+ target_pass_ctrlc ();
}
/* See target.h. */
@@ -3360,6 +3365,22 @@ target_interrupt (ptid_t ptid)
/* See target.h. */
void
+target_pass_ctrlc (void)
+{
+ (*current_target.to_pass_ctrlc) (¤t_target);
+}
+
+/* See target.h. */
+
+void
+default_target_pass_ctrlc (struct target_ops *ops)
+{
+ target_interrupt (inferior_ptid);
+}
+
+/* See target.h. */
+
+void
target_check_pending_interrupt (void)
{
(*current_target.to_check_pending_interrupt) (¤t_target);
diff --git a/gdb/target.h b/gdb/target.h
index 26c8579..00625fec 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -645,6 +645,8 @@ struct target_ops
TARGET_DEFAULT_IGNORE ();
void (*to_interrupt) (struct target_ops *, ptid_t)
TARGET_DEFAULT_IGNORE ();
+ void (*to_pass_ctrlc) (struct target_ops *)
+ TARGET_DEFAULT_FUNC (default_target_pass_ctrlc);
void (*to_check_pending_interrupt) (struct target_ops *)
TARGET_DEFAULT_IGNORE ();
void (*to_rcmd) (struct target_ops *,
@@ -1716,6 +1718,17 @@ extern void target_stop (ptid_t ptid);
extern void target_interrupt (ptid_t ptid);
+/* Pass a ^C, as determined to have been pressed by checking the quit
+ flag, to the target. Normally calls target_interrupt, but remote
+ targets may take the opportunity to detect the remote side is not
+ responding and offer to disconnect. */
+
+extern void target_pass_ctrlc (void);
+
+/* The default target_ops::to_pass_ctrlc implementation. Simply calls
+ target_interrupt. */
+extern void default_target_pass_ctrlc (struct target_ops *ops);
+
/* Some targets install their own SIGINT handler while the target is
running. This method is called from the QUIT macro to give such
targets a chance to process a Ctrl-C. The target may e.g., choose
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 12/30] Don't call clear_quit_flag in command_handler
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (15 preceding siblings ...)
2016-03-18 19:24 ` [PATCH 17/30] Pass Ctrl-C to the target in target_terminal_inferior Pedro Alves
@ 2016-03-18 19:25 ` Pedro Alves
2016-03-18 19:25 ` [PATCH 13/30] Don't call clear_quit_flag in prepare_to_throw_exception Pedro Alves
` (13 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:25 UTC (permalink / raw)
To: gdb-patches
This just looks totally wrong to me, for completetly discarding a
user-requested Ctrl-C. I can't think of why we'd want do this here.
Actually, I digged the history, and found out that this has been here
since at least 7b4ac7e1ed2c (gdb-2.4, the initial revision, 1988), at
a time were we had a top level setjmp/longjmp, long before that got
wrapped in throw_exception and friends, and this code was in an
explicit loop, with the quit_flag cleared on every iteration, before
executing a command...
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* event-top.c (command_handler): Don't call clear_quit_flag.
---
gdb/event-top.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/gdb/event-top.c b/gdb/event-top.c
index da72b1d..7fedb48 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -459,7 +459,6 @@ command_handler (char *command)
struct cleanup *stat_chain;
char *c;
- clear_quit_flag ();
if (instream == stdin)
reinitialize_more_filter ();
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 13/30] Don't call clear_quit_flag in prepare_to_throw_exception
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (16 preceding siblings ...)
2016-03-18 19:25 ` [PATCH 12/30] Don't call clear_quit_flag in command_handler Pedro Alves
@ 2016-03-18 19:25 ` Pedro Alves
2016-03-18 19:25 ` [PATCH 04/30] Don't set immediate_quit in prompt_for_continue Pedro Alves
` (12 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:25 UTC (permalink / raw)
To: gdb-patches
I think this is reminiscent of the time when a longjmp would always
jump to the top level. Nowaways code that throw exceptions other than
a quit, which may even be caught and handled without reaching the top
level. Certainly such exceptions shouldn't clear an interrupt
request...
(We also need to get rid of prepare_to_throw_exception in order to be
able to just do "throw ex;" in C++.)
One could argue that we should clear the quit flag when we throw a
quit from the SIGINT handler, when immediate_quit is in effect, to
handle a race, here:
immediate_quit++;
QUIT;
... that's the usual pattern code must use when enabling
immediate_quit. The QUIT is there to catch the case of Ctrl-C having
already been pressed before immediate_quit was enabled. However, this
can happen:
immediate_quit++;
<< Ctrl-C pressed here too.
QUIT;
And in that case, if the quit flag was already set, it'll stay set
even after throwing a quit from the SIGINT handler. The end result is
a double quit. But OTOH, the user did press Ctrl-C two times. Since
I'm getting rid of immediate_quit, I'm not bothering with this.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* exceptions.c (prepare_to_throw_exception): Don't clear the quit
flag.
---
gdb/exceptions.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index 8ba86fc..b457838 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -30,7 +30,6 @@
void
prepare_to_throw_exception (void)
{
- clear_quit_flag ();
immediate_quit = 0;
}
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 04/30] Don't set immediate_quit in prompt_for_continue
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (17 preceding siblings ...)
2016-03-18 19:25 ` [PATCH 13/30] Don't call clear_quit_flag in prepare_to_throw_exception Pedro Alves
@ 2016-03-18 19:25 ` Pedro Alves
2016-03-18 19:25 ` [PATCH 08/30] Fix signal handler/event-loop races Pedro Alves
` (11 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:25 UTC (permalink / raw)
To: gdb-patches
immediate_quit used to be necessary back when prompt_for_continue used
blocking fread, but nowadays it uses gdb_readline_wrapper, which is
implemented in terms of a nested event loop, which already knows how
to react to SIGINT:
#0 throw_it (reason=RETURN_QUIT, error=GDB_NO_ERROR, fmt=0x9d6d7e "Quit", ap=0x7fffffffcb88)
at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/common/common-exceptions.c:324
#1 0x00000000007bab5d in throw_vquit (fmt=0x9d6d7e "Quit", ap=0x7fffffffcb88) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/common/common-exceptions.c:366
#2 0x00000000007bac9f in throw_quit (fmt=0x9d6d7e "Quit") at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/common/common-exceptions.c:385
#3 0x0000000000773a2d in quit () at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/utils.c:1039
#4 0x000000000065d81b in async_request_quit (arg=0x0) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-top.c:893
#5 0x000000000065c27b in invoke_async_signal_handlers () at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-loop.c:949
#6 0x000000000065aeef in gdb_do_one_event () at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-loop.c:280
#7 0x0000000000770838 in gdb_readline_wrapper (prompt=0x7fffffffcd40 "---Type <return> to continue, or q <return> to quit---")
at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/top.c:873
The need for the QUIT in stdin_event_handler is then exposed by the
gdb.base/double-prompt-target-event-error.exp test, which has:
# We're now stopped in a pagination query while handling a
# target event (printing where the program stopped). Quitting
# the pagination should result in only one prompt being
# output.
send_gdb "\003p 1\n"
Without that change we'd get:
Continuing.
---Type <return> to continue, or q <return> to quit---PASS: gdb.base/double-prompt-target-event-error.exp: ctrlc target event: continue: continue to pagination
^CpQuit
(gdb) 1
Undefined command: "1". Try "help".
(gdb) PASS: gdb.base/double-prompt-target-event-error.exp: ctrlc target event: continue: first prompt
ERROR: Undefined command "".
UNRESOLVED: gdb.base/double-prompt-target-event-error.exp: ctrlc target event: continue: no double prompt
Vs:
Continuing.
---Type <return> to continue, or q <return> to quit---PASS: gdb.base/double-prompt-target-event-error.exp: ctrlc target event: continue: continue to pagination
^CQuit
(gdb) p 1
$1 = 1
(gdb) PASS: gdb.base/double-prompt-target-event-error.exp: ctrlc target event: continue: first prompt
PASS: gdb.base/double-prompt-target-event-error.exp: ctrlc target event: continue: no double prompt
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* event-top.c (stdin_event_handler): Call QUIT;
(prompt_for_continue): Don't run with immediate_quit set.
---
gdb/event-top.c | 8 ++++++++
gdb/utils.c | 4 ----
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/gdb/event-top.c b/gdb/event-top.c
index eb4f0b9..769485f 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -403,6 +403,14 @@ stdin_event_handler (int error, gdb_client_data client_data)
}
else
{
+ /* This makes sure a ^C immediately followed by further input is
+ always processed in that order. E.g,. with input like "^Cprint
+ 1\n", the SIGINT handler runs, marks the async signal handler,
+ and then select/poll may return with stdin ready, instead of
+ -1/EINTR. The gdb.base/double-prompt-target-event-error.exp
+ test exercises this. */
+ QUIT;
+
do
{
call_stdin_event_handler_again_p = 0;
diff --git a/gdb/utils.c b/gdb/utils.c
index 97e5133..a909b38 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1836,9 +1836,6 @@ prompt_for_continue (void)
beyond the end of the screen. */
reinitialize_more_filter ();
- immediate_quit++;
- QUIT;
-
/* We'll need to handle input. */
target_terminal_ours ();
@@ -1866,7 +1863,6 @@ prompt_for_continue (void)
throw_quit ("Quit");
xfree (ignore);
}
- immediate_quit--;
/* Now we have to do this again, so that GDB will know that it doesn't
need to save the ---Type <return>--- line at the top of the screen. */
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 08/30] Fix signal handler/event-loop races
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (18 preceding siblings ...)
2016-03-18 19:25 ` [PATCH 04/30] Don't set immediate_quit in prompt_for_continue Pedro Alves
@ 2016-03-18 19:25 ` Pedro Alves
2016-03-18 19:26 ` [PATCH 07/30] Introduce a serial interface for select'able events Pedro Alves
` (10 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:25 UTC (permalink / raw)
To: gdb-patches
GDB's core signal handling suffers from a classical signal handler /
mainline code race:
int
gdb_do_one_event (void)
{
...
/* First let's see if there are any asynchronous signal handlers
that are ready. These would be the result of invoking any of the
signal handlers. */
if (invoke_async_signal_handlers ())
return 1;
...
/* Block waiting for a new event. (...). */
if (gdb_wait_for_event (1) < 0)
return -1;
...
}
If a signal is delivered while gdb is blocked in the poll/select
inside gdb_wait_for_event, then the select/poll breaks with EINTR,
we'll loop back around and call invoke_async_signal_handlers.
However, if the signal handler runs between
invoke_async_signal_handlers and gdb_wait_for_event,
gdb_wait_for_event will block, until the next unrelated event...
The fix is to a struct serial_event, and register it in the set of
files that select/poll in gdb_wait_for_event waits on. The signal
handlers that defer work to invoke_async_signal_handlers call
mark_async_signal_handler, which is adjusted to also set the new
serial event in addition to setting a flag, and is thus now is
garanteed to immediately unblock the next gdb_select/poll call, up
until invoke_async_signal_handlers is called and the event is cleared.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* event-loop.c: Include "ser-event.h".
(async_signal_handlers_serial_event): New global.
(async_signals_handler, initialize_async_signal_handlers): New
functions.
(mark_async_signal_handler): Set
async_signal_handlers_serial_event.
(invoke_async_signal_handlers): Clear
async_signal_handlers_serial_event.
* event-top.c (async_init_signals): Call
initialize_async_signal_handlers.
---
gdb/event-loop.c | 32 +++++++++++++++++++++++++++++++-
gdb/event-loop.h | 2 ++
gdb/event-top.c | 2 ++
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/gdb/event-loop.c b/gdb/event-loop.c
index 0e1cb2b..052d535 100644
--- a/gdb/event-loop.c
+++ b/gdb/event-loop.c
@@ -21,6 +21,7 @@
#include "event-loop.h"
#include "event-top.h"
#include "queue.h"
+#include "ser-event.h"
#ifdef HAVE_POLL
#if defined (HAVE_POLL_H)
@@ -262,6 +263,28 @@ static int update_wait_timeout (void);
static int poll_timers (void);
\f
+/* This event is signalled whenever an asynchronous handler needs to
+ defer an action to the event loop. */
+static struct serial_event *async_signal_handlers_serial_event;
+
+/* Callback registered with ASYNC_SIGNAL_HANDLERS_SERIAL_EVENT. */
+
+static void
+async_signals_handler (int error, gdb_client_data client_data)
+{
+ /* Do nothing. Handlers are run by invoke_async_signal_handlers
+ from instead. */
+}
+
+void
+initialize_async_signal_handlers (void)
+{
+ async_signal_handlers_serial_event = make_serial_event ();
+
+ add_file_handler (serial_event_fd (async_signal_handlers_serial_event),
+ async_signals_handler, NULL);
+}
+
/* Process one high level event. If nothing is ready at this time,
wait for something to happen (via gdb_wait_for_event), then process
it. Returns >0 if something was done otherwise returns <0 (this
@@ -905,6 +928,7 @@ void
mark_async_signal_handler (async_signal_handler * async_handler_ptr)
{
async_handler_ptr->ready = 1;
+ serial_event_set (async_signal_handlers_serial_event);
}
/* See event-loop.h. */
@@ -925,13 +949,19 @@ async_signal_handler_is_marked (async_signal_handler *async_handler_ptr)
/* Call all the handlers that are ready. Returns true if any was
indeed ready. */
+
static int
invoke_async_signal_handlers (void)
{
async_signal_handler *async_handler_ptr;
int any_ready = 0;
- /* Invoke ready handlers. */
+ /* We're going to handle all pending signals, so no need to wake up
+ the event loop again the next time around. Note this must be
+ cleared _before_ calling the callbacks, to avoid races. */
+ serial_event_clear (async_signal_handlers_serial_event);
+
+ /* Invoke all ready handlers. */
while (1)
{
diff --git a/gdb/event-loop.h b/gdb/event-loop.h
index 1b765e1..155dafa 100644
--- a/gdb/event-loop.h
+++ b/gdb/event-loop.h
@@ -143,3 +143,5 @@ extern void mark_async_event_handler (struct async_event_handler *handler);
/* Mark the handler (ASYNC_HANDLER_PTR) as NOT ready. */
extern void clear_async_event_handler (struct async_event_handler *handler);
+
+extern void initialize_async_signal_handlers (void);
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 769485f..9fff2be 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -747,6 +747,8 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
void
async_init_signals (void)
{
+ initialize_async_signal_handlers ();
+
signal (SIGINT, handle_sigint);
sigint_token =
create_async_signal_handler (async_request_quit, NULL);
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 07/30] Introduce a serial interface for select'able events
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (19 preceding siblings ...)
2016-03-18 19:25 ` [PATCH 08/30] Fix signal handler/event-loop races Pedro Alves
@ 2016-03-18 19:26 ` Pedro Alves
2016-03-18 19:26 ` [PATCH 24/30] Add missing cleanups to defaulted_query and prompt_for_continue Pedro Alves
` (9 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:26 UTC (permalink / raw)
To: gdb-patches
This patch adds a new "event" struct serial type, that is an
abstraction specifically for waking up blocking waits/selects,
implemented on top of a pipe on POSIX, and on top of a native Windows
event (CreateEvent, etc.) on Windows.
This will be used to plug signal handler / mainline code races.
For example, GDB can indefinitely delay handling a quit request if the
user presses Ctrl-C between the last QUIT call and the next (blocking)
gdb_select call in the event loop:
QUIT;
<<< press ctrl-c here and end up blocked in gdb_select
indefinitely.
gdb_select (...); // whoops, SIGINT was already handled, no EINTR.
A global alone (either the quit flag, or the "ready" flag of the async
signal handlers in the event loop) is not sufficient.
To plug races such as these on POSIX systems, we have to register some
waitable file descriptor in the set of files gdb_select waits on, and
write to it from the signal handler. This is classically a pipe, and
the pattern called the self-pipe trick. On Linux, it could be a more
efficient eventfd instead, but I'm sticking with a pipe for
simplifity, as we need it for portability anyway.
(Alternatively, we could use pselect/ppoll, and block signals until
the pselect. The latter is not a design I think GDB could use,
because we want the QUIT macro to be super cheap, as it is used in
loops. Plus, Windows.)
This is a "struct serial" because Windows's gdb_select relies on that.
Windows's gdb_select, our "select" replacement, knows how to wait on
all kinds of handles (regular files, pipes, sockets, console, etc.)
unlike the native Windows "select" function, which can only wait on
sockets. Each file descriptor for a "serial" type that is not
normally waitable with WaitForMultipleObjects must have a
corresponding struct serial instance. gdb_select then internally
looks up the struct serial instance that wraps each file descriptor,
and asks it for the corresponding Windows waitable handle.
We could use serial_pipe() to create a "struct serial"-wrapped pipe
that is usable everywhere, including Windows. That's what currently
python/python.c uses for cross-thread posting of events.
However, serial_write and serial_readchar are not designed to be
async-signal-safe on POSIX hosts. It's easier to bypass those when
setting/clearing the event source.
And writing and a serial pipe is a bit heavy weight on Windows.
gdb_select requires an extra thread to wait on the pipe and several
Windows events, when a single manual-reset Windows event, with no
extra thread is sufficient.
The intended usage is simply:
- Call make_serial_event to create a serial event object.
- From the signal handler call serial_event_set to set the event.
- From mainline code, have select/poll wait for serial_event_fd(), in
addition to whatever other files you're about to wait for.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* Makefile.in (SFILES): Add ser-event.c.
(HFILES_NO_SRCDIR): Add ser-event.h.
(COMMON_OBS): Add ser-event.o.
* ser-event.c, ser-event.h: New files.
* serial.c (new_serial): New function, factored out from
(serial_fdopen_ops): ... this.
(serial_open_ops_1): New function, factored out from
(serial_open): ... this.
(serial_open_ops): New function.
* serial.h (struct serial): Forware declare.
(serial_open_ops): New declaration.
---
gdb/Makefile.in | 6 +-
gdb/ser-event.c | 220 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
gdb/ser-event.h | 51 +++++++++++++
gdb/serial.c | 61 ++++++++++------
gdb/serial.h | 7 ++
5 files changed, 320 insertions(+), 25 deletions(-)
create mode 100644 gdb/ser-event.c
create mode 100644 gdb/ser-event.h
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index ccd5c23..2af78a5 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -870,7 +870,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
prologue-value.c psymtab.c \
regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c reverse.c \
sentinel-frame.c \
- serial.c ser-base.c ser-unix.c skip.c \
+ serial.c ser-base.c ser-unix.c ser-event.c skip.c \
solib.c solib-target.c source.c \
stabsread.c stack.c probe.c stap-probe.c std-regs.c \
symfile.c symfile-debug.c symfile-mem.c symmisc.c symtab.c \
@@ -987,7 +987,7 @@ common/common-exceptions.h target/target.h common/symbol.h \
common/common-regcache.h fbsd-tdep.h nat/linux-personality.h \
common/fileio.h nat/x86-linux.h nat/x86-linux-dregs.h nat/amd64-linux-siginfo.h\
nat/linux-namespaces.h arch/arm.h common/gdb_sys_time.h arch/aarch64-insn.h \
-tid-parse.h
+tid-parse.h ser-event.h
# Header files that already have srcdir in them, or which are in objdir.
@@ -1065,7 +1065,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
ada-typeprint.o c-typeprint.o f-typeprint.o m2-typeprint.o \
ada-valprint.o c-valprint.o cp-valprint.o d-valprint.o f-valprint.o \
m2-valprint.o \
- serial.o mdebugread.o top.o utils.o \
+ ser-event.o serial.o mdebugread.o top.o utils.o \
ui-file.o \
user-regs.o \
frame.o frame-unwind.o doublest.o \
diff --git a/gdb/ser-event.c b/gdb/ser-event.c
new file mode 100644
index 0000000..4851672
--- /dev/null
+++ b/gdb/ser-event.c
@@ -0,0 +1,220 @@
+/* Serial interface for a selectable event.
+ Copyright (C) 2016 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 "ser-event.h"
+#include "serial.h"
+#include "common/filestuff.h"
+
+/* On POSIX hosts, a serial_event is basically an abstraction for the
+ classical self-pipe trick.
+
+ On Windows, a serial_event is a wrapper around a native Windows
+ event object. Because we want to interface with gdb_select, which
+ takes file descriptors, we need to wrap that Windows event object
+ in a file descriptor. As _open_osfhandle can not be used with
+ event objects, we instead create a dummy file wrap that in a file
+ descriptor with _open_osfhandle, and pass that as selectable
+ descriptor to callers. As Windows' gdb_select converts file
+ descriptors back to Windows handles by calling serial->wait_handle,
+ nothing ever actually waits on that file descriptor. */
+
+struct serial_event_state
+ {
+#ifdef USE_WIN32API
+ /* The Windows event object, created with CreateEvent. */
+ HANDLE event;
+#else
+ /* The write side of the pipe. The read side is in
+ serial->fd. */
+ int write_fd;
+#endif
+ };
+
+/* Open a new serial event. */
+
+static int
+serial_event_open (struct serial *scb, const char *name)
+{
+ struct serial_event_state *state;
+
+ state = XNEW (struct serial_event_state);
+ scb->state = state;
+
+#ifndef USE_WIN32API
+ {
+ int fds[2];
+
+ if (gdb_pipe_cloexec (fds) == -1)
+ internal_error (__FILE__, __LINE__,
+ "creating serial event pipe failed.");
+
+ fcntl (fds[0], F_SETFL, O_NONBLOCK);
+ fcntl (fds[1], F_SETFL, O_NONBLOCK);
+
+ scb->fd = fds[0];
+ state->write_fd = fds[1];
+ }
+#else
+ {
+ /* A dummy file object that can be wrapped in a file descriptor.
+ We don't need to store this handle because closing the file
+ descriptor automatically closes this. */
+ HANDLE dummy_file;
+
+ /* A manual-reset event. */
+ state->event = CreateEvent (0, TRUE, FALSE, 0);
+
+ /* The dummy file handle. Created just so we have something
+ wrappable in a file descriptor. */
+ dummy_file = CreateFile ("nul", 0, 0, NULL, OPEN_EXISTING, 0, NULL);
+ scb->fd = _open_osfhandle ((intptr_t) dummy_file, 0);
+ }
+#endif
+
+ return 0;
+}
+
+static void
+serial_event_close (struct serial *scb)
+{
+ struct serial_event_state *state = (struct serial_event_state *) scb->state;
+
+ close (scb->fd);
+#ifndef USE_WIN32API
+ close (state->write_fd);
+#else
+ CloseHandle (state->event);
+#endif
+
+ scb->fd = -1;
+
+ xfree (state);
+ scb->state = NULL;
+}
+
+#ifdef USE_WIN32API
+
+/* Implementation of the wait_handle method. Returns the native
+ Windows event object handle. */
+
+static void
+serial_event_wait_handle (struct serial *scb, HANDLE *read, HANDLE *except)
+{
+ struct serial_event_state *state = (struct serial_event_state *) scb->state;
+
+ *read = state->event;
+}
+
+#endif
+
+/* The serial_ops for struct serial_event objects. Note we never
+ register this serial type with serial_add_interface, because this
+ is internal implementation detail never to be used by remote
+ targets for protocol transport. */
+
+static const struct serial_ops serial_event_ops =
+{
+ "event",
+ serial_event_open,
+ serial_event_close,
+ NULL, /* fdopen */
+ NULL, /* readchar */
+ NULL, /* write */
+ NULL, /* flush_output */
+ NULL, /* flush_input */
+ NULL, /* send_break */
+ NULL, /* go_raw */
+ NULL, /* get_tty_state */
+ NULL, /* copy_tty_state */
+ NULL, /* set_tty_state */
+ NULL, /* print_tty_state */
+ NULL, /* noflush_set_tty_state */
+ NULL, /* setbaudrate */
+ NULL, /* setstopbits */
+ NULL, /* setparity */
+ NULL, /* drain_output */
+ NULL, /* async */
+ NULL, /* read_prim */
+ NULL, /* write_prim */
+ NULL, /* avail */
+#ifdef USE_WIN32API
+ serial_event_wait_handle,
+#endif
+};
+
+/* See ser-event.h. */
+
+struct serial_event *
+make_serial_event (void)
+{
+ return (struct serial_event *) serial_open_ops (&serial_event_ops);
+}
+
+/* See ser-event.h. */
+
+int
+serial_event_fd (struct serial_event *event)
+{
+ struct serial *ser = (struct serial *) event;
+
+ return ser->fd;
+}
+
+/* See ser-event.h. */
+
+void
+serial_event_set (struct serial_event *event)
+{
+ struct serial *ser = (struct serial *) event;
+ struct serial_event_state *state = (struct serial_event_state *) ser->state;
+#ifndef USE_WIN32API
+ int r;
+ char c = '+'; /* Anything. */
+
+ do
+ {
+ r = write (state->write_fd, &c, 1);
+ }
+ while (r < 0 && errno == EINTR);
+#else
+ SetEvent (state->event);
+#endif
+}
+
+/* See ser-event.h. */
+
+void
+serial_event_clear (struct serial_event *event)
+{
+ struct serial *ser = (struct serial *) event;
+ struct serial_event_state *state = (struct serial_event_state *) ser->state;
+#ifndef USE_WIN32API
+ int r;
+
+ do
+ {
+ char c;
+
+ r = read (ser->fd, &c, 1);
+ }
+ while (r > 0 || (r < 0 && errno == EINTR));
+#else
+ ResetEvent (state->event);
+#endif
+}
diff --git a/gdb/ser-event.h b/gdb/ser-event.h
new file mode 100644
index 0000000..b6654c4
--- /dev/null
+++ b/gdb/ser-event.h
@@ -0,0 +1,51 @@
+/* Serial interface for a selectable event.
+ Copyright (C) 2016 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 SER_EVENT_H
+#define SER_EVENT_H
+
+/* This is used to be able to signal the event loop (or any other
+ select/poll) of events, in a race-free manner.
+
+ For example, a signal handler can defer non-async-signal-safe work
+ to the event loop, by having the signal handler set a struct
+ serial_event object, and having the event loop wait for that same
+ object to the readable. Once readable, the event loop breaks out
+ of select/poll and calls a registered callback that does the
+ deferred work. */
+
+struct serial_event;
+
+/* Make a new serial_event object. */
+struct serial_event *make_serial_event (void);
+
+/* Return the FD that can be used by select/poll to wait for the
+ event. The only valid operation on this object is to wait until it
+ is readable. */
+extern int serial_event_fd (struct serial_event *event);
+
+/* Set the event. This signals the file descriptor returned by
+ serial_event_fd as readable. */
+extern void serial_event_set (struct serial_event *event);
+
+/* Clear the event. The file descriptor returned by serial_event_fd
+ is not longer readable after this, until a new serial_event_set
+ call is made. */
+extern void serial_event_clear (struct serial_event *event);
+
+#endif
diff --git a/gdb/serial.c b/gdb/serial.c
index 5d242b9..a95f85e 100644
--- a/gdb/serial.c
+++ b/gdb/serial.c
@@ -179,6 +179,27 @@ serial_for_fd (int fd)
return NULL;
}
+/* Create a new serial for OPS. */
+
+static struct serial *
+new_serial (const struct serial_ops *ops)
+{
+ struct serial *scb;
+
+ scb = XCNEW (struct serial);
+
+ scb->ops = ops;
+
+ scb->bufp = scb->buf;
+ scb->error_fd = -1;
+ scb->refcnt = 1;
+
+ return scb;
+}
+
+static struct serial *serial_open_ops_1 (const struct serial_ops *ops,
+ const char *open_name);
+
/* Open up a device or a network socket, depending upon the syntax of NAME. */
struct serial *
@@ -210,14 +231,17 @@ serial_open (const char *name)
if (!ops)
return NULL;
- scb = XNEW (struct serial);
+ return serial_open_ops_1 (ops, open_name);
+}
- scb->ops = ops;
+/* Open up a serial for OPS, passing OPEN_NAME to the open method. */
- scb->bufcnt = 0;
- scb->bufp = scb->buf;
- scb->error_fd = -1;
- scb->refcnt = 1;
+static struct serial *
+serial_open_ops_1 (const struct serial_ops *ops, const char *open_name)
+{
+ struct serial *scb;
+
+ scb = new_serial (ops);
/* `...->open (...)' would get expanded by the open(2) syscall macro. */
if ((*scb->ops->open) (scb, open_name))
@@ -227,10 +251,6 @@ serial_open (const char *name)
}
scb->next = scb_base;
- scb->debug_p = 0;
- scb->async_state = 0;
- scb->async_handler = NULL;
- scb->async_context = NULL;
scb_base = scb;
if (serial_logfile != NULL)
@@ -243,6 +263,14 @@ serial_open (const char *name)
return scb;
}
+/* See serial.h. */
+
+struct serial *
+serial_open_ops (const struct serial_ops *ops)
+{
+ return serial_open_ops_1 (ops, NULL);
+}
+
/* Open a new serial stream using a file handle, using serial
interface ops OPS. */
@@ -261,20 +289,9 @@ serial_fdopen_ops (const int fd, const struct serial_ops *ops)
if (!ops)
return NULL;
- scb = XCNEW (struct serial);
-
- scb->ops = ops;
-
- scb->bufcnt = 0;
- scb->bufp = scb->buf;
- scb->error_fd = -1;
- scb->refcnt = 1;
+ scb = new_serial (ops);
scb->next = scb_base;
- scb->debug_p = 0;
- scb->async_state = 0;
- scb->async_handler = NULL;
- scb->async_context = NULL;
scb_base = scb;
if ((ops->fdopen) != NULL)
diff --git a/gdb/serial.h b/gdb/serial.h
index b339f66..10b0643 100644
--- a/gdb/serial.h
+++ b/gdb/serial.h
@@ -34,6 +34,9 @@ struct ui_file;
typedef void *serial_ttystate;
struct serial;
+struct serial_ops;
+
+/* Create a new serial for OPS. The new serial is not opened. */
/* Try to open NAME. Returns a new `struct serial *' on success, NULL
on failure. The new serial object has a reference count of 1.
@@ -44,6 +47,10 @@ struct serial;
extern struct serial *serial_open (const char *name);
+/* Open a new serial stream using OPS. */
+
+extern struct serial *serial_open_ops (const struct serial_ops *ops);
+
/* Returns true if SCB is open. */
extern int serial_is_open (struct serial *scb);
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 24/30] Add missing cleanups to defaulted_query and prompt_for_continue
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (20 preceding siblings ...)
2016-03-18 19:26 ` [PATCH 07/30] Introduce a serial interface for select'able events Pedro Alves
@ 2016-03-18 19:26 ` Pedro Alves
2016-03-18 19:26 ` [PATCH 10/30] Make Python use a struct serial event Pedro Alves
` (8 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:26 UTC (permalink / raw)
To: gdb-patches
Some of the error paths in these functions leak.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* utils.c (defaulted_query, prompt_for_continue): Free temporary
strings with cleanups, instead of xfree.
---
gdb/utils.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/gdb/utils.c b/gdb/utils.c
index 4d81c23..46f61c1 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1212,6 +1212,7 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
/* Used to add duration we waited for user to respond to
prompt_for_continue_wait_time. */
struct timeval prompt_started, prompt_ended, prompt_delta;
+ struct cleanup *old_chain;
/* Set up according to which answer is the default. */
if (defchar == '\0')
@@ -1266,13 +1267,16 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
return deprecated_query_hook (ctlstr, args);
}
+ old_chain = make_cleanup (null_cleanup, NULL);
+
/* Format the question outside of the loop, to avoid reusing args. */
question = xstrvprintf (ctlstr, args);
+ make_cleanup (xfree, question);
prompt = xstrprintf (_("%s%s(%s or %s) %s"),
annotation_level > 1 ? "\n\032\032pre-query\n" : "",
question, y_string, n_string,
annotation_level > 1 ? "\n\032\032query\n" : "");
- xfree (question);
+ make_cleanup (xfree, prompt);
/* Used for calculating time spend waiting for user. */
gettimeofday (&prompt_started, NULL);
@@ -1323,9 +1327,9 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
timeval_add (&prompt_for_continue_wait_time,
&prompt_for_continue_wait_time, &prompt_delta);
- xfree (prompt);
if (annotation_level > 1)
printf_filtered (("\n\032\032post-query\n"));
+ do_cleanups (old_chain);
return retval;
}
\f
@@ -1830,6 +1834,7 @@ prompt_for_continue (void)
/* Used to add duration we waited for user to respond to
prompt_for_continue_wait_time. */
struct timeval prompt_started, prompt_ended, prompt_delta;
+ struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
gettimeofday (&prompt_started, NULL);
@@ -1852,6 +1857,7 @@ prompt_for_continue (void)
/* Call gdb_readline_wrapper, not readline, in order to keep an
event loop running. */
ignore = gdb_readline_wrapper (cont_prompt);
+ make_cleanup (xfree, ignore);
/* Add time spend in this routine to prompt_for_continue_wait_time. */
gettimeofday (&prompt_ended, NULL);
@@ -1862,7 +1868,7 @@ prompt_for_continue (void)
if (annotation_level > 1)
printf_unfiltered (("\n\032\032post-prompt-for-continue\n"));
- if (ignore)
+ if (ignore != NULL)
{
char *p = ignore;
@@ -1871,7 +1877,6 @@ prompt_for_continue (void)
if (p[0] == 'q')
/* Do not call quit here; there is no possibility of SIGINT. */
throw_quit ("Quit");
- xfree (ignore);
}
/* Now we have to do this again, so that GDB will know that it doesn't
@@ -1879,6 +1884,8 @@ prompt_for_continue (void)
reinitialize_more_filter ();
dont_repeat (); /* Forget prev cmd -- CR won't repeat it. */
+
+ do_cleanups (old_chain);
}
/* Initalize timer to keep track of how long we waited for the user. */
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 10/30] Make Python use a struct serial event
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (21 preceding siblings ...)
2016-03-18 19:26 ` [PATCH 24/30] Add missing cleanups to defaulted_query and prompt_for_continue Pedro Alves
@ 2016-03-18 19:26 ` Pedro Alves
2016-03-18 19:27 ` [PATCH 15/30] Eliminate clear_quit_flag Pedro Alves
` (7 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:26 UTC (permalink / raw)
To: gdb-patches
From: Pedro Alves <pedro@cascais.lan>
Now that we have an abstract for wakeable events, use it instead of a
(heavier) serial pipe.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* python/python.c: Include "ser-event.h".
(gdbpy_event_fds): Delete.
(gdbpy_serial_event): New.
(gdbpy_run_events): Change prototype. Use serial_event_clear
instead of serial_readchar.
(gdbpy_post_event): Use serial_event_set instead of serial_write.
(gdbpy_initialize_events): Use make_serial_event instead of
serial_pipe.
---
gdb/python/python.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 7202105..ab3aa0a 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -35,6 +35,7 @@
#include "cli/cli-utils.h"
#include <ctype.h>
#include "location.h"
+#include "ser-event.h"
/* Declared constants and enum for python stack printing. */
static const char python_excp_none[] = "none";
@@ -922,26 +923,25 @@ static struct gdbpy_event *gdbpy_event_list;
/* The final link of the event list. */
static struct gdbpy_event **gdbpy_event_list_end;
-/* We use a file handler, and not an async handler, so that we can
- wake up the main thread even when it is blocked in poll(). */
-static struct serial *gdbpy_event_fds[2];
+/* So that we can wake up the main thread even when it is blocked in
+ poll(). */
+static struct serial_event *gdbpy_serial_event;
/* The file handler callback. This reads from the internal pipe, and
then processes the Python event queue. This will always be run in
the main gdb thread. */
static void
-gdbpy_run_events (struct serial *scb, void *context)
+gdbpy_run_events (int error, gdb_client_data client_data)
{
struct cleanup *cleanup;
cleanup = ensure_python_env (get_current_arch (), current_language);
- /* Flush the fd. Do this before flushing the events list, so that
- any new event post afterwards is sure to re-awake the event
+ /* Clear the event fd. Do this before flushing the events list, so
+ that any new event post afterwards is sure to re-awake the event
loop. */
- while (serial_readchar (gdbpy_event_fds[0], 0) >= 0)
- ;
+ serial_event_clear (gdbpy_serial_event);
while (gdbpy_event_list)
{
@@ -999,12 +999,7 @@ gdbpy_post_event (PyObject *self, PyObject *args)
/* Wake up gdb when needed. */
if (wakeup)
- {
- char c = 'q'; /* Anything. */
-
- if (serial_write (gdbpy_event_fds[1], &c, 1))
- return PyErr_SetFromErrno (PyExc_IOError);
- }
+ serial_event_set (gdbpy_serial_event);
Py_RETURN_NONE;
}
@@ -1013,11 +1008,11 @@ gdbpy_post_event (PyObject *self, PyObject *args)
static int
gdbpy_initialize_events (void)
{
- if (serial_pipe (gdbpy_event_fds) == 0)
- {
- gdbpy_event_list_end = &gdbpy_event_list;
- serial_async (gdbpy_event_fds[0], gdbpy_run_events, NULL);
- }
+ gdbpy_event_list_end = &gdbpy_event_list;
+
+ gdbpy_serial_event = make_serial_event ();
+ add_file_handler (serial_event_fd (gdbpy_serial_event),
+ gdbpy_run_events, NULL);
return 0;
}
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 15/30] Eliminate clear_quit_flag
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (22 preceding siblings ...)
2016-03-18 19:26 ` [PATCH 10/30] Make Python use a struct serial event Pedro Alves
@ 2016-03-18 19:27 ` Pedro Alves
2016-03-18 19:27 ` [PATCH 21/30] Use target_terminal_ours_for_output in exceptions.c Pedro Alves
` (6 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:27 UTC (permalink / raw)
To: gdb-patches
Nothing calls this anymore.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* defs.h (clear_quit_flag): Remove declaration.
* extension-priv.h (struct extension_language_ops)
<clear_quit_flag>: Remove field and update comments.
* extension.c (clear_quit_flag): Delete.
* guile/guile.c (guile_extension_ops): Adjust.
* python/python.c (python_extension_ops): Adjust.
(gdbpy_clear_quit_flag): Delete.
---
gdb/defs.h | 2 --
gdb/extension-priv.h | 5 +----
gdb/extension.c | 19 -------------------
gdb/guile/guile.c | 1 -
gdb/python/python.c | 11 -----------
5 files changed, 1 insertion(+), 37 deletions(-)
diff --git a/gdb/defs.h b/gdb/defs.h
index ad9b259..006f660 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -139,8 +139,6 @@ extern char *debug_file_directory;
These functions use the extension_language_ops API to allow extension
language(s) and GDB SIGINT handling to coexist seamlessly. */
-/* * Clear the quit flag. */
-extern void clear_quit_flag (void);
/* * Evaluate to non-zero if the quit flag is set, zero otherwise. This
will clear the quit flag as a side effect. */
extern int check_quit_flag (void);
diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
index 5ccbc29..d7bc572 100644
--- a/gdb/extension-priv.h
+++ b/gdb/extension-priv.h
@@ -232,7 +232,7 @@ struct extension_language_ops
enum ext_lang_bp_stop (*breakpoint_cond_says_stop)
(const struct extension_language_defn *, struct breakpoint *);
- /* The next three are used to connect GDB's SIGINT handling with the
+ /* The next two are used to connect GDB's SIGINT handling with the
extension language's.
Terminology: If an extension language can use GDB's SIGINT handling then
@@ -242,9 +242,6 @@ struct extension_language_ops
These need not be implemented, but if one of them is implemented
then they all must be. */
- /* Clear the SIGINT indicator. */
- void (*clear_quit_flag) (const struct extension_language_defn *);
-
/* Set the SIGINT indicator.
This is called by GDB's SIGINT handler and must be async-safe. */
void (*set_quit_flag) (const struct extension_language_defn *);
diff --git a/gdb/extension.c b/gdb/extension.c
index c00db47..17268d6 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -794,25 +794,6 @@ restore_active_ext_lang (struct active_ext_lang_state *previous)
xfree (previous);
}
-/* Clear the quit flag.
- The flag is cleared in all extension languages,
- not just the currently active one. */
-
-void
-clear_quit_flag (void)
-{
- int i;
- const struct extension_language_defn *extlang;
-
- ALL_ENABLED_EXTENSION_LANGUAGES (i, extlang)
- {
- if (extlang->ops->clear_quit_flag != NULL)
- extlang->ops->clear_quit_flag (extlang);
- }
-
- quit_flag = 0;
-}
-
/* Set the quit flag.
This only sets the flag in the currently active extension language.
If the currently active extension language does not have cooperative
diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index 7352b57..f9481c9 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -155,7 +155,6 @@ const struct extension_language_ops guile_extension_ops =
gdbscm_breakpoint_cond_says_stop,
NULL, /* gdbscm_check_quit_flag, */
- NULL, /* gdbscm_clear_quit_flag, */
NULL, /* gdbscm_set_quit_flag, */
};
diff --git a/gdb/python/python.c b/gdb/python/python.c
index ab3aa0a..69d30de 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -146,7 +146,6 @@ static enum ext_lang_rc gdbpy_apply_type_printers
const struct ext_lang_type_printers *, struct type *, char **);
static void gdbpy_free_type_printers (const struct extension_language_defn *,
struct ext_lang_type_printers *);
-static void gdbpy_clear_quit_flag (const struct extension_language_defn *);
static void gdbpy_set_quit_flag (const struct extension_language_defn *);
static int gdbpy_check_quit_flag (const struct extension_language_defn *);
static enum ext_lang_rc gdbpy_before_prompt_hook
@@ -184,7 +183,6 @@ const struct extension_language_ops python_extension_ops =
gdbpy_breakpoint_has_cond,
gdbpy_breakpoint_cond_says_stop,
- gdbpy_clear_quit_flag,
gdbpy_set_quit_flag,
gdbpy_check_quit_flag,
@@ -270,15 +268,6 @@ ensure_python_env (struct gdbarch *gdbarch,
return make_cleanup (restore_python_env, env);
}
-/* Clear the quit flag. */
-
-static void
-gdbpy_clear_quit_flag (const struct extension_language_defn *extlang)
-{
- /* This clears the flag as a side effect. */
- PyOS_InterruptOccurred ();
-}
-
/* Set the quit flag. */
static void
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 21/30] Use target_terminal_ours_for_output in exceptions.c
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (23 preceding siblings ...)
2016-03-18 19:27 ` [PATCH 15/30] Eliminate clear_quit_flag Pedro Alves
@ 2016-03-18 19:27 ` Pedro Alves
2016-03-18 19:27 ` [PATCH 06/30] Remove unused struct serial::name field Pedro Alves
` (5 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:27 UTC (permalink / raw)
To: gdb-patches
We're only doing output here, so leave raw/cooked mode alone, as well
as the SIGINT handler.
Restore terminal settings after output, while at it.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* exceptions.c (print_flush): Use target_terminal_ours_for_output
instead of target_terminal_ours, and restore target terminal with
a cleanup.
---
gdb/exceptions.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index b457838..ffdd1f37 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -37,12 +37,16 @@ static void
print_flush (void)
{
struct serial *gdb_stdout_serial;
+ struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
if (deprecated_error_begin_hook)
deprecated_error_begin_hook ();
if (target_supports_terminal_ours ())
- target_terminal_ours ();
+ {
+ make_cleanup_restore_target_terminal ();
+ target_terminal_ours_for_output ();
+ }
/* We want all output to appear now, before we print the error. We
have 3 levels of buffering we have to flush (it's possible that
@@ -66,6 +70,8 @@ print_flush (void)
}
annotate_error_begin ();
+
+ do_cleanups (old_chain);
}
static void
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 06/30] Remove unused struct serial::name field
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (24 preceding siblings ...)
2016-03-18 19:27 ` [PATCH 21/30] Use target_terminal_ours_for_output in exceptions.c Pedro Alves
@ 2016-03-18 19:27 ` Pedro Alves
2016-03-18 19:28 ` [PATCH 05/30] Stop remote-fileio.c from throwing from SIGINT handler Pedro Alves
` (4 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:27 UTC (permalink / raw)
To: gdb-patches
From: Pedro Alves <pedro@cascais.lan>
Not used by anything.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* serial.c (serial_open, serial_fdopen_ops, do_serial_close):
Remove references to name.
* serial.h (struct serial) <name>: Delete.
---
gdb/serial.c | 5 -----
gdb/serial.h | 1 -
2 files changed, 6 deletions(-)
diff --git a/gdb/serial.c b/gdb/serial.c
index 19b1c2f..5d242b9 100644
--- a/gdb/serial.c
+++ b/gdb/serial.c
@@ -226,7 +226,6 @@ serial_open (const char *name)
return NULL;
}
- scb->name = xstrdup (name);
scb->next = scb_base;
scb->debug_p = 0;
scb->async_state = 0;
@@ -271,7 +270,6 @@ serial_fdopen_ops (const int fd, const struct serial_ops *ops)
scb->error_fd = -1;
scb->refcnt = 1;
- scb->name = NULL;
scb->next = scb_base;
scb->debug_p = 0;
scb->async_state = 0;
@@ -315,9 +313,6 @@ do_serial_close (struct serial *scb, int really_close)
if (really_close)
scb->ops->close (scb);
- if (scb->name)
- xfree (scb->name);
-
/* For serial_is_open. */
scb->bufp = NULL;
diff --git a/gdb/serial.h b/gdb/serial.h
index 495b04d..b339f66 100644
--- a/gdb/serial.h
+++ b/gdb/serial.h
@@ -248,7 +248,6 @@ struct serial
int timeout_remaining; /* (ser-unix.c termio{,s} only), we
still need to wait for this many
more seconds. */
- char *name; /* The name of the device or host */
struct serial *next; /* Pointer to the next `struct serial *' */
int debug_p; /* Trace this serial devices operation. */
int async_state; /* Async internal state. */
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 05/30] Stop remote-fileio.c from throwing from SIGINT handler
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (25 preceding siblings ...)
2016-03-18 19:27 ` [PATCH 06/30] Remove unused struct serial::name field Pedro Alves
@ 2016-03-18 19:28 ` Pedro Alves
2016-03-18 19:28 ` [PATCH 11/30] Don't call clear_quit_flag after check_quit_flag Pedro Alves
` (3 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:28 UTC (permalink / raw)
To: gdb-patches
This code installs a custom signal handler that throws a quit
exception if remote_fio_no_longjmp is not set.
AFAICS, the only real reason for this might have been to unblock the
ui_file_read call, in remote_fileio_func_read. But ever since:
2009-11-13 Daniel Jacobowitz <dan@codesourcery.com>
* ui-file.c (stdio_file_read): Call gdb_select before read.
at:
https://sourceware.org/ml/gdb-patches/2009-11/msg00321.html
that call is interruptible.
This is not only useful for switching to native C++ exceptions, but
AFAICS, also fixes a potential mess up of the remote protocol
connection, since there are target_read_memory calls done while
remote_fio_no_longjmp is clear. If the user presses ctrl-c while GDB
is sending or receiving a packet, we'll stop the communication
immediately, at a point where it isn't safe.
gdbserver doesn't support the File I/O remote protocol extension so I
can't test this.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* remote-fileio.c (sigint_fileio_token, remote_fio_no_longjmp):
Delete.
(async_remote_fileio_interrupt): Delete.
(remote_fileio_ctrl_c_signal_handler): Don't call the async signal
handler. Instead just always set the ctrl_c flag.
(remote_fileio_reply): Clear remote_fio_ctrl_c_flag before
re-enabling the SIGINT handler.
(remote_fileio_func_open, remote_fileio_func_close)
(remote_fileio_func_read, remote_fileio_func_write)
(remote_fileio_func_lseek, remote_fileio_func_rename)
(remote_fileio_func_unlink, remote_fileio_func_stat)
(remote_fileio_func_fstat, remote_fileio_func_gettimeofday)
(remote_fileio_func_isatty, remote_fileio_func_system)
(remote_fileio_request): Remove references to
remote_fio_no_longjmp.
(initialize_remote_fileio): Don't create an async signal handler.
---
gdb/remote-fileio.c | 34 ++--------------------------------
1 file changed, 2 insertions(+), 32 deletions(-)
diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
index 94c552b..44817df 100644
--- a/gdb/remote-fileio.c
+++ b/gdb/remote-fileio.c
@@ -48,8 +48,6 @@ static struct {
static int remote_fio_system_call_allowed = 0;
-static struct async_signal_handler *sigint_fileio_token;
-
static int
remote_fileio_init_fd_map (void)
{
@@ -301,7 +299,6 @@ remote_fileio_to_fio_timeval (struct timeval *tv, struct fio_timeval *ftv)
}
static int remote_fio_ctrl_c_flag = 0;
-static int remote_fio_no_longjmp = 0;
#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
static struct sigaction remote_fio_sa;
@@ -347,19 +344,10 @@ remote_fileio_sig_exit (void)
}
static void
-async_remote_fileio_interrupt (gdb_client_data arg)
-{
- quit ();
-}
-
-static void
remote_fileio_ctrl_c_signal_handler (int signo)
{
- remote_fileio_sig_set (SIG_IGN);
- remote_fio_ctrl_c_flag = 1;
- if (!remote_fio_no_longjmp)
- gdb_call_async_signal_handler (sigint_fileio_token, 1);
remote_fileio_sig_set (remote_fileio_ctrl_c_signal_handler);
+ remote_fio_ctrl_c_flag = 1;
}
static void
@@ -388,6 +376,7 @@ remote_fileio_reply (int retcode, int error)
if (remote_fio_ctrl_c_flag)
strcat (buf, ",C");
}
+ remote_fio_ctrl_c_flag = 0;
remote_fileio_sig_set (remote_fileio_ctrl_c_signal_handler);
putpkt (buf);
}
@@ -475,7 +464,6 @@ remote_fileio_func_open (char *buf)
}
}
- remote_fio_no_longjmp = 1;
fd = gdb_open_cloexec (pathname, flags, mode);
if (fd < 0)
{
@@ -506,7 +494,6 @@ remote_fileio_func_close (char *buf)
return;
}
- remote_fio_no_longjmp = 1;
if (fd != FIO_FD_CONSOLE_IN && fd != FIO_FD_CONSOLE_OUT && close (fd))
remote_fileio_return_errno (-1);
remote_fileio_close_target_fd ((int) num);
@@ -564,7 +551,6 @@ remote_fileio_func_read (char *buf)
buffer = (gdb_byte *) xmalloc (16384);
if (remaining_buf)
{
- remote_fio_no_longjmp = 1;
if (remaining_length > length)
{
memcpy (buffer, remaining_buf, length);
@@ -595,7 +581,6 @@ remote_fileio_func_read (char *buf)
safe margin, in case the limit depends on system
resources or version. */
ret = ui_file_read (gdb_stdtargin, (char *) buffer, 16383);
- remote_fio_no_longjmp = 1;
if (ret > 0 && (size_t)ret > length)
{
remaining_buf = (char *) xmalloc (ret - length);
@@ -614,7 +599,6 @@ remote_fileio_func_read (char *buf)
Therefore a complete solution must check how many bytes have been
read on EINTR to return a more reliable value to the target */
old_offset = lseek (fd, 0, SEEK_CUR);
- remote_fio_no_longjmp = 1;
ret = read (fd, buffer, length);
if (ret < 0 && errno == EINTR)
{
@@ -687,7 +671,6 @@ remote_fileio_func_write (char *buf)
return;
}
- remote_fio_no_longjmp = 1;
switch (fd)
{
case FIO_FD_CONSOLE_IN:
@@ -761,7 +744,6 @@ remote_fileio_func_lseek (char *buf)
return;
}
- remote_fio_no_longjmp = 1;
ret = lseek (fd, offset, flag);
if (ret == (off_t) -1)
@@ -819,7 +801,6 @@ remote_fileio_func_rename (char *buf)
return;
}
- remote_fio_no_longjmp = 1;
ret = rename (oldpath, newpath);
if (ret == -1)
@@ -895,7 +876,6 @@ remote_fileio_func_unlink (char *buf)
return;
}
- remote_fio_no_longjmp = 1;
ret = unlink (pathname);
if (ret == -1)
@@ -937,7 +917,6 @@ remote_fileio_func_stat (char *buf)
return;
}
- remote_fio_no_longjmp = 1;
ret = stat (pathname, &st);
if (ret == -1)
@@ -997,7 +976,6 @@ remote_fileio_func_fstat (char *buf)
}
ptrval = (CORE_ADDR) lnum;
- remote_fio_no_longjmp = 1;
if (fd == FIO_FD_CONSOLE_IN || fd == FIO_FD_CONSOLE_OUT)
{
host_to_fileio_uint (1, fst.fst_dev);
@@ -1073,7 +1051,6 @@ remote_fileio_func_gettimeofday (char *buf)
return;
}
- remote_fio_no_longjmp = 1;
ret = gettimeofday (&tv, NULL);
if (ret == -1)
@@ -1108,7 +1085,6 @@ remote_fileio_func_isatty (char *buf)
remote_fileio_ioerror ();
return;
}
- remote_fio_no_longjmp = 1;
fd = remote_fileio_map_fd ((int) target_fd);
remote_fileio_return_success (fd == FIO_FD_CONSOLE_IN ||
fd == FIO_FD_CONSOLE_OUT ? 1 : 0);
@@ -1152,7 +1128,6 @@ remote_fileio_func_system (char *buf)
return;
}
- remote_fio_no_longjmp = 1;
ret = system (cmdline);
if (!length)
@@ -1244,13 +1219,11 @@ remote_fileio_request (char *buf, int ctrlc_pending_p)
asynchronously earlier, take this opportunity to send the
Ctrl-C synchronously. */
remote_fio_ctrl_c_flag = 1;
- remote_fio_no_longjmp = 0;
remote_fileio_reply (-1, FILEIO_EINTR);
}
else
{
remote_fio_ctrl_c_flag = 0;
- remote_fio_no_longjmp = 0;
ex = catch_exceptions (current_uiout,
do_remote_fileio_request, (void *)buf,
@@ -1366,9 +1339,6 @@ void
initialize_remote_fileio (struct cmd_list_element *remote_set_cmdlist,
struct cmd_list_element *remote_show_cmdlist)
{
- sigint_fileio_token =
- create_async_signal_handler (async_remote_fileio_interrupt, NULL);
-
add_cmd ("system-call-allowed", no_class,
set_system_call_allowed,
_("Set if the host system(3) call is allowed for the target."),
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 11/30] Don't call clear_quit_flag after check_quit_flag
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (26 preceding siblings ...)
2016-03-18 19:28 ` [PATCH 05/30] Stop remote-fileio.c from throwing from SIGINT handler Pedro Alves
@ 2016-03-18 19:28 ` Pedro Alves
2016-03-18 19:37 ` [PATCH 16/30] Decouple target_interrupt from all-stop/non-stop modes Pedro Alves
` (2 subsequent siblings)
30 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:28 UTC (permalink / raw)
To: gdb-patches
Obviously not necessary since check_quit_flag clears the flag as side
effect.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* remote-sim.c (gdb_os_poll_quit): Don't call clear_quit_flag.
* remote.c (remote_wait_as): Don't call clear_quit_flag.
---
gdb/remote-sim.c | 5 +----
gdb/remote.c | 5 +----
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index 8489eb6..11d36eb 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -955,10 +955,7 @@ gdb_os_poll_quit (host_callback *p)
deprecated_ui_loop_hook (0);
if (check_quit_flag ()) /* gdb's idea of quit */
- {
- clear_quit_flag (); /* we've stolen it */
- return 1;
- }
+ return 1;
return 0;
}
diff --git a/gdb/remote.c b/gdb/remote.c
index af0a08a..d43293b 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -6935,10 +6935,7 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
/* If the user hit C-c before this packet, or between packets,
pretend that it was hit right here. */
if (check_quit_flag ())
- {
- clear_quit_flag ();
- sync_remote_interrupt (SIGINT);
- }
+ sync_remote_interrupt (SIGINT);
}
/* FIXME: cagney/1999-09-27: If we're in async mode we should
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 16/30] Decouple target_interrupt from all-stop/non-stop modes
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (27 preceding siblings ...)
2016-03-18 19:28 ` [PATCH 11/30] Don't call clear_quit_flag after check_quit_flag Pedro Alves
@ 2016-03-18 19:37 ` Pedro Alves
2016-03-21 18:21 ` Simon Marchi
2016-03-18 19:37 ` [PATCH 09/30] Introduce interruptible_select Pedro Alves
2016-03-21 19:52 ` [PATCH 00/30] Stop throwing exceptions from signal handlers Simon Marchi
30 siblings, 1 reply; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:37 UTC (permalink / raw)
To: gdb-patches
In non-stop mode, "interrupt" results in a "stop with no mode, it
results in a remote interrupt request / stop with SIGINT. This is
currently implemented in both the Linux and remote target backends.
Move it to the core code instead, making target_interrupt specifically
always about "Interrupting as if with Ctrl-C", just like it is
documented.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* infcmd.c (interrupt_target_1): Call target_stop is in non-stop
mode.
* linux-nat.c (linux_nat_interrupt): Delete.
(linux_nat_add_target): Don't install linux_nat_interrupt.
* remote.c (remote_interrupt_ns): Change return type to void.
Throw error if interrupting the target is not supported.
(remote_interrupt): Don't call the remote_stop_ns/remote_stop_as.
---
gdb/infcmd.c | 6 +++++-
gdb/linux-nat.c | 10 ----------
gdb/remote.c | 35 +++++++++--------------------------
3 files changed, 14 insertions(+), 37 deletions(-)
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index d687116..3a0265f 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -3013,7 +3013,11 @@ interrupt_target_1 (int all_threads)
ptid = minus_one_ptid;
else
ptid = inferior_ptid;
- target_interrupt (ptid);
+
+ if (non_stop)
+ target_stop (ptid);
+ else
+ target_interrupt (ptid);
/* Tag the thread as having been explicitly requested to stop, so
other parts of gdb know not to resume this thread automatically,
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 0829bcb..b75153c 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4463,15 +4463,6 @@ linux_nat_stop (struct target_ops *self, ptid_t ptid)
}
static void
-linux_nat_interrupt (struct target_ops *self, ptid_t ptid)
-{
- if (non_stop)
- iterate_over_lwps (ptid, linux_nat_stop_lwp, NULL);
- else
- linux_ops->to_interrupt (linux_ops, ptid);
-}
-
-static void
linux_nat_close (struct target_ops *self)
{
/* Unregister from the event loop. */
@@ -4672,7 +4663,6 @@ linux_nat_add_target (struct target_ops *t)
t->to_close = linux_nat_close;
t->to_stop = linux_nat_stop;
- t->to_interrupt = linux_nat_interrupt;
t->to_supports_multi_process = linux_nat_supports_multi_process;
diff --git a/gdb/remote.c b/gdb/remote.c
index d43293b..f932455 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5811,10 +5811,10 @@ remote_interrupt_as (void)
/* Non-stop version of target_interrupt. Uses `vCtrlC' to interrupt
the remote target. It is undefined which thread of which process
- reports the interrupt. Returns true if the packet is supported by
- the server, false otherwise. */
+ reports the interrupt. Throws an error if the packet is not
+ supported by the server. */
-static int
+static void
remote_interrupt_ns (void)
{
struct remote_state *rs = get_remote_state ();
@@ -5833,12 +5833,10 @@ remote_interrupt_ns (void)
case PACKET_OK:
break;
case PACKET_UNKNOWN:
- return 0;
+ error (_("No support for interrupting the remote target."));
case PACKET_ERROR:
error (_("Interrupting target failed: %s"), rs->buf);
}
-
- return 1;
}
/* Implement the to_stop function for the remote targets. */
@@ -5864,30 +5862,15 @@ remote_stop (struct target_ops *self, ptid_t ptid)
static void
remote_interrupt (struct target_ops *self, ptid_t ptid)
{
+ struct remote_state *rs = get_remote_state ();
+
if (remote_debug)
fprintf_unfiltered (gdb_stdlog, "remote_interrupt called\n");
- if (non_stop)
- {
- /* In non-stop mode, we always stop with no signal instead. */
- remote_stop_ns (ptid);
- }
+ if (target_is_non_stop_p ())
+ remote_interrupt_ns ();
else
- {
- /* In all-stop, we emulate ^C-ing the remote target's
- terminal. */
- if (target_is_non_stop_p ())
- {
- if (!remote_interrupt_ns ())
- {
- /* No support for ^C-ing the remote target. Stop it
- (with no signal) instead. */
- remote_stop_ns (ptid);
- }
- }
- else
- remote_interrupt_as ();
- }
+ remote_interrupt_as ();
}
/* Ask the user what to do when an interrupt is received. */
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 16/30] Decouple target_interrupt from all-stop/non-stop modes
2016-03-18 19:37 ` [PATCH 16/30] Decouple target_interrupt from all-stop/non-stop modes Pedro Alves
@ 2016-03-21 18:21 ` Simon Marchi
2016-03-21 18:24 ` Pedro Alves
0 siblings, 1 reply; 41+ messages in thread
From: Simon Marchi @ 2016-03-21 18:21 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 16-03-18 03:18 PM, Pedro Alves wrote:
> In non-stop mode, "interrupt" results in a "stop with no mode, it
> results in a remote interrupt request / stop with SIGINT. This is
I don't really understand the first sentence, is there something missing?
> currently implemented in both the Linux and remote target backends.
> Move it to the core code instead, making target_interrupt specifically
> always about "Interrupting as if with Ctrl-C", just like it is
> documented.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 16/30] Decouple target_interrupt from all-stop/non-stop modes
2016-03-21 18:21 ` Simon Marchi
@ 2016-03-21 18:24 ` Pedro Alves
0 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-21 18:24 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 03/21/2016 06:21 PM, Simon Marchi wrote:
> On 16-03-18 03:18 PM, Pedro Alves wrote:
>> In non-stop mode, "interrupt" results in a "stop with no mode, it
>> results in a remote interrupt request / stop with SIGINT. This is
>
> I don't really understand the first sentence, is there something missing?
Urgh, yes, looks like I deleted a line or something.
It should have read:
In non-stop mode, "interrupt" results in a "stop with no signal",
while in all-stop mode, it results in a remote interrupt
request / stop with SIGINT. This is (...)
>
>> currently implemented in both the Linux and remote target backends.
>> Move it to the core code instead, making target_interrupt specifically
>> always about "Interrupting as if with Ctrl-C", just like it is
>> documented.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 09/30] Introduce interruptible_select
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (28 preceding siblings ...)
2016-03-18 19:37 ` [PATCH 16/30] Decouple target_interrupt from all-stop/non-stop modes Pedro Alves
@ 2016-03-18 19:37 ` Pedro Alves
2016-03-21 17:59 ` Simon Marchi
2016-03-21 19:48 ` Simon Marchi
2016-03-21 19:52 ` [PATCH 00/30] Stop throwing exceptions from signal handlers Simon Marchi
30 siblings, 2 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-18 19:37 UTC (permalink / raw)
To: gdb-patches
We have places where we call a blocking gdb_select expecting that a
Ctrl-C will unblock it. However, if the Ctrl-C is pressed just before
gdb_select, the SIGINT handler runs before gdb_select, and thus
gdb_select won't return.
For example gdb_readline_no_editing:
QUIT;
/* Wait until at least one byte of data is available. Control-C
can interrupt gdb_select, but not fgetc. */
FD_ZERO (&readfds);
FD_SET (fd, &readfds);
if (gdb_select (fd + 1, &readfds, NULL, NULL, NULL) == -1)
and stdio_file_read:
/* For the benefit of Windows, call gdb_select before reading from
the file. Wait until at least one byte of data is available.
Control-C can interrupt gdb_select, but not read. */
{
fd_set readfds;
FD_ZERO (&readfds);
FD_SET (stdio->fd, &readfds);
if (gdb_select (stdio->fd + 1, &readfds, NULL, NULL, NULL) == -1)
return -1;
}
return read (stdio->fd, buf, length_buf);
This is a race classically fixed with either the self-pipe trick, or
by blocking SIGINT and then using pselect instead of select.
Blocking SIGINT most of the time would mean that check_quit_flag (and
thus QUIT) would need to do a syscall every time it is called, which
sounds best avoided, since QUIT is called in many loops. Thus we take
the self-pipe trick route (wrapped in a serial event).
Instead of having all places that need this manually add an extra file
descriptor to the set of gdb_select's watched file descriptors, we
introduce a wrapper, interruptible_select, that does that.
The Windows version of gdb_select actually does not suffer from this,
because mingw-hdep.c:gdb_call_async_signal_handler sets a Windows
event that gdb_select always waits on. So this patch can be seen as
generalization of that technique. We can't remove that extra event
from mingw-hdep.c until we get rid of immediate_quit though.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* defs.h: Extend QUIT-related comments to mention
interruptible_select.
(quit_serial_event_set, quit_serial_event_clear): Declare.
* event-top.c: Include "ser-event.h" and "gdb_select.h".
(quit_serial_event): New global.
(async_init_signals): Make quit_serial_event.
(quit_serial_event_set, quit_serial_event_clear)
(quit_serial_event_fd, interruptible_select): New functions.
* extension.c (set_quit_flag): Set the quit serial event.
(check_quit_flag): Clear the quit serial event.
* gdb_select.h (interruptible_select): New declaration.
* guile/scm-ports.c (ioscm_input_waiting): Use
interruptible_select instead of gdb_select.
* top.c (gdb_readline_no_editing): Likewise.
* ui-file.c (stdio_file_read): Likewise.
---
gdb/defs.h | 11 ++++++++
gdb/event-top.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++--
gdb/extension.c | 15 ++++++++++-
gdb/gdb_select.h | 15 +++++++++++
gdb/guile/scm-ports.c | 4 ++-
gdb/top.c | 4 +--
gdb/ui-file.c | 8 +++---
7 files changed, 122 insertions(+), 10 deletions(-)
diff --git a/gdb/defs.h b/gdb/defs.h
index b94df30..ad9b259 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -131,6 +131,11 @@ extern char *debug_file_directory;
take a long time, and which ought to be interruptible, checks this
flag using the QUIT macro.
+ In addition to setting a flag, the SIGINT handler also marks a
+ select/poll-able file descriptor as read-ready. That is used by
+ interruptible_select in order to support interrupting blocking I/O
+ in a race-free manner.
+
These functions use the extension_language_ops API to allow extension
language(s) and GDB SIGINT handling to coexist seamlessly. */
@@ -159,6 +164,12 @@ extern void maybe_quit (void);
connection. */
#define QUIT maybe_quit ()
+/* Set the serial event associated with the quit flag. */
+extern void quit_serial_event_set (void);
+
+/* Clear the serial event associated with the quit flag. */
+extern void quit_serial_event_clear (void);
+
/* * Languages represented in the symbol table and elsewhere.
This should probably be in language.h, but since enum's can't
be forward declared to satisfy opaque references before their
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 9fff2be..da72b1d 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -38,6 +38,8 @@
#include "annotate.h"
#include "maint.h"
#include "buffer.h"
+#include "ser-event.h"
+#include "gdb_select.h"
/* readline include files. */
#include "readline/readline.h"
@@ -732,6 +734,12 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
}
\f
+/* The serial event associated with the QUIT flag. set_quit_flag sets
+ this, and check_quit_flag clears it. Used by interruptible_select
+ to be able to do interruptible I/O with no race with the SIGINT
+ handler. */
+static struct serial_event *quit_serial_event;
+
/* Initialization of signal handlers and tokens. There is a function
handle_sig* for each of the signals GDB cares about. Specifically:
SIGINT, SIGFPE, SIGQUIT, SIGTSTP, SIGHUP, SIGWINCH. These
@@ -749,6 +757,8 @@ async_init_signals (void)
{
initialize_async_signal_handlers ();
+ quit_serial_event = make_serial_event ();
+
signal (SIGINT, handle_sigint);
sigint_token =
create_async_signal_handler (async_request_quit, NULL);
@@ -793,8 +803,33 @@ async_init_signals (void)
#endif
}
-/* Tell the event loop what to do if SIGINT is received.
- See event-signal.c. */
+/* See defs.h. */
+
+void
+quit_serial_event_set (void)
+{
+ serial_event_set (quit_serial_event);
+}
+
+/* See defs.h. */
+
+void
+quit_serial_event_clear (void)
+{
+ serial_event_clear (quit_serial_event);
+}
+
+/* Return the selectable file descriptor of the serial event
+ associated with the quit flag. */
+
+static int
+quit_serial_event_fd (void)
+{
+ return serial_event_fd (quit_serial_event);
+}
+
+/* Handle a SIGINT. */
+
void
handle_sigint (int sig)
{
@@ -818,6 +853,42 @@ handle_sigint (int sig)
gdb_call_async_signal_handler (sigint_token, immediate_quit);
}
+/* See gdb_select.h. */
+
+int
+interruptible_select (int n,
+ fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
+ struct timeval *timeout)
+{
+ fd_set my_readfds;
+ int fd;
+ int res;
+
+ if (readfds == NULL)
+ {
+ readfds = &my_readfds;
+ FD_ZERO (&my_readfds);
+ }
+
+ fd = quit_serial_event_fd ();
+ FD_SET (fd, readfds);
+ if (n <= fd)
+ n = fd + 1;
+
+ do
+ {
+ res = gdb_select (n, readfds, writefds, exceptfds, timeout);
+ }
+ while (res == -1 && errno == EINTR);
+
+ if (res == 1 && FD_ISSET (fd, readfds))
+ {
+ errno = EINTR;
+ return -1;
+ }
+ return res;
+}
+
/* Handle GDB exit upon receiving SIGTERM if target_can_async_p (). */
static void
diff --git a/gdb/extension.c b/gdb/extension.c
index d2c5669..c00db47 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -828,7 +828,16 @@ set_quit_flag (void)
&& active_ext_lang->ops->set_quit_flag != NULL)
active_ext_lang->ops->set_quit_flag (active_ext_lang);
else
- quit_flag = 1;
+ {
+ quit_flag = 1;
+
+ /* Now wake up the event loop, or any interruptible_select. Do
+ this after setting the flag, because signals on Windows
+ actually run on a separate thread, and thus otherwise the
+ main code could be woken up and find quit_flag still
+ clear. */
+ quit_serial_event_set ();
+ }
}
/* Return true if the quit flag has been set, false otherwise.
@@ -852,6 +861,10 @@ check_quit_flag (void)
/* This is written in a particular way to avoid races. */
if (quit_flag)
{
+ /* No longer need to wake up the event loop or any
+ interruptible_select. The caller handles the quit
+ request. */
+ quit_serial_event_clear ();
quit_flag = 0;
result = 1;
}
diff --git a/gdb/gdb_select.h b/gdb/gdb_select.h
index e00a332..d346c60 100644
--- a/gdb/gdb_select.h
+++ b/gdb/gdb_select.h
@@ -33,4 +33,19 @@
extern int gdb_select (int n, fd_set *readfds, fd_set *writefds,
fd_set *exceptfds, struct timeval *timeout);
+/* Convenience wrapper around gdb_select that returns -1/EINTR if
+ set_quit_flag is set, either on entry or from a signal handler or
+ from a different thread while select is blocked. The quit flag is
+ not cleared on exit -- the caller is responsible to check it with
+ check_quit_flag or QUIT.
+
+ Note this does NOT return -1/EINTR if any signal handler other than
+ SIGINT runs, nor if the current SIGINT handler does not call
+ set_quit_flag. */
+extern int interruptible_select (int n,
+ fd_set *readfds,
+ fd_set *writefds,
+ fd_set *exceptfds,
+ struct timeval *timeout);
+
#endif /* !defined(GDB_SELECT_H) */
diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index a7d61af..b0f576e 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -201,7 +201,9 @@ ioscm_input_waiting (SCM port)
FD_ZERO (&input_fds);
FD_SET (fdes, &input_fds);
- num_found = gdb_select (num_fds, &input_fds, NULL, NULL, &timeout);
+ num_found = interruptible_select (num_fds,
+ &input_fds, NULL, NULL,
+ &timeout);
if (num_found < 0)
{
/* Guile doesn't export SIGINT hooks like Python does.
diff --git a/gdb/top.c b/gdb/top.c
index 41ff6b2..f5ef718 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -613,10 +613,10 @@ gdb_readline_no_editing (const char *prompt)
QUIT;
/* Wait until at least one byte of data is available. Control-C
- can interrupt gdb_select, but not fgetc. */
+ can interrupt interruptible_select, but not fgetc. */
FD_ZERO (&readfds);
FD_SET (fd, &readfds);
- if (gdb_select (fd + 1, &readfds, NULL, NULL, NULL) == -1)
+ if (interruptible_select (fd + 1, &readfds, NULL, NULL, NULL) == -1)
{
if (errno == EINTR)
{
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index c86994d..4260710 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -567,14 +567,14 @@ stdio_file_read (struct ui_file *file, char *buf, long length_buf)
internal_error (__FILE__, __LINE__,
_("stdio_file_read: bad magic number"));
- /* For the benefit of Windows, call gdb_select before reading from
- the file. Wait until at least one byte of data is available.
- Control-C can interrupt gdb_select, but not read. */
+ /* Wait until at least one byte of data is available, or we get
+ interrupted with Control-C. */
{
fd_set readfds;
+
FD_ZERO (&readfds);
FD_SET (stdio->fd, &readfds);
- if (gdb_select (stdio->fd + 1, &readfds, NULL, NULL, NULL) == -1)
+ if (interruptible_select (stdio->fd + 1, &readfds, NULL, NULL, NULL) == -1)
return -1;
}
--
2.5.0
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 09/30] Introduce interruptible_select
2016-03-18 19:37 ` [PATCH 09/30] Introduce interruptible_select Pedro Alves
@ 2016-03-21 17:59 ` Simon Marchi
2016-03-21 18:33 ` Pedro Alves
2016-03-21 19:48 ` Simon Marchi
1 sibling, 1 reply; 41+ messages in thread
From: Simon Marchi @ 2016-03-21 17:59 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 16-03-18 03:18 PM, Pedro Alves wrote:
> @@ -749,6 +757,8 @@ async_init_signals (void)
> {
> initialize_async_signal_handlers ();
>
> + quit_serial_event = make_serial_event ();
> +
Just above these line is this comment:
/* NOTE: 1999-04-30 This is the asynchronous version of init_signals.
init_signals will become obsolete as we move to have to event loop
as the default for gdb. */
Is is still relevant?
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 09/30] Introduce interruptible_select
2016-03-21 17:59 ` Simon Marchi
@ 2016-03-21 18:33 ` Pedro Alves
0 siblings, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-21 18:33 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 03/21/2016 05:59 PM, Simon Marchi wrote:
> On 16-03-18 03:18 PM, Pedro Alves wrote:
>> @@ -749,6 +757,8 @@ async_init_signals (void)
>> {
>> initialize_async_signal_handlers ();
>>
>> + quit_serial_event = make_serial_event ();
>> +
>
> Just above these line is this comment:
>
> /* NOTE: 1999-04-30 This is the asynchronous version of init_signals.
> init_signals will become obsolete as we move to have to event loop
> as the default for gdb. */
>
>
> Is is still relevant?
That comment is stale since (I think):
2004-09-13 Andrew Cagney <cagney@gnu.org>
...
Eliminate event_loop_p, always has the value 1.
* defs.h (event_loop_p): Delete macro.
* breakpoint.c (until_break_command): Simplify.
* utils.c (prompt_for_continue): Simplify.
* tracepoint.c (read_actions): Simplify.
* top.c (throw_exception, execute_command, gdb_readline_wrapper)
(gdb_rl_operate_and_get_next, command_line_input, get_prompt)
(set_prompt, init_main): Simplify.
(init_signals, disconnect): Delete, unused.
That is, init_signals is long gone, and we always have an event loop
running.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 09/30] Introduce interruptible_select
2016-03-18 19:37 ` [PATCH 09/30] Introduce interruptible_select Pedro Alves
2016-03-21 17:59 ` Simon Marchi
@ 2016-03-21 19:48 ` Simon Marchi
2016-03-21 19:49 ` Pedro Alves
1 sibling, 1 reply; 41+ messages in thread
From: Simon Marchi @ 2016-03-21 19:48 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 16-03-18 03:18 PM, Pedro Alves wrote:
> +int
> +interruptible_select (int n,
> + fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
> + struct timeval *timeout)
> +{
> + fd_set my_readfds;
> + int fd;
Just a nit. For clarity, you could name this variable "quit_fd" instead of "fd".
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 00/30] Stop throwing exceptions from signal handlers
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
` (29 preceding siblings ...)
2016-03-18 19:37 ` [PATCH 09/30] Introduce interruptible_select Pedro Alves
@ 2016-03-21 19:52 ` Simon Marchi
2016-03-31 14:43 ` Pedro Alves
30 siblings, 1 reply; 41+ messages in thread
From: Simon Marchi @ 2016-03-21 19:52 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 16-03-18 03:18 PM, Pedro Alves wrote:
> Aka, get rid of immediate_quit.
>
> Throwing exceptions from signal handlers is undefined C++. So for C++
> conversion we need to replace the immediate_quit mechanism by
> something else.
>
> This series:
>
> - Replaces all immediate_quit uses with something else.
>
> - Removes a few custom SIGINT handlers, centralizing more on the
> "quit" flag and GDB's default SIGINT handler.
>
> - Fixes several classical signal handler / mainline code races, using
> the self-pipe trick.
>
> - Introduces a "struct serial_event" abstraction to hide the self-pipe
> trick -- on Windows we don't use pipes at all, and on Linux, we
> could use eventfd (but I didn't do that, at least yet).
>
> - Stumbles on a few bogus clear_quit_flag uses, and thus completely
> eliminates it, for good measure.
>
> - Decouples target_interrupt from all-stop/non-stop modes
>
> - Fixes a few cases of indefinitely-delayed Ctrl-C handling.
>
> - Fixes/incorrect handling of EINTR in ser-*.c backends
>
> - Goes through the tree fixing all target_terminal_ours calls that
> should be target_terminal_ours_for_output instead. We only end up
> with a few target_terminal_ours calls, at places were we need to
> handle input.
>
> - Plugs a few leaks along the way.
>
> - Fixes a bug where you can't answer y/n to an internal error if the
> target has the terminal.
>
> - Introduces the concept of quit handlers.
>
> - Does a few clean ups along the way.
>
> Tested on x86-64 Fedora 23, native and gdbserver.
>
> A slightly older version manually test on mingw64/Windows 7, to make
> sure Ctrl-C still works as expected there.
>
> This applies on top of the "target mips" elimination patch:
> https://sourceware.org/ml/gdb-patches/2016-03/msg00313.html
>
> I've pushed the whole series to users/palves/immediate_quit.
>
> Pedro Alves (30):
> Don't rely on immediate_quit in command_line_input
> Inline command_loop in read_command_line
> TUI: check whether in secondary prompt instead of immediate_quit
> Don't set immediate_quit in prompt_for_continue
> Stop remote-fileio.c from throwing from SIGINT handler
> Remove unused struct serial::name field
> Introduce a serial interface for select'able events
> Fix signal handler/event-loop races
> Introduce interruptible_select
> Make Python use a struct serial event
> Don't call clear_quit_flag after check_quit_flag
> Don't call clear_quit_flag in command_handler
> Don't call clear_quit_flag in prepare_to_throw_exception
> Don't call clear_quit_flag in captured_main
> Eliminate clear_quit_flag
> Decouple target_interrupt from all-stop/non-stop modes
> Pass Ctrl-C to the target in target_terminal_inferior
> Fix inconsistent handling of EINTR in ser-*.c backends
> ada-lang.c: Introduce type_as_string and use it
> Use target_terminal_ours_for_output in cp-support.c
> Use target_terminal_ours_for_output in exceptions.c
> Use target_terminal_ours_for_output in infcmd.c
> Use target_terminal_ours_for_output in warning/internal_error
> Add missing cleanups to defaulted_query and prompt_for_continue
> Do target_terminal_ours in query & friends instead of in all callers
> Use target_terminal_ours_for_output in MI
> TUI: GC tui_target_has_run
> target remote: Don't rely on immediate_quit (introduce quit handlers)
> Eliminate immediate_quit
> Eliminate target_check_pending_interrupt
>
> gdb/Makefile.in | 6 +-
> gdb/ada-lang.c | 74 ++++++-----
> gdb/cp-support.c | 4 +-
> gdb/defs.h | 58 +++++++--
> gdb/event-loop.c | 41 ++++--
> gdb/event-loop.h | 26 +---
> gdb/event-top.c | 165 ++++++++++++++++++++++---
> gdb/exceptions.c | 17 ++-
> gdb/extension-priv.h | 5 +-
> gdb/extension.c | 34 +++--
> gdb/gdb_select.h | 15 +++
> gdb/gnu-nat.c | 12 +-
> gdb/guile/guile.c | 1 -
> gdb/guile/scm-ports.c | 4 +-
> gdb/i386-tdep.c | 49 ++------
> gdb/infcmd.c | 10 +-
> gdb/linux-nat.c | 10 --
> gdb/linux-record.c | 54 +++-----
> gdb/main.c | 1 -
> gdb/mi/mi-interp.c | 125 ++++++++++++++++---
> gdb/mi/mi-main.c | 7 +-
> gdb/mingw-hdep.c | 52 +-------
> gdb/nto-procfs.c | 4 -
> gdb/posix-hdep.c | 13 --
> gdb/python/python.c | 44 +++----
> gdb/record-full.c | 22 ++--
> gdb/remote-fileio.c | 34 +----
> gdb/remote-sim.c | 5 +-
> gdb/remote.c | 329 +++++++++++++++++++++----------------------------
> gdb/ser-base.c | 30 ++++-
> gdb/ser-event.c | 220 +++++++++++++++++++++++++++++++++
> gdb/ser-event.h | 51 ++++++++
> gdb/ser-go32.c | 4 +
> gdb/ser-unix.c | 108 ++--------------
> gdb/serial.c | 66 ++++++----
> gdb/serial.h | 8 +-
> gdb/target-delegates.c | 25 ++--
> gdb/target.c | 17 ++-
> gdb/target.h | 19 +--
> gdb/top.c | 67 +++++-----
> gdb/tui/tui-hooks.c | 24 ----
> gdb/tui/tui-io.c | 2 +-
> gdb/tui/tui.c | 4 +-
> gdb/ui-file.c | 8 +-
> gdb/utils.c | 66 ++++++----
> 45 files changed, 1115 insertions(+), 825 deletions(-)
> create mode 100644 gdb/ser-event.c
> create mode 100644 gdb/ser-event.h
>
I went over the series, and I understood some of the words used. What I understood
looked very good. For the rest, I trust you are doing the right thing. :)
Thanks a lot for doing this, since this is the major roadblock before we can't start
using C++!
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 00/30] Stop throwing exceptions from signal handlers
2016-03-21 19:52 ` [PATCH 00/30] Stop throwing exceptions from signal handlers Simon Marchi
@ 2016-03-31 14:43 ` Pedro Alves
2016-03-31 18:44 ` Pedro Alves
2016-04-12 16:15 ` Pedro Alves
0 siblings, 2 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-31 14:43 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 03/21/2016 07:52 PM, Simon Marchi wrote:
> I went over the series, and I understood some of the words used. What I understood
> looked very good. For the rest, I trust you are doing the right thing. :)
:-) Feel free to ask about anything, and I'll try to clarify.
> Thanks a lot for doing this, since this is the major roadblock before we can't start
> using C++!
Yup! I'm looking forward to have this in, and switch back
TRY/CATCH to be backed by real C++ exceptions, again.
I think that this might bring in the speed up Yichun Zhang
wanted here:
https://sourceware.org/ml/gdb-patches/2015-02/msg00272.html
if he builds in C++ mode, of course.
If anyone else wants to take a look at the series,
please let me know. Otherwise, I'd like to push it in soon.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 00/30] Stop throwing exceptions from signal handlers
2016-03-31 14:43 ` Pedro Alves
@ 2016-03-31 18:44 ` Pedro Alves
2016-04-12 16:15 ` Pedro Alves
1 sibling, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-03-31 18:44 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 03/31/2016 03:43 PM, Pedro Alves wrote:
> On 03/21/2016 07:52 PM, Simon Marchi wrote:
>> I went over the series, and I understood some of the words used. What I understood
>> looked very good. For the rest, I trust you are doing the right thing. :)
>
> :-) Feel free to ask about anything, and I'll try to clarify.
>
>> Thanks a lot for doing this, since this is the major roadblock before we can't start
>> using C++!
>
> Yup! I'm looking forward to have this in, and switch back
> TRY/CATCH to be backed by real C++ exceptions, again.
>
> I think that this might bring in the speed up Yichun Zhang
> wanted here:
>
> https://sourceware.org/ml/gdb-patches/2015-02/msg00272.html
>
> if he builds in C++ mode, of course.
Actually, this series makes it possible to use setjmp instead of
sigsetjump for all GDB exceptions and thus get the speedup in C
mode as well.
> If anyone else wants to take a look at the series,
> please let me know. Otherwise, I'd like to push it in soon.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 00/30] Stop throwing exceptions from signal handlers
2016-03-31 14:43 ` Pedro Alves
2016-03-31 18:44 ` Pedro Alves
@ 2016-04-12 16:15 ` Pedro Alves
1 sibling, 0 replies; 41+ messages in thread
From: Pedro Alves @ 2016-04-12 16:15 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 03/31/2016 03:43 PM, Pedro Alves wrote:
> If anyone else wants to take a look at the series,
> please let me know. Otherwise, I'd like to push it in soon.
FYI, I've pushed this in now.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 41+ messages in thread