From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 8B4583858439 for ; Mon, 7 Mar 2022 15:13:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8B4583858439 Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-610-rEIc9KIxM4GbO2PsGRXs_g-1; Mon, 07 Mar 2022 10:13:24 -0500 X-MC-Unique: rEIc9KIxM4GbO2PsGRXs_g-1 Received: by mail-wm1-f71.google.com with SMTP id l13-20020a7bcf0d000000b0038982c6bf8fso4824850wmg.7 for ; Mon, 07 Mar 2022 07:13:24 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=PsbcEkZ4l5gHetMq56E9+mJWCQ05rDCfi4vrUeLloXc=; b=yN979i2Yzplp9oq4GLNxsjoh5ITYcL6l5H6d2aynFjwW6OwjWKQsaw5HCNKwhu8Otc JAehNPAOWZilFFbLUGCi6L+R+NnnU4ldoBzG8PVDITdDfYeRymDn9P1yDuX/ZvxCFyzs RsbgwmdVzeIjg386EhYq+UlY6kjRO0PG1Y1wjRzrks1e1WQJW79I7hhZhygpuaXBRxwg p/AhLjeYl3l7O/cprYqxip5CdHe/odFmCjYaAqV4z+pVR+M32xt5GtuulGdcInUsqaGI oF+tOJp7wKTQNmSJdAHGt941iKNT5FVeWjtOK3i/W9K7Qch8Mdxm+acVq3MXuZAZpY7E AcHg== X-Gm-Message-State: AOAM530VvGwSNsFKjLsfk5ODNMfSCcqpzNjmnxdPZWNCRvfGfsf1oPDY 9YZFcSC8cpwKfHqkoSF13eP+aamjYPeUut7yjNxhvZRg6u9T82mVCc+nnl4mMNUAvR3stNkTLTV /QWyhfg7KEOb66za60+LlwRqGOJOYsbjNYs/jpXagdbXpqp3/hy6JPUh+GC1f3UChNnHffBMOcQ == X-Received: by 2002:a05:600c:3391:b0:389:9c6e:c248 with SMTP id o17-20020a05600c339100b003899c6ec248mr7872905wmp.97.1646666002819; Mon, 07 Mar 2022 07:13:22 -0800 (PST) X-Google-Smtp-Source: ABdhPJweRaagNynM8vE37I2R/EhNb1muwv3ofLm6yNhd7YzNa+PGI1icehOqcRiVukIrR09cBb5jtg== X-Received: by 2002:a05:600c:3391:b0:389:9c6e:c248 with SMTP id o17-20020a05600c339100b003899c6ec248mr7872870wmp.97.1646666002328; Mon, 07 Mar 2022 07:13:22 -0800 (PST) Received: from localhost (host86-134-151-205.range86-134.btcentralplus.com. [86.134.151.205]) by smtp.gmail.com with ESMTPSA id r16-20020a05600c35d000b00389a826abd3sm2592547wmq.42.2022.03.07.07.13.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Mar 2022 07:13:21 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 2/2] gdb: handle bracketed-paste-mode and ctrl-d correctly Date: Mon, 7 Mar 2022 15:13:15 +0000 Message-Id: <9427f341bb8730756a98d491ffd33a3f883e1dba.1646665813.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org 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: , X-List-Received-Date: Mon, 07 Mar 2022 15:13:30 -0000 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> 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 &&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 . + +# 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