From patchwork Wed Oct 30 15:53:35 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 99829 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 84749385782C for ; Wed, 30 Oct 2024 15:54:24 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id B86133857C5D for ; Wed, 30 Oct 2024 15:53:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B86133857C5D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org B86133857C5D Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730303636; cv=none; b=G8pRVF66//lzDs5dXLT/4dHDy2XbXsN3Gtfunqbrg+VwyhlJ15HpqWGMZ7eAd8m54RFGoasFh3InnMt392D0nga5L4S09FuW23undHPy3du2qBaKl/8deNJ4AZA6QgcbdZrUj/V35GXmUX1VrZfBDao3KmrYBbgxR1EWYMysmlk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730303636; c=relaxed/simple; bh=NdY3WXb54I4YR6T8OF9Nykrz/hYDp/9kCCw5PYArNko=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=f/FZKm7HIQd4WdiXom7OvAT53n3mGlIN02NOECy84Q4/wOuAzxTZNKI2x4I3uZif2qhsITwIvI1J8Lpo9h5vik3t5Oj5N1L2F/FxJcR9BsK3osCbbqzEPvsx6Ov75/wXh8/nIQt9cs5trlO3oxYxex1gALy0xyljTr3wHaPpq4s= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1730303627; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=aOxFpEA7iX+N0JwvjZfYXKlj7WNfD+pl0sVLsupdGZw=; b=ec29XMfEpN+r6PnrF48NQkx6Z5Z2TMrjYfWfBpjv1A/6Qe9Z2k/T4M5LGwE+jIYLqOSnph PVspxHw4Dwm4bKxkHu5wxPD8z2z1ov+cKGzzXW1rceFl3ca6ZfJOx03EMQ9+dSa7m02Pj3 YjVkkTVZEEoqegRm2Nl2If3VC/jsLAk= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-417-M9Q87pWGPwO-YxKnqNSKCQ-1; Wed, 30 Oct 2024 11:53:43 -0400 X-MC-Unique: M9Q87pWGPwO-YxKnqNSKCQ-1 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-37d563a1af4so13231f8f.2 for ; Wed, 30 Oct 2024 08:53:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730303622; x=1730908422; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=aOxFpEA7iX+N0JwvjZfYXKlj7WNfD+pl0sVLsupdGZw=; b=s1VGudRFqGzWi/3yc/oJ9VteYEQH1OzWGIVmB5iZCYZyHWMI4birE/v/Q5TU/e8iDf kTh/yNtfzXrQVPQz/qlOQRdIytQoEKJvst3jn5HnaB6m9o35YPm8YHlotKA1hdvZGbOq RFs+ymooH8arr7OjwUofJRlsbmg7s4eKZQipsg3Rm9jpz+RGfdKK9dUH8JReVtf3THT0 hnDU00Nnpo6BGLuUV05LN+aB+x9uvDpifVKL3CVpHRgP9NT6IYrwXmK7kIP06Mm2dh6M +xavjd1wx8+Ex2Rlh3/3Na/03zu2HenACgkixqy7YFaEuiFaIzqGzEY6z8ZvxWJF2ILK Tl2Q== X-Gm-Message-State: AOJu0Yw+y0mwRmflJ114uz3zkzfXN26aNvNtb+md/z1ErfvSB2xxrkCp OdoF9iPEQNtjn0YlJojwERxFa1dY7oWOHeX4EbwMFfohcBC/gwY0KodU8f4W+5yjsWJixE6A/b6 1hkBsKJU/GBVazh7QYiHK7EKJww+c+UyEs8amo+WFR4xQJxFiaZt4Kx99H3OF6cV/ITy7nROGIT oA59KsrjjIdtzYt5tdjaBte0MdpYyov90WUjQhG5p8Abk= X-Received: by 2002:adf:cf0a:0:b0:37d:3e8c:f6fa with SMTP id ffacd0b85a97d-38061159c5emr12642926f8f.31.1730303621915; Wed, 30 Oct 2024 08:53:41 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG+32QwDS8rQC/e+uZ8O9afAv4eITTz2W+l9HjYXiKgnEZBmvzlN9kJYm+I6phQYf+yA549XA== X-Received: by 2002:adf:cf0a:0:b0:37d:3e8c:f6fa with SMTP id ffacd0b85a97d-38061159c5emr12642889f8f.31.1730303621213; Wed, 30 Oct 2024 08:53:41 -0700 (PDT) Received: from localhost (197.209.200.146.dyn.plus.net. [146.200.209.197]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38058b1c4d8sm15762977f8f.2.2024.10.30.08.53.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Oct 2024 08:53:39 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 2/2] gdb/readline: don't get stuck thinking an EOF arrived Date: Wed, 30 Oct 2024 15:53:35 +0000 Message-Id: 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 X-Spam-Status: No, score=-11.9 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_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~patchwork=sourceware.org@sourceware.org It was brought to my attention[1] that if a user makes use of Ctrl+d to quit from a secondary prompt (e.g. the prompt used to enter lines for the 'commands' command) then GDB will start displaying some unexpected blank lines. Here's an example: Reading symbols from /tmp/hello.x... (gdb) break main Breakpoint 1 at 0x401198: file hello.c, line 18. (gdb) commands Type commands for breakpoint(s) 1, one per line. End with a line saying just "end". >quit # <----------- Use Ctrl+d to quit here. (gdb) show architecture # <----------- This blank line is unexpected. The target architecture is set to "auto" (currently "i386:x86-64"). (gdb) I've marked up where I press 'Ctrl+d', and also the unexpected blank line. This issue will only happen if bracketed-paste-mode is in use. If this has been disabled (e.g. in ~/.inputrc) then this issue will not occur. The blank line is not just emitted for the 'show architecture' command. The blank line is actually caused by an extra '\n' character emitted by readline after it has gathered a complete line of input, and so will occur for any command. The problem is caused by readline getting "stuck" in a state where it thinks that an EOF has just been delivered. This state is set when the 'Ctrl+d' does deliver an EOF, but then this state is never fully reset. As a result, every time readline completes a line, it thinks that the line was completed due to an EOF and so adds an extra '\n' character. Obviously the real fix for this issue is to patch readline, and I do have a patch for that[2], however, version 8.2 of readline has been released, and contains this issue. As such, if GDB is linked against the system readline, and that system readline is 8.2, then we can expect to see this issue. There's a pretty simple, and cheap workaround that we can add to GDB that will mitigate this issue. I propose that we add this workaround to GDB. If/when the readline patch is accepted then I'll back-port this to our local readline copy, but retaining the workaround will be harmless, and will make GDB play nicer with system readline libraries (version 8.2). [1] https://inbox.sourceware.org/gdb-patches/34ef5438-8644-44cd-8537-5068e0e5e434@redhat.com [2] https://lists.gnu.org/archive/html/bug-readline/2024-10/msg00014.html Reviewed-by: Keith Seitz --- gdb/event-top.c | 17 +++ .../gdb.base/readline-commands-eof.c | 22 +++ .../gdb.base/readline-commands-eof.exp | 128 ++++++++++++++++++ gdb/testsuite/lib/gdb.exp | 28 ++++ 4 files changed, 195 insertions(+) create mode 100644 gdb/testsuite/gdb.base/readline-commands-eof.c create mode 100644 gdb/testsuite/gdb.base/readline-commands-eof.exp diff --git a/gdb/event-top.c b/gdb/event-top.c index 0d46dec1774..adc17508c64 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -387,6 +387,23 @@ gdb_rl_callback_handler_install (const char *prompt) therefore loses input. */ gdb_assert (!callback_handler_installed); +#ifdef RL_STATE_EOF + /* Some versions of readline contain a but where the rl_eof_found flag + would not be reset back to 0 in rl_initialize, despite the + RL_STATE_EOF flag being cleared in this function. + + The consequence of this mistake is that readline will appear to get + stuck in the EOF state, and will emit an extra '\n' character each + time an input line is completed. + + Work around this by clearing the EOF state now ourselves. */ + if (RL_ISSTATE (RL_STATE_EOF)) + { + RL_UNSETSTATE (RL_STATE_EOF); + rl_eof_found = 0; + } +#endif /* RL_STATE_EOF */ + rl_callback_handler_install (prompt, gdb_rl_callback_handler); callback_handler_installed = true; } diff --git a/gdb/testsuite/gdb.base/readline-commands-eof.c b/gdb/testsuite/gdb.base/readline-commands-eof.c new file mode 100644 index 00000000000..4061e6a15c1 --- /dev/null +++ b/gdb/testsuite/gdb.base/readline-commands-eof.c @@ -0,0 +1,22 @@ +/* This test program is part of GDB, the GNU debugger. + + Copyright 2024 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 . */ + +int +main (void) +{ + return 0; +} diff --git a/gdb/testsuite/gdb.base/readline-commands-eof.exp b/gdb/testsuite/gdb.base/readline-commands-eof.exp new file mode 100644 index 00000000000..c1efbbfe123 --- /dev/null +++ b/gdb/testsuite/gdb.base/readline-commands-eof.exp @@ -0,0 +1,128 @@ +# Copyright 2024 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 . + +# If a user uses 'Ctrl+d' to exit from a secondary prompt, then +# readline can get stuck thinking that an EOF has arrived. As a +# consequence of this readline will output an extra newline everytime +# that it exits bracketed-paste-mode (which is done after every line +# of input). The result is the user will see some unexpected blank +# lines in the output. + +standard_testfile + +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } { + return -1 +} + +# The fix for this issue relies on GDB being able to adjust the EOF +# flag state within readline. Access to this state was added for +# readline 8.2, but was also backported to out internal readline. If +# this feature is not available then this test might not pass. +if { ![readline_supports_eof_flag] } { + unsupported "readline is not eof flag aware" + return -1 +} + +# Create a breakpoint then issue the 'commands' commands. When the +# secondary prompt is displayed, use Ctrl+d to send EOF to readline +# and cancel the input. +# +# Then check that readline is not stuck thinking that an EOF has +# arrived. If it is then GDB will start displaying extra blank lines +# after each line of input. +proc run_test {} { + clean_restart $::binfile + + gdb_breakpoint main + + # Issue the 'commands' command, and wait for the secondary prompt + # to be displayed. + gdb_test_multiple "commands" "start b/p commands" { + -re "Type commands for breakpoint\\(s\\) 1, one per line\\.\r\n" { + exp_continue + } + -re "^End with a line saying just \"end\"\\.\r\n" { + exp_continue + } + -re "^\[^\r\n\]*>$" { + pass $gdb_test_name + } + } + + # Send Ctrl+d to GDB and wait for the 'quit' message, and then for + # the GDB prompt to be displayed. + # + # As this test runs (sometimes) with bracketed-paste-mode on then + # we need to accept a control sequence before the prompt. This + # control sequence can contain '\r', which is why we only check + # for '\n' here, which is different than what we do in the rest of + # the testsuite, where we usually check for '\r\n' together. + send_gdb "\004" + gdb_test_multiple "" "quit b/p commands" { + -re "^quit\r\n\[^\n\]*$::gdb_prompt $" { + pass $gdb_test_name + } + } + + # Now issue any other command. If readline is stuck in EOF mode + # (thinking that an EOF has just arrived), then we'll see an extra + # blank line afer the command, and before any command output. + # + # As described above we scan for '\n' only in some patterns here + # as we're allowing for a control sequence that might include + # '\r'. + gdb_test_multiple "show architecture" "check for excessive blank lines" { + -re "^show architecture\r\n" { + exp_continue + } + -re "^\[^\n\]*The target architecture is set to \[^\r\n\]+\\.\r\n\[^\n\]*$::gdb_prompt $" { + pass $gdb_test_name + } + -re "^\[^\n\]*\nThe target architecture is set to \[^\r\n\]+\\.\r\n\[^\n\]*$::gdb_prompt" { + fail $gdb_test_name + } + } +} + +# Run the test in various different terminal modes. +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 a438a101fc9..c16aac9845a 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -3740,6 +3740,34 @@ proc readline_is_used { } { } } +# Return true if readline has support for the EOF flag. + +proc readline_supports_eof_flag { } { + gdb_test_multiple "maint info readline" "" { + -re -wrap "^version: ($::decimal)\\.($::decimal)\\s+\\((internal|system)\\)" { + set major $expect_out(1,string) + set minor $expect_out(2,string) + set type $expect_out(3,string) + + # The internal readline was patched with EOF support ahead + # of this landing in upstream readline. + if { $type eq "internal" } { + return true + } + + # The EOF flag support was added in readline 8.2. + if { $major > 8 || $major == 8 && $minor >= 2 } { + return true + } + + return false + } + -re ".*$::gdb_prompt $" { + return false + } + } +} + # Return 1 if target is ELF. gdb_caching_proc is_elf_target {} { set me "is_elf_target"