public inbox for gdb-cvs@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] gdb: work around prompt corruption caused by bracketed-paste-mode
@ 2022-03-16 20:44 Andrew Burgess
  0 siblings, 0 replies; only message in thread
From: Andrew Burgess @ 2022-03-16 20:44 UTC (permalink / raw)
  To: gdb-cvs

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a6b413d24ccc5d76179bab866834e11fd6fec294

commit a6b413d24ccc5d76179bab866834e11fd6fec294
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Fri Mar 11 14:44:03 2022 +0000

    gdb: work around prompt corruption caused by bracketed-paste-mode
    
    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

Diff:
---
 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(-)

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


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-03-16 20:44 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-16 20:44 [binutils-gdb] gdb: work around prompt corruption caused by bracketed-paste-mode Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).