gdb/gdb-readline-6.0-signal.patch

http://sourceware.org/ml/gdb-patches/2009-11/msg00596.html
Subject: [gdb FYI-patch] callback-mode readline-6.0 regression

Hi Chet,

FSF GDB currently ships bundled with readline-5.2 which works fine.
But using --with-system-readline and readline-6.0-patchlevel4 has
a regression:

readline-5.2: Run `gdb -nx -q' and type CTRL-C:
(gdb) Quit
(gdb) _

readline-6.0: Run `gdb -nx -q' and type CTRL-C:
(gdb) _
 = nothing happens (it gets buffered and executed later)
	(It does also FAIL on gdb.gdb/selftest.exp.)

It is because GDB waits in its own poll() mainloop and readline uses via
rl_callback_handler_install and rl_callback_handler_remove.  This way the
readline internal variable _rl_interrupt_immediately remains 0 and CTRL-C gets
only stored to _rl_caught_signal but not executed.

Seen in rl_signal_handler even if _rl_interrupt_immediately is set and
_rl_handle_signal is called then the signal is still stored to
_rl_caught_signal.  In the _rl_interrupt_immediately case it should not be
stored when it was already processed.

rl_signal_handler does `_rl_interrupt_immediately = 0;' - while I am not aware
of its meaning it breaks the nest-counting of other routines which do
`_rl_interrupt_immediately++;' and `_rl_interrupt_immediately--;' possibly
creating problematic `_rl_interrupt_immediately == -1'.

`_rl_interrupt_immediately' is an internal variable, how it could be accessed
by a readline application? (OK, maybe it should not be used.)

Attaching a current GDB-side patch but it must access readline internal
variable _rl_caught_signal and it is generally just a workaround.  Could you
please include support for signals in this asynchronous mode in readline-6.1?
I find it would be enough to make RL_CHECK_SIGNALS public?


GDB: No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.
But this is not a patch intended to be accepted.


Thanks,
Jan


gdb/
2009-11-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* config.in, configure: Regenerate.
	* configure.ac (for readline_echoing_p): Move inside $LIBS change.
	(for _rl_caught_signal): New.
	* event-loop.c: Include readline/readline.h.
	(gdb_do_one_event) [HAVE_READLINE_CAUGHT_SIGNAL]: New.

gdb/testsuite/
2009-11-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.gdb/selftest.exp (backtrace through signal handler): Move before
	SIGINT pass, drop the timeout case.
	(send SIGINT signal to child process): Use gdb_test.
	(backtrace through readline handler): New.

Index: gdb-7.0.50.20100115/gdb/config.in
===================================================================
--- gdb-7.0.50.20100115.orig/gdb/config.in	2010-01-15 12:48:04.000000000 +0100
+++ gdb-7.0.50.20100115/gdb/config.in	2010-01-15 12:48:40.000000000 +0100
@@ -359,6 +359,9 @@
 /* Define if Python interpreter is being linked in. */
 #undef HAVE_PYTHON
 
+/* readline-6.0 workaround of blocked signals. */
+#undef HAVE_READLINE_CAUGHT_SIGNAL
+
 /* Define to 1 if you have the `realpath' function. */
 #undef HAVE_REALPATH
 
Index: gdb-7.0.50.20100115/gdb/configure.ac
===================================================================
--- gdb-7.0.50.20100115.orig/gdb/configure.ac	2010-01-15 12:48:04.000000000 +0100
+++ gdb-7.0.50.20100115/gdb/configure.ac	2010-01-15 12:48:40.000000000 +0100
@@ -777,17 +777,25 @@ if test "$with_system_readline" = yes; t
   # readline-6.0 started to use the name `_rl_echoing_p'.
   # `$(READLINE_DIR)/' of bundled readline would not resolve in configure.
 
-  AC_MSG_CHECKING([for readline_echoing_p])
   save_LIBS=$LIBS
   LIBS="$LIBS $READLINE"
+  AC_MSG_CHECKING([for readline_echoing_p])
   AC_LINK_IFELSE(AC_LANG_PROGRAM(,[[extern int readline_echoing_p;
 				    return readline_echoing_p;]]),
 		 [READLINE_ECHOING_P=yes],
 		 [READLINE_ECHOING_P=no
 		  AC_DEFINE([readline_echoing_p], [_rl_echoing_p],
 			    [readline-6.0 started to use different name.])])
-  LIBS="$save_LIBS"
   AC_MSG_RESULT([$READLINE_ECHOING_P])
+  AC_MSG_CHECKING([for _rl_caught_signal])
+  AC_LINK_IFELSE(AC_LANG_PROGRAM(,[[extern int volatile _rl_caught_signal;
+				    return _rl_caught_signal;]]),
+		 [READLINE_CAUGHT_SIGNAL=yes
+		  AC_DEFINE([HAVE_READLINE_CAUGHT_SIGNAL],,
+			    [readline-6.0 workaround of blocked signals.])],
+		 [READLINE_CAUGHT_SIGNAL=no])
+  AC_MSG_RESULT([$READLINE_CAUGHT_SIGNAL])
+  LIBS="$save_LIBS"
 else
   READLINE='$(READLINE_DIR)/libreadline.a'
   READLINE_DEPS='$(READLINE)'
Index: gdb-7.0.50.20100115/gdb/event-loop.c
===================================================================
--- gdb-7.0.50.20100115.orig/gdb/event-loop.c	2010-01-01 08:31:31.000000000 +0100
+++ gdb-7.0.50.20100115/gdb/event-loop.c	2010-01-15 12:48:40.000000000 +0100
@@ -37,6 +37,7 @@
 #include "exceptions.h"
 #include "gdb_assert.h"
 #include "gdb_select.h"
+#include "readline/readline.h"
 
 /* Data point to pass to the event handler.  */
 typedef union event_data
@@ -411,6 +412,9 @@ gdb_do_one_event (void *data)
   static int event_source_head = 0;
   const int number_of_sources = 3;
   int current = 0;
+#ifdef HAVE_READLINE_CAUGHT_SIGNAL
+  extern int volatile _rl_caught_signal;
+#endif
 
   /* Any events already waiting in the queue?  */
   if (process_event ())
@@ -455,6 +459,16 @@ gdb_do_one_event (void *data)
   if (gdb_wait_for_event (1) < 0)
     return -1;
 
+#ifdef HAVE_READLINE_CAUGHT_SIGNAL
+  if (async_command_editing_p && RL_ISSTATE (RL_STATE_CALLBACK)
+      && _rl_caught_signal)
+    {
+      /* Call RL_CHECK_SIGNALS this way.  */
+      rl_callback_handler_remove ();
+      rl_callback_handler_install (NULL, input_handler);
+    }
+#endif
+
   /* Handle any new events occurred while waiting.  */
   if (process_event ())
     return 1;
Index: gdb-7.0.50.20100115/gdb/testsuite/gdb.gdb/selftest.exp
===================================================================
--- gdb-7.0.50.20100115.orig/gdb/testsuite/gdb.gdb/selftest.exp	2010-01-15 12:48:01.000000000 +0100
+++ gdb-7.0.50.20100115/gdb/testsuite/gdb.gdb/selftest.exp	2010-01-15 12:48:40.000000000 +0100
@@ -471,31 +471,42 @@ GDB.*Copyright \[0-9\]+ Free Software Fo
 	    fail "$description (timeout)"
 	}
     }
-    
-    set description "send SIGINT signal to child process"
-    send_gdb "signal SIGINT\n"
-    gdb_expect {
-	-re "Continuing with signal SIGINT.*$gdb_prompt $" {
+
+    # get a stack trace with the poll function
+    #
+    # This fails on some linux systems for unknown reasons.  On the
+    # systems where it fails, sometimes it works fine when run manually.
+    # The testsuite failures may not be limited to just aout systems.
+    setup_xfail "i*86-pc-linuxaout-gnu"
+    set description "backtrace through signal handler"
+    gdb_test_multiple "backtrace" $description {
+	-re "#0.*(read|poll).*in main \\(.*\\) at .*gdb\\.c.*$gdb_prompt $" {
 	    pass "$description"
 	}
 	-re ".*$gdb_prompt $" {
+	    # On the alpha, we hit the infamous problem about gdb
+	    # being unable to get the frame pointer (mentioned in
+	    # gdb/README).  As it is intermittent, there is no way to
+	    # XFAIL it which will give us an XPASS if the problem goes
+	    # away.
+	    setup_xfail "alpha*-*-osf*"
 	    fail "$description"
 	}
-	timeout {
-	    fail "$description (timeout)"
-	}
     }
     
-    # get a stack trace
+    gdb_test "signal SIGINT" "Continuing with signal SIGINT.*" \
+	     "send SIGINT signal to child process"
+    
+    # get a stack trace being redelivered by readline
     #
     # This fails on some linux systems for unknown reasons.  On the
     # systems where it fails, sometimes it works fine when run manually.
     # The testsuite failures may not be limited to just aout systems.
+    # Optional system readline may not have symbols to be shown.
     setup_xfail "i*86-pc-linuxaout-gnu"
-    set description "backtrace through signal handler"
-    send_gdb "backtrace\n"
-    gdb_expect {
-	-re "#0.*(read|poll).*in main \\(.*\\) at .*gdb\\.c.*$gdb_prompt $" {
+    set description "backtrace through readline handler"
+    gdb_test_multiple "backtrace" $description {
+	-re "#0.*gdb_do_one_event.*in main \\(.*\\) at .*gdb\\.c.*$gdb_prompt $" {
 	    pass "$description"
 	}
 	-re ".*$gdb_prompt $" {
@@ -507,9 +518,6 @@ GDB.*Copyright \[0-9\]+ Free Software Fo
 	    setup_xfail "alpha*-*-osf*"
 	    fail "$description"
 	}
-	timeout {
-	    fail "$description (timeout)"
-	}
     }
 
 
Index: gdb-7.0.50.20100115/gdb/configure
===================================================================
--- gdb-7.0.50.20100115.orig/gdb/configure	2010-01-15 12:48:04.000000000 +0100
+++ gdb-7.0.50.20100115/gdb/configure	2010-01-15 12:48:46.000000000 +0100
@@ -6772,6 +6772,185 @@ else
 fi
 
 
+
+
+
+
+# Check whether --with-separate-debug-dir was given.
+if test "${with_separate_debug_dir+set}" = set; then :
+  withval=$with_separate_debug_dir;
+    DEBUGDIR=$withval
+else
+  DEBUGDIR=${libdir}/debug
+fi
+
+
+  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
+  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
+  ac_define_dir=`eval echo $DEBUGDIR`
+  ac_define_dir=`eval echo $ac_define_dir`
+
+cat >>confdefs.h <<_ACEOF
+#define DEBUGDIR "$ac_define_dir"
+_ACEOF
+
+
+
+  if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}'; then
+     if test "x$prefix" = xNONE; then
+     	test_prefix=/usr/local
+     else
+	test_prefix=$prefix
+     fi
+  else
+     test_prefix=$exec_prefix
+  fi
+  value=0
+  case ${ac_define_dir} in
+     "${test_prefix}"|"${test_prefix}/"*|\
+	'${exec_prefix}'|'${exec_prefix}/'*)
+     value=1
+     ;;
+  esac
+
+cat >>confdefs.h <<_ACEOF
+#define DEBUGDIR_RELOCATABLE $value
+_ACEOF
+
+
+
+# GDB's datadir relocation
+
+
+
+# Check whether --with-gdb-datadir was given.
+if test "${with_gdb_datadir+set}" = set; then :
+  withval=$with_gdb_datadir;
+    GDB_DATADIR=$withval
+else
+  GDB_DATADIR=${datadir}/gdb
+fi
+
+
+  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
+  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
+  ac_define_dir=`eval echo $GDB_DATADIR`
+  ac_define_dir=`eval echo $ac_define_dir`
+
+cat >>confdefs.h <<_ACEOF
+#define GDB_DATADIR "$ac_define_dir"
+_ACEOF
+
+
+
+  if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}'; then
+     if test "x$prefix" = xNONE; then
+     	test_prefix=/usr/local
+     else
+	test_prefix=$prefix
+     fi
+  else
+     test_prefix=$exec_prefix
+  fi
+  value=0
+  case ${ac_define_dir} in
+     "${test_prefix}"|"${test_prefix}/"*|\
+	'${exec_prefix}'|'${exec_prefix}/'*)
+     value=1
+     ;;
+  esac
+
+cat >>confdefs.h <<_ACEOF
+#define GDB_DATADIR_RELOCATABLE $value
+_ACEOF
+
+
+
+
+# Check whether --with-relocated-sources was given.
+if test "${with_relocated_sources+set}" = set; then :
+  withval=$with_relocated_sources; reloc_srcdir="${withval}"
+
+  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
+  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
+  ac_define_dir=`eval echo $reloc_srcdir`
+  ac_define_dir=`eval echo $ac_define_dir`
+
+cat >>confdefs.h <<_ACEOF
+#define RELOC_SRCDIR "$ac_define_dir"
+_ACEOF
+
+
+
+fi
+
+
+# GDB's datadir relocation
+
+gdbdatadir=${datadir}/gdb
+
+
+# Check whether --with-gdb-datadir was given.
+if test "${with_gdb_datadir+set}" = set; then :
+  withval=$with_gdb_datadir; gdbdatadir="${withval}"
+fi
+
+
+
+  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
+  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
+  ac_define_dir=`eval echo $gdbdatadir`
+  ac_define_dir=`eval echo $ac_define_dir`
+
+cat >>confdefs.h <<_ACEOF
+#define GDB_DATADIR "$ac_define_dir"
+_ACEOF
+
+
+
+if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}'; then
+  if test "x$prefix" = xNONE; then
+    test_prefix=/usr/local
+  else
+    test_prefix=$prefix
+  fi
+else
+  test_prefix=$exec_prefix
+fi
+
+case ${gdbdatadir} in
+  "${test_prefix}"|"${test_prefix}/"*|\
+  '${exec_prefix}'|'${exec_prefix}/'*)
+
+$as_echo "#define GDB_DATADIR_RELOCATABLE 1" >>confdefs.h
+
+  ;;
+esac
+GDB_DATADIR_PATH=${gdbdatadir}
+
+
+
+# Check whether --with-pythondir was given.
+if test "${with_pythondir+set}" = set; then :
+  withval=$with_pythondir; pythondir="${withval}"
+else
+  pythondir=no
+fi
+
+
+# If the user passed in a path, define it.  Otherwise, compute it at
+# runtime based on the possibly-relocatable datadir.
+if test "$pythondir" = "no"; then
+  pythondir='$(GDB_DATADIR_PATH)/python'
+else
+
+cat >>confdefs.h <<_ACEOF
+#define PYTHONDIR "$pythondir"
+_ACEOF
+
+fi
+
+
 # Integration with rpm library to support missing debuginfo suggestions.
 # --without-rpm: Disable any rpm support.
 # --with-rpm=libname.so: Try to dynamically open `libname.so' during runtime.
@@ -7255,185 +7434,6 @@ fi
 
 
 
-
-# Check whether --with-separate-debug-dir was given.
-if test "${with_separate_debug_dir+set}" = set; then :
-  withval=$with_separate_debug_dir;
-    DEBUGDIR=$withval
-else
-  DEBUGDIR=${libdir}/debug
-fi
-
-
-  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
-  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
-  ac_define_dir=`eval echo $DEBUGDIR`
-  ac_define_dir=`eval echo $ac_define_dir`
-
-cat >>confdefs.h <<_ACEOF
-#define DEBUGDIR "$ac_define_dir"
-_ACEOF
-
-
-
-  if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}'; then
-     if test "x$prefix" = xNONE; then
-     	test_prefix=/usr/local
-     else
-	test_prefix=$prefix
-     fi
-  else
-     test_prefix=$exec_prefix
-  fi
-  value=0
-  case ${ac_define_dir} in
-     "${test_prefix}"|"${test_prefix}/"*|\
-	'${exec_prefix}'|'${exec_prefix}/'*)
-     value=1
-     ;;
-  esac
-
-cat >>confdefs.h <<_ACEOF
-#define DEBUGDIR_RELOCATABLE $value
-_ACEOF
-
-
-
-# GDB's datadir relocation
-
-
-
-# Check whether --with-gdb-datadir was given.
-if test "${with_gdb_datadir+set}" = set; then :
-  withval=$with_gdb_datadir;
-    GDB_DATADIR=$withval
-else
-  GDB_DATADIR=${datadir}/gdb
-fi
-
-
-  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
-  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
-  ac_define_dir=`eval echo $GDB_DATADIR`
-  ac_define_dir=`eval echo $ac_define_dir`
-
-cat >>confdefs.h <<_ACEOF
-#define GDB_DATADIR "$ac_define_dir"
-_ACEOF
-
-
-
-  if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}'; then
-     if test "x$prefix" = xNONE; then
-     	test_prefix=/usr/local
-     else
-	test_prefix=$prefix
-     fi
-  else
-     test_prefix=$exec_prefix
-  fi
-  value=0
-  case ${ac_define_dir} in
-     "${test_prefix}"|"${test_prefix}/"*|\
-	'${exec_prefix}'|'${exec_prefix}/'*)
-     value=1
-     ;;
-  esac
-
-cat >>confdefs.h <<_ACEOF
-#define GDB_DATADIR_RELOCATABLE $value
-_ACEOF
-
-
-
-
-# Check whether --with-relocated-sources was given.
-if test "${with_relocated_sources+set}" = set; then :
-  withval=$with_relocated_sources; reloc_srcdir="${withval}"
-
-  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
-  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
-  ac_define_dir=`eval echo $reloc_srcdir`
-  ac_define_dir=`eval echo $ac_define_dir`
-
-cat >>confdefs.h <<_ACEOF
-#define RELOC_SRCDIR "$ac_define_dir"
-_ACEOF
-
-
-
-fi
-
-
-# GDB's datadir relocation
-
-gdbdatadir=${datadir}/gdb
-
-
-# Check whether --with-gdb-datadir was given.
-if test "${with_gdb_datadir+set}" = set; then :
-  withval=$with_gdb_datadir; gdbdatadir="${withval}"
-fi
-
-
-
-  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
-  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
-  ac_define_dir=`eval echo $gdbdatadir`
-  ac_define_dir=`eval echo $ac_define_dir`
-
-cat >>confdefs.h <<_ACEOF
-#define GDB_DATADIR "$ac_define_dir"
-_ACEOF
-
-
-
-if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}'; then
-  if test "x$prefix" = xNONE; then
-    test_prefix=/usr/local
-  else
-    test_prefix=$prefix
-  fi
-else
-  test_prefix=$exec_prefix
-fi
-
-case ${gdbdatadir} in
-  "${test_prefix}"|"${test_prefix}/"*|\
-  '${exec_prefix}'|'${exec_prefix}/'*)
-
-$as_echo "#define GDB_DATADIR_RELOCATABLE 1" >>confdefs.h
-
-  ;;
-esac
-GDB_DATADIR_PATH=${gdbdatadir}
-
-
-
-# Check whether --with-pythondir was given.
-if test "${with_pythondir+set}" = set; then :
-  withval=$with_pythondir; pythondir="${withval}"
-else
-  pythondir=no
-fi
-
-
-# If the user passed in a path, define it.  Otherwise, compute it at
-# runtime based on the possibly-relocatable datadir.
-if test "$pythondir" = "no"; then
-  pythondir='$(GDB_DATADIR_PATH)/python'
-else
-
-cat >>confdefs.h <<_ACEOF
-#define PYTHONDIR "$pythondir"
-_ACEOF
-
-fi
-
-
-
-
-
 subdirs="$subdirs doc testsuite"
 
 
@@ -9290,10 +9290,10 @@ if test "$with_system_readline" = yes; t
   # readline-6.0 started to use the name `_rl_echoing_p'.
   # `$(READLINE_DIR)/' of bundled readline would not resolve in configure.
 
-  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for readline_echoing_p" >&5
-$as_echo_n "checking for readline_echoing_p... " >&6; }
   save_LIBS=$LIBS
   LIBS="$LIBS $READLINE"
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for readline_echoing_p" >&5
+$as_echo_n "checking for readline_echoing_p... " >&6; }
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
@@ -9316,9 +9316,35 @@ $as_echo "#define readline_echoing_p _rl
 fi
 rm -f core conftest.err conftest.$ac_objext \
     conftest$ac_exeext conftest.$ac_ext
-  LIBS="$save_LIBS"
   { $as_echo "$as_me:${as_lineno-$LINENO}: result: $READLINE_ECHOING_P" >&5
 $as_echo "$READLINE_ECHOING_P" >&6; }
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for _rl_caught_signal" >&5
+$as_echo_n "checking for _rl_caught_signal... " >&6; }
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+int
+main ()
+{
+extern int volatile _rl_caught_signal;
+				    return _rl_caught_signal;
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  READLINE_CAUGHT_SIGNAL=yes
+
+$as_echo "#define HAVE_READLINE_CAUGHT_SIGNAL /**/" >>confdefs.h
+
+else
+  READLINE_CAUGHT_SIGNAL=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $READLINE_CAUGHT_SIGNAL" >&5
+$as_echo "$READLINE_CAUGHT_SIGNAL" >&6; }
+  LIBS="$save_LIBS"
 else
   READLINE='$(READLINE_DIR)/libreadline.a'
   READLINE_DEPS='$(READLINE)'