* DevNotes: Add entry cgf-000022.

* cygtls.h (_cygtls::func): Define as a sa_sigaction style function.
* exceptions.cc (sig_handle_tty_stop): Ditto.
(_cygtls::interrupt_setup): Fix coercion to accommodate 'func' change.
(ctrl_c_handler): Use tty kill_pgrp to send a signal.
(sigpacket::process): Don't process sigflush here.
(_cygtls::call_signal_handler): Reorganize to avoid a race.  Always call
sa_sigaction style function.
* fhandler_termios.cc (is_flush_sig): Define new function.
(tty_min::kill_pgrp): Handle tty flush when signal detected.
(fhandler_termios::bg_check): Be slightly more paranoid about checking for
valid tty.
(fhandler_termios::sigflush): Don't flush unless tty owner.
* fhandler_tty.cc (fhandler_pty_slave::ioctl): Use tty kill_pgrp to send
signal.
(fhandler_pty_master::ioctl): Ditto.
* signal.cc (killsys): Delete definition.
* sigproc.h (killsys): Delete declaration.
* include/cygwin/signal.h (siginfo_t): Simplify union/struct nesting slightly.
Implement mechanism to allow cygwin data passing.

9 files changed, 100 insertions, 68 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 873b94346..a50875fca 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,26 @@
+2013-01-31  Christopher Faylor  <me.cygwin2013@cgf.cx>
+
+	* DevNotes: Add entry cgf-000022.
+	* cygtls.h (_cygtls::func): Define as a sa_sigaction style function.
+	* exceptions.cc (sig_handle_tty_stop): Ditto.
+	(_cygtls::interrupt_setup): Fix coercion to accommodate 'func' change.
+	(ctrl_c_handler): Use tty kill_pgrp to send a signal.
+	(sigpacket::process): Don't process sigflush here.
+	(_cygtls::call_signal_handler): Reorganize to avoid a race.  Always
+	call sa_sigaction style function.
+	* fhandler_termios.cc (is_flush_sig): Define new function.
+	(tty_min::kill_pgrp): Handle tty flush when signal detected.
+	(fhandler_termios::bg_check): Be slightly more paranoid about checking
+	for valid tty.
+	(fhandler_termios::sigflush): Don't flush unless tty owner.
+	* fhandler_tty.cc (fhandler_pty_slave::ioctl): Use tty kill_pgrp to
+	send signal.
+	(fhandler_pty_master::ioctl): Ditto.
+	* signal.cc (killsys): Delete definition.
+	* sigproc.h (killsys): Delete declaration.
+	* include/cygwin/signal.h (siginfo_t): Simplify union/struct nesting
+	slightly.  Implement mechanism to allow cygwin data passing.
+
 2013-01-23  Christopher Faylor  <me.cygwin2013@cgf.cx>
 
 	* miscfuncs.cc (__import_address): Check if malloc field points
diff --git a/winsup/cygwin/DevNotes b/winsup/cygwin/DevNotes
index e477e614a..70f4ec22a 100644
--- a/winsup/cygwin/DevNotes
+++ b/winsup/cygwin/DevNotes
@@ -1,3 +1,18 @@
+2013-01-31  cgf-000022
+
+While researching the lftp behavior reported here:
+
+http://cygwin.com/ml/cygwin/2013-01/msg00390.html
+
+after a frenzy of rewriting sigflush handling to avoid blocking in the
+signal thread (which is now and should ever have been illegal), it
+dawned on me that we're not supposed to be flushing the tty input buffer
+every time a signal is received.  We're supposed to do this only when
+the user hits a character (e.g., CTRL-C) which initiates a signal
+action.  So, I removed sigflush from sigpacket::process and moved it to
+tc ()->kill_pgrp ().  This function should only be called to send
+signals related to the tty so this should have the desired effect.
+
 2013-01-11  cgf-000021
 
 Apparently I got the signal handling semantics of select() wrong again
