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

[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.

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