[RFC][gdb] Handle ^D when terminal is not prepped

Tom de Vries tdevries@suse.de
Fri Apr 8 15:36:58 GMT 2022


Hi,

When running test-case gdb.base/eof-exit.exp, I get:
...
(gdb) ^M
(gdb) quit^M
PASS: gdb.base/eof-exit.exp: with non-dump terminal: close GDB with eof
...
but when run in combination with taskset -c 0, I get instead:
...
(gdb) ^M
(gdb) FAIL: gdb.base/eof-exit.exp: default: close GDB with eof (timeout)
...

The test-case is a bit unusual, in the sense that most test-cases wait for
gdb to present the prompt '(gdb) ' before sending input.  Instead, this
test-case first sends a newline to the prompt:
...
    send_gdb "\n"
...
to generate a new prompt, but rather than waiting for the new prompt, it just
waits for the resulting newline:
...
    gdb_test_multiple "" "discard newline" {
        -re "^\r\n" {
        }
    }
...
and then sends the End-of-Transmission character (ascii code 4, caret notation
'^D'):
...
    send_gdb "\004"
...

The purpose of this is to verify that the result is:
...
(gdb) quit
...
instead of:
...
quit)
...

Putting this together with the failure log above, it seems like the ^D has no
observable effect.

After adding some debugging code in readline, to verify what chars are read
from stdin (call to read in rl_getc), we find that in the passing case we get
'^D', but in the failing case '^@' (ascii code 0, '\0') instead.

Readline treats the '^D' as EOF, and calls gdb's installed handler with NULL
(meaning EOF), which then issues the quit.

But readline does not invoke gdb's installed handler for the '^@', AFAIU
because as per default keymap it treats it as the 'set mark' function.

So, why does ^D end up as ^@?

My theory is that this is due to "canonical mode" (
"https://man7.org/linux/man-pages/man3/tcflow.3.html" ):
...
Input is made available line by line.  An input line is
available when one of the line delimiters is typed (NL, EOL,
EOL2; or EOF at the start of line).  Except in the case of EOF,
the line delimiter is included in the buffer returned by read(2).
...

So, if the line is empty to start out with, and then ^D is issued and
interpreted by the terminal as EOF, it'll make available an empty line, in
other words a pointer to char ^@.

The canonical mode seems to be on by default, but is switched off when
readline puts the terminal in "prepped" mode.

Gdb switches forth and back between having the readline handler installed and
not (which makes readline switch back and forth between "prepped" and
"deprepped" mode), and even readline itself seems to switch back and forth
internally, in rl_callback_read_char.  So, apparantly the '^D' arrives at a
moment when the terminal happens to be unprepped.

Indeed, adding a 'usleep (100 * 1000)' at the end of rl_deprep_terminal, makes
it more likely to catch the terminal in unprepped mode, and triggers the FAIL
without taskset.

At this point, it's good to point out that I have no idea whether this
behaviour is as expected, or should be considered a bug, and if so whether the
bug is in gdb, readline, or the terminal emulator.

Either way, I suppose it would be nice if we treat the ^D the same regardless.

This patch has a way of achieving this, by setting the terminal to
non-canonical mode before initializing readline, such that both the prepped
and deprepped mode keep the terminal in non-canonical mode.

But terminal settings are sticky so they also need to be undone.

The patch adds functions termios_enable_readline and termios_disable_readline,
which are called at points found necessary using trial-and-error.

Tested on x86_64-linux, both with and without taskset -c 0.

I have an open question whether it would be necessary or a good idea to change
more that just the canonical mode.

Andrew also noted that a potential readline fix / workaround could be:
...
@@ -630,7 +630,7 @@ readline_internal_charloop (void)
 	 previous character is interpreted as EOF.  This doesn't work when
 	 READLINE_CALLBACKS is defined, so hitting a series of ^Ds will
 	 erase all the chars on the line and then return EOF. */
-      if (((c == _rl_eof_char && lastc != c) || c == EOF) && rl_end == 0)
+      if (((c == _rl_eof_char && lastc != c) || c == EOF || c == 0) && rl_end == 0)
 	{
 #if defined (READLINE_CALLBACKS)
 	  RL_SETSTATE(RL_STATE_DONE);
...
which indeed fixes the test-case, but broader testing runs into trouble in
gdb.tui, so that might need more work, but could of course be trivial to fix.

Also, the patch assumes that canonical mode is on at gdb start, which might be
incorrect, so perhaps we need to cater for that possibility as well.

Any comments?

Thanks,
- Tom

[gdb] Handle ^D when terminal is not prepped

---
 gdb/event-top.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index b628f0397cb..05e3a4d9039 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -43,6 +43,7 @@
 #include "async-event.h"
 #include "bt-utils.h"
 #include "pager.h"
+#include "termios.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
@@ -51,6 +52,38 @@
 /* readline defines this.  */
 #undef savestring
 
+static void
+update_termios_c_lflag (FILE *stream, tcflag_t flag, const char *flag_name,
+			bool set_p)
+{
+  struct termios termios;
+  tcgetattr (fileno (stream), &termios);
+
+  if (set_p)
+    termios.c_lflag |= flag;
+  else
+    termios.c_lflag &= ~(flag);
+
+  tcsetattr (fileno (stream), TCSANOW, &termios);
+
+  if (0)
+    fprintf (stderr, "%s %s on fileno %d\n",
+	     set_p ? "Setting" : "Clearing", flag_name,
+	     fileno (stream));
+}
+
+static void
+termios_enable_readline (FILE * stream)
+{
+  update_termios_c_lflag (stream, ICANON, "ICANON", false);
+}
+
+static void
+termios_disable_readline (FILE * stream)
+{
+  update_termios_c_lflag (stream, ICANON, "ICANON", true);
+}
+
 static std::string top_level_prompt ();
 
 /* Signal handlers.  */
@@ -280,6 +313,8 @@ change_line_handler (int editing)
 
       /* Turn on editing by using readline.  */
       ui->call_readline = gdb_rl_callback_read_char_wrapper;
+
+      termios_enable_readline (ui->instream);
     }
   else
     {
@@ -287,6 +322,8 @@ change_line_handler (int editing)
       if (ui->command_editing)
 	gdb_rl_callback_handler_remove ();
       ui->call_readline = gdb_readline_no_editing_callback;
+
+      termios_disable_readline (ui->instream);
     }
   ui->command_editing = editing;
 }
@@ -1312,6 +1349,8 @@ gdb_setup_readline (int editing)
      one instance of readline.  */
   if (ISATTY (ui->instream) && editing && ui == main_ui)
     {
+      termios_enable_readline (ui->instream);
+
       /* Tell gdb that we will be using the readline library.  This
 	 could be overwritten by a command in .gdbinit like 'set
 	 editing on' or 'off'.  */
@@ -1362,6 +1401,8 @@ gdb_disable_readline (void)
   if (ui->command_editing)
     gdb_rl_callback_handler_remove ();
   delete_file_handler (ui->input_fd);
+
+  termios_disable_readline (ui->instream);
 }
 
 scoped_segv_handler_restore::scoped_segv_handler_restore (segv_handler_t new_handler)


More information about the Gdb-patches mailing list