Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 05/30] Stop remote-fileio.c from throwing from SIGINT handler
Date: Fri, 18 Mar 2016 19:28:00 -0000	[thread overview]
Message-ID: <1458328714-4938-6-git-send-email-palves@redhat.com> (raw)
In-Reply-To: <1458328714-4938-1-git-send-email-palves@redhat.com>

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


  parent reply	other threads:[~2016-03-18 19:28 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` [PATCH 19/30] ada-lang.c: Introduce type_as_string and use it Pedro Alves
2016-03-18 19:19 ` [PATCH 14/30] Don't call clear_quit_flag in captured_main Pedro Alves
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 ` [PATCH 28/30] target remote: Don't rely on immediate_quit (introduce quit handlers) Pedro Alves
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 ` [PATCH 27/30] TUI: GC tui_target_has_run Pedro Alves
2016-03-18 19:19 ` [PATCH 30/30] Eliminate target_check_pending_interrupt Pedro Alves
2016-03-18 19:19 ` [PATCH 26/30] Use target_terminal_ours_for_output in MI Pedro Alves
2016-03-18 19:19 ` [PATCH 02/30] Inline command_loop in read_command_line Pedro Alves
2016-03-18 19:19 ` [PATCH 29/30] Eliminate immediate_quit Pedro Alves
2016-03-18 19:19 ` [PATCH 22/30] Use target_terminal_ours_for_output in infcmd.c Pedro Alves
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 ` [PATCH 25/30] Do target_terminal_ours in query & friends instead of in all callers Pedro Alves
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 ` [PATCH 04/30] Don't set immediate_quit in prompt_for_continue Pedro Alves
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 ` [PATCH 12/30] Don't call clear_quit_flag in command_handler Pedro Alves
2016-03-18 19:25 ` [PATCH 08/30] Fix signal handler/event-loop races Pedro Alves
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 ` [PATCH 10/30] Make Python use a struct serial event Pedro Alves
2016-03-18 19:26 ` [PATCH 07/30] Introduce a serial interface for select'able events Pedro Alves
2016-03-18 19:27 ` [PATCH 15/30] Eliminate clear_quit_flag Pedro Alves
2016-03-18 19:27 ` [PATCH 06/30] Remove unused struct serial::name field Pedro Alves
2016-03-18 19:27 ` [PATCH 21/30] Use target_terminal_ours_for_output in exceptions.c Pedro Alves
2016-03-18 19:28 ` [PATCH 11/30] Don't call clear_quit_flag after check_quit_flag Pedro Alves
2016-03-18 19:28 ` Pedro Alves [this message]
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
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
2016-03-21 19:49     ` Pedro Alves
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1458328714-4938-6-git-send-email-palves@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox