PATCH: PR tui/2173: Arrow keys no longer works in breakpoint command list
Jan Kratochvil
jan.kratochvil@redhat.com
Tue Jun 26 13:49:00 GMT 2007
Hi,
Chet, the patch would be ported later from the current GDB readline-5.1 to 5.2.
On Mon, 04 Jun 2007 03:06:33 +0200, Daniel Jacobowitz wrote:
> On Mon, Jun 04, 2007 at 10:54:25AM +1200, Nick Roberts wrote:
> >
> > This change breaks the behaviour of annotations with commands that span
> > multiple lines, like if, while, etc:
>
> Sorry, I didn't even know we had special annotations for this (and the
> testsuite must not cover it). I'm not sure how to fix it, either -
> the patch you cited was very tricky to come up with already :-(
The regression happened as the patch was very complex:
http://sourceware.org/ml/gdb-cvs/2007-01/msg00022.html
The synchronous readline() call has no asynchronous counterpart in libreadline.
Any asynchronous readline() emulation on top of the current libreadline API may
work but it is hard to make it precisely backward compatible.
Facts review:
* longjmp() out of readline() is so far documented API usage of libreadline:
@node Readline Signal Handling
If the application's signal handler does more than update its idea of
the terminal size and return (for example, a @code{longjmp} back to
a main processing loop),
* Application (GDB) cannot maintain the RL_STATE_CALLBACK flag as it is
undocumented and therefore internal to libreadline.
* Any (non-unwinder) based libreadline code cannot set the RL_STATE_CALLBACK
flag properly as it cannot find if the application longjmp()ed into a stack
depth originally being in callback or non-callback mode. Proof pseudocode:
http://sources.redhat.com/ml/gdb-patches/2006-12/msg00224.html
Multiple solutions exist:
(1) Code a new libreadline API function readline_async() which will try to
be backward compatible with readline().
- Such function complies with active RL_STATE_CALLBACK requreiment but
a synchronous function would better fit the current use in GDB.
(2) Provide a new documented way to abort readline() from a signal handler.
Provided rl_readline_unwind() needed to be called right before longjmp().
Proposed, patch attached.
+ Fully crossplatform, backward compatible.
+ Not affecting most of the apps as they do not longjmp() out of readline().
- New libreadline API function.
(3) Provide C++ try/catch handling (attached, GDB -lstd++ needed, deprecated).
+ Transparent to the application.
- libreadline.a has no backward compatible linking (-lstdc++ is needed)
- Systems without C++ will link but the current bug remains present there.
(4) Custom C based unwinder registration (such as C++ but without -lstdc++)
- gcc dependent, generally noncrossplatform.
Regards,
Jan
-------------- next part --------------
2007-06-26 Jan Kratochvil <jan.kratochvil@redhat.com>
* top.c (gdb_readline_wrapper) Wrap readline() by readline_cleanup().
(readline_cleanup): New function.
2007-06-26 Jan Kratochvil <jan.kratochvil@redhat.com>
H.J. Lu <hongjiu.lu@intel.com>
PR tui/2173
* readline.c (readline): Save/restore RL_STATE_CALLBACK into STATE_LIST.
(rl_readline_unwind, struct state, state_list): New.
* readline.h (rl_readline_unwind): New declaration.
2007-06-26 Jan Kratochvil <jan.kratochvil@redhat.com>
* rltech.texi (Readline Signal Handling): Describe rl_readline_unwind().
--- sources-unpatched/gdb/top.c 2007-06-26 13:04:11.000000000 +0200
+++ sources/gdb/top.c 2007-06-26 12:38:43.000000000 +0200
@@ -706,6 +706,13 @@ The filename in which to record the comm
value);
}
+static void
+readline_cleanup (void *unused)
+{
+ /* No need to check for it by autoconf as libreadline is bundled with GDB. */
+ rl_readline_unwind ();
+}
+
/* This is like readline(), but it has some gdb-specific behavior.
gdb can use readline in both the synchronous and async modes during
a single gdb invocation. At the ordinary top-level prompt we might
@@ -719,6 +726,9 @@ The filename in which to record the comm
char *
gdb_readline_wrapper (char *prompt)
{
+ char *retval;
+ struct cleanup *cleanups;
+
/* Set the hook that works in this case. */
if (after_char_processing_hook)
{
@@ -726,7 +736,11 @@ gdb_readline_wrapper (char *prompt)
after_char_processing_hook = NULL;
}
- return readline (prompt);
+ cleanups = make_cleanup (readline_cleanup, NULL);
+ retval = readline (prompt);
+ discard_cleanups (cleanups);
+
+ return retval;
}
--- sources-unpatched/readline/doc/rltech.texi 2007-06-26 12:54:57.000000000 +0200
+++ sources/readline/doc/rltech.texi 2007-06-26 12:40:50.000000000 +0200
@@ -1336,7 +1336,16 @@ resetting the terminal to its original s
handler does more than update its idea of the terminal size and return (for
example, a @code{longjmp} back to a main processing loop), it @emph{must}
call @code{rl_cleanup_after_signal()} (described below), to restore the
-terminal state.
+terminal state. Each @code{longjmp} call must be also protected by
+@code{rl_readline_unwind()}:
+
+@deftypefun void rl_readline_unwind (void)
+Each @code{readline()} call you jump out of by @code{longjmp} must be matched
+by one call of this function. It is appropriate to call it from your
+@code{catch} part of the C++ exception handler. This functiona must not be
+called if @code{readline()} returns itself. If you do not abort
+@code{readline()} by @code{longjmp} you must never call this function.
+@end deftypefun
Readline provides two variables that allow application writers to
control whether or not it will catch certain signals and act on them
--- sources-unpatched/readline/readline.c 2007-06-26 12:54:52.000000000 +0200
+++ sources/readline/readline.c 2007-06-26 12:33:57.000000000 +0200
@@ -287,7 +287,27 @@ rl_set_prompt (prompt)
rl_visible_prompt_length = rl_expand_prompt (rl_prompt);
return 0;
}
-
+
+struct state {
+ struct state *next;
+ int in_callback;
+};
+static struct state *state_list;
+
+/* To be called once for each readline () call which you have jumped out of by
+ longjmp() or siglongjmp(). Function needs to be called BEFORE the jump. */
+void
+rl_readline_unwind ()
+{
+ /* NULL should not happen. */
+ if (state_list != NULL)
+ {
+ if (state_list->in_callback)
+ RL_SETSTATE (RL_STATE_CALLBACK);
+ state_list = state_list->next;
+ }
+}
+
/* Read a line of input. Prompt with PROMPT. An empty PROMPT means
none. A return value of NULL means that EOF was encountered. */
char *
@@ -295,6 +315,7 @@ readline (prompt)
const char *prompt;
{
char *value;
+ struct state state;
/* If we are at EOF return a NULL string. */
if (rl_pending_input == EOF)
@@ -303,6 +324,15 @@ readline (prompt)
return ((char *)NULL);
}
+ /* When we call readline, we have to make sure that readline isn't in
+ the callback state. Otherwise, it will get really confused.
+ PR gdb tui/2173. */
+ state.next = state_list;
+ state_list = &state;
+ state.in_callback = RL_ISSTATE (RL_STATE_CALLBACK);
+ if (state.in_callback)
+ RL_UNSETSTATE (RL_STATE_CALLBACK);
+
rl_set_prompt (prompt);
rl_initialize ();
@@ -321,6 +351,8 @@ readline (prompt)
rl_clear_signals ();
#endif
+ rl_readline_unwind ();
+
return (value);
}
--- sources-unpatched/readline/readline.h 2007-06-26 12:54:52.000000000 +0200
+++ sources/readline/readline.h 2007-06-26 12:34:00.000000000 +0200
@@ -280,6 +280,10 @@ extern int rl_vi_eword PARAMS((int, int)
/* Read a line of input. Prompt with PROMPT. A NULL PROMPT means none. */
extern char *readline PARAMS((const char *));
+/* To be called once for each readline () call which you have jumped out of by
+ longjmp() or siglongjmp(). Function needs to be called BEFORE the jump. */
+extern void rl_readline_unwind PARAMS((void));
+
extern int rl_set_prompt PARAMS((const char *));
extern int rl_expand_prompt PARAMS((char *));
-------------- next part --------------
2007-06-26 Jan Kratochvil <jan.kratochvil@redhat.com>
* top.c (gdb_readline_wrapper_done, gdb_readline_wrapper_result)
(saved_after_char_processing_hook, gdb_readline_wrapper_line)
(struct gdb_readline_wrapper_cleanup, gdb_readline_wrapper_cleanup)
(gdb_readline_wrapper): Revert the asynchronous readline patches from
2007-02-28 and 2007-01-03 back to the synchronous state.
* Makefile.in (top.o): Likewise.
--- ./gdb/top.c 28 Feb 2007 15:55:54 -0000 1.119
+++ ./gdb/top.c 9 Jan 2007 17:58:59 -0000 1.118
@@ -770,6 +770,7 @@ gdb_readline_wrapper_cleanup (void *arg)
{
struct gdb_readline_wrapper_cleanup *cleanup = arg;
+ gdb_assert (rl_already_prompted == 1);
rl_already_prompted = cleanup->already_prompted_orig;
PROMPT (0) = cleanup->prompt_orig;
--- ./gdb/Makefile.in 3 Jan 2007 21:46:11 -0000 1.865
+++ ./gdb/Makefile.in 3 Jan 2007 18:05:43 -0000 1.864
@@ -2782,7 +2782,7 @@ top.o: top.c $(defs_h) $(gdbcmd_h) $(cal
$(annotate_h) $(completer_h) $(top_h) $(version_h) $(serial_h) \
$(doublest_h) $(gdb_assert_h) $(readline_h) $(readline_history_h) \
$(event_top_h) $(gdb_string_h) $(gdb_stat_h) $(ui_out_h) \
- $(cli_out_h) $(main_h) $(event_loop_h)
+ $(cli_out_h) $(main_h)
tracepoint.o: tracepoint.c $(defs_h) $(symtab_h) $(frame_h) $(gdbtypes_h) \
$(expression_h) $(gdbcmd_h) $(value_h) $(target_h) $(language_h) \
$(gdb_string_h) $(inferior_h) $(tracepoint_h) $(remote_h) \
--- ./gdb/top.c 3 Jan 2007 21:46:11 -0000 1.117
+++ ./gdb/top.c 1 Jan 2007 05:57:49 -0000 1.116
@@ -47,7 +47,6 @@
#include "doublest.h"
#include "gdb_assert.h"
#include "main.h"
-#include "event-loop.h"
/* readline include files */
#include "readline/readline.h"
@@ -713,111 +712,26 @@ The filename in which to record the comm
}
/* This is like readline(), but it has some gdb-specific behavior.
- gdb may want readline in both the synchronous and async modes during
+ gdb can use readline in both the synchronous and async modes during
a single gdb invocation. At the ordinary top-level prompt we might
be using the async readline. That means we can't use
rl_pre_input_hook, since it doesn't work properly in async mode.
However, for a secondary prompt (" >", such as occurs during a
- `define'), gdb wants a synchronous response.
-
- We used to call readline() directly, running it in synchronous
- mode. But mixing modes this way is not supported, and as of
- readline 5.x it no longer works; the arrow keys come unbound during
- the synchronous call. So we make a nested call into the event
- loop. That's what gdb_readline_wrapper is for. */
-
-/* A flag set as soon as gdb_readline_wrapper_line is called; we can't
- rely on gdb_readline_wrapper_result, which might still be NULL if
- the user types Control-D for EOF. */
-static int gdb_readline_wrapper_done;
-
-/* The result of the current call to gdb_readline_wrapper, once a newline
- is seen. */
-static char *gdb_readline_wrapper_result;
-
-/* Any intercepted hook. Operate-and-get-next sets this, expecting it
- to be called after the newline is processed (which will redisplay
- the prompt). But in gdb_readline_wrapper we will not get a new
- prompt until the next call, or until we return to the event loop.
- So we disable this hook around the newline and restore it before we
- return. */
-static void (*saved_after_char_processing_hook) (void);
-
-/* This function is called when readline has seen a complete line of
- text. */
-
-static void
-gdb_readline_wrapper_line (char *line)
-{
- gdb_assert (!gdb_readline_wrapper_done);
- gdb_readline_wrapper_result = line;
- gdb_readline_wrapper_done = 1;
-
- /* Prevent operate-and-get-next from acting too early. */
- saved_after_char_processing_hook = after_char_processing_hook;
- after_char_processing_hook = NULL;
-}
-
-struct gdb_readline_wrapper_cleanup
- {
- void (*handler_orig) (char *);
- char *prompt_orig;
- int already_prompted_orig;
- };
-
-static void
-gdb_readline_wrapper_cleanup (void *arg)
-{
- struct gdb_readline_wrapper_cleanup *cleanup = arg;
-
- gdb_assert (rl_already_prompted == 1);
- rl_already_prompted = cleanup->already_prompted_orig;
- PROMPT (0) = cleanup->prompt_orig;
-
- gdb_assert (input_handler == gdb_readline_wrapper_line);
- input_handler = cleanup->handler_orig;
- gdb_readline_wrapper_result = NULL;
- gdb_readline_wrapper_done = 0;
-
- after_char_processing_hook = saved_after_char_processing_hook;
- saved_after_char_processing_hook = NULL;
-
- xfree (cleanup);
-}
-
+ `define'), gdb just calls readline() directly, running it in
+ synchronous mode. So for operate-and-get-next to work in this
+ situation, we have to switch the hooks around. That is what
+ gdb_readline_wrapper is for. */
char *
gdb_readline_wrapper (char *prompt)
{
- struct cleanup *back_to;
- struct gdb_readline_wrapper_cleanup *cleanup;
- char *retval;
-
- cleanup = xmalloc (sizeof (*cleanup));
- cleanup->handler_orig = input_handler;
- input_handler = gdb_readline_wrapper_line;
-
- cleanup->prompt_orig = get_prompt ();
- PROMPT (0) = prompt;
- cleanup->already_prompted_orig = rl_already_prompted;
-
- back_to = make_cleanup (gdb_readline_wrapper_cleanup, cleanup);
-
- /* Display our prompt and prevent double prompt display. */
- display_gdb_prompt (NULL);
- rl_already_prompted = 1;
-
+ /* Set the hook that works in this case. */
if (after_char_processing_hook)
- (*after_char_processing_hook) ();
- gdb_assert (after_char_processing_hook == NULL);
+ {
+ rl_pre_input_hook = (Function *) after_char_processing_hook;
+ after_char_processing_hook = NULL;
+ }
- /* gdb_do_one_event argument is unused. */
- while (gdb_do_one_event (NULL) >= 0)
- if (gdb_readline_wrapper_done)
- break;
-
- retval = gdb_readline_wrapper_result;
- do_cleanups (back_to);
- return retval;
+ return readline (prompt);
}
-------------- next part --------------
Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/readline/Makefile.in,v
retrieving revision 1.9
diff -u -p -r1.9 Makefile.in
--- Makefile.in 27 Mar 2007 18:09:35 -0000 1.9
+++ Makefile.in 26 Jun 2007 09:42:55 -0000
@@ -36,6 +36,7 @@ INSTALL_PROGRAM = @INSTALL_PROGRAM@
INSTALL_DATA = @INSTALL_DATA@
CC = @CC@
+CXX = @CXX@
RANLIB = @RANLIB@
AR = @AR@
ARFLAGS = @ARFLAGS@
@@ -70,6 +71,7 @@ ETAGS = etags -tw
CTAGS = ctags -tw
CFLAGS = @CFLAGS@
+CXXFLAGS = @CXXFLAGS@
LOCAL_CFLAGS = @LOCAL_CFLAGS@ -DRL_LIBRARY_VERSION='"$(RL_LIBRARY_VERSION)"'
CPPFLAGS = @CPPFLAGS@
@@ -94,12 +96,21 @@ GCC_LINT_CFLAGS = $(XCCFLAGS) $(GCC_LINT
${RM} $@
$(CC) -c $(CCFLAGS) $<
+.C.o:
+ ${RM} $@
+ $(CXX) -c $(CXXFLAGS) $<
+
# The name of the main library target.
LIBRARY_NAME = libreadline.a
STATIC_LIBS = libreadline.a libhistory.a
WCWIDTH_OBJ = @WCWIDTH_OBJ@
+@CPLUSPLUS_TRUE@OPTCSOURCES = cleanup-unwind.C
+@CPLUSPLUS_TRUE@OPTOBJECTS = cleanup-unwind.o
+@CPLUSPLUS_FALSE@OPTCSOURCES = cleanup-none.c
+@CPLUSPLUS_FALSE@OPTOBJECTS = cleanup-none.o
+
# The C code source files for this library.
CSOURCES = $(srcdir)/readline.c $(srcdir)/funmap.c $(srcdir)/keymaps.c \
$(srcdir)/vi_mode.c $(srcdir)/parens.c $(srcdir)/rltty.c \
@@ -112,13 +123,13 @@ CSOURCES = $(srcdir)/readline.c $(srcdir
$(srcdir)/histfile.c $(srcdir)/nls.c $(srcdir)/search.c \
$(srcdir)/shell.c $(srcdir)/savestring.c $(srcdir)/tilde.c \
$(srcdir)/text.c $(srcdir)/misc.c $(srcdir)/compat.c \
- $(srcdir)/mbutil.c $(srcdir)/support/wcwidth.c
+ $(srcdir)/mbutil.c $(srcdir)/support/wcwidth.c $(OPTCSOURCES)
# The header files for this library.
HSOURCES = readline.h rldefs.h chardefs.h keymaps.h history.h histlib.h \
posixstat.h posixdir.h posixjmp.h tilde.h rlconf.h rltty.h \
ansi_stdlib.h tcap.h rlstdc.h xmalloc.h rlprivate.h rlshell.h \
- rltypedefs.h rlmbutil.h
+ rltypedefs.h rlmbutil.h cleanup.h
HISTOBJ = history.o histexpand.o histfile.o histsearch.o shell.o mbutil.o
TILDEOBJ = tilde.o
@@ -126,7 +137,7 @@ OBJECTS = readline.o vi_mode.o funmap.o
rltty.o complete.o bind.o isearch.o display.o signals.o \
util.o kill.o undo.o macro.o input.o callback.o terminal.o \
text.o nls.o misc.o compat.o xmalloc.o $(HISTOBJ) $(TILDEOBJ) \
- $(WCWIDTH_OBJ)
+ $(WCWIDTH_OBJ) $(OPTOBJECTS)
# The texinfo files which document this library.
DOCSOURCE = doc/rlman.texinfo doc/rltech.texinfo doc/rluser.texinfo
@@ -484,6 +495,10 @@ search.o: rlmbutil.h
text.o: rlmbutil.h
vi_mode.o: rlmbutil.h
+readline.o: cleanup.h
+cleanup-none.o: cleanup.h
+cleanup-unwind.o: cleanup.h
+
bind.o: $(srcdir)/bind.c
callback.o: $(srcdir)/callback.c
compat.o: $(srcdir)/compat.c
@@ -518,6 +533,9 @@ histfile.o: $(srcdir)/histfile.c
history.o: $(srcdir)/history.c
histsearch.o: $(srcdir)/histsearch.c
+cleanup-none.o: $(srcdir)/cleanup-none.c
+cleanup-unwind.o: $(srcdir)/cleanup-unwind.C
+
bind.o: bind.c
callback.o: callback.c
compat.o: compat.c
@@ -551,3 +569,6 @@ histexpand.o: histexpand.c
histfile.o: histfile.c
history.o: history.c
histsearch.o: histsearch.c
+
+cleanup-none.o: cleanup-none.c
+cleanup-unwind.o: cleanup-unwind.C
Index: cleanup-none.c
===================================================================
RCS file: cleanup-none.c
diff -N cleanup-none.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ cleanup-none.c 26 Jun 2007 09:42:55 -0000
@@ -0,0 +1,11 @@
+#include "cleanup.h"
+
+void *_rl_cleanup (void *(*body) (void *data), void (*cleanup) (void *data),
+ void *data)
+{
+ void *retval;
+
+ retval = (*body) (data);
+ (*cleanup) (data);
+ return retval;
+}
Index: cleanup-unwind.C
===================================================================
RCS file: cleanup-unwind.C
diff -N cleanup-unwind.C
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ cleanup-unwind.C 26 Jun 2007 09:42:55 -0000
@@ -0,0 +1,14 @@
+#include "cleanup.h"
+
+void *_rl_cleanup (void *(*body) (void *data), void (*cleanup) (void *data),
+ void *data)
+{
+ try
+ {
+ return (*body) (data);
+ }
+ catch (...)
+ {
+ (*cleanup) (data);
+ }
+}
Index: cleanup.h
===================================================================
RCS file: cleanup.h
diff -N cleanup.h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ cleanup.h 26 Jun 2007 09:42:55 -0000
@@ -0,0 +1,8 @@
+#ifdef __cplusplus
+extern "C" {
+#endif
+void *_rl_cleanup (void *(*body) (void *data), void (*cleanup) (void *data),
+ void *data);
+#ifdef __cplusplus
+}
+#endif
Index: configure.in
===================================================================
RCS file: /cvs/src/src/readline/configure.in,v
retrieving revision 1.10
diff -u -p -r1.10 configure.in
--- configure.in 5 May 2006 18:26:12 -0000 1.10
+++ configure.in 26 Jun 2007 09:42:56 -0000
@@ -112,6 +112,26 @@ AC_PROG_CC
dnl AC_AIX
AC_MINIX
+m4_pushdef([AC_MSG_ERROR], [cxx_error=yes])
+AC_PROG_CXX
+AC_LANG_PUSH([C++])
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[const char hw[] = "hello world\n";]],
+ [[cout << hw;]])],
+ [AC_PROG_CXXCPP
+ cxx_available=no],
+ [cxx_available=yes])
+AC_LANG_POP([C++])
+m4_popdef([AC_MSG_ERROR])
+if test "x$cxx_available" = "xyes"; then
+ CPLUSPLUS_TRUE=""
+ CPLUSPLUS_FALSE="#"
+else
+ CPLUSPLUS_TRUE="#"
+ CPLUSPLUS_FALSE=""
+fi
+AC_SUBST(CPLUSPLUS_TRUE)
+AC_SUBST(CPLUSPLUS_FALSE)
+
dnl BEGIN changes for CYGNUS cross-building for Cygwin
dnl NOTE: Some of these changes may no longer be necessary.
Index: readline.c
===================================================================
RCS file: /cvs/src/src/readline/readline.c,v
retrieving revision 1.10
diff -u -p -r1.10 readline.c
--- readline.c 5 May 2006 18:26:12 -0000 1.10
+++ readline.c 26 Jun 2007 09:42:56 -0000
@@ -66,6 +66,7 @@
#include "rlprivate.h"
#include "rlshell.h"
#include "xmalloc.h"
+#include "cleanup.h"
#ifndef RL_LIBRARY_VERSION
# define RL_LIBRARY_VERSION "5.1"
@@ -287,6 +288,14 @@ rl_set_prompt (prompt)
rl_visible_prompt_length = rl_expand_prompt (rl_prompt);
return 0;
}
+
+static void
+readline_cleanup (in_callback_pointer)
+ int *in_callback_pointer;
+{
+ if (*in_callback_pointer)
+ RL_SETSTATE (RL_STATE_CALLBACK);
+}
/* Read a line of input. Prompt with PROMPT. An empty PROMPT means
none. A return value of NULL means that EOF was encountered. */
@@ -295,6 +304,7 @@ readline (prompt)
const char *prompt;
{
char *value;
+ int in_callback;
/* If we are at EOF return a NULL string. */
if (rl_pending_input == EOF)
@@ -303,6 +313,13 @@ readline (prompt)
return ((char *)NULL);
}
+ /* When we call readline, we have to make sure that readline isn't in
+ the callback state. Otherwise, it will get really confused.
+ PR gdb tui/2173. */
+ in_callback = RL_ISSTATE (RL_STATE_CALLBACK);
+ if (in_callback)
+ RL_UNSETSTATE (RL_STATE_CALLBACK);
+
rl_set_prompt (prompt);
rl_initialize ();
@@ -313,7 +330,10 @@ readline (prompt)
rl_set_signals ();
#endif
- value = readline_internal ();
+ /* readline_internal () has no parameters. */
+ value = _rl_cleanup ((void *(*) ()) readline_internal, readline_cleanup,
+ &in_callback);
+
if (rl_deprep_term_function)
(*rl_deprep_term_function) ();
More information about the Gdb-patches
mailing list