[PATCH 0/2] Fix GDB prompt corruption issue

[PATCH 0/2] Fix GDB prompt corruption issue

[Andrew Burgess]

This series fixes PR cli/28833.  The first commit is a patch to
readline, which is based on an existing, upstream, readline commit.
See my commit message for more details.

The second patch fixes GDB to make use of the new reaadline features
in order to fix the prompt corruption issue.

Thanks,
Andrew


---

Andrew Burgess (2):
  readline: back-port changes needed to properly detect EOF
  gdb: handle bracketed-paste-mode and ctrl-d correctly

 gdb/event-top.c                     | 61 ++++++++++++++++++++
 gdb/event-top.h                     |  6 ++
 gdb/testsuite/gdb.base/eof-exit.exp | 88 +++++++++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp           |  9 +++
 gdb/top.c                           |  1 +
 readline/readline/callback.c        |  8 ++-
 readline/readline/doc/rltech.texi   | 11 ++++
 readline/readline/readline.c        | 19 ++++---
 readline/readline/readline.h        |  8 ++-
 readline/readline/rlprivate.h       |  1 -
 readline/readline/rltty.c           |  4 +-
 11 files changed, 204 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/eof-exit.exp

-- 
2.25.4

[PATCH 1/2] readline: back-port changes needed to properly detect EOF

[Andrew Burgess]

This commit is a partial back-port of this upstream readline commit:

  commit 002d31aa5f5929eb32d0e0e2e8b8d35d99e59961
  Author: Chet Ramey <chet.ramey@case.edu>
  Date:   Thu Mar 3 11:11:47 2022 -0500

      add rl_eof_found to public API; fix pointer aliasing problems  \
            with history-search-backward; fix a display problem with \
            runs of invisible characters at the end of a physical    \
            screen line

I have only pulled in the parts of this commit that relate to the new
rl_eof_found global, and the RL_STATE_EOF state flag.  These changes
are needed in order to fix PR cli/28833, and are discussed in this
thread to the bug-readline mailing list:

  https://lists.gnu.org/archive/html/bug-readline/2022-02/msg00021.html

The above commit is not yet in any official readline release, but my
hope is that now it has been merged into the readline tree it should
be safe enough to back port this fix to GDB's tree.

At some point in the future we will inevitably want to roll forward
the version of readline that we maintain in the binutils-gdb
repository.  When that day comes the changes in this commit can be
replaced with the latest upstream readline code, as I have not changed
the meaning of this code at all from what is in upstream readline.

This commit alone does not fix the PR cli/28833 issue, for that the
next commit, which changes GDB itself, is needed.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833
---
 readline/readline/callback.c      |  8 +++++++-
 readline/readline/doc/rltech.texi | 11 +++++++++++
 readline/readline/readline.c      | 19 ++++++++++++-------
 readline/readline/readline.h      |  8 +++++++-
 readline/readline/rlprivate.h     |  1 -
 readline/readline/rltty.c         |  4 ++--
 6 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/readline/readline/callback.c b/readline/readline/callback.c
index a466cf9b6ef..58b84d2e2ad 100644
--- a/readline/readline/callback.c
+++ b/readline/readline/callback.c
@@ -1,6 +1,6 @@
 /* callback.c -- functions to use readline as an X `callback' mechanism. */
 
-/* Copyright (C) 1987-2017 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.
@@ -136,6 +136,8 @@ rl_callback_read_char (void)
       abort ();
     }
 
+  eof = 0;
+
   memcpy ((void *)olevel, (void *)_rl_top_level, sizeof (procenv_t));
 #if defined (HAVE_POSIX_SIGSETJMP)
   jcode = sigsetjmp (_rl_top_level, 0);
@@ -268,6 +270,10 @@ rl_callback_read_char (void)
 	  _rl_want_redisplay = 0;
 	}
 
+      /* Make sure application hooks can see whether we saw EOF. */
+      if (rl_eof_found = eof)
+	RL_SETSTATE(RL_STATE_EOF);
+
       if (rl_done)
 	{
 	  line = readline_internal_teardown (eof);
diff --git a/readline/readline/doc/rltech.texi b/readline/readline/doc/rltech.texi
index bbf57c239c9..797e34d95e0 100644
--- a/readline/readline/doc/rltech.texi
+++ b/readline/readline/doc/rltech.texi
@@ -323,6 +323,14 @@
 @deftypevar int rl_done
 Setting this to a non-zero value causes Readline to return the current
 line immediately.
+Readline will set this variable when it has read a key sequence bound
+to @code{accept-line} and is about to return the line to the caller.
+@end deftypevar
+
+@deftypevar int rl_eof_found
+Readline will set this variable when it has read an EOF character (e.g., the
+stty @samp{EOF} character) on an empty line or encountered a read error and
+is about to return a NULL line to the caller.
 @end deftypevar
 
 @deftypevar int rl_num_chars_to_read
@@ -588,6 +596,9 @@
 @item RL_STATE_DONE
 Readline has read a key sequence bound to @code{accept-line}
 and is about to return the line to the caller.
+@item RL_STATE_EOF
+Readline has read an EOF character (e.g., the stty @samp{EOF} character)
+or encountered a read error and is about to return a NULL line to the caller.
 @end table
 
 @end deftypevar
diff --git a/readline/readline/readline.c b/readline/readline/readline.c
index e61d188bbe9..0e33587f234 100644
--- a/readline/readline/readline.c
+++ b/readline/readline/readline.c
@@ -1,7 +1,7 @@
 /* readline.c -- a general facility for reading lines of input
    with emacs style editing and completion. */
 
-/* Copyright (C) 1987-2020 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.      
@@ -165,6 +165,9 @@ int rl_end;
 /* Make this non-zero to return the current input_line. */
 int rl_done;
 
+/* If non-zero when readline_internal returns, it means we found EOF */
+int rl_eof_found = 0;
+
 /* The last function executed by readline. */
 rl_command_func_t *rl_last_func = (rl_command_func_t *)NULL;
 
@@ -218,9 +221,6 @@ int _rl_eof_char = CTRL ('D');
 /* Non-zero makes this the next keystroke to read. */
 int rl_pending_input = 0;
 
-/* If non-zero when readline_internal returns, it means we found EOF */
-int _rl_eof_found = 0;
-
 /* Pointer to a useful terminal name. */
 const char *rl_terminal_name = (const char *)NULL;
 
@@ -474,6 +474,9 @@ readline_internal_teardown (int eof)
 
   RL_CHECK_SIGNALS ();
 
+  if (eof)
+    RL_SETSTATE (RL_STATE_EOF);		/* XXX */
+
   /* Restore the original of this history line, iff the line that we
      are editing was originally in the history, AND the line has changed. */
   entry = current_history ();
@@ -596,6 +599,7 @@ readline_internal_charloop (void)
 	  RL_SETSTATE(RL_STATE_DONE);
 	  return (rl_done = 1);
 #else
+	  RL_SETSTATE(RL_STATE_EOF);
 	  eof_found = 1;
 	  break;
 #endif
@@ -636,6 +640,7 @@ readline_internal_charloop (void)
 	  RL_SETSTATE(RL_STATE_DONE);
 	  return (rl_done = 1);
 #else
+	  RL_SETSTATE(RL_STATE_EOF);
 	  eof_found = 1;
 	  break;
 #endif
@@ -703,8 +708,8 @@ static char *
 readline_internal (void)
 {
   readline_internal_setup ();
-  _rl_eof_found = readline_internal_charloop ();
-  return (readline_internal_teardown (_rl_eof_found));
+  rl_eof_found = readline_internal_charloop ();
+  return (readline_internal_teardown (rl_eof_found));
 }
 
 void
@@ -1161,7 +1166,7 @@ rl_initialize (void)
 
   /* We aren't done yet.  We haven't even gotten started yet! */
   rl_done = 0;
-  RL_UNSETSTATE(RL_STATE_DONE);
+  RL_UNSETSTATE(RL_STATE_DONE|RL_STATE_EOF);
 
   /* Tell the history routines what is going on. */
   _rl_start_using_history ();
diff --git a/readline/readline/readline.h b/readline/readline/readline.h
index 78fa39d02a1..9f79810f290 100644
--- a/readline/readline/readline.h
+++ b/readline/readline/readline.h
@@ -1,6 +1,6 @@
 /* Readline.h -- the names of functions callable from within readline. */
 
-/* Copyright (C) 1987-2020 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.      
@@ -553,6 +553,10 @@ extern int rl_mark;
    line and should return it. */
 extern int rl_done;
 
+/* Flag to indicate that readline has read an EOF character or read has
+   returned 0 or error, and is returning a NULL line as a result. */
+extern int rl_eof_found;
+
 /* If set to a character value, that will be the next keystroke read. */
 extern int rl_pending_input;
 
@@ -906,6 +910,8 @@ extern int rl_persistent_signal_handlers;
 #define RL_STATE_REDISPLAYING	0x1000000	/* updating terminal display */
 
 #define RL_STATE_DONE		0x2000000	/* done; accepted line */
+#define RL_STATE_EOF		0x8000000	/* done; got eof on read */
+
 
 #define RL_SETSTATE(x)		(rl_readline_state |= (x))
 #define RL_UNSETSTATE(x)	(rl_readline_state &= ~(x))
diff --git a/readline/readline/rlprivate.h b/readline/readline/rlprivate.h
index 23ab2d8cec0..02838ae21ae 100644
--- a/readline/readline/rlprivate.h
+++ b/readline/readline/rlprivate.h
@@ -544,7 +544,6 @@ extern FILE *_rl_in_stream;
 extern FILE *_rl_out_stream;
 extern int _rl_last_command_was_kill;
 extern int _rl_eof_char;
-extern int _rl_eof_found;
 extern procenv_t _rl_top_level;
 extern _rl_keyseq_cxt *_rl_kscxt;
 extern int _rl_keyseq_timeout;
diff --git a/readline/readline/rltty.c b/readline/readline/rltty.c
index d0cd572713a..11997b7c9d8 100644
--- a/readline/readline/rltty.c
+++ b/readline/readline/rltty.c
@@ -1,7 +1,7 @@
 /* rltty.c -- functions to prepare and restore the terminal for readline's
    use. */
 
-/* Copyright (C) 1992-2017 Free Software Foundation, Inc.
+/* Copyright (C) 1992-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.
@@ -692,7 +692,7 @@ rl_deprep_terminal (void)
   if (terminal_prepped & TPX_BRACKPASTE)
     {
       fprintf (rl_outstream, BRACK_PASTE_FINI);
-      if (_rl_eof_found)
+      if (rl_eof_found)
  	fprintf (rl_outstream, "\n");
     }
 
-- 
2.25.4

[PATCH 2/2] gdb: handle bracketed-paste-mode and ctrl-d correctly

[Andrew Burgess]

Currently, when readline is using bracketed-paste-mode, and the user
quits gdb with EOF (typically bound to ctrl-d), then we end up with
some corruption of the output.

If the user is sat at a prompt, like this:

  (gdb)

And then the user sends EOF, we currently see this:

  quit)
  ... gdb terminates ...

Notice, that the 'quit' is printed over the (gdb) prompt.  This is not
what we wanted, what we want to see is:

  (gdb) quit
  ... gdb terminates ...

The problem is that, when readline detects that the user has entered a
complete line, either by entering a newline, or by spotting EOF,
readline will restore the terminal to a pre-readline state.  When
bracketed-paste-mode is on, this includes switching
bracketed-paste-mode back off.

In the case where a real line of input is entered, and then a newline
sent, the cursor will have moved onto the next line before the
terminal settings are restored.  However, when an EOF is detected, the
cursor remains on the current line when the terminal settings are
restored.

This difference, of whether the cursor is on a new line or not, is
important.  In command_line_handler (event-top.c), this is where we
print the final "quit" message, or more accurately, we print "quit\n",
when the user sends EOF.  The final "\n" on that string moves the
cursor onto the next line so that we're in a similar state as if the
user had manually entered "quit" and then sent a newline.

And this is where bracketed-paste-mode causes a problem.  For
background reading, see:

  https://lists.gnu.org/archive/html/bug-bash/2018-01/msg00097.html

The outcome of that discussion, is that, when readline disables
bracketed-paste-mode, it also sends a "\r" to the terminal.

Now, in the case where the user sent a real command and a newline,
this isn't a problem.  The cursor has moved to the next line
anyway (which would be empty), and the "\r" simply returns us to the
start of the (empty) line.  No problem there.

However, in the EOF case, we don't move to a newline.  So the "\r"
after disabling bracketed-paste-mode returns the cursor to the start
of the prompt line.  Now in command_line_handler, when we print
"quit\n", this will overwrite the prompt.

It turns out that there is actually some problems with readline in
this area.  Disabling of bracketed paste mode is done in the readline
function rl_deprep_terminal.  As part of this readline should print a
"\n" after disabling bracketed paste mode if an EOF was received.
This is to work around specifically the issue we see above.
Unfortunately, how readline detects that the EOF was seen doesn't work
correctly when using the callback API, as GDB does.  As a result the
"\n" is not printed from rl_deprep_terminal.

If we fix readline's detection of EOF then rl_deprep_terminal will now
print the "\n" as expected, however, this has a knock on problem for
GDB.  rl_deprep_terminal is called before the command line is
dispatched back to GDB, this means that the "\n" will be printed from
rl_deprep_terminal before GDB prints "quit\n", the result is that when
the user sends EOF GDB's output will be:

  (gdb)
  quit

Which is now what we wanted; we wanted the quit on the same line as
the prompt.

To fix this problem I propose that within GDB we override the
rl_deprep_terminal function.  This is a valid thing to do, the
readline API exposes rl_deprep_term_function, which can be used to
replace the deprep function.

Now, we can move the printing of "quit" into GDB's new deprep
function, before calling on to readlines default rl_deprep_terminal
function, which will then print the trailing "\n" for us.

The only problem with this is that readline doesn't expose whether it
is handling EOF as part of its API.  However, in this thread:

  https://lists.gnu.org/archive/html/bug-readline/2022-02/msg00021.html

changes to readline were proposed, and accepted, that extended the
readline API such that information about whether we're handling EOF is
now available.

With this information available we can now use rl_deprep_term_function
to print the "quit" message.

There's just a couple of final cases we need to cover.

First, GDB can link against the system readline.  The system readline
might not have all of the fixes to expose the EOF status.  If this is
the case then we can't print "quit" from our rl_deprep_term_function
hook, as we can't know if we are handling EOF or not.

In this case, I have left GDB printing "quit" from the old location in
command_line_handler.  To avoid prompt corruption, then, if bracketed
paste mode is on, then we now deliberately print an extra "\n",
forcing the quit onto the next line, like this:

  (gdb)
  quit

This isn't ideal, but is, I think, the best we can do in this case.

The other edge case is from gdb_rl_deprep_term_function, our new
deprep hook.  We have to consider two cases, bracketed paste mode on,
and bracketed paste mode off.  As readline emits a "\n" after
leaving bracketed paste mode, then if we print "quit\n" from
gdb_rl_deprep_term_function we will end up with an extra blank line,
like this:

  (gdb) quit
  		# <--- This line left blank
  [Shell-Prompt]>

However, if we only print "quit" from gdb_rl_deprep_term_function, but
the user has bracketed paste mode disabled (say from .inputrc), then
we end up with this:

  (gdb) quit[Shell-Prompt]>

Which obviously is not right.

My solution here is to temporarily set rl_eof_found back to 0 before
calling the builtin rl_deprep_terminal function.  Right now, the only
impact this has is to prevent readline printing the extra "\n", which
solves the above problems.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833
---
 gdb/event-top.c                     | 61 ++++++++++++++++++++
 gdb/event-top.h                     |  6 ++
 gdb/testsuite/gdb.base/eof-exit.exp | 88 +++++++++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp           |  9 +++
 gdb/top.c                           |  1 +
 5 files changed, 165 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/eof-exit.exp

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 8890c4fae2d..e14c00cf485 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -745,6 +745,25 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
     return cmd;
 }
 
+/* See event-top.h.  */
+
+void
+gdb_rl_deprep_term_function (void)
+{
+#ifdef RL_STATE_EOF
+  gdb::optional<scoped_restore_tmpl<int>> restore_eof_found;
+
+  if (RL_ISSTATE (RL_STATE_EOF))
+    {
+      printf_unfiltered ("quit\n");
+      restore_eof_found.emplace (&rl_eof_found, 0);
+    }
+
+#endif /* RL_STATE_EOF */
+
+  rl_deprep_terminal ();
+}
+
 /* Handle a complete line of input.  This is called by the callback
    mechanism within the readline library.  Deal with incomplete
    commands as well, by saving the partial input in a global
@@ -767,7 +786,49 @@ command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
 	 This happens at the end of a testsuite run, after Expect has
 	 hung up but GDB is still alive.  In such a case, we just quit
 	 gdb killing the inferior program too.  */
+
+#ifndef RL_STATE_EOF
+      /* When readline is using bracketed paste mode, then, when eof is
+	 received, readline will emit the control sequence to leave
+	 bracketed paste mode.
+
+	 This control sequence ends with \r, which means that the "quit" we
+	 are about to print will overwrite the prompt on this line.
+
+	 The solution to this problem is to actually print the "quit"
+	 message from gdb_rl_deprep_term_function (see above), however, we
+	 can only do that if we can know, in that function, when eof was
+	 received.
+
+	 Unfortunately, with older versions of readline, it is not possible
+	 in the gdb_rl_deprep_term_function to know if eof was received or
+	 not, and, as GDB can be built against the system readline, which
+	 could be older than the readline in GDB's repository, then we
+	 can't be sure that we can work around this prompt corruption in
+	 the gdb_rl_deprep_term_function function.
+
+	 If we get here, RL_STATE_EOF is not defined.  This indicates that
+	 we are using an older readline, and couldn't print the quit
+	 message in gdb_rl_deprep_term_function.  So, what we do here is
+	 check to see if bracketed paste mode is on or not.  If it's on
+	 then we print a \n and then the quit, this means the user will
+	 see:
+
+	 (gdb)
+	 quit
+
+	 Rather than the usual:
+
+	 (gdb) quit
+
+	 Which we will get with a newer readline, but this really is the
+	 best we can do with older versions of readline.  */
+      const char *value = rl_variable_value ("enable-bracketed-paste");
+      if (value != nullptr && strcmp (value, "on") == 0)
+	printf_unfiltered ("\n");
       printf_unfiltered ("quit\n");
+#endif
+
       execute_command ("quit", 1);
     }
   else if (cmd == NULL)
