From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id qA4WENBWUGKWJwAAWB0awg (envelope-from ) for ; Fri, 08 Apr 2022 11:37:52 -0400 Received: by simark.ca (Postfix, from userid 112) id 3F8071F377; Fri, 8 Apr 2022 11:37:52 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 4EDB41ED17 for ; Fri, 8 Apr 2022 11:37:51 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E7F6F3860825 for ; Fri, 8 Apr 2022 15:37:50 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E7F6F3860825 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1649432270; bh=LlxxR71Tf8vM02sTphA0bRDdlEFhV4BcOGbD6t4d6zQ=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=eKJiaFe8v5FgffKlTLGBcjU243Fd9OxR0Ah/r//32Rcxu1tytomixZknoxs7UQO7c NwMVxlUwtv/RAfY+IoSIjo32K5n5dKDFRbRy0sfF8fJf4ZxDSuepsW3thTpoVuMD+x WSY3zP0uFYPIOFLJf82h+QIs0svqqscCWOi87aa4= Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 0AC3C383942B for ; Fri, 8 Apr 2022 15:37:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0AC3C383942B Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 34EA821603; Fri, 8 Apr 2022 15:37:00 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 1927213314; Fri, 8 Apr 2022 15:37:00 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id Wh4BBZxWUGLFXQAAMHmgww (envelope-from ); Fri, 08 Apr 2022 15:37:00 +0000 Date: Fri, 8 Apr 2022 17:36:58 +0200 To: gdb-patches@sourceware.org Subject: [RFC][gdb] Handle ^D when terminal is not prepped Message-ID: <20220408153656.GA32258@delia> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13) X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Tom de Vries via Gdb-patches Reply-To: Tom de Vries Cc: Andrew Burgess , Pedro Alves Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" 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)