public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCHv4 3/3] gdb: handle bracketed-paste-mode and EOF correctly
Date: Tue, 29 Mar 2022 15:26:29 +0100	[thread overview]
Message-ID: <3b752f6f0ca3c5b8fcac9576a1577addbd011b1c.1648563831.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1648563831.git.aburgess@redhat.com>

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


  parent reply	other threads:[~2022-03-29 14:26 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-07 15:13 [PATCH 0/2] Fix GDB prompt corruption issue Andrew Burgess
2022-03-07 15:13 ` [PATCH 1/2] readline: back-port changes needed to properly detect EOF Andrew Burgess
2022-03-09 17:45   ` Tom Tromey
2022-03-07 15:13 ` [PATCH 2/2] gdb: handle bracketed-paste-mode and ctrl-d correctly Andrew Burgess
2022-03-07 17:10   ` Eli Zaretskii
2022-03-08  9:45     ` Andrew Burgess
2022-03-08 12:10       ` Eli Zaretskii
2022-03-14 10:23 ` [PATCHv2 0/3] Fix GDB prompt corruption issue Andrew Burgess
2022-03-14 10:23   ` [PATCHv2 1/3] gdb: work around prompt corruption caused by bracketed-paste-mode Andrew Burgess
2022-03-16 17:31     ` Tom Tromey
2022-03-16 20:45       ` Andrew Burgess
2022-03-17  8:22         ` Aktemur, Tankut Baris
2022-03-25 18:32     ` Simon Marchi
2022-03-26 14:02       ` Andrew Burgess
2022-03-27  0:59         ` Simon Marchi
2022-03-14 10:23   ` [PATCHv2 2/3] readline: back-port changes needed to properly detect EOF Andrew Burgess
2022-03-14 10:23   ` [PATCHv2 3/3] gdb: handle bracketed-paste-mode and EOF correctly Andrew Burgess
2022-03-21 15:58   ` [PATCHv3 0/2] Fix GDB prompt corruption issue Andrew Burgess
2022-03-21 15:58     ` [PATCHv3 1/2] readline: back-port changes needed to properly detect EOF Andrew Burgess
2022-03-21 15:58     ` [PATCHv3 2/2] gdb: handle bracketed-paste-mode and EOF correctly Andrew Burgess
2022-03-29 14:26     ` [PATCHv4 0/3] Fix GDB prompt corruption issue Andrew Burgess
2022-03-29 14:26       ` [PATCHv4 1/3] gdb: improved EOF handling when using readline 7 Andrew Burgess
2022-03-29 14:26       ` [PATCHv4 2/3] readline: back-port changes needed to properly detect EOF Andrew Burgess
2022-03-29 14:26       ` Andrew Burgess [this message]
2022-04-21 16:49       ` [PATCHv4 0/3] Fix GDB prompt corruption issue Andrew Burgess
2022-04-22 17:52         ` Andrew Burgess
2022-04-26 14:27           ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3b752f6f0ca3c5b8fcac9576a1577addbd011b1c.1648563831.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).