diff --git a/gdb/event-top.h b/gdb/event-top.h
index 078482b79aa..722e636a110 100644
--- a/gdb/event-top.h
+++ b/gdb/event-top.h
@@ -70,6 +70,12 @@ extern void gdb_rl_callback_handler_install (const char *prompt);
    currently installed.  */
 extern void gdb_rl_callback_handler_reinstall (void);
 
+/* Called by readline after a complete line has been gathered from the
+   user, but before the line is dispatched to back to GDB.  This function
+   is a wrapper around readline's builtin rl_deprep_terminal function, and
+   handles the case where readline received EOF.  */
+extern void gdb_rl_deprep_term_function (void);
+
 typedef void (*segv_handler_t) (int);
 
 /* On construction, replaces the current thread's SIGSEGV handler with
diff --git a/gdb/testsuite/gdb.base/eof-exit.exp b/gdb/testsuite/gdb.base/eof-exit.exp
new file mode 100644
index 00000000000..2434031a63d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/eof-exit.exp
@@ -0,0 +1,88 @@
+# Copyright (C) 2022 Free Software Foundation, Inc.
+
+# 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/>.
+
+# This test script checks how GDB handles exiting with ctrl-d.
+
+# Start GDB, and then close it by sendig ctrl-d.  Check that the
+# string 'quit' appears where we expect it too.
+proc run_test {} {
+    clean_restart
+
+    # The gdb_start call above swallows the GDB prompt, and we want
+    # the prompt for the test below.
+    #
+    # Send a newline character, which will cause GDB to redisplay the
+    # prompt.
+    send_gdb "\n"
+    gdb_test_multiple "" "discard newline" {
+	-re "^\r\n" {
+	}
+    }
+
+    # Send GDB a ctrl-d.  Check that we see the 'quit' string in the
+    # expected place.
+    send_gdb "\004"
+    gdb_test_multiple "" "close GDB with eof" {
+	-re "$::gdb_prompt \[^\n\]*\r\[^\n\]*quit" {
+	    fail "$gdb_test_name (misplaced \\r)"
+	}
+	-re "$::gdb_prompt \[^\n\]*\r\[^\n\]*\r\nquit\r\n" {
+	    # For versions of readline that don't include the
+	    # RL_STATE_EOF patch, then the 'quit' is printed on the
+	    # subsequent line.
+	    pass "$gdb_test_name (older version of readline)"
+	}
+	-re "$::gdb_prompt quit\r\n\[^\n\]*\r\n$" {
+	    # There was a bug where GDB would print an extra blank
+	    # line after the 'quit', this catches that case.
+	    fail $gdb_test_name
+	}
+	-re "$::gdb_prompt quit\r\n\[^\n\]*$" {
+	    pass $gdb_test_name
+	}
+	eof {
+	    fail "$gdb_test_name (missed the prompt)"
+	}
+    }
+}
+
+with_test_prefix "default" {
+    run_test
+}
+
+save_vars { env(TERM) } {
+    setenv TERM ansi
+
+    with_test_prefix "with non-dump terminal" {
+	run_test
+
+	save_vars { env(INPUTRC) } {
+
+	    # Create an inputrc file that turns bracketed paste mode
+	    # on.  This is usually turned off (see lib/gdb.exp), but
+	    # for the next test we want to see what happens with this
+	    # on.
+	    set inputrc [standard_output_file inputrc]
+	    set fd [open "$inputrc" w]
+	    puts $fd "set enable-bracketed-paste on"
+	    close $fd
+
+	    setenv INPUTRC "$inputrc"
+	    with_test_prefix "with bracketed-paste-mode on" {
+		run_test
+	    }
+	}
+    }
+}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index a35d08a05de..08726f78563 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2151,6 +2151,15 @@ proc default_gdb_start { } {
 	-re "\[\r\n\]$gdb_prompt $" {
 	    verbose "GDB initialized."
 	}
+	-re "\[\r\n\]\033\\\[.2004h$gdb_prompt $" {
+	    # This special case detects what happens when GDB is
+	    # started with bracketed paste mode enabled.  This mode is
+	    # usually forced off (see setting of INPUTRC in
+	    # default_gdb_init), but for at least one test we turn
+	    # bracketed paste mode back on, and then start GDB.  In
+	    # that case, this case is hit.
+	    verbose "GDB initialized."
+	}
 	-re "$gdb_prompt $"	{
 	    perror "GDB never initialized."
 	    unset gdb_spawn_id
diff --git a/gdb/top.c b/gdb/top.c
index a94ed5cebdb..3af35daa9ca 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -2229,6 +2229,7 @@ init_main (void)
   rl_completion_display_matches_hook = cli_display_match_list;
   rl_readline_name = "gdb";
   rl_terminal_name = getenv ("TERM");
+  rl_deprep_term_function = gdb_rl_deprep_term_function;
 
   /* The name for this defun comes from Bash, where it originated.
      15 is Control-o, the same binding this function has in Bash.  */
-- 
2.25.4

Re: [PATCH 2/2] gdb: handle bracketed-paste-mode and ctrl-d correctly

[Eli Zaretskii]

> Date: Mon,  7 Mar 2022 15:13:15 +0000
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Andrew Burgess <aburgess@redhat.com>
> 
> Currently, when readline is using bracketed-paste-mode, and the user
> quits gdb with EOF (typically bound to ctrl-d), then we end up with
> some corruption of the output.
> 
> If the user is sat at a prompt, like this:
> 
>   (gdb)
> 
> And then the user sends EOF, we currently see this:
> 
>   quit)
>   ... gdb terminates ...
> 
> Notice, that the 'quit' is printed over the (gdb) prompt.  This is not
> what we wanted, what we want to see is:
> 
>   (gdb) quit
>   ... gdb terminates ...
> 
> The problem is that, when readline detects that the user has entered a
> complete line, either by entering a newline, or by spotting EOF,
> readline will restore the terminal to a pre-readline state.  When
> bracketed-paste-mode is on, this includes switching
> bracketed-paste-mode back off.
> 
> In the case where a real line of input is entered, and then a newline
> sent, the cursor will have moved onto the next line before the
> terminal settings are restored.  However, when an EOF is detected, the
> cursor remains on the current line when the terminal settings are
> restored.
> 
> This difference, of whether the cursor is on a new line or not, is
> important.  In command_line_handler (event-top.c), this is where we
> print the final "quit" message, or more accurately, we print "quit\n",
> when the user sends EOF.  The final "\n" on that string moves the
> cursor onto the next line so that we're in a similar state as if the
> user had manually entered "quit" and then sent a newline.
> 
> And this is where bracketed-paste-mode causes a problem.  For
> background reading, see:
> 
>   https://lists.gnu.org/archive/html/bug-bash/2018-01/msg00097.html
> 
> The outcome of that discussion, is that, when readline disables
> bracketed-paste-mode, it also sends a "\r" to the terminal.
> 
> Now, in the case where the user sent a real command and a newline,
> this isn't a problem.  The cursor has moved to the next line
> anyway (which would be empty), and the "\r" simply returns us to the
> start of the (empty) line.  No problem there.
> 
> However, in the EOF case, we don't move to a newline.  So the "\r"
> after disabling bracketed-paste-mode returns the cursor to the start
> of the prompt line.  Now in command_line_handler, when we print
> "quit\n", this will overwrite the prompt.
> 
> It turns out that there is actually some problems with readline in
> this area.  Disabling of bracketed paste mode is done in the readline
> function rl_deprep_terminal.  As part of this readline should print a
> "\n" after disabling bracketed paste mode if an EOF was received.
> This is to work around specifically the issue we see above.
> Unfortunately, how readline detects that the EOF was seen doesn't work
> correctly when using the callback API, as GDB does.  As a result the
> "\n" is not printed from rl_deprep_terminal.
> 
> If we fix readline's detection of EOF then rl_deprep_terminal will now
> print the "\n" as expected, however, this has a knock on problem for
> GDB.  rl_deprep_terminal is called before the command line is
> dispatched back to GDB, this means that the "\n" will be printed from
> rl_deprep_terminal before GDB prints "quit\n", the result is that when
> the user sends EOF GDB's output will be:
> 
>   (gdb)
>   quit
> 
> Which is now what we wanted; we wanted the quit on the same line as
> the prompt.
> 
> To fix this problem I propose that within GDB we override the
> rl_deprep_terminal function.  This is a valid thing to do, the
> readline API exposes rl_deprep_term_function, which can be used to
> replace the deprep function.
> 
> Now, we can move the printing of "quit" into GDB's new deprep
> function, before calling on to readlines default rl_deprep_terminal
> function, which will then print the trailing "\n" for us.
> 
> The only problem with this is that readline doesn't expose whether it
> is handling EOF as part of its API.  However, in this thread:
> 
>   https://lists.gnu.org/archive/html/bug-readline/2022-02/msg00021.html
> 
> changes to readline were proposed, and accepted, that extended the
> readline API such that information about whether we're handling EOF is
> now available.
> 
> With this information available we can now use rl_deprep_term_function
> to print the "quit" message.
> 
> There's just a couple of final cases we need to cover.
> 
> First, GDB can link against the system readline.  The system readline
> might not have all of the fixes to expose the EOF status.  If this is
> the case then we can't print "quit" from our rl_deprep_term_function
> hook, as we can't know if we are handling EOF or not.
> 
> In this case, I have left GDB printing "quit" from the old location in
> command_line_handler.  To avoid prompt corruption, then, if bracketed
> paste mode is on, then we now deliberately print an extra "\n",
> forcing the quit onto the next line, like this:
> 
>   (gdb)
>   quit
> 
> This isn't ideal, but is, I think, the best we can do in this case.
> 
> The other edge case is from gdb_rl_deprep_term_function, our new
> deprep hook.  We have to consider two cases, bracketed paste mode on,
> and bracketed paste mode off.  As readline emits a "\n" after
> leaving bracketed paste mode, then if we print "quit\n" from
> gdb_rl_deprep_term_function we will end up with an extra blank line,
> like this:
> 
>   (gdb) quit
>   		# <--- This line left blank
>   [Shell-Prompt]>
> 
> However, if we only print "quit" from gdb_rl_deprep_term_function, but
> the user has bracketed paste mode disabled (say from .inputrc), then
> we end up with this:
> 
>   (gdb) quit[Shell-Prompt]>
> 
> Which obviously is not right.
> 
> My solution here is to temporarily set rl_eof_found back to 0 before
> calling the builtin rl_deprep_terminal function.  Right now, the only
> impact this has is to prevent readline printing the extra "\n", which
> solves the above problems.

Let me play the devil's advocate role and ask: isn't this too much of
a hassle for something that is basically an aesthetic issue, and in a
corner use case at that?  We are making non-trivial surgery in
Readline, something that was never tried before, we are importing a
partial change to upstream Readline, and we do all that a minute or so
before branching for GDB 12.  Are we sure this won't destabilize GDB
12 in ways we will never be able to discover and fix before the
release?

Re: [PATCH 2/2] gdb: handle bracketed-paste-mode and ctrl-d correctly

[Andrew Burgess]

Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> writes:

>> Date: Mon,  7 Mar 2022 15:13:15 +0000
>> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: Andrew Burgess <aburgess@redhat.com>
>> 
>> Currently, when readline is using bracketed-paste-mode, and the user
>> quits gdb with EOF (typically bound to ctrl-d), then we end up with
>> some corruption of the output.
>> 
>> If the user is sat at a prompt, like this:
>> 
>>   (gdb)
>> 
>> And then the user sends EOF, we currently see this:
>> 
>>   quit)
>>   ... gdb terminates ...
>> 
>> Notice, that the 'quit' is printed over the (gdb) prompt.  This is not
>> what we wanted, what we want to see is:
>> 
>>   (gdb) quit
>>   ... gdb terminates ...
>> 
>> The problem is that, when readline detects that the user has entered a
>> complete line, either by entering a newline, or by spotting EOF,
>> readline will restore the terminal to a pre-readline state.  When
>> bracketed-paste-mode is on, this includes switching
>> bracketed-paste-mode back off.
>> 
>> In the case where a real line of input is entered, and then a newline
>> sent, the cursor will have moved onto the next line before the
>> terminal settings are restored.  However, when an EOF is detected, the
>> cursor remains on the current line when the terminal settings are
>> restored.
>> 
>> This difference, of whether the cursor is on a new line or not, is
>> important.  In command_line_handler (event-top.c), this is where we
>> print the final "quit" message, or more accurately, we print "quit\n",
>> when the user sends EOF.  The final "\n" on that string moves the
>> cursor onto the next line so that we're in a similar state as if the
>> user had manually entered "quit" and then sent a newline.
>> 
>> And this is where bracketed-paste-mode causes a problem.  For
>> background reading, see:
>> 
>>   https://lists.gnu.org/archive/html/bug-bash/2018-01/msg00097.html
>> 
>> The outcome of that discussion, is that, when readline disables
>> bracketed-paste-mode, it also sends a "\r" to the terminal.
>> 
>> Now, in the case where the user sent a real command and a newline,
>> this isn't a problem.  The cursor has moved to the next line
>> anyway (which would be empty), and the "\r" simply returns us to the
>> start of the (empty) line.  No problem there.
>> 
>> However, in the EOF case, we don't move to a newline.  So the "\r"
>> after disabling bracketed-paste-mode returns the cursor to the start
>> of the prompt line.  Now in command_line_handler, when we print
>> "quit\n", this will overwrite the prompt.
>> 
>> It turns out that there is actually some problems with readline in
>> this area.  Disabling of bracketed paste mode is done in the readline
>> function rl_deprep_terminal.  As part of this readline should print a
>> "\n" after disabling bracketed paste mode if an EOF was received.
>> This is to work around specifically the issue we see above.
>> Unfortunately, how readline detects that the EOF was seen doesn't work
>> correctly when using the callback API, as GDB does.  As a result the
>> "\n" is not printed from rl_deprep_terminal.
>> 
>> If we fix readline's detection of EOF then rl_deprep_terminal will now
>> print the "\n" as expected, however, this has a knock on problem for
>> GDB.  rl_deprep_terminal is called before the command line is
>> dispatched back to GDB, this means that the "\n" will be printed from
>> rl_deprep_terminal before GDB prints "quit\n", the result is that when
>> the user sends EOF GDB's output will be:
>> 
>>   (gdb)
>>   quit
>> 
>> Which is now what we wanted; we wanted the quit on the same line as
>> the prompt.
>> 
>> To fix this problem I propose that within GDB we override the
>> rl_deprep_terminal function.  This is a valid thing to do, the
>> readline API exposes rl_deprep_term_function, which can be used to
>> replace the deprep function.
>> 
>> Now, we can move the printing of "quit" into GDB's new deprep
>> function, before calling on to readlines default rl_deprep_terminal
>> function, which will then print the trailing "\n" for us.
>> 
>> The only problem with this is that readline doesn't expose whether it
>> is handling EOF as part of its API.  However, in this thread:
>> 
>>   https://lists.gnu.org/archive/html/bug-readline/2022-02/msg00021.html
>> 
>> changes to readline were proposed, and accepted, that extended the
>> readline API such that information about whether we're handling EOF is
>> now available.
>> 
>> With this information available we can now use rl_deprep_term_function
>> to print the "quit" message.
>> 
>> There's just a couple of final cases we need to cover.
>> 
>> First, GDB can link against the system readline.  The system readline
>> might not have all of the fixes to expose the EOF status.  If this is
>> the case then we can't print "quit" from our rl_deprep_term_function
>> hook, as we can't know if we are handling EOF or not.
>> 
>> In this case, I have left GDB printing "quit" from the old location in
>> command_line_handler.  To avoid prompt corruption, then, if bracketed
>> paste mode is on, then we now deliberately print an extra "\n",
>> forcing the quit onto the next line, like this:
>> 
>>   (gdb)
>>   quit
>> 
>> This isn't ideal, but is, I think, the best we can do in this case.
>> 
>> The other edge case is from gdb_rl_deprep_term_function, our new
>> deprep hook.  We have to consider two cases, bracketed paste mode on,
>> and bracketed paste mode off.  As readline emits a "\n" after
>> leaving bracketed paste mode, then if we print "quit\n" from
>> gdb_rl_deprep_term_function we will end up with an extra blank line,
>> like this:
>> 
>>   (gdb) quit
>>   		# <--- This line left blank
>>   [Shell-Prompt]>
>> 
>> However, if we only print "quit" from gdb_rl_deprep_term_function, but
>> the user has bracketed paste mode disabled (say from .inputrc), then
>> we end up with this:
>> 
>>   (gdb) quit[Shell-Prompt]>
>> 
>> Which obviously is not right.
>> 
>> My solution here is to temporarily set rl_eof_found back to 0 before
>> calling the builtin rl_deprep_terminal function.  Right now, the only
>> impact this has is to prevent readline printing the extra "\n", which
>> solves the above problems.
>
> Let me play the devil's advocate role and ask: isn't this too much of
> a hassle for something that is basically an aesthetic issue, and in a
> corner use case at that?  We are making non-trivial surgery in
> Readline, something that was never tried before, we are importing a
> partial change to upstream Readline, and we do all that a minute or so
> before branching for GDB 12.  Are we sure this won't destabilize GDB
> 12 in ways we will never be able to discover and fix before the
> release?

I certainly don't have strong feelings either way.  I guess because I've
been working on the fix for a while I don't see it as too risky.  But I
also recognise that I'm not impartial here.

Below is the smallest fix which avoids the prompt corruption; simply
moving the "quit" message to the line after the prompt, like:

  (gdb)
  quit

I don't mind which fix people prefer.

If the preference is for the patch below then I can port the new test
from the original patch to include with this.

Thanks,
Andrew

---

commit 5cbf93bc3f8c3871612a5c640972bd448167eeaf
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Mar 8 09:37:07 2022 +0000

    gdb: work around issue of prompt corruption
    
    But PR cli/28833 identifies an issue of prompt corruption.  This
    occurs when readline is making use of bracketed paste mode.
    
    The problem is that when readline deactivates bracketed paste mode,
    the escape sequence used ends in "\r".  For the background of why this
    is, see:
    
      https://lists.gnu.org/archive/html/bug-bash/2018-01/msg00097.html
    
    Normally, this isn't an issue.  By the time readline deactivates
    bracketed paste mode the user will have sent a newline to readline,
    which will have moved the cursor to the next (empty) line, and so the
    "\r" simply returns the cursor to the start of the empty line.
    
    However, if readline receives EOF then the cursor is not moved to the
    next line, and so the "\r" returns the cursor to the start of the
    current prompt line.
    
    In GDB, when we spot the EOF in command_line_handler, we print
    "quit\n", so that it appears like the user entered the "quit"
    command.  However, due to the "\r", when bracketed paste mode is being
    used, the quit is printed over the GDB prompt.
    
    Fixing this issue fully requires changes to readline.
    
    This commit simply changes the "quit\n" to "\nquit\n", which forces
    GDB to print the quit on the line after the prompt in all cases, this
    avoid the prompt corruption problem.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 8890c4fae2d..cc077c000ad 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -767,7 +767,7 @@ command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
 	 This happens at the end of a testsuite run, after Expect has
 	 hung up but GDB is still alive.  In such a case, we just quit
 	 gdb killing the inferior program too.  */
-      printf_unfiltered ("quit\n");
+      printf_unfiltered ("\nquit\n");
       execute_command ("quit", 1);
     }
   else if (cmd == NULL)

Re: [PATCH 2/2] gdb: handle bracketed-paste-mode and ctrl-d correctly

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Tue, 08 Mar 2022 09:45:57 +0000
> 
> Below is the smallest fix which avoids the prompt corruption; simply
> moving the "quit" message to the line after the prompt, like:
> 
>   (gdb)
>   quit

Thanks, I think this is much safer for GDB 12, and seems to solve 95%
of the original problem.

I don't mind if, after branching for GDB 12, your original full
changeset will be applied to the master branch, of course.

Re: [PATCH 1/2] readline: back-port changes needed to properly detect EOF

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> This commit is a partial back-port of this upstream readline commit:

I think it's fine to do backports like this.

Last time I upgraded readline I made a repository to make this simpler,
but I see I didn't really document my plan.  Anyway, as long as local
divergences are related to something upstream, it shouldn't pose too
many problems.

Tom

[PATCHv2 0/3] Fix GDB prompt corruption issue

[Andrew Burgess]

Here's a revised plan for PR gdb/28833.

I've split the series into 3 patches.  The first is (I claim) the
smallest patch that mitigates against the problem reported in
gdb/28833, but isn't a complete fix.  This first patch is what I'd
like to push soon (before GDB 12 branches).

Patches 2 and 3 are the full fix.  These I'd like to push after GDB 12
branches.

Thoughts?

Thanks,
Andrew




---

Andrew Burgess (3):
  gdb: work around prompt corruption caused by bracketed-paste-mode
  readline: back-port changes needed to properly detect EOF
  gdb: handle bracketed-paste-mode and EOF correctly

 gdb/event-top.c                     | 64 ++++++++++++++++++++-
 gdb/event-top.h                     |  6 ++
 gdb/testsuite/gdb.base/eof-exit.exp | 88 +++++++++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp           |  9 +++
 gdb/top.c                           |  1 +
 readline/readline/callback.c        |  8 ++-
 readline/readline/doc/rltech.texi   | 11 ++++
 readline/readline/readline.c        | 19 ++++---
 readline/readline/readline.h        |  8 ++-
 readline/readline/rlprivate.h       |  1 -
 readline/readline/rltty.c           |  4 +-
 11 files changed, 206 insertions(+), 13 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/eof-exit.exp

-- 
2.25.4

[PATCHv2 1/3] gdb: work around prompt corruption caused by bracketed-paste-mode

[Andrew Burgess]

In this commit:

  commit b4f26d541aa7224b70d363932e816e6e1a857633
  Date:   Tue Mar 2 13:42:37 2021 -0700

      Import GNU Readline 8.1

We imported readline 8.1 into GDB.  As a consequence bug PR cli/28833
was reported.  This bug spotted that, when the user terminated GDB by
sending EOF (usually bound to Ctrl+d), the last prompt would become
corrupted.  Here's what happens, the user is sat at a prompt like
this:

  (gdb)

And then the user sends EOF (Ctrl+d), we now see this:

  quit)
  ... gdb terminates, and we return to the shell ...

Notice the 'quit' was printed over the prompt.

This problem is a result of readline 8.1 enabling bracketed paste mode
by default.  This problem is present in readline 8.0 too, but in that
version of readline bracketed paste mode is off by default, so a user
will not see the bug unless they specifically enable the feature.

Bracketed paste mode is available in readline 7.0 too, but the bug
is not present in this version of readline, see below for why.

What causes this problem is how readline disables bracketed paste
mode.  Bracketed paste mode is a terminal feature that is enabled and
disabled by readline emitting a specific escape sequence.  The problem
for GDB is that the escape sequence to disable bracketed paste mode
includes a '\r' character at the end, see this thread for more
details:

  https://lists.gnu.org/archive/html/bug-bash/2018-01/msg00097.html

The change to add the '\r' character to the escape sequence used to
disable bracketed paste mode was introduced between readline 7.0 and
readline 8.0, this is why the bug would not occur when using older
versions of readline (note: I don't know if its even possible to build
GDB using readline 7.0.  That really isn't important, I'm just
documenting the history of this issue).

So, the escape sequence to disable bracketed paste mode is emitted
from the readline function rl_deprep_terminal, this is called after
the user has entered a complete command and pressed return, or, if the
user sends EOF.

However, these two cases are slightly different.  In the first case,
when the user has entered a command and pressed return, the cursor
will have moved to the next, empty, line, before readline emits the
escape sequence to leave bracketed paste mode.  The final '\r'
character moves the cursor back to the beginning of this empty line,
which is harmless.

For the EOF case though, this is not what happens.  Instead, the
escape sequence to leave bracketed paste mode is emitted on the same
line as the prompt.  The final '\r' moves the cursor back to the start
of the prompt line.  This leaves us ready to override the prompt.

It is worth noting, that this is not the intended behaviour of
readline, in rl_deprep_terminal, readline should emit a '\n' character
when EOF is seen.  However, due to a bug in readline this does not
happen (the _rl_eof_found flag is never set).  This is the first
readline bug that effects GDB.

GDB prints the 'quit' message from command_line_handler (in
event-top.c), this function is called (indirectly) from readline to
process the complete command line, but also in the EOF case (in which
case the command line is set to nullptr).  As this is part of the
callback to process a complete command, this is called after readline
has disabled bracketed paste mode (by calling rl_deprep_terminal).

And so, when bracketed paste mode is in use, rl_deprep_terminal leaves
the cursor at the start of the prompt line (in the EOF case), and
command_line_handler then prints 'quit', which overwrites the prompt.

The solution to this problem is to print the 'quit' message earlier,
before rl_deprep_terminal is called.  This is easy to do by using the
rl_deprep_term_function hook.  It is this hook that usually calls
rl_deprep_terminal, however, if we replace this with a new function,
we can print the 'quit' string, and then call rl_deprep_terminal
ourselves.  This allows the 'quit' to be printed before
rl_deprep_terminal is called.

The problem here is that there is no way in rl_deprep_terminal to know
if readline is processing EOF or not, and as a result, we don't know
when we should print 'quit'.  This is the second readline bug that
effects GDB.

Both of these readline issues are discussed in this thread:

  https://lists.gnu.org/archive/html/bug-readline/2022-02/msg00021.html

The result of that thread was that readline was patched to address
both of these issues.

Now it should be easy to backport the readline fix to GDB's in tree
copy of readline, and then change GDB to make use of these fixes to
correctly print the 'quit' string.

However, we are just about to branch GDB 12, and there is concern from
some that changing readline this close to a new release is a risky
idea, see this thread:

  https://sourceware.org/pipermail/gdb-patches/2022-March/186391.html

So, this commit doesn't change readline at all.  Instead, this commit
is the smallest possible GDB change in order to avoid the prompt
corruption.

In this commit I change GDB to print the 'quit' string on the line
after the prompt, but only when bracketed paste mode is on.  This
avoids the overwriting issue, the user sees this:

  (gdb)
  quit
  ... gdb terminates, and returns to the shell ...

This isn't ideal, but is better than the existing behaviour.  After
GDB 12 has branched, we can backport the readline fix, and apply a
real fix to GDB.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833
---
 gdb/event-top.c                     | 20 ++++++-
 gdb/testsuite/gdb.base/eof-exit.exp | 88 +++++++++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp           |  9 +++
 3 files changed, 116 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/eof-exit.exp

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 8890c4fae2d..c1a95a4b748 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -766,7 +766,25 @@ command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
       /* stdin closed.  The connection with the terminal is gone.
 	 This happens at the end of a testsuite run, after Expect has
 	 hung up but GDB is still alive.  In such a case, we just quit
-	 gdb killing the inferior program too.  */
+	 gdb killing the inferior program too.  This also happens if the
+	 user sends EOF, which is usually bound to ctrl+d.
+
+	 What we want to do in this case is print "quit" after the GDB
+	 prompt, as if the user had just typed "quit" and pressed return.
+
+	 This used to work just fine, but unfortunately, doesn't play well
+	 with readline's bracketed paste mode.  By the time we get here,
+	 readline has already sent the control sequence to leave bracketed
+	 paste mode, and this sequence ends with a '\r' character.  As a
+	 result, if bracketed paste mode is on, and we print quit here,
+	 then this will overwrite the prompt.
+
+	 To work around this issue, when bracketed paste mode is enabled,
+	 we first print '\n' to move to the next line, and then print the
+	 quit.  This isn't ideal, but avoids corrupting the prompt.  */
+      const char *value = rl_variable_value ("enable-bracketed-paste");
+      if (value != nullptr && strcmp (value, "on") == 0)
+	printf_unfiltered ("\n");
       printf_unfiltered ("quit\n");
       execute_command ("quit", 1);
     }
diff --git a/gdb/testsuite/gdb.base/eof-exit.exp b/gdb/testsuite/gdb.base/eof-exit.exp
new file mode 100644
index 00000000000..d604d029974
--- /dev/null
+++ b/gdb/testsuite/gdb.base/eof-exit.exp
@@ -0,0 +1,88 @@
+# Copyright (C) 2022 Free Software Foundation, Inc.
+
+# 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/>.
+
+# This test script checks how GDB handles exiting with ctrl-d.
+
+# Start GDB, and then close it by sendig ctrl-d.  Check that the
+# string 'quit' appears where we expect it too.
+proc run_test {} {
+    clean_restart
+
+    # The gdb_start call above swallows the GDB prompt, and we want
+    # the prompt for the test below.
+    #
+    # Send a newline character, which will cause GDB to redisplay the
+    # prompt.
+    send_gdb "\n"
+    gdb_test_multiple "" "discard newline" {
+	-re "^\r\n" {
+	}
+    }
+
+    # Send GDB a ctrl-d.  Check that we see the 'quit' string in the
+    # expected place.
+    send_gdb "\004"
+    gdb_test_multiple "" "close GDB with eof" {
+	-re "$::gdb_prompt \[^\n\]*\r\[^\n\]*quit" {
+	    fail "$gdb_test_name (misplaced \\r)"
+	}
+	-re "$::gdb_prompt \[^\n\]*\r\[^\n\]*\r\nquit\r\n" {
+	    # For versions of readline that don't include the
+	    # RL_STATE_EOF patch, then the 'quit' is printed on the
+	    # subsequent line.
+	    kfail gdb/28833 "$gdb_test_name (older version of readline)"
+	}
+	-re "$::gdb_prompt quit\[^\n\]*\r\n\[^\n\]*\r\n$" {
+	    # There was a bug where GDB would print an extra blank
+	    # line after the 'quit', this catches that case.
+	    fail $gdb_test_name
+	}
+	-re "$::gdb_prompt quit\[^\n\]*\r\n\[^\n\]*$" {
+	    pass $gdb_test_name
+	}
+	eof {
+	    fail "$gdb_test_name (missed the prompt)"
+	}
+    }
+}
+
+with_test_prefix "default" {
+    run_test
+}
+
+save_vars { env(TERM) } {
+    setenv TERM ansi
+
+    with_test_prefix "with non-dump terminal" {
+	run_test
+
+	save_vars { env(INPUTRC) } {
+
+	    # Create an inputrc file that turns bracketed paste mode
+	    # on.  This is usually turned off (see lib/gdb.exp), but
+	    # for the next test we want to see what happens with this
+	    # on.
+	    set inputrc [standard_output_file inputrc]
+	    set fd [open "$inputrc" w]
+	    puts $fd "set enable-bracketed-paste on"
+	    close $fd
+
+	    setenv INPUTRC "$inputrc"
+	    with_test_prefix "with bracketed-paste-mode on" {
+		run_test
+	    }
+	}
+    }
+}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index a35d08a05de..08726f78563 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2151,6 +2151,15 @@ proc default_gdb_start { } {
 	-re "\[\r\n\]$gdb_prompt $" {
 	    verbose "GDB initialized."
 	}
+	-re "\[\r\n\]\033\\\[.2004h$gdb_prompt $" {
+	    # This special case detects what happens when GDB is
+	    # started with bracketed paste mode enabled.  This mode is
+	    # usually forced off (see setting of INPUTRC in
+	    # default_gdb_init), but for at least one test we turn
+	    # bracketed paste mode back on, and then start GDB.  In
+	    # that case, this case is hit.
+	    verbose "GDB initialized."
+	}
 	-re "$gdb_prompt $"	{
 	    perror "GDB never initialized."
 	    unset gdb_spawn_id
-- 
2.25.4

[PATCHv2 2/3] readline: back-port changes needed to properly detect EOF

[Andrew Burgess]

This commit is a partial back-port of this upstream readline commit:

  commit 002d31aa5f5929eb32d0e0e2e8b8d35d99e59961
  Author: Chet Ramey <chet.ramey@case.edu>
  Date:   Thu Mar 3 11:11:47 2022 -0500

      add rl_eof_found to public API; fix pointer aliasing problems  \
            with history-search-backward; fix a display problem with \
            runs of invisible characters at the end of a physical    \
            screen line

I have only pulled in the parts of this commit that relate to the new
rl_eof_found global, and the RL_STATE_EOF state flag.  These changes
are needed in order to fix PR cli/28833, and are discussed in this
thread to the bug-readline mailing list:

  https://lists.gnu.org/archive/html/bug-readline/2022-02/msg00021.html

The above commit is not yet in any official readline release, but my
hope is that now it has been merged into the readline tree it should
be safe enough to back port this fix to GDB's tree.

At some point in the future we will inevitably want to roll forward
the version of readline that we maintain in the binutils-gdb
repository.  When that day comes the changes in this commit can be
replaced with the latest upstream readline code, as I have not changed
the meaning of this code at all from what is in upstream readline.

This commit alone does not fix the PR cli/28833 issue, for that see
the next commit, which changes GDB itself.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833
---
 readline/readline/callback.c      |  8 +++++++-
 readline/readline/doc/rltech.texi | 11 +++++++++++
 readline/readline/readline.c      | 19 ++++++++++++-------
 readline/readline/readline.h      |  8 +++++++-
 readline/readline/rlprivate.h     |  1 -
 readline/readline/rltty.c         |  4 ++--
 6 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/readline/readline/callback.c b/readline/readline/callback.c
index a466cf9b6ef..58b84d2e2ad 100644
--- a/readline/readline/callback.c
+++ b/readline/readline/callback.c
@@ -1,6 +1,6 @@
 /* callback.c -- functions to use readline as an X `callback' mechanism. */
 
-/* Copyright (C) 1987-2017 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.
@@ -136,6 +136,8 @@ rl_callback_read_char (void)
       abort ();
     }
 
+  eof = 0;
+
   memcpy ((void *)olevel, (void *)_rl_top_level, sizeof (procenv_t));
 #if defined (HAVE_POSIX_SIGSETJMP)
   jcode = sigsetjmp (_rl_top_level, 0);
@@ -268,6 +270,10 @@ rl_callback_read_char (void)
 	  _rl_want_redisplay = 0;
 	}
 
+      /* Make sure application hooks can see whether we saw EOF. */
+      if (rl_eof_found = eof)
+	RL_SETSTATE(RL_STATE_EOF);
+
       if (rl_done)
 	{
 	  line = readline_internal_teardown (eof);
diff --git a/readline/readline/doc/rltech.texi b/readline/readline/doc/rltech.texi
index bbf57c239c9..797e34d95e0 100644
--- a/readline/readline/doc/rltech.texi
+++ b/readline/readline/doc/rltech.texi
@@ -323,6 +323,14 @@
 @deftypevar int rl_done
 Setting this to a non-zero value causes Readline to return the current
 line immediately.
+Readline will set this variable when it has read a key sequence bound
+to @code{accept-line} and is about to return the line to the caller.
+@end deftypevar
+
+@deftypevar int rl_eof_found
+Readline will set this variable when it has read an EOF character (e.g., the
+stty @samp{EOF} character) on an empty line or encountered a read error and
+is about to return a NULL line to the caller.
 @end deftypevar
 
 @deftypevar int rl_num_chars_to_read
@@ -588,6 +596,9 @@
 @item RL_STATE_DONE
 Readline has read a key sequence bound to @code{accept-line}
 and is about to return the line to the caller.
+@item RL_STATE_EOF
+Readline has read an EOF character (e.g., the stty @samp{EOF} character)
+or encountered a read error and is about to return a NULL line to the caller.
 @end table
 
 @end deftypevar
diff --git a/readline/readline/readline.c b/readline/readline/readline.c
index e61d188bbe9..0e33587f234 100644
--- a/readline/readline/readline.c
+++ b/readline/readline/readline.c
@@ -1,7 +1,7 @@
 /* readline.c -- a general facility for reading lines of input
    with emacs style editing and completion. */
 
-/* Copyright (C) 1987-2020 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.      
@@ -165,6 +165,9 @@ int rl_end;
 /* Make this non-zero to return the current input_line. */
 int rl_done;
 
+/* If non-zero when readline_internal returns, it means we found EOF */
+int rl_eof_found = 0;
+
 /* The last function executed by readline. */
 rl_command_func_t *rl_last_func = (rl_command_func_t *)NULL;
 
@@ -218,9 +221,6 @@ int _rl_eof_char = CTRL ('D');
 /* Non-zero makes this the next keystroke to read. */
 int rl_pending_input = 0;
 
-/* If non-zero when readline_internal returns, it means we found EOF */
-int _rl_eof_found = 0;
-
 /* Pointer to a useful terminal name. */
 const char *rl_terminal_name = (const char *)NULL;
 
@@ -474,6 +474,9 @@ readline_internal_teardown (int eof)
 
   RL_CHECK_SIGNALS ();
 
+  if (eof)
+    RL_SETSTATE (RL_STATE_EOF);		/* XXX */
+
   /* Restore the original of this history line, iff the line that we
      are editing was originally in the history, AND the line has changed. */
   entry = current_history ();
@@ -596,6 +599,7 @@ readline_internal_charloop (void)
 	  RL_SETSTATE(RL_STATE_DONE);
 	  return (rl_done = 1);
 #else
+	  RL_SETSTATE(RL_STATE_EOF);
 	  eof_found = 1;
 	  break;
 #endif
@@ -636,6 +640,7 @@ readline_internal_charloop (void)
 	  RL_SETSTATE(RL_STATE_DONE);
 	  return (rl_done = 1);
 #else
+	  RL_SETSTATE(RL_STATE_EOF);
 	  eof_found = 1;
 	  break;
 #endif
@@ -703,8 +708,8 @@ static char *
 readline_internal (void)
 {
   readline_internal_setup ();
-  _rl_eof_found = readline_internal_charloop ();
-  return (readline_internal_teardown (_rl_eof_found));
+  rl_eof_found = readline_internal_charloop ();
+  return (readline_internal_teardown (rl_eof_found));
 }
 
 void
@@ -1161,7 +1166,7 @@ rl_initialize (void)
 
   /* We aren't done yet.  We haven't even gotten started yet! */
   rl_done = 0;
-  RL_UNSETSTATE(RL_STATE_DONE);
+  RL_UNSETSTATE(RL_STATE_DONE|RL_STATE_EOF);
 
   /* Tell the history routines what is going on. */
   _rl_start_using_history ();
diff --git a/readline/readline/readline.h b/readline/readline/readline.h
index 78fa39d02a1..9f79810f290 100644
--- a/readline/readline/readline.h
+++ b/readline/readline/readline.h
@@ -1,6 +1,6 @@
 /* Readline.h -- the names of functions callable from within readline. */
 
-/* Copyright (C) 1987-2020 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.      
@@ -553,6 +553,10 @@ extern int rl_mark;
    line and should return it. */
 extern int rl_done;
 
+/* Flag to indicate that readline has read an EOF character or read has
+   returned 0 or error, and is returning a NULL line as a result. */
+extern int rl_eof_found;
+
 /* If set to a character value, that will be the next keystroke read. */
 extern int rl_pending_input;
 
@@ -906,6 +910,8 @@ extern int rl_persistent_signal_handlers;
 #define RL_STATE_REDISPLAYING	0x1000000	/* updating terminal display */
 
 #define RL_STATE_DONE		0x2000000	/* done; accepted line */
+#define RL_STATE_EOF		0x8000000	/* done; got eof on read */
+
 
 #define RL_SETSTATE(x)		(rl_readline_state |= (x))
 #define RL_UNSETSTATE(x)	(rl_readline_state &= ~(x))
diff --git a/readline/readline/rlprivate.h b/readline/readline/rlprivate.h
index 23ab2d8cec0..02838ae21ae 100644
--- a/readline/readline/rlprivate.h
+++ b/readline/readline/rlprivate.h
@@ -544,7 +544,6 @@ extern FILE *_rl_in_stream;
 extern FILE *_rl_out_stream;
 extern int _rl_last_command_was_kill;
 extern int _rl_eof_char;
-extern int _rl_eof_found;
 extern procenv_t _rl_top_level;
 extern _rl_keyseq_cxt *_rl_kscxt;
 extern int _rl_keyseq_timeout;
diff --git a/readline/readline/rltty.c b/readline/readline/rltty.c
index d0cd572713a..11997b7c9d8 100644
--- a/readline/readline/rltty.c
+++ b/readline/readline/rltty.c
@@ -1,7 +1,7 @@
 /* rltty.c -- functions to prepare and restore the terminal for readline's
    use. */
 
-/* Copyright (C) 1992-2017 Free Software Foundation, Inc.
+/* Copyright (C) 1992-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.
@@ -692,7 +692,7 @@ rl_deprep_terminal (void)
   if (terminal_prepped & TPX_BRACKPASTE)
     {
       fprintf (rl_outstream, BRACK_PASTE_FINI);
-      if (_rl_eof_found)
+      if (rl_eof_found)
  	fprintf (rl_outstream, "\n");
     }
 
-- 
2.25.4

[PATCHv2 3/3] gdb: handle bracketed-paste-mode and EOF correctly

[Andrew Burgess]

This commit replaces an earlier commit that worked around the issues
reported in bug PR gdb/28833.

The previous commit just implemented a work around in order to avoid
the worst results of the bug, but was not a complete solution.  The
full solution was considered too risky to merge close to branching GDB
12.  This improved fix has been applied after GDB 12 branched.  See
this thread for more details:

  https://sourceware.org/pipermail/gdb-patches/2022-March/186391.html

This commit replaces this earlier commit:

  commit 74a159a420d4b466cc81061c16d444568e36740c
  Date:   Fri Mar 11 14:44:03 2022 +0000

      gdb: work around prompt corruption caused by bracketed-paste-mode

Please read that commit for a full description of the bug, and why is
occurs.

In this commit I extend GDB to use readline's rl_deprep_term_function
hook to call a new function gdb_rl_deprep_term_function.  From this
new function we can now print the 'quit' message, this replaces the
old printing of 'quit' in command_line_handler.  Of course, we only
print 'quit' in gdb_rl_deprep_term_function when we are handling EOF,
but thanks to the previous commit (to readline) we now know when this
is.

There are two aspects of this commit that are worth further
discussion, the first is in the new gdb_rl_deprep_term_function
function.  In here I have used a scoped_restore_tmpl to disable the
readline global variable rl_eof_found.

The reason for this is that, in rl_deprep_terminal, readline will
print an extra '\n' character before printing the escape sequence to
leave bracketed paste mode.  You might then think that in the
gdb_rl_deprep_term_function function, we could simply print "quit" and
rely on rl_deprep_terminal to print the trailing '\n'.  However,
rl_deprep_terminal only prints the '\n' when bracketed paste mode is
on.  If the user has turned this feature off, no '\n' is printed.
This means that in gdb_rl_deprep_term_function we need to print
"quit" when bracketed paste mode is on, and "quit\n" when bracketed
paste mode is off.

We could absolutely do that, no problem, but given we know how
rl_deprep_terminal is implemented, it's easier (I think) to just
temporarily clear rl_eof_found, this prevents the '\n' being printed
from rl_deprep_terminal, and so in gdb_rl_deprep_term_function, we can
now always print "quit\n" and this works for all cases.

The second issue that should be discussed is backwards compatibility
with older versions of readline.  GDB can be built against the system
readline, which might be older than the version contained within GDB's
tree.  If this is the case then the system readline might not contain
the fixes needed to support correctly printing the 'quit' string.

To handle this situation I have retained the existing code in
command_line_handler for printing 'quit', however, this code is only
used now if the version of readline we are using doesn't not include
the required fixes.  And so, if a user is using an older version of
readline, and they have bracketed paste mode on, then they will see
the 'quit' sting printed on the line below the prompt, like this:

  (gdb)
  quit

I think this is the best we can do when someone builds GDB against an
older version of readline.

Using a newer version of readline, or the patched version of readline
that is in-tree, will now give a result like this in all cases:

  (gdb) quit

Which is what we want.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833
---
 gdb/event-top.c | 68 ++++++++++++++++++++++++++++++++++++++++---------
 gdb/event-top.h |  6 +++++
 gdb/top.c       |  1 +
 3 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index c1a95a4b748..a85049e87e4 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -745,6 +745,25 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
     return cmd;
 }
 
+/* See event-top.h.  */
+
+void
+gdb_rl_deprep_term_function (void)
+{
+#ifdef RL_STATE_EOF
+  gdb::optional<scoped_restore_tmpl<int>> restore_eof_found;
+
+  if (RL_ISSTATE (RL_STATE_EOF))
+    {
+      printf_unfiltered ("quit\n");
+      restore_eof_found.emplace (&rl_eof_found, 0);
+    }
+
+#endif /* RL_STATE_EOF */
+
+  rl_deprep_terminal ();
+}
+
 /* Handle a complete line of input.  This is called by the callback
    mechanism within the readline library.  Deal with incomplete
    commands as well, by saving the partial input in a global
@@ -767,25 +786,50 @@ command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
 	 This happens at the end of a testsuite run, after Expect has
 	 hung up but GDB is still alive.  In such a case, we just quit
 	 gdb killing the inferior program too.  This also happens if the
-	 user sends EOF, which is usually bound to ctrl+d.
+	 user sends EOF, which is usually bound to ctrl+d.  */
+
+#ifndef RL_STATE_EOF
+      /* When readline is using bracketed paste mode, then, when eof is
+	 received, readline will emit the control sequence to leave
+	 bracketed paste mode.
+
+	 This control sequence ends with \r, which means that the "quit" we
+	 are about to print will overwrite the prompt on this line.
+
+	 The solution to this problem is to actually print the "quit"
+	 message from gdb_rl_deprep_term_function (see above), however, we
+	 can only do that if we can know, in that function, when eof was
+	 received.
+
+	 Unfortunately, with older versions of readline, it is not possible
+	 in the gdb_rl_deprep_term_function to know if eof was received or
+	 not, and, as GDB can be built against the system readline, which
+	 could be older than the readline in GDB's repository, then we
+	 can't be sure that we can work around this prompt corruption in
+	 the gdb_rl_deprep_term_function function.
 
-	 What we want to do in this case is print "quit" after the GDB
-	 prompt, as if the user had just typed "quit" and pressed return.
+	 If we get here, RL_STATE_EOF is not defined.  This indicates that
+	 we are using an older readline, and couldn't print the quit
+	 message in gdb_rl_deprep_term_function.  So, what we do here is
+	 check to see if bracketed paste mode is on or not.  If it's on
+	 then we print a \n and then the quit, this means the user will
+	 see:
 
-	 This used to work just fine, but unfortunately, doesn't play well
-	 with readline's bracketed paste mode.  By the time we get here,
-	 readline has already sent the control sequence to leave bracketed
-	 paste mode, and this sequence ends with a '\r' character.  As a
-	 result, if bracketed paste mode is on, and we print quit here,
-	 then this will overwrite the prompt.
+	 (gdb)
+	 quit
 
-	 To work around this issue, when bracketed paste mode is enabled,
-	 we first print '\n' to move to the next line, and then print the
-	 quit.  This isn't ideal, but avoids corrupting the prompt.  */
+	 Rather than the usual:
+
+	 (gdb) quit
+
+	 Which we will get with a newer readline, but this really is the
+	 best we can do with older versions of readline.  */
       const char *value = rl_variable_value ("enable-bracketed-paste");
       if (value != nullptr && strcmp (value, "on") == 0)
 	printf_unfiltered ("\n");
       printf_unfiltered ("quit\n");
+#endif
+
       execute_command ("quit", 1);
     }
   else if (cmd == NULL)
diff --git a/gdb/event-top.h b/gdb/event-top.h
index 078482b79aa..722e636a110 100644
--- a/gdb/event-top.h
+++ b/gdb/event-top.h
@@ -70,6 +70,12 @@ extern void gdb_rl_callback_handler_install (const char *prompt);
    currently installed.  */
 extern void gdb_rl_callback_handler_reinstall (void);
 
+/* Called by readline after a complete line has been gathered from the
+   user, but before the line is dispatched to back to GDB.  This function
+   is a wrapper around readline's builtin rl_deprep_terminal function, and
+   handles the case where readline received EOF.  */
+extern void gdb_rl_deprep_term_function (void);
+
 typedef void (*segv_handler_t) (int);
 
 /* On construction, replaces the current thread's SIGSEGV handler with
diff --git a/gdb/top.c b/gdb/top.c
index a94ed5cebdb..3af35daa9ca 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -2229,6 +2229,7 @@ init_main (void)
   rl_completion_display_matches_hook = cli_display_match_list;
   rl_readline_name = "gdb";
   rl_terminal_name = getenv ("TERM");
+  rl_deprep_term_function = gdb_rl_deprep_term_function;
 
   /* The name for this defun comes from Bash, where it originated.
      15 is Control-o, the same binding this function has in Bash.  */
-- 
2.25.4

Re: [PATCHv2 1/3] gdb: work around prompt corruption caused by bracketed-paste-mode

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> This isn't ideal, but is better than the existing behaviour.  After
Andrew> GDB 12 has branched, we can backport the readline fix, and apply a
Andrew> real fix to GDB.

FWIW this patch looks good to me.  I appreciate how much research you
did into this.  Thank you.

Tom

Re: [PATCHv2 1/3] gdb: work around prompt corruption caused by bracketed-paste-mode

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> This isn't ideal, but is better than the existing behaviour.  After
> Andrew> GDB 12 has branched, we can backport the readline fix, and apply a
> Andrew> real fix to GDB.
>
> FWIW this patch looks good to me.  I appreciate how much research you
> did into this.  Thank you.

Thanks.  I've now pushed this patch.  After GDB 12 branches I'll ping
patches #2 and #3 and get those merged to fully address this bug.

Thanks,
Andrew

RE: [PATCHv2 1/3] gdb: work around prompt corruption caused by bracketed-paste-mode

[Aktemur, Tankut Baris]

On Wednesday, March 16, 2022 9:45 PM, Andrew Burgess wrote:
> Tom Tromey <tom@tromey.com> writes:
> 
> >>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
> >
> > Andrew> This isn't ideal, but is better than the existing behaviour.  After
> > Andrew> GDB 12 has branched, we can backport the readline fix, and apply a
> > Andrew> real fix to GDB.
> >
> > FWIW this patch looks good to me.  I appreciate how much research you
> > did into this.  Thank you.
> 
> Thanks.  I've now pushed this patch.  After GDB 12 branches I'll ping
> patches #2 and #3 and get those merged to fully address this bug.

Thank you for fixing this.  It was indeed quite involved.

-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCHv3 0/2] Fix GDB prompt corruption issue

[Andrew Burgess]

Now GDB 12 has branched, I'm reposting the final two patches in this
series.

These provide a real fix for the prompt corruption issue reported in
bug PR cli/28833.

All I've done for v3 is rebase onto current master, and retest.

Thanks,
Andrew



---

Andrew Burgess (2):
  readline: back-port changes needed to properly detect EOF
  gdb: handle bracketed-paste-mode and EOF correctly

 gdb/event-top.c                   | 68 +++++++++++++++++++++++++------
 gdb/event-top.h                   |  6 +++
 gdb/top.c                         |  1 +
 readline/readline/callback.c      |  8 +++-
 readline/readline/doc/rltech.texi | 11 +++++
 readline/readline/readline.c      | 19 +++++----
 readline/readline/readline.h      |  8 +++-
 readline/readline/rlprivate.h     |  1 -
 readline/readline/rltty.c         |  4 +-
 9 files changed, 102 insertions(+), 24 deletions(-)

-- 
2.25.4

[PATCHv3 1/2] readline: back-port changes needed to properly detect EOF

[Andrew Burgess]

This commit is a partial back-port of this upstream readline commit:

  commit 002d31aa5f5929eb32d0e0e2e8b8d35d99e59961
  Author: Chet Ramey <chet.ramey@case.edu>
  Date:   Thu Mar 3 11:11:47 2022 -0500

      add rl_eof_found to public API; fix pointer aliasing problems  \
            with history-search-backward; fix a display problem with \
            runs of invisible characters at the end of a physical    \
            screen line

I have only pulled in the parts of this commit that relate to the new
rl_eof_found global, and the RL_STATE_EOF state flag.  These changes
are needed in order to fix PR cli/28833, and are discussed in this
thread to the bug-readline mailing list:

  https://lists.gnu.org/archive/html/bug-readline/2022-02/msg00021.html

The above commit is not yet in any official readline release, but my
hope is that now it has been merged into the readline tree it should
be safe enough to back port this fix to GDB's tree.

At some point in the future we will inevitably want to roll forward
the version of readline that we maintain in the binutils-gdb
repository.  When that day comes the changes in this commit can be
replaced with the latest upstream readline code, as I have not changed
the meaning of this code at all from what is in upstream readline.

This commit alone does not fix the PR cli/28833 issue, for that see
the next commit, which changes GDB itself.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833
---
 readline/readline/callback.c      |  8 +++++++-
 readline/readline/doc/rltech.texi | 11 +++++++++++
 readline/readline/readline.c      | 19 ++++++++++++-------
 readline/readline/readline.h      |  8 +++++++-
 readline/readline/rlprivate.h     |  1 -
 readline/readline/rltty.c         |  4 ++--
 6 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/readline/readline/callback.c b/readline/readline/callback.c
index a466cf9b6ef..58b84d2e2ad 100644
--- a/readline/readline/callback.c
+++ b/readline/readline/callback.c
@@ -1,6 +1,6 @@
 /* callback.c -- functions to use readline as an X `callback' mechanism. */
 
-/* Copyright (C) 1987-2017 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.
@@ -136,6 +136,8 @@ rl_callback_read_char (void)
       abort ();
     }
 
+  eof = 0;
+
   memcpy ((void *)olevel, (void *)_rl_top_level, sizeof (procenv_t));
 #if defined (HAVE_POSIX_SIGSETJMP)
   jcode = sigsetjmp (_rl_top_level, 0);
@@ -268,6 +270,10 @@ rl_callback_read_char (void)
 	  _rl_want_redisplay = 0;
 	}
 
+      /* Make sure application hooks can see whether we saw EOF. */
+      if (rl_eof_found = eof)
+	RL_SETSTATE(RL_STATE_EOF);
+
       if (rl_done)
 	{
 	  line = readline_internal_teardown (eof);
diff --git a/readline/readline/doc/rltech.texi b/readline/readline/doc/rltech.texi
index bbf57c239c9..797e34d95e0 100644
--- a/readline/readline/doc/rltech.texi
+++ b/readline/readline/doc/rltech.texi
@@ -323,6 +323,14 @@
 @deftypevar int rl_done
 Setting this to a non-zero value causes Readline to return the current
 line immediately.
+Readline will set this variable when it has read a key sequence bound
+to @code{accept-line} and is about to return the line to the caller.
+@end deftypevar
+
+@deftypevar int rl_eof_found
+Readline will set this variable when it has read an EOF character (e.g., the
+stty @samp{EOF} character) on an empty line or encountered a read error and
+is about to return a NULL line to the caller.
 @end deftypevar
 
 @deftypevar int rl_num_chars_to_read
@@ -588,6 +596,9 @@
 @item RL_STATE_DONE
 Readline has read a key sequence bound to @code{accept-line}
 and is about to return the line to the caller.
+@item RL_STATE_EOF
+Readline has read an EOF character (e.g., the stty @samp{EOF} character)
+or encountered a read error and is about to return a NULL line to the caller.
 @end table
 
 @end deftypevar
diff --git a/readline/readline/readline.c b/readline/readline/readline.c
index e61d188bbe9..0e33587f234 100644
--- a/readline/readline/readline.c
+++ b/readline/readline/readline.c
@@ -1,7 +1,7 @@
 /* readline.c -- a general facility for reading lines of input
    with emacs style editing and completion. */
 
-/* Copyright (C) 1987-2020 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.      
@@ -165,6 +165,9 @@ int rl_end;
 /* Make this non-zero to return the current input_line. */
 int rl_done;
 
+/* If non-zero when readline_internal returns, it means we found EOF */
+int rl_eof_found = 0;
+
 /* The last function executed by readline. */
 rl_command_func_t *rl_last_func = (rl_command_func_t *)NULL;
 
@@ -218,9 +221,6 @@ int _rl_eof_char = CTRL ('D');
 /* Non-zero makes this the next keystroke to read. */
 int rl_pending_input = 0;
 
-/* If non-zero when readline_internal returns, it means we found EOF */
-int _rl_eof_found = 0;
-
 /* Pointer to a useful terminal name. */
 const char *rl_terminal_name = (const char *)NULL;
 
@@ -474,6 +474,9 @@ readline_internal_teardown (int eof)
 
   RL_CHECK_SIGNALS ();
 
+  if (eof)
+    RL_SETSTATE (RL_STATE_EOF);		/* XXX */
+
   /* Restore the original of this history line, iff the line that we
      are editing was originally in the history, AND the line has changed. */
   entry = current_history ();
@@ -596,6 +599,7 @@ readline_internal_charloop (void)
 	  RL_SETSTATE(RL_STATE_DONE);
 	  return (rl_done = 1);
 #else
+	  RL_SETSTATE(RL_STATE_EOF);
 	  eof_found = 1;
 	  break;
 #endif
@@ -636,6 +640,7 @@ readline_internal_charloop (void)
 	  RL_SETSTATE(RL_STATE_DONE);
 	  return (rl_done = 1);
 #else
+	  RL_SETSTATE(RL_STATE_EOF);
 	  eof_found = 1;
 	  break;
 #endif
@@ -703,8 +708,8 @@ static char *
 readline_internal (void)
 {
   readline_internal_setup ();
-  _rl_eof_found = readline_internal_charloop ();
-  return (readline_internal_teardown (_rl_eof_found));
+  rl_eof_found = readline_internal_charloop ();
+  return (readline_internal_teardown (rl_eof_found));
 }
 
 void
@@ -1161,7 +1166,7 @@ rl_initialize (void)
 
   /* We aren't done yet.  We haven't even gotten started yet! */
   rl_done = 0;
-  RL_UNSETSTATE(RL_STATE_DONE);
+  RL_UNSETSTATE(RL_STATE_DONE|RL_STATE_EOF);
 
   /* Tell the history routines what is going on. */
   _rl_start_using_history ();
diff --git a/readline/readline/readline.h b/readline/readline/readline.h
index 78fa39d02a1..9f79810f290 100644
--- a/readline/readline/readline.h
+++ b/readline/readline/readline.h
@@ -1,6 +1,6 @@
 /* Readline.h -- the names of functions callable from within readline. */
 
-/* Copyright (C) 1987-2020 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.      
@@ -553,6 +553,10 @@ extern int rl_mark;
    line and should return it. */
 extern int rl_done;
 
+/* Flag to indicate that readline has read an EOF character or read has
+   returned 0 or error, and is returning a NULL line as a result. */
+extern int rl_eof_found;
+
 /* If set to a character value, that will be the next keystroke read. */
 extern int rl_pending_input;
 
@@ -906,6 +910,8 @@ extern int rl_persistent_signal_handlers;
 #define RL_STATE_REDISPLAYING	0x1000000	/* updating terminal display */
 
 #define RL_STATE_DONE		0x2000000	/* done; accepted line */
+#define RL_STATE_EOF		0x8000000	/* done; got eof on read */
+
 
 #define RL_SETSTATE(x)		(rl_readline_state |= (x))
 #define RL_UNSETSTATE(x)	(rl_readline_state &= ~(x))
diff --git a/readline/readline/rlprivate.h b/readline/readline/rlprivate.h
index 23ab2d8cec0..02838ae21ae 100644
--- a/readline/readline/rlprivate.h
+++ b/readline/readline/rlprivate.h
@@ -544,7 +544,6 @@ extern FILE *_rl_in_stream;
 extern FILE *_rl_out_stream;
 extern int _rl_last_command_was_kill;
 extern int _rl_eof_char;
-extern int _rl_eof_found;
 extern procenv_t _rl_top_level;
 extern _rl_keyseq_cxt *_rl_kscxt;
 extern int _rl_keyseq_timeout;
diff --git a/readline/readline/rltty.c b/readline/readline/rltty.c
index d0cd572713a..11997b7c9d8 100644
--- a/readline/readline/rltty.c
+++ b/readline/readline/rltty.c
@@ -1,7 +1,7 @@
 /* rltty.c -- functions to prepare and restore the terminal for readline's
    use. */
 
-/* Copyright (C) 1992-2017 Free Software Foundation, Inc.
+/* Copyright (C) 1992-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.
@@ -692,7 +692,7 @@ rl_deprep_terminal (void)
   if (terminal_prepped & TPX_BRACKPASTE)
     {
       fprintf (rl_outstream, BRACK_PASTE_FINI);
-      if (_rl_eof_found)
+      if (rl_eof_found)
  	fprintf (rl_outstream, "\n");
     }
 
-- 
2.25.4

[PATCHv3 2/2] gdb: handle bracketed-paste-mode and EOF correctly

[Andrew Burgess]

This commit replaces an earlier commit that worked around the issues
reported in bug PR gdb/28833.

The previous commit just implemented a work around in order to avoid
the worst results of the bug, but was not a complete solution.  The
full solution was considered too risky to merge close to branching GDB
12.  This improved fix has been applied after GDB 12 branched.  See
this thread for more details:

  https://sourceware.org/pipermail/gdb-patches/2022-March/186391.html

This commit replaces this earlier commit:

  commit 74a159a420d4b466cc81061c16d444568e36740c
  Date:   Fri Mar 11 14:44:03 2022 +0000

      gdb: work around prompt corruption caused by bracketed-paste-mode

Please read that commit for a full description of the bug, and why is
occurs.

In this commit I extend GDB to use readline's rl_deprep_term_function
hook to call a new function gdb_rl_deprep_term_function.  From this
new function we can now print the 'quit' message, this replaces the
old printing of 'quit' in command_line_handler.  Of course, we only
print 'quit' in gdb_rl_deprep_term_function when we are handling EOF,
but thanks to the previous commit (to readline) we now know when this
is.

There are two aspects of this commit that are worth further
discussion, the first is in the new gdb_rl_deprep_term_function
function.  In here I have used a scoped_restore_tmpl to disable the
readline global variable rl_eof_found.

The reason for this is that, in rl_deprep_terminal, readline will
print an extra '\n' character before printing the escape sequence to
leave bracketed paste mode.  You might then think that in the
gdb_rl_deprep_term_function function, we could simply print "quit" and
rely on rl_deprep_terminal to print the trailing '\n'.  However,
rl_deprep_terminal only prints the '\n' when bracketed paste mode is
on.  If the user has turned this feature off, no '\n' is printed.
This means that in gdb_rl_deprep_term_function we need to print
"quit" when bracketed paste mode is on, and "quit\n" when bracketed
paste mode is off.

We could absolutely do that, no problem, but given we know how
rl_deprep_terminal is implemented, it's easier (I think) to just
temporarily clear rl_eof_found, this prevents the '\n' being printed
from rl_deprep_terminal, and so in gdb_rl_deprep_term_function, we can
now always print "quit\n" and this works for all cases.

The second issue that should be discussed is backwards compatibility
with older versions of readline.  GDB can be built against the system
readline, which might be older than the version contained within GDB's
tree.  If this is the case then the system readline might not contain
the fixes needed to support correctly printing the 'quit' string.

To handle this situation I have retained the existing code in
command_line_handler for printing 'quit', however, this code is only
used now if the version of readline we are using doesn't not include
the required fixes.  And so, if a user is using an older version of
readline, and they have bracketed paste mode on, then they will see
the 'quit' sting printed on the line below the prompt, like this:

  (gdb)
  quit

I think this is the best we can do when someone builds GDB against an
older version of readline.

Using a newer version of readline, or the patched version of readline
that is in-tree, will now give a result like this in all cases:

  (gdb) quit

Which is what we want.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833
---
 gdb/event-top.c | 68 ++++++++++++++++++++++++++++++++++++++++---------
 gdb/event-top.h |  6 +++++
 gdb/top.c       |  1 +
 3 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index c1a95a4b748..a85049e87e4 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -745,6 +745,25 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
     return cmd;
 }
 
+/* See event-top.h.  */
+
+void
+gdb_rl_deprep_term_function (void)
+{
+#ifdef RL_STATE_EOF
+  gdb::optional<scoped_restore_tmpl<int>> restore_eof_found;
+
+  if (RL_ISSTATE (RL_STATE_EOF))
+    {
+      printf_unfiltered ("quit\n");
+      restore_eof_found.emplace (&rl_eof_found, 0);
+    }
+
+#endif /* RL_STATE_EOF */
+
+  rl_deprep_terminal ();
+}
+
 /* Handle a complete line of input.  This is called by the callback
    mechanism within the readline library.  Deal with incomplete
    commands as well, by saving the partial input in a global
@@ -767,25 +786,50 @@ command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
 	 This happens at the end of a testsuite run, after Expect has
 	 hung up but GDB is still alive.  In such a case, we just quit
 	 gdb killing the inferior program too.  This also happens if the
-	 user sends EOF, which is usually bound to ctrl+d.
+	 user sends EOF, which is usually bound to ctrl+d.  */
+
+#ifndef RL_STATE_EOF
+      /* When readline is using bracketed paste mode, then, when eof is
+	 received, readline will emit the control sequence to leave
+	 bracketed paste mode.
+
+	 This control sequence ends with \r, which means that the "quit" we
+	 are about to print will overwrite the prompt on this line.
+
+	 The solution to this problem is to actually print the "quit"
+	 message from gdb_rl_deprep_term_function (see above), however, we
+	 can only do that if we can know, in that function, when eof was
+	 received.
+
+	 Unfortunately, with older versions of readline, it is not possible
+	 in the gdb_rl_deprep_term_function to know if eof was received or
+	 not, and, as GDB can be built against the system readline, which
+	 could be older than the readline in GDB's repository, then we
+	 can't be sure that we can work around this prompt corruption in
+	 the gdb_rl_deprep_term_function function.
 
-	 What we want to do in this case is print "quit" after the GDB
-	 prompt, as if the user had just typed "quit" and pressed return.
+	 If we get here, RL_STATE_EOF is not defined.  This indicates that
+	 we are using an older readline, and couldn't print the quit
+	 message in gdb_rl_deprep_term_function.  So, what we do here is
+	 check to see if bracketed paste mode is on or not.  If it's on
+	 then we print a \n and then the quit, this means the user will
+	 see:
 
-	 This used to work just fine, but unfortunately, doesn't play well
-	 with readline's bracketed paste mode.  By the time we get here,
-	 readline has already sent the control sequence to leave bracketed
-	 paste mode, and this sequence ends with a '\r' character.  As a
-	 result, if bracketed paste mode is on, and we print quit here,
-	 then this will overwrite the prompt.
+	 (gdb)
+	 quit
 
-	 To work around this issue, when bracketed paste mode is enabled,
-	 we first print '\n' to move to the next line, and then print the
-	 quit.  This isn't ideal, but avoids corrupting the prompt.  */
+	 Rather than the usual:
+
+	 (gdb) quit
+
+	 Which we will get with a newer readline, but this really is the
+	 best we can do with older versions of readline.  */
       const char *value = rl_variable_value ("enable-bracketed-paste");
       if (value != nullptr && strcmp (value, "on") == 0)
 	printf_unfiltered ("\n");
       printf_unfiltered ("quit\n");
+#endif
+
       execute_command ("quit", 1);
     }
   else if (cmd == NULL)
diff --git a/gdb/event-top.h b/gdb/event-top.h
index 078482b79aa..722e636a110 100644
--- a/gdb/event-top.h
+++ b/gdb/event-top.h
@@ -70,6 +70,12 @@ extern void gdb_rl_callback_handler_install (const char *prompt);
    currently installed.  */
 extern void gdb_rl_callback_handler_reinstall (void);
 
+/* Called by readline after a complete line has been gathered from the
+   user, but before the line is dispatched to back to GDB.  This function
+   is a wrapper around readline's builtin rl_deprep_terminal function, and
+   handles the case where readline received EOF.  */
+extern void gdb_rl_deprep_term_function (void);
+
 typedef void (*segv_handler_t) (int);
 
 /* On construction, replaces the current thread's SIGSEGV handler with
diff --git a/gdb/top.c b/gdb/top.c
index 7831b4f96ca..d3e5391df6a 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -2222,6 +2222,7 @@ init_main (void)
   rl_completion_display_matches_hook = cli_display_match_list;
   rl_readline_name = "gdb";
   rl_terminal_name = getenv ("TERM");
+  rl_deprep_term_function = gdb_rl_deprep_term_function;
 
   /* The name for this defun comes from Bash, where it originated.
      15 is Control-o, the same binding this function has in Bash.  */
-- 
2.25.4

Re: [PATCHv2 1/3] gdb: work around prompt corruption caused by bracketed-paste-mode

[Simon Marchi]

On 2022-03-14 06:23, Andrew Burgess via Gdb-patches wrote:
> In this commit:
>
>   commit b4f26d541aa7224b70d363932e816e6e1a857633
>   Date:   Tue Mar 2 13:42:37 2021 -0700
>
>       Import GNU Readline 8.1
>
> We imported readline 8.1 into GDB.  As a consequence bug PR cli/28833
> was reported.  This bug spotted that, when the user terminated GDB by
> sending EOF (usually bound to Ctrl+d), the last prompt would become
> corrupted.  Here's what happens, the user is sat at a prompt like
> this:
>
>   (gdb)
>
> And then the user sends EOF (Ctrl+d), we now see this:
>
>   quit)
>   ... gdb terminates, and we return to the shell ...
>
> Notice the 'quit' was printed over the prompt.
>
> This problem is a result of readline 8.1 enabling bracketed paste mode
> by default.  This problem is present in readline 8.0 too, but in that
> version of readline bracketed paste mode is off by default, so a user
> will not see the bug unless they specifically enable the feature.
>
> Bracketed paste mode is available in readline 7.0 too, but the bug
> is not present in this version of readline, see below for why.
>
> What causes this problem is how readline disables bracketed paste
> mode.  Bracketed paste mode is a terminal feature that is enabled and
> disabled by readline emitting a specific escape sequence.  The problem
> for GDB is that the escape sequence to disable bracketed paste mode
> includes a '\r' character at the end, see this thread for more
> details:
>
>   https://lists.gnu.org/archive/html/bug-bash/2018-01/msg00097.html
>
> The change to add the '\r' character to the escape sequence used to
> disable bracketed paste mode was introduced between readline 7.0 and
> readline 8.0, this is why the bug would not occur when using older
> versions of readline (note: I don't know if its even possible to build
> GDB using readline 7.0.  That really isn't important, I'm just
> documenting the history of this issue).
>
> So, the escape sequence to disable bracketed paste mode is emitted
> from the readline function rl_deprep_terminal, this is called after
> the user has entered a complete command and pressed return, or, if the
> user sends EOF.
>
> However, these two cases are slightly different.  In the first case,
> when the user has entered a command and pressed return, the cursor
> will have moved to the next, empty, line, before readline emits the
> escape sequence to leave bracketed paste mode.  The final '\r'
> character moves the cursor back to the beginning of this empty line,
> which is harmless.
>
> For the EOF case though, this is not what happens.  Instead, the
> escape sequence to leave bracketed paste mode is emitted on the same
> line as the prompt.  The final '\r' moves the cursor back to the start
> of the prompt line.  This leaves us ready to override the prompt.
>
> It is worth noting, that this is not the intended behaviour of
> readline, in rl_deprep_terminal, readline should emit a '\n' character
> when EOF is seen.  However, due to a bug in readline this does not
> happen (the _rl_eof_found flag is never set).  This is the first
> readline bug that effects GDB.
>
> GDB prints the 'quit' message from command_line_handler (in
> event-top.c), this function is called (indirectly) from readline to
> process the complete command line, but also in the EOF case (in which
> case the command line is set to nullptr).  As this is part of the
> callback to process a complete command, this is called after readline
> has disabled bracketed paste mode (by calling rl_deprep_terminal).
>
> And so, when bracketed paste mode is in use, rl_deprep_terminal leaves
> the cursor at the start of the prompt line (in the EOF case), and
> command_line_handler then prints 'quit', which overwrites the prompt.
>
> The solution to this problem is to print the 'quit' message earlier,
> before rl_deprep_terminal is called.  This is easy to do by using the
> rl_deprep_term_function hook.  It is this hook that usually calls
> rl_deprep_terminal, however, if we replace this with a new function,
> we can print the 'quit' string, and then call rl_deprep_terminal
> ourselves.  This allows the 'quit' to be printed before
> rl_deprep_terminal is called.
>
> The problem here is that there is no way in rl_deprep_terminal to know
> if readline is processing EOF or not, and as a result, we don't know
> when we should print 'quit'.  This is the second readline bug that
> effects GDB.
>
> Both of these readline issues are discussed in this thread:
>
>   https://lists.gnu.org/archive/html/bug-readline/2022-02/msg00021.html
>
> The result of that thread was that readline was patched to address
> both of these issues.
>
> Now it should be easy to backport the readline fix to GDB's in tree
> copy of readline, and then change GDB to make use of these fixes to
> correctly print the 'quit' string.
>
> However, we are just about to branch GDB 12, and there is concern from
> some that changing readline this close to a new release is a risky
> idea, see this thread:
>
>   https://sourceware.org/pipermail/gdb-patches/2022-March/186391.html
>
> So, this commit doesn't change readline at all.  Instead, this commit
> is the smallest possible GDB change in order to avoid the prompt
> corruption.
>
> In this commit I change GDB to print the 'quit' string on the line
> after the prompt, but only when bracketed paste mode is on.  This
> avoids the overwriting issue, the user sees this:
>
>   (gdb)
>   quit
>   ... gdb terminates, and returns to the shell ...
>
> This isn't ideal, but is better than the existing behaviour.  After
> GDB 12 has branched, we can backport the readline fix, and apply a
> real fix to GDB.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833

Hi Andrew,

I see this test fail on the following setup:

 - Ubuntu 18.04
 - Building with --with-system-readline (distro package libreadline-dev
   installed, which is readline 7.0)


^[[?2004h(gdb) set height 0^M
^[[?2004l^[[?2004h(gdb) set width 0^M
^[[?2004l^[[?2004h(gdb) dir^M
^[[?2004l^[[?2004hReinitialize source path to empty? (y or n) y^M
^[[?2004lSource directories searched: $cdir:$cwd^M
^[[?2004h(gdb) dir /build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base^M
^[[?2004lSource directories searched: /build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base:$cdir:$cwd^M
^[[?2004h(gdb) ^M
^[[?2004l^[[?2004h(gdb) ^[[?2004l^M
quit^M
FAIL: gdb.base/eof-exit.exp: with non-dump terminal: with bracketed-paste-mode on: close GDB with eof (missed the prompt)


It's clearly related to bracketed-paste-mode, since these 2004 escape
sequences turn bracketed-paste-mode on and off (accordin to
https://wiki2.org/en/ISO/IEC_6429).  But I don't know what happens
exactly.

Here's a Dockerfile that can be used to reproduce the failure, hopefully
it makes it easier:

    FROM ubuntu:18.04
    RUN apt-get -y update && \
      apt-get -y full-upgrade && \
      DEBIAN_FRONTEND=noninteractive TZ=Etc/UTC apt-get -y install build-essential flex bison git texinfo libreadline-dev dejagnu libexpat1-dev libgmp-dev
    RUN git clone --depth 1 git://sourceware.org/git/binutils-gdb.git
    RUN mkdir build
    WORKDIR /build
    RUN ../binutils-gdb/configure --with-system-readline
    RUN make -j $(nproc) all-gdb
    WORKDIR /build/gdb
    RUN make check TESTS="gdb.base/eof-exit.exp"

Simon

Re: [PATCHv2 1/3] gdb: work around prompt corruption caused by bracketed-paste-mode

[Andrew Burgess]

* Simon Marchi <simark@simark.ca> [2022-03-25 14:32:42 -0400]:

> On 2022-03-14 06:23, Andrew Burgess via Gdb-patches wrote:
> > In this commit:
> >
> >   commit b4f26d541aa7224b70d363932e816e6e1a857633
> >   Date:   Tue Mar 2 13:42:37 2021 -0700
> >
> >       Import GNU Readline 8.1
> >
> > We imported readline 8.1 into GDB.  As a consequence bug PR cli/28833
> > was reported.  This bug spotted that, when the user terminated GDB by
> > sending EOF (usually bound to Ctrl+d), the last prompt would become
> > corrupted.  Here's what happens, the user is sat at a prompt like
> > this:
> >
> >   (gdb)
> >
> > And then the user sends EOF (Ctrl+d), we now see this:
> >
> >   quit)
> >   ... gdb terminates, and we return to the shell ...
> >
> > Notice the 'quit' was printed over the prompt.
> >
> > This problem is a result of readline 8.1 enabling bracketed paste mode
> > by default.  This problem is present in readline 8.0 too, but in that
> > version of readline bracketed paste mode is off by default, so a user
> > will not see the bug unless they specifically enable the feature.
> >
> > Bracketed paste mode is available in readline 7.0 too, but the bug
> > is not present in this version of readline, see below for why.
> >
> > What causes this problem is how readline disables bracketed paste
> > mode.  Bracketed paste mode is a terminal feature that is enabled and
> > disabled by readline emitting a specific escape sequence.  The problem
> > for GDB is that the escape sequence to disable bracketed paste mode
> > includes a '\r' character at the end, see this thread for more
> > details:
> >
> >   https://lists.gnu.org/archive/html/bug-bash/2018-01/msg00097.html
> >
> > The change to add the '\r' character to the escape sequence used to
> > disable bracketed paste mode was introduced between readline 7.0 and
> > readline 8.0, this is why the bug would not occur when using older
> > versions of readline (note: I don't know if its even possible to build
> > GDB using readline 7.0.  That really isn't important, I'm just
> > documenting the history of this issue).
> >
> > So, the escape sequence to disable bracketed paste mode is emitted
> > from the readline function rl_deprep_terminal, this is called after
> > the user has entered a complete command and pressed return, or, if the
> > user sends EOF.
> >
> > However, these two cases are slightly different.  In the first case,
> > when the user has entered a command and pressed return, the cursor
> > will have moved to the next, empty, line, before readline emits the
> > escape sequence to leave bracketed paste mode.  The final '\r'
> > character moves the cursor back to the beginning of this empty line,
> > which is harmless.
> >
> > For the EOF case though, this is not what happens.  Instead, the
> > escape sequence to leave bracketed paste mode is emitted on the same
> > line as the prompt.  The final '\r' moves the cursor back to the start
> > of the prompt line.  This leaves us ready to override the prompt.
> >
> > It is worth noting, that this is not the intended behaviour of
> > readline, in rl_deprep_terminal, readline should emit a '\n' character
> > when EOF is seen.  However, due to a bug in readline this does not
> > happen (the _rl_eof_found flag is never set).  This is the first
> > readline bug that effects GDB.
> >
> > GDB prints the 'quit' message from command_line_handler (in
> > event-top.c), this function is called (indirectly) from readline to
> > process the complete command line, but also in the EOF case (in which
> > case the command line is set to nullptr).  As this is part of the
> > callback to process a complete command, this is called after readline
> > has disabled bracketed paste mode (by calling rl_deprep_terminal).
> >
> > And so, when bracketed paste mode is in use, rl_deprep_terminal leaves
> > the cursor at the start of the prompt line (in the EOF case), and
> > command_line_handler then prints 'quit', which overwrites the prompt.
> >
> > The solution to this problem is to print the 'quit' message earlier,
> > before rl_deprep_terminal is called.  This is easy to do by using the
> > rl_deprep_term_function hook.  It is this hook that usually calls
> > rl_deprep_terminal, however, if we replace this with a new function,
> > we can print the 'quit' string, and then call rl_deprep_terminal
> > ourselves.  This allows the 'quit' to be printed before
> > rl_deprep_terminal is called.
> >
> > The problem here is that there is no way in rl_deprep_terminal to know
> > if readline is processing EOF or not, and as a result, we don't know
> > when we should print 'quit'.  This is the second readline bug that
> > effects GDB.
> >
> > Both of these readline issues are discussed in this thread:
> >
> >   https://lists.gnu.org/archive/html/bug-readline/2022-02/msg00021.html
> >
> > The result of that thread was that readline was patched to address
> > both of these issues.
> >
> > Now it should be easy to backport the readline fix to GDB's in tree
> > copy of readline, and then change GDB to make use of these fixes to
> > correctly print the 'quit' string.
> >
> > However, we are just about to branch GDB 12, and there is concern from
> > some that changing readline this close to a new release is a risky
> > idea, see this thread:
> >
> >   https://sourceware.org/pipermail/gdb-patches/2022-March/186391.html
> >
> > So, this commit doesn't change readline at all.  Instead, this commit
> > is the smallest possible GDB change in order to avoid the prompt
> > corruption.
> >
> > In this commit I change GDB to print the 'quit' string on the line
> > after the prompt, but only when bracketed paste mode is on.  This
> > avoids the overwriting issue, the user sees this:
> >
> >   (gdb)
> >   quit
> >   ... gdb terminates, and returns to the shell ...
> >
> > This isn't ideal, but is better than the existing behaviour.  After
> > GDB 12 has branched, we can backport the readline fix, and apply a
> > real fix to GDB.
> >
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833
> 
> Hi Andrew,
> 
> I see this test fail on the following setup:
> 
>  - Ubuntu 18.04
>  - Building with --with-system-readline (distro package libreadline-dev
>    installed, which is readline 7.0)
> 
> 
> ^[[?2004h(gdb) set height 0^M
> ^[[?2004l^[[?2004h(gdb) set width 0^M
> ^[[?2004l^[[?2004h(gdb) dir^M
> ^[[?2004l^[[?2004hReinitialize source path to empty? (y or n) y^M
> ^[[?2004lSource directories searched: $cdir:$cwd^M
> ^[[?2004h(gdb) dir /build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base^M
> ^[[?2004lSource directories searched: /build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base:$cdir:$cwd^M
> ^[[?2004h(gdb) ^M
> ^[[?2004l^[[?2004h(gdb) ^[[?2004l^M
> quit^M
> FAIL: gdb.base/eof-exit.exp: with non-dump terminal: with bracketed-paste-mode on: close GDB with eof (missed the prompt)
> 
> 
> It's clearly related to bracketed-paste-mode, since these 2004 escape
> sequences turn bracketed-paste-mode on and off (accordin to
> https://wiki2.org/en/ISO/IEC_6429).  But I don't know what happens
> exactly.
> 
> Here's a Dockerfile that can be used to reproduce the failure, hopefully
> it makes it easier:
> 
>     FROM ubuntu:18.04
>     RUN apt-get -y update && \
>       apt-get -y full-upgrade && \
>       DEBIAN_FRONTEND=noninteractive TZ=Etc/UTC apt-get -y install build-essential flex bison git texinfo libreadline-dev dejagnu libexpat1-dev libgmp-dev
>     RUN git clone --depth 1 git://sourceware.org/git/binutils-gdb.git
>     RUN mkdir build
>     WORKDIR /build
>     RUN ../binutils-gdb/configure --with-system-readline
>     RUN make -j $(nproc) all-gdb
>     WORKDIR /build/gdb
>     RUN make check TESTS="gdb.base/eof-exit.exp"

Thanks for the awesome steps to reproduce!  This made it super easy to
track down the problem.

Basically the problem is that I only took readline 8+ into account
when writing the test, this spin of ubuntu is on readline 7.

The patch below changes the test FAIL into a KFAIL, which, if you were
on readline 8 would be the best we could do.

But given you're on readline 7, we should be able to do better,
getting this to a PASS!  But that will require changes to GDB itself.

I'd like to propose that first we merge the patch below, this removes
the FAIL, then next week I'll post a follow on patch for GDB that
should get this test PASSing for readline 7.

Thoughts?

Thanks,
Andrew

---

commit 7a0add556420e2ef814a6cd58501a832fdb1fb90
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Sat Mar 26 13:41:33 2022 +0000

    gdb/testsuite: fix test failure when building against readline v7
    
    The test added in the commit:
    
      commit a6b413d24ccc5d76179bab866834e11fd6fec294
      Date:   Fri Mar 11 14:44:03 2022 +0000
    
          gdb: work around prompt corruption caused by bracketed-paste-mode
    
    Was not written with readline 7 in mind, only readline 8+.  Between
    readline 7 and 8 the escape sequence used to disable bracketed paste
    mode changed, an additional '\r' character was added to the end.  In
    fact, it was the addition of this '\r' character that triggered the
    issue for which the above commit is part of the solution.
    
    Anyway, the test tries to spot the case where the output from GDB is
    not perfect, but does have the above work around applied.  However,
    the pattern in the test assume that the problematic '\r' will be
    present, and this is only true for readline 8+.  With readline 7 the
    test was failing.
    
    In this commit I generalise the pattern a little so that the test will
    still KFAIL with readline 7.
    
    It's a little unfortunate that the test is KFAILing with readline 7,
    as without the problematic '\r' there's actually no reason that GDB
    couldn't "do the right thing" in this case, in which case, the test
    would PASS, but that would require changes within GDB itself.
    
    My preference then is that initially we patch the test to get it
    KFAILing, then in a separate commit I can modify GDB so that it can
    PASS with readline 7.

diff --git a/gdb/testsuite/gdb.base/eof-exit.exp b/gdb/testsuite/gdb.base/eof-exit.exp
index d604d029974..2d9530ccebe 100644
--- a/gdb/testsuite/gdb.base/eof-exit.exp
+++ b/gdb/testsuite/gdb.base/eof-exit.exp
@@ -38,7 +38,7 @@ proc run_test {} {
 	-re "$::gdb_prompt \[^\n\]*\r\[^\n\]*quit" {
 	    fail "$gdb_test_name (misplaced \\r)"
 	}
-	-re "$::gdb_prompt \[^\n\]*\r\[^\n\]*\r\nquit\r\n" {
+	-re "$::gdb_prompt (?:\[^\n\]*\r)?\[^\n\]*\r\nquit\r\n" {
 	    # For versions of readline that don't include the
 	    # RL_STATE_EOF patch, then the 'quit' is printed on the
 	    # subsequent line.

Re: [PATCHv2 1/3] gdb: work around prompt corruption caused by bracketed-paste-mode

[Simon Marchi]

> Thanks for the awesome steps to reproduce!  This made it super easy to
> track down the problem.
> 
> Basically the problem is that I only took readline 8+ into account
> when writing the test, this spin of ubuntu is on readline 7.
> 
> The patch below changes the test FAIL into a KFAIL, which, if you were
> on readline 8 would be the best we could do.
> 
> But given you're on readline 7, we should be able to do better,
> getting this to a PASS!  But that will require changes to GDB itself.
> 
> I'd like to propose that first we merge the patch below, this removes
> the FAIL, then next week I'll post a follow on patch for GDB that
> should get this test PASSing for readline 7.
> 
> Thoughts?

Hi,

I am fine with the patch below.  Unfortunately I am not up to speed on
what this issue with the prompt was, so I can't really help further.
Thanks for coming back with a "fix" quickly, that helps.

Just one nit in the commit message below.

> commit 7a0add556420e2ef814a6cd58501a832fdb1fb90
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Sat Mar 26 13:41:33 2022 +0000
> 
>     gdb/testsuite: fix test failure when building against readline v7
>     
>     The test added in the commit:
>     
>       commit a6b413d24ccc5d76179bab866834e11fd6fec294
>       Date:   Fri Mar 11 14:44:03 2022 +0000
>     
>           gdb: work around prompt corruption caused by bracketed-paste-mode
>     
>     Was not written with readline 7 in mind, only readline 8+.  Between
>     readline 7 and 8 the escape sequence used to disable bracketed paste
>     mode changed, an additional '\r' character was added to the end.  In
>     fact, it was the addition of this '\r' character that triggered the
>     issue for which the above commit is part of the solution.
>     
>     Anyway, the test tries to spot the case where the output from GDB is
>     not perfect, but does have the above work around applied.  However,
>     the pattern in the test assume that the problematic '\r' will be

assume -> assumes

Simon

[PATCHv4 0/3] Fix GDB prompt corruption issue

[Andrew Burgess]

Changes since v3:

 - New patch #1, this was inspired by the conversation here:
   https://sourceware.org/pipermail/gdb-patches/2022-March/186996.html
   where it was pointed out that the work around was not actually
   needed for readline 7,

 - Patches #2 and #3 are unchanged,

 - Everything is rebased onto current master.

Thanks,
Andrew

---

Andrew Burgess (3):
  gdb: improved EOF handling when using readline 7
  readline: back-port changes needed to properly detect EOF
  gdb: handle bracketed-paste-mode and EOF correctly

 gdb/event-top.c                     | 71 +++++++++++++++++++++++------
 gdb/event-top.h                     |  6 +++
 gdb/testsuite/gdb.base/eof-exit.exp |  2 +-
 gdb/top.c                           |  1 +
 readline/readline/callback.c        |  8 +++-
 readline/readline/doc/rltech.texi   | 11 +++++
 readline/readline/readline.c        | 19 +++++---
 readline/readline/readline.h        |  8 +++-
 readline/readline/rlprivate.h       |  1 -
 readline/readline/rltty.c           |  4 +-
 10 files changed, 105 insertions(+), 26 deletions(-)

-- 
2.25.4

[PATCHv4 1/3] gdb: improved EOF handling when using readline 7

[Andrew Burgess]

In this commit:

  commit a6b413d24ccc5d76179bab866834e11fd6fec294
  Date:   Fri Mar 11 14:44:03 2022 +0000

      gdb: work around prompt corruption caused by bracketed-paste-mode

a change was made to GDB to work around bug PR gdb/28833.  The
consequence of this work around is that, when bracketed paste mode is
enabled in readline, and GDB is quit by sending EOF, then the output
will look like this:

  (gdb)
  quit

The ideal output, which is what we get when bracketed paste mode is
off, is this:

  (gdb) quit

The reason we need to make this change is explained in the original
commit referenced above.  What isn't mentioned in the above commit, is
that the change that motivated this work around was only added in
readline 8, older versions of readline don't require the change.

In later commits in this series I will add a fix to GDB's in-tree copy
of readline (this fix is back-ported from upstream readline), and then
I will change GDB so that, when using the (patched) in-tree readline,
we can have the ideal output in all cases.

However, GDB can be built against the system readline.  When this is
done, and the system readline is version 8, then we will still have to
use the work around (two line) style output.

But, if GDB is built against the system readline, and the system
readline is an older version 7 readline, then there's no reason why we
can't have the ideal output, after all, readline 7 doesn't include the
change that we need to work around.

This commit changes GDB so that, when using readline 7 we get the
ideal output in all cases.  This change is trivial (a simple check
against the readline version number) so I think this should be fine to
include.

For testing this commit, you need to configure GDB including the
'--with-system-readline' flag, and build GDB on a system that uses
readline 7, for example 'Ubuntu 18.04'.  Then run the test
'gdb.base/eof-exit.exp', you should expect everything to PASS.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833
---
 gdb/event-top.c                     | 3 ++-
 gdb/testsuite/gdb.base/eof-exit.exp | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 3ba7239518e..a5686488976 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -783,7 +783,8 @@ command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
 	 we first print '\n' to move to the next line, and then print the
 	 quit.  This isn't ideal, but avoids corrupting the prompt.  */
       const char *value = rl_variable_value ("enable-bracketed-paste");
-      if (value != nullptr && strcmp (value, "on") == 0)
+      if (value != nullptr && strcmp (value, "on") == 0
+	  && ((rl_readline_version >> 8) & 0xff) > 0x07)
 	printf_unfiltered ("\n");
       printf_unfiltered ("quit\n");
       execute_command ("quit", 1);
diff --git a/gdb/testsuite/gdb.base/eof-exit.exp b/gdb/testsuite/gdb.base/eof-exit.exp
index 2d9530ccebe..ad5f33d2f10 100644
--- a/gdb/testsuite/gdb.base/eof-exit.exp
+++ b/gdb/testsuite/gdb.base/eof-exit.exp
@@ -49,7 +49,7 @@ proc run_test {} {
 	    # line after the 'quit', this catches that case.
 	    fail $gdb_test_name
 	}
-	-re "$::gdb_prompt quit\[^\n\]*\r\n\[^\n\]*$" {
+       -re "$::gdb_prompt \[^\n\r\]*quit\[^\n\]*\r\n\[^\n\]*$" {
 	    pass $gdb_test_name
 	}
 	eof {
-- 
2.25.4

[PATCHv4 2/3] readline: back-port changes needed to properly detect EOF

[Andrew Burgess]

This commit is a partial back-port of this upstream readline commit:

  commit 002d31aa5f5929eb32d0e0e2e8b8d35d99e59961
  Author: Chet Ramey <chet.ramey@case.edu>
  Date:   Thu Mar 3 11:11:47 2022 -0500

      add rl_eof_found to public API; fix pointer aliasing problems  \
            with history-search-backward; fix a display problem with \
            runs of invisible characters at the end of a physical    \
            screen line

I have only pulled in the parts of this commit that relate to the new
rl_eof_found global, and the RL_STATE_EOF state flag.  These changes
are needed in order to fix PR cli/28833, and are discussed in this
thread to the bug-readline mailing list:

  https://lists.gnu.org/archive/html/bug-readline/2022-02/msg00021.html

The above commit is not yet in any official readline release, but my
hope is that now it has been merged into the readline tree it should
be safe enough to back port this fix to GDB's tree.

At some point in the future we will inevitably want to roll forward
the version of readline that we maintain in the binutils-gdb
repository.  When that day comes the changes in this commit can be
replaced with the latest upstream readline code, as I have not changed
the meaning of this code at all from what is in upstream readline.

This commit alone does not fix the PR cli/28833 issue, for that see
the next commit, which changes GDB itself.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833
---
 readline/readline/callback.c      |  8 +++++++-
 readline/readline/doc/rltech.texi | 11 +++++++++++
 readline/readline/readline.c      | 19 ++++++++++++-------
 readline/readline/readline.h      |  8 +++++++-
 readline/readline/rlprivate.h     |  1 -
 readline/readline/rltty.c         |  4 ++--
 6 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/readline/readline/callback.c b/readline/readline/callback.c
index a466cf9b6ef..58b84d2e2ad 100644
--- a/readline/readline/callback.c
+++ b/readline/readline/callback.c
@@ -1,6 +1,6 @@
 /* callback.c -- functions to use readline as an X `callback' mechanism. */
 
-/* Copyright (C) 1987-2017 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.
@@ -136,6 +136,8 @@ rl_callback_read_char (void)
       abort ();
     }
 
+  eof = 0;
+
   memcpy ((void *)olevel, (void *)_rl_top_level, sizeof (procenv_t));
 #if defined (HAVE_POSIX_SIGSETJMP)
   jcode = sigsetjmp (_rl_top_level, 0);
@@ -268,6 +270,10 @@ rl_callback_read_char (void)
 	  _rl_want_redisplay = 0;
 	}
 
+      /* Make sure application hooks can see whether we saw EOF. */
+      if (rl_eof_found = eof)
+	RL_SETSTATE(RL_STATE_EOF);
+
       if (rl_done)
 	{
 	  line = readline_internal_teardown (eof);
diff --git a/readline/readline/doc/rltech.texi b/readline/readline/doc/rltech.texi
index bbf57c239c9..797e34d95e0 100644
--- a/readline/readline/doc/rltech.texi
+++ b/readline/readline/doc/rltech.texi
@@ -323,6 +323,14 @@
 @deftypevar int rl_done
 Setting this to a non-zero value causes Readline to return the current
 line immediately.
+Readline will set this variable when it has read a key sequence bound
+to @code{accept-line} and is about to return the line to the caller.
+@end deftypevar
+
+@deftypevar int rl_eof_found
+Readline will set this variable when it has read an EOF character (e.g., the
+stty @samp{EOF} character) on an empty line or encountered a read error and
+is about to return a NULL line to the caller.
 @end deftypevar
 
 @deftypevar int rl_num_chars_to_read
@@ -588,6 +596,9 @@
 @item RL_STATE_DONE
 Readline has read a key sequence bound to @code{accept-line}
 and is about to return the line to the caller.
+@item RL_STATE_EOF
+Readline has read an EOF character (e.g., the stty @samp{EOF} character)
+or encountered a read error and is about to return a NULL line to the caller.
 @end table
 
 @end deftypevar
diff --git a/readline/readline/readline.c b/readline/readline/readline.c
index e61d188bbe9..0e33587f234 100644
--- a/readline/readline/readline.c
+++ b/readline/readline/readline.c
@@ -1,7 +1,7 @@
 /* readline.c -- a general facility for reading lines of input
    with emacs style editing and completion. */
 
-/* Copyright (C) 1987-2020 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.      
@@ -165,6 +165,9 @@ int rl_end;
 /* Make this non-zero to return the current input_line. */
 int rl_done;
 
+/* If non-zero when readline_internal returns, it means we found EOF */
+int rl_eof_found = 0;
+
 /* The last function executed by readline. */
 rl_command_func_t *rl_last_func = (rl_command_func_t *)NULL;
 
@@ -218,9 +221,6 @@ int _rl_eof_char = CTRL ('D');
 /* Non-zero makes this the next keystroke to read. */
 int rl_pending_input = 0;
 
-/* If non-zero when readline_internal returns, it means we found EOF */
-int _rl_eof_found = 0;
-
 /* Pointer to a useful terminal name. */
 const char *rl_terminal_name = (const char *)NULL;
 
@@ -474,6 +474,9 @@ readline_internal_teardown (int eof)
 
   RL_CHECK_SIGNALS ();
 
+  if (eof)
+    RL_SETSTATE (RL_STATE_EOF);		/* XXX */
+
   /* Restore the original of this history line, iff the line that we
      are editing was originally in the history, AND the line has changed. */
   entry = current_history ();
@@ -596,6 +599,7 @@ readline_internal_charloop (void)
 	  RL_SETSTATE(RL_STATE_DONE);
 	  return (rl_done = 1);
 #else
+	  RL_SETSTATE(RL_STATE_EOF);
 	  eof_found = 1;
 	  break;
 #endif
@@ -636,6 +640,7 @@ readline_internal_charloop (void)
 	  RL_SETSTATE(RL_STATE_DONE);
 	  return (rl_done = 1);
 #else
+	  RL_SETSTATE(RL_STATE_EOF);
 	  eof_found = 1;
 	  break;
 #endif
@@ -703,8 +708,8 @@ static char *
 readline_internal (void)
 {
   readline_internal_setup ();
-  _rl_eof_found = readline_internal_charloop ();
-  return (readline_internal_teardown (_rl_eof_found));
+  rl_eof_found = readline_internal_charloop ();
+  return (readline_internal_teardown (rl_eof_found));
 }
 
 void
@@ -1161,7 +1166,7 @@ rl_initialize (void)
 
   /* We aren't done yet.  We haven't even gotten started yet! */
   rl_done = 0;
-  RL_UNSETSTATE(RL_STATE_DONE);
+  RL_UNSETSTATE(RL_STATE_DONE|RL_STATE_EOF);
 
   /* Tell the history routines what is going on. */
   _rl_start_using_history ();
diff --git a/readline/readline/readline.h b/readline/readline/readline.h
index 78fa39d02a1..9f79810f290 100644
--- a/readline/readline/readline.h
+++ b/readline/readline/readline.h
@@ -1,6 +1,6 @@
 /* Readline.h -- the names of functions callable from within readline. */
 
-/* Copyright (C) 1987-2020 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.      
@@ -553,6 +553,10 @@ extern int rl_mark;
    line and should return it. */
 extern int rl_done;
 
+/* Flag to indicate that readline has read an EOF character or read has
+   returned 0 or error, and is returning a NULL line as a result. */
+extern int rl_eof_found;
+
 /* If set to a character value, that will be the next keystroke read. */
 extern int rl_pending_input;
 
@@ -906,6 +910,8 @@ extern int rl_persistent_signal_handlers;
 #define RL_STATE_REDISPLAYING	0x1000000	/* updating terminal display */
 
 #define RL_STATE_DONE		0x2000000	/* done; accepted line */
+#define RL_STATE_EOF		0x8000000	/* done; got eof on read */
+
 
 #define RL_SETSTATE(x)		(rl_readline_state |= (x))
 #define RL_UNSETSTATE(x)	(rl_readline_state &= ~(x))
diff --git a/readline/readline/rlprivate.h b/readline/readline/rlprivate.h
index 23ab2d8cec0..02838ae21ae 100644
--- a/readline/readline/rlprivate.h
+++ b/readline/readline/rlprivate.h
@@ -544,7 +544,6 @@ extern FILE *_rl_in_stream;
 extern FILE *_rl_out_stream;
 extern int _rl_last_command_was_kill;
 extern int _rl_eof_char;
-extern int _rl_eof_found;
 extern procenv_t _rl_top_level;
 extern _rl_keyseq_cxt *_rl_kscxt;
 extern int _rl_keyseq_timeout;
diff --git a/readline/readline/rltty.c b/readline/readline/rltty.c
index d0cd572713a..11997b7c9d8 100644
--- a/readline/readline/rltty.c
+++ b/readline/readline/rltty.c
@@ -1,7 +1,7 @@
 /* rltty.c -- functions to prepare and restore the terminal for readline's
    use. */
 
-/* Copyright (C) 1992-2017 Free Software Foundation, Inc.
+/* Copyright (C) 1992-2022 Free Software Foundation, Inc.
 
    This file is part of the GNU Readline Library (Readline), a library
    for reading lines of text with interactive input and history editing.
@@ -692,7 +692,7 @@ rl_deprep_terminal (void)
   if (terminal_prepped & TPX_BRACKPASTE)
     {
       fprintf (rl_outstream, BRACK_PASTE_FINI);
-      if (_rl_eof_found)
+      if (rl_eof_found)
  	fprintf (rl_outstream, "\n");
     }
 
-- 
2.25.4

[PATCHv4 3/3] gdb: handle bracketed-paste-mode and EOF correctly

[Andrew Burgess]

This commit replaces an earlier commit that worked around the issues
reported in bug PR gdb/28833.

The previous commit just implemented a work around in order to avoid
the worst results of the bug, but was not a complete solution.  The
full solution was considered too risky to merge close to branching GDB
12.  This improved fix has been applied after GDB 12 branched.  See
this thread for more details:

  https://sourceware.org/pipermail/gdb-patches/2022-March/186391.html

This commit replaces this earlier commit:

  commit 74a159a420d4b466cc81061c16d444568e36740c
  Date:   Fri Mar 11 14:44:03 2022 +0000

      gdb: work around prompt corruption caused by bracketed-paste-mode

Please read that commit for a full description of the bug, and why is
occurs.

In this commit I extend GDB to use readline's rl_deprep_term_function
hook to call a new function gdb_rl_deprep_term_function.  From this
new function we can now print the 'quit' message, this replaces the
old printing of 'quit' in command_line_handler.  Of course, we only
print 'quit' in gdb_rl_deprep_term_function when we are handling EOF,
but thanks to the previous commit (to readline) we now know when this
is.

There are two aspects of this commit that are worth further
discussion, the first is in the new gdb_rl_deprep_term_function
function.  In here I have used a scoped_restore_tmpl to disable the
readline global variable rl_eof_found.

The reason for this is that, in rl_deprep_terminal, readline will
print an extra '\n' character before printing the escape sequence to
leave bracketed paste mode.  You might then think that in the
gdb_rl_deprep_term_function function, we could simply print "quit" and
rely on rl_deprep_terminal to print the trailing '\n'.  However,
rl_deprep_terminal only prints the '\n' when bracketed paste mode is
on.  If the user has turned this feature off, no '\n' is printed.
This means that in gdb_rl_deprep_term_function we need to print
"quit" when bracketed paste mode is on, and "quit\n" when bracketed
paste mode is off.

We could absolutely do that, no problem, but given we know how
rl_deprep_terminal is implemented, it's easier (I think) to just
temporarily clear rl_eof_found, this prevents the '\n' being printed
from rl_deprep_terminal, and so in gdb_rl_deprep_term_function, we can
now always print "quit\n" and this works for all cases.

The second issue that should be discussed is backwards compatibility
with older versions of readline.  GDB can be built against the system
readline, which might be older than the version contained within GDB's
tree.  If this is the case then the system readline might not contain
the fixes needed to support correctly printing the 'quit' string.

To handle this situation I have retained the existing code in
command_line_handler for printing 'quit', however, this code is only
used now if the version of readline we are using doesn't not include
the required fixes.  And so, if a user is using an older version of
readline, and they have bracketed paste mode on, then they will see
the 'quit' sting printed on the line below the prompt, like this:

  (gdb)
  quit

I think this is the best we can do when someone builds GDB against an
older version of readline.

Using a newer version of readline, or the patched version of readline
that is in-tree, will now give a result like this in all cases:

  (gdb) quit

Which is what we want.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833
---
 gdb/event-top.c | 68 ++++++++++++++++++++++++++++++++++++++++---------
 gdb/event-top.h |  6 +++++
 gdb/top.c       |  1 +
 3 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index a5686488976..c3154b6ec07 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -745,6 +745,25 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
     return cmd;
 }
 
+/* See event-top.h.  */
+
+void
+gdb_rl_deprep_term_function (void)
+{
+#ifdef RL_STATE_EOF
+  gdb::optional<scoped_restore_tmpl<int>> restore_eof_found;
+
+  if (RL_ISSTATE (RL_STATE_EOF))
+    {
+      printf_unfiltered ("quit\n");
+      restore_eof_found.emplace (&rl_eof_found, 0);
+    }
+
+#endif /* RL_STATE_EOF */
+
+  rl_deprep_terminal ();
+}
+
 /* Handle a complete line of input.  This is called by the callback
    mechanism within the readline library.  Deal with incomplete
    commands as well, by saving the partial input in a global
@@ -767,26 +786,51 @@ command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
 	 This happens at the end of a testsuite run, after Expect has
 	 hung up but GDB is still alive.  In such a case, we just quit
 	 gdb killing the inferior program too.  This also happens if the
-	 user sends EOF, which is usually bound to ctrl+d.
+	 user sends EOF, which is usually bound to ctrl+d.  */
+
+#ifndef RL_STATE_EOF
+      /* When readline is using bracketed paste mode, then, when eof is
+	 received, readline will emit the control sequence to leave
+	 bracketed paste mode.
+
+	 This control sequence ends with \r, which means that the "quit" we
+	 are about to print will overwrite the prompt on this line.
+
+	 The solution to this problem is to actually print the "quit"
+	 message from gdb_rl_deprep_term_function (see above), however, we
+	 can only do that if we can know, in that function, when eof was
+	 received.
+
+	 Unfortunately, with older versions of readline, it is not possible
+	 in the gdb_rl_deprep_term_function to know if eof was received or
+	 not, and, as GDB can be built against the system readline, which
+	 could be older than the readline in GDB's repository, then we
+	 can't be sure that we can work around this prompt corruption in
+	 the gdb_rl_deprep_term_function function.
 
-	 What we want to do in this case is print "quit" after the GDB
-	 prompt, as if the user had just typed "quit" and pressed return.
+	 If we get here, RL_STATE_EOF is not defined.  This indicates that
+	 we are using an older readline, and couldn't print the quit
+	 message in gdb_rl_deprep_term_function.  So, what we do here is
+	 check to see if bracketed paste mode is on or not.  If it's on
+	 then we print a \n and then the quit, this means the user will
+	 see:
 
-	 This used to work just fine, but unfortunately, doesn't play well
-	 with readline's bracketed paste mode.  By the time we get here,
-	 readline has already sent the control sequence to leave bracketed
-	 paste mode, and this sequence ends with a '\r' character.  As a
-	 result, if bracketed paste mode is on, and we print quit here,
-	 then this will overwrite the prompt.
+	 (gdb)
+	 quit
 
-	 To work around this issue, when bracketed paste mode is enabled,
-	 we first print '\n' to move to the next line, and then print the
-	 quit.  This isn't ideal, but avoids corrupting the prompt.  */
+	 Rather than the usual:
+
+	 (gdb) quit
+
+	 Which we will get with a newer readline, but this really is the
+	 best we can do with older versions of readline.  */
       const char *value = rl_variable_value ("enable-bracketed-paste");
       if (value != nullptr && strcmp (value, "on") == 0
 	  && ((rl_readline_version >> 8) & 0xff) > 0x07)
 	printf_unfiltered ("\n");
       printf_unfiltered ("quit\n");
+#endif
+
       execute_command ("quit", 1);
     }
   else if (cmd == NULL)
diff --git a/gdb/event-top.h b/gdb/event-top.h
index 078482b79aa..722e636a110 100644
--- a/gdb/event-top.h
+++ b/gdb/event-top.h
@@ -70,6 +70,12 @@ extern void gdb_rl_callback_handler_install (const char *prompt);
    currently installed.  */
 extern void gdb_rl_callback_handler_reinstall (void);
 
+/* Called by readline after a complete line has been gathered from the
+   user, but before the line is dispatched to back to GDB.  This function
+   is a wrapper around readline's builtin rl_deprep_terminal function, and
+   handles the case where readline received EOF.  */
+extern void gdb_rl_deprep_term_function (void);
+
 typedef void (*segv_handler_t) (int);
 
 /* On construction, replaces the current thread's SIGSEGV handler with
diff --git a/gdb/top.c b/gdb/top.c
index 7831b4f96ca..d3e5391df6a 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -2222,6 +2222,7 @@ init_main (void)
   rl_completion_display_matches_hook = cli_display_match_list;
   rl_readline_name = "gdb";
   rl_terminal_name = getenv ("TERM");
+  rl_deprep_term_function = gdb_rl_deprep_term_function;
 
   /* The name for this defun comes from Bash, where it originated.
      15 is Control-o, the same binding this function has in Bash.  */
-- 
2.25.4

Re: [PATCHv4 0/3] Fix GDB prompt corruption issue

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> Changes since v3:
>
>  - New patch #1, this was inspired by the conversation here:
>    https://sourceware.org/pipermail/gdb-patches/2022-March/186996.html
>    where it was pointed out that the work around was not actually
>    needed for readline 7,
>
>  - Patches #2 and #3 are unchanged,
>
>  - Everything is rebased onto current master.

I'm going to check this in tomorrow.

Thanks,
Andrew

Re: [PATCHv4 0/3] Fix GDB prompt corruption issue

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> Andrew Burgess <aburgess@redhat.com> writes:
>
>> Changes since v3:
>>
>>  - New patch #1, this was inspired by the conversation here:
>>    https://sourceware.org/pipermail/gdb-patches/2022-March/186996.html
>>    where it was pointed out that the work around was not actually
>>    needed for readline 7,
>>
>>  - Patches #2 and #3 are unchanged,
>>
>>  - Everything is rebased onto current master.
>
> I'm going to check this in tomorrow.

I've now checked this in.

Thanks
Andrew

Re: [PATCHv4 0/3] Fix GDB prompt corruption issue

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> Andrew Burgess <aburgess@redhat.com> writes:
>
>> Andrew Burgess <aburgess@redhat.com> writes:
>>
>>> Changes since v3:
>>>
>>>  - New patch #1, this was inspired by the conversation here:
>>>    https://sourceware.org/pipermail/gdb-patches/2022-March/186996.html
>>>    where it was pointed out that the work around was not actually
>>>    needed for readline 7,
>>>
>>>  - Patches #2 and #3 are unchanged,
>>>
>>>  - Everything is rebased onto current master.
>>
>> I'm going to check this in tomorrow.
>
> I've now checked this in.

I'm aware that this series has caused a regression where an extra 'quit'
message will be printed in some situations.

I have posted this patch that should address this issue:

  https://sourceware.org/pipermail/gdb-patches/2022-April/188386.html

Thanks,
Andrew