diff --git a/winsup/cygwin/cygtls.h b/winsup/cygwin/cygtls.h
index e2e926ff8..a5b622111 100644
--- a/winsup/cygwin/cygtls.h
+++ b/winsup/cygwin/cygtls.h
@@ -178,7 +178,7 @@ public:
     char __dontuse[8 * ((sizeof(struct _reent) + 4) / 8)];
   };
   /**/
-  void (*func) /*gentls_offsets*/(int)/*gentls_offsets*/;
+  void (*func) /*gentls_offsets*/(int, siginfo_t *, void *)/*gentls_offsets*/;
   int saved_errno;
   int sa_flags;
   sigset_t oldmask;
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 56e9bc106..b3e327779 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -688,7 +688,7 @@ extern DWORD exec_exit;		// Possible exit value for exec
 
 extern "C" {
 static void
-sig_handle_tty_stop (int sig)
+sig_handle_tty_stop (int sig, siginfo_t *, void *)
 {
   _my_tls.incyg = 1;
   /* Silently ignore attempts to suspend if there is no accommodating
@@ -748,7 +748,7 @@ _cygtls::interrupt_setup (siginfo_t& si, void *handler, struct sigaction& siga)
   push ((__stack_t) sigdelayed);
   deltamask = siga.sa_mask & ~SIG_NONMASKABLE;
   sa_flags = siga.sa_flags;
-  func = (void (*) (int)) handler;
+  func = (void (*) (int, siginfo_t *, void *)) handler;
   if (siga.sa_flags & SA_RESETHAND)
     siga.sa_handler = SIG_DFL;
   saved_errno = -1;		// Flag: no errno to save
@@ -956,7 +956,7 @@ ctrl_c_handler (DWORD type)
 	  && t->ti.c_cc[VINTR] == 3 && t->ti.c_cc[VQUIT] == 3)
 	sig = SIGQUIT;
       t->last_ctrl_c = GetTickCount ();
-      killsys (-myself->pid, sig);
+      t->kill_pgrp (sig);
       t->last_ctrl_c = GetTickCount ();
       return TRUE;
     }
@@ -1145,19 +1145,6 @@ sigpacket::process ()
       sig_clear (SIGTTOU);
     }
 
-  switch (si.si_signo)
-    {
-    case SIGINT:
-    case SIGQUIT:
-    case SIGSTOP:
-    case SIGTSTP:
-      if (cygheap->ctty)
-	cygheap->ctty->sigflush ();
-      break;
-    default:
-      break;
-    }
-
   int rc = 1;
 
   sigproc_printf ("signal %d processing", si.si_signo);
@@ -1301,28 +1288,23 @@ _cygtls::call_signal_handler ()
 
       debug_only_printf ("dealing with signal %d", sig);
       this_sa_flags = sa_flags;
+
+      /* Save information locally on stack to pass to handler. */
       int thissig = sig;
-      void (*thisfunc) (int) = func;
+      siginfo_t thissi = infodata;
+      void (*thisfunc) (int, siginfo_t *, void *) = func;
 
       sigset_t this_oldmask = set_process_mask_delta ();
       int this_errno = saved_errno;
-      sig = 0;
+      sig = 0;		/* Flag that we can accept another signal */
       reset_signal_arrived ();
-      unlock ();	// make sure synchronized
-      if (!(this_sa_flags & SA_SIGINFO))
-	{
-	  incyg = false;
-	  thisfunc (thissig);
-	}
-      else
-	{
-	  siginfo_t thissi = infodata;
-	  void (*sigact) (int, siginfo_t *, void *) = (void (*) (int, siginfo_t *, void *)) thisfunc;
-	  /* no ucontext_t information provided yet */
-	  incyg = false;
-	  sigact (thissig, &thissi, NULL);
-	}
+      unlock ();	/* unlock signal stack */
+
+      incyg = false;
+      /* no ucontext_t information provided yet, so third arg is NULL */
+      thisfunc (thissig, &thissi, NULL);
       incyg = true;
+
       set_signal_mask (_my_tls.sigmask, this_oldmask);
       if (this_errno >= 0)
 	set_errno (this_errno);
diff --git a/winsup/cygwin/fhandler_termios.cc b/winsup/cygwin/fhandler_termios.cc
index bd75ee804..1383d5e37 100644
--- a/winsup/cygwin/fhandler_termios.cc
+++ b/winsup/cygwin/fhandler_termios.cc
@@ -114,14 +114,23 @@ fhandler_pty_master::tcgetpgrp ()
   return tc ()->pgid;
 }
 
+static inline bool
+is_flush_sig (int sig)
+{
+  return sig == SIGINT || sig == SIGQUIT || sig == SIGTSTP;
+}
+
 void
 tty_min::kill_pgrp (int sig)
 {
   bool killself = false;
+  if (is_flush_sig (sig) && cygheap->ctty)
+    cygheap->ctty->sigflush ();
   winpids pids ((DWORD) PID_MAP_RW);
   siginfo_t si = {0};
   si.si_signo = sig;
   si.si_code = SI_KERNEL;
+
   for (unsigned i = 0; i < pids.npids; i++)
     {
       _pinfo *p = pids[i];
@@ -163,7 +172,7 @@ tty_min::is_orphaned_process_group (int pgid)
 bg_check_types
 fhandler_termios::bg_check (int sig)
 {
-  if (!myself->pgid || tc ()->getpgid () == myself->pgid ||
+  if (!myself->pgid || !tc () || tc ()->getpgid () == myself->pgid ||
 	myself->ctty != tc ()->ntty ||
 	((sig == SIGTTOU) && !(tc ()->ti.c_lflag & TOSTOP)))
     return bg_ok;
@@ -396,8 +405,9 @@ fhandler_termios::sigflush ()
   /* FIXME: Checking get_ttyp() for NULL is not right since it should not
      be NULL while this is alive.  However, we can conceivably close a
      ctty while exiting and that will zero this. */
-  if ((!have_execed || have_execed_cygwin) && get_ttyp ()
-      && !(get_ttyp ()->ti.c_lflag & NOFLSH))
+  if ((!have_execed || have_execed_cygwin) && tc ()
+      && (tc ()->getpgid () == myself->pgid)
+      && !(tc ()->ti.c_lflag & NOFLSH))
     tcflush (TCIFLUSH);
 }
 
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 06a64739d..e34707944 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -1053,7 +1053,7 @@ fhandler_pty_slave::ioctl (unsigned int cmd, void *arg)
 	{
 	  get_ttyp ()->arg.winsize = *(struct winsize *) arg;
 	  get_ttyp ()->winsize = *(struct winsize *) arg;
-	  killsys (-get_ttyp ()->getpgid (), SIGWINCH);
+	  get_ttyp ()->kill_pgrp (SIGWINCH);
 	}
       break;
     }
@@ -1425,7 +1425,7 @@ fhandler_pty_master::ioctl (unsigned int cmd, void *arg)
 	  || get_ttyp ()->winsize.ws_col != ((struct winsize *) arg)->ws_col)
 	{
 	  get_ttyp ()->winsize = *(struct winsize *) arg;
-	  killsys (-get_ttyp ()->getpgid (), SIGWINCH);
+	  get_ttyp ()->kill_pgrp (SIGWINCH);
 	}
       break;
     case TIOCGPGRP:
diff --git a/winsup/cygwin/include/cygwin/signal.h b/winsup/cygwin/include/cygwin/signal.h
index 31fdf1296..268b5eb74 100644
--- a/winsup/cygwin/include/cygwin/signal.h
+++ b/winsup/cygwin/include/cygwin/signal.h
@@ -1,7 +1,7 @@
 /* signal.h
 
-  Copyright 2003, 2004, 2005, 2006, 2007, 2008, 2010, 2011, 2012, 2013 Red
-  Hat, Inc.
+  Copyright 2003, 2004, 2005, 2006, 2007, 2008, 2010, 2011, 2012, 2013
+  Red Hat, Inc.
 
   This file is part of Cygwin.
 
@@ -90,6 +90,19 @@ struct _sigcommune
   };
 };
 
+#define __SI_PAD_SIZE 32
+#ifdef __INSIDE_CYGWIN__
+# ifndef max
+#   define max(a,b) (((a) > (b)) ? (a) : (b))
+# endif /*max*/
+# define __uint32_size(__x) (max(sizeof (__x) / sizeof (uint32_t), 1))
+
+/* This padding represents the elements of the last struct in siginfo_t,
+   aligning the elements to the end to avoid conflicts with other struct
+   members. */
+# define __SI_CYG_PAD (__SI_PAD_SIZE - __uint32_size (void *))
+#endif /*__INSIDE_CYGWIN__*/
+
 typedef struct
 {
   int si_signo;				/* signal number */
@@ -100,26 +113,21 @@ typedef struct
 
   __extension__ union
   {
-    __uint32_t __pad[32];		/* plan for future growth */
+    __uint32_t __pad[__SI_PAD_SIZE];	/* plan for future growth */
     struct _sigcommune _si_commune;	/* cygwin ipc */
-    __extension__ union
+    __extension__ struct
     {
-      /* timers */
-      struct
+      __extension__ union
       {
-	union
-	{
-	  struct
-	  {
-	    timer_t si_tid;		/* timer id */
-	    unsigned int si_overrun;	/* overrun count */
-	  };
-	  sigval_t si_sigval;		/* signal value */
-	  sigval_t si_value;		/* signal value */
-	};
+	sigval_t si_sigval;		/* signal value */
+	sigval_t si_value;		/* signal value */
+      };
+      __extension__ struct
+      {
+	timer_t si_tid;			/* timer id */
+	unsigned int si_overrun;	/* overrun count */
       };
     };
-
     /* SIGCHLD */
     __extension__ struct
     {
@@ -128,13 +136,17 @@ typedef struct
       clock_t si_stime;			/* system time */
     };
 
-    __extension__ struct
+    void *si_addr;			/* faulting address for core dumping
+					   signals */
+    /* Cygwin internal fields */
+#ifdef __INSIDE_CYGWIN__
+    __extension__ struct 
     {
-      /* core dumping signals */
-      void *si_addr;			/* faulting address */
+      __uint32_t __pad2[__SI_CYG_PAD];	/* Locate at end of struct */
       void *si_cyg;			/* pointer to block containing
 					   cygwin-special info */
     };
+#endif /*__INSIDE_CYGWIN__*/
   };
 } siginfo_t;
 #pragma pack(pop)
diff --git a/winsup/cygwin/signal.cc b/winsup/cygwin/signal.cc
index 444d56a34..c99220d35 100644
--- a/winsup/cygwin/signal.cc
+++ b/winsup/cygwin/signal.cc
@@ -300,15 +300,6 @@ kill0 (pid_t pid, siginfo_t& si)
 }
 
 int
-killsys (pid_t pid, int sig)
-{
-  siginfo_t si = {0};
-  si.si_signo = sig;
-  si.si_code = SI_KERNEL;
-  return kill0 (pid, si);
-}
-
-int
 kill (pid_t pid, int sig)
 {
   siginfo_t si = {0};
diff --git a/winsup/cygwin/sigproc.h b/winsup/cygwin/sigproc.h
index df10ed30c..9c70c2c2d 100644
--- a/winsup/cygwin/sigproc.h
+++ b/winsup/cygwin/sigproc.h
@@ -80,7 +80,6 @@ void __stdcall signal_fixup_after_exec ();
 void __stdcall sigalloc ();
 
 int kill_pgrp (pid_t, siginfo_t&);
-int killsys (pid_t, int);
 void __reg1 exit_thread (DWORD) __attribute__ ((noreturn));
 void __reg1 setup_signal_exit (int);