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.129.124]) by sourceware.org (Postfix) with ESMTPS id F22233857C72 for ; Tue, 8 Mar 2022 09:46:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F22233857C72 Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-671-3F3dFUavOFq5bQaerPZRtA-1; Tue, 08 Mar 2022 04:46:01 -0500 X-MC-Unique: 3F3dFUavOFq5bQaerPZRtA-1 Received: by mail-wm1-f69.google.com with SMTP id v125-20020a1cac83000000b0037e3d70e7e1so947873wme.1 for ; Tue, 08 Mar 2022 01:46:00 -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:in-reply-to:references:date :message-id:mime-version; bh=HWcZG0gQQ7jEDfjx2xFUL7YU7z2yeDYD7AfEV8L/57U=; b=uN2Uhmhxxi8g1U1D+LPC8RDZ01+JsqsiagA7guAnafCSbNCi5PQE9NwPHYmOG5tMO5 EN3l4ydMc0TP6R5PbcFUeeg9KQBwp/Bgi2HWMsrfWTir8+dIhnB5qZTgIgCoJ2+Z9wKq +HiAvLsasm53MFXr/pr7u1YjdWuSUlw71prO+DTUdejLXznzH2mp1DKQNjqqtR6siL2S 7BUJAk/LLScV2PHh6M5cChfnYT5knabFoDbUp8KdReTWUV6UHvgtD9ykDvA5XGX3fsAD OvJg5PJryb2dFrJwfMdC47xDXyi5OLVgyf5tGq5FlR7czItaPQUSxBTpG4owX5M7FHP+ fpHQ== X-Gm-Message-State: AOAM532SGbYYxepU27RwIb9FnOYNLys4DXEV/r/JhQ2h1FYt475uaMRr 6eMOhcs+4crbc05/flD/VASHa5XkaDAx3WVOdiKS10Z2ycNsj81BicjmPYQv2s30B3pXdMgrN5r qGGN3D41dVchf+GB+3RF+/A== X-Received: by 2002:a05:6000:100c:b0:1f0:64de:422 with SMTP id a12-20020a056000100c00b001f064de0422mr11792898wrx.111.1646732759338; Tue, 08 Mar 2022 01:45:59 -0800 (PST) X-Google-Smtp-Source: ABdhPJwrGNUrHXaj4kAav5Ap3g7aHR4E0raMewmFeiK0JAtBL6pk9wSbaEQnShEVnx6CBTlgGO8olQ== X-Received: by 2002:a05:6000:100c:b0:1f0:64de:422 with SMTP id a12-20020a056000100c00b001f064de0422mr11792887wrx.111.1646732759131; Tue, 08 Mar 2022 01:45:59 -0800 (PST) Received: from localhost (host86-134-151-205.range86-134.btcentralplus.com. [86.134.151.205]) by smtp.gmail.com with ESMTPSA id a10-20020a7bc1ca000000b00389bc87db45sm1679908wmj.7.2022.03.08.01.45.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 08 Mar 2022 01:45:58 -0800 (PST) From: Andrew Burgess To: Eli Zaretskii Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/2] gdb: handle bracketed-paste-mode and ctrl-d correctly In-Reply-To: <83mti1g042.fsf@gnu.org> References: <9427f341bb8730756a98d491ffd33a3f883e1dba.1646665813.git.aburgess@redhat.com> <83mti1g042.fsf@gnu.org> Date: Tue, 08 Mar 2022 09:45:57 +0000 Message-ID: <87cziwwze2.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain 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, 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: Tue, 08 Mar 2022 09:46:07 -0000 Eli Zaretskii via Gdb-patches writes: >> Date: Mon, 7 Mar 2022 15:13:15 +0000 >> From: Andrew Burgess via Gdb-patches >> Cc: Andrew Burgess >> >> 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. > > Let me play the devil's advocate role and ask: isn't this too much of > a hassle for something that is basically an aesthetic issue, and in a > corner use case at that? We are making non-trivial surgery in > Readline, something that was never tried before, we are importing a > partial change to upstream Readline, and we do all that a minute or so > before branching for GDB 12. Are we sure this won't destabilize GDB > 12 in ways we will never be able to discover and fix before the > release? I certainly don't have strong feelings either way. I guess because I've been working on the fix for a while I don't see it as too risky. But I also recognise that I'm not impartial here. Below is the smallest fix which avoids the prompt corruption; simply moving the "quit" message to the line after the prompt, like: (gdb) quit I don't mind which fix people prefer. If the preference is for the patch below then I can port the new test from the original patch to include with this. Thanks, Andrew --- commit 5cbf93bc3f8c3871612a5c640972bd448167eeaf Author: Andrew Burgess Date: Tue Mar 8 09:37:07 2022 +0000 gdb: work around issue of prompt corruption But PR cli/28833 identifies an issue of prompt corruption. This occurs when readline is making use of bracketed paste mode. The problem is that when readline deactivates bracketed paste mode, the escape sequence used ends in "\r". For the background of why this is, see: https://lists.gnu.org/archive/html/bug-bash/2018-01/msg00097.html Normally, this isn't an issue. By the time readline deactivates bracketed paste mode the user will have sent a newline to readline, which will have moved the cursor to the next (empty) line, and so the "\r" simply returns the cursor to the start of the empty line. However, if readline receives EOF then the cursor is not moved to the next line, and so the "\r" returns the cursor to the start of the current prompt line. In GDB, when we spot the EOF in command_line_handler, we print "quit\n", so that it appears like the user entered the "quit" command. However, due to the "\r", when bracketed paste mode is being used, the quit is printed over the GDB prompt. Fixing this issue fully requires changes to readline. This commit simply changes the "quit\n" to "\nquit\n", which forces GDB to print the quit on the line after the prompt in all cases, this avoid the prompt corruption problem. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28833 diff --git a/gdb/event-top.c b/gdb/event-top.c index 8890c4fae2d..cc077c000ad 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -767,7 +767,7 @@ 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. */ - printf_unfiltered ("quit\n"); + printf_unfiltered ("\nquit\n"); execute_command ("quit", 1); } else if (cmd == NULL)