From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id MISzAXEfeWKVQgUAWB0awg (envelope-from ) for ; Mon, 09 May 2022 10:04:33 -0400 Received: by simark.ca (Postfix, from userid 112) id 01F511E21F; Mon, 9 May 2022 10:04:32 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=OHPoVSTr; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (server2.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 E72751E143 for ; Mon, 9 May 2022 10:04:31 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6DC4E3948A4C for ; Mon, 9 May 2022 14:04:31 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6DC4E3948A4C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1652105071; bh=zvUqk7/V3A5y6fikZsOxFtKbrJVMbkgvoPYCdhEpg8k=; h=To:Subject:In-Reply-To:References:Date:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=OHPoVSTrYduXXpz4FHlC9C0f2AplmkwPrDVZWogERA5mZ7Meag5LfLxjBiKqub4J7 dkfOAArJzhFCYRLYs/1ZroX8pouPLir1Hy6s+cWaUY5POozfbMntF2DBKiMpPMms7P D2/MeOKGSukgcyr1vIHF71UrvxTa327zHNRfIyM0= 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 C8D4D3948831 for ; Mon, 9 May 2022 14:04:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C8D4D3948831 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-79-LBkd69RFORC2Fy_FkXt-Ig-1; Mon, 09 May 2022 10:04:10 -0400 X-MC-Unique: LBkd69RFORC2Fy_FkXt-Ig-1 Received: by mail-wm1-f71.google.com with SMTP id bi5-20020a05600c3d8500b0039489e1d18dso2225350wmb.5 for ; Mon, 09 May 2022 07:04:10 -0700 (PDT) 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=zvUqk7/V3A5y6fikZsOxFtKbrJVMbkgvoPYCdhEpg8k=; b=iSyhEy9zpJI9UmCpdHFCxVdBtgMRgP4uKikzmdnjySx6C3EqYbsnypy63+tofirATU ADWHc2IRt3+ZB77O50p5UV45RE9n8GjRQD/flAk8uO0vuLAp22rvO28z6u5RfuG3oHrQ 4DwcHu1PGZmXz1m1g8FjGV1YqdwyjevcMsYu8sMN6NNYSuy9/9hXCcVdnabHQiGzx57X foytCPvTD5AoBEonhiQvqY1ZPiitRR8jo91EG2eFmWggoKY+mkrVgGcIQYMHycHesHgq oeGjo8NMM3X9WUaP4Wwbh1UWh+TGVmIDLYZCrzC+s7+YrdFnMGpi/BkusFlmi+x8UG+J RSgA== X-Gm-Message-State: AOAM531llmILrTQRb+9IUUoLBv6keqGOxE+fW3CFSb9Bn8PBZjG1FRxU Hm5yFE0Q2QsUFvUsnHuj1xvQdO+cS4vM2w18ZBBj3jObyWHt2NJLFW/vOuv/E4B/5PKS4aO00jG nn6YqPHrcn8c+Efwom5YW4A== X-Received: by 2002:a1c:38c6:0:b0:392:a4f2:20b1 with SMTP id f189-20020a1c38c6000000b00392a4f220b1mr16299654wma.7.1652105048449; Mon, 09 May 2022 07:04:08 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyNfNF0fB5hfTqPgcdCnAPIgYmmE+qXDtXkYGtBHyH4Vd0Ir9U+q7z2hXy7RYEi3/fgnarb4g== X-Received: by 2002:a1c:38c6:0:b0:392:a4f2:20b1 with SMTP id f189-20020a1c38c6000000b00392a4f220b1mr16299604wma.7.1652105047950; Mon, 09 May 2022 07:04:07 -0700 (PDT) Received: from localhost (host81-136-113-48.range81-136.btcentralplus.com. [81.136.113.48]) by smtp.gmail.com with ESMTPSA id g24-20020a05600c4c9800b003942a244ee4sm13523009wmp.41.2022.05.09.07.04.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 May 2022 07:04:07 -0700 (PDT) To: Tom de Vries , gdb-patches@sourceware.org Subject: Re: [RFC][gdb] Handle ^D when terminal is not prepped In-Reply-To: <0e439ffb-5ba2-8b7b-0a83-55fbba1a21cc@suse.de> References: <20220408153656.GA32258@delia> <87y200q836.fsf@redhat.com> <0e439ffb-5ba2-8b7b-0a83-55fbba1a21cc@suse.de> Date: Mon, 09 May 2022 15:04:06 +0100 Message-ID: <87r152ls4p.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain 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: Andrew Burgess via Gdb-patches Reply-To: Andrew Burgess Cc: Tom Tromey , Andrew Burgess , Pedro Alves Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" Tom de Vries via Gdb-patches writes: > On 4/20/22 13:59, Andrew Burgess wrote: >> Tom de Vries via Gdb-patches writes: >> >>> 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. >> >> I looked at this a little more. If you check out tui_getc_1 (tui-io.c) >> you'll see that we sometimes return 0 to indicate "ignore this >> character", so clearly my idea above for handling 0 is not a good one. >> > > I see that indeed, I do wonder though if that in itself is correct. > > I managed to manually feed ^@ (char with ascii code 0) to a tui session > in a meaningful way: > - gdb -tui > - c-x o to put focus in command window > - type "word1" + space > - type c-2 to get ^@, to set mark at start of "word2" > - type "word2" + space + "word3" > - move cursor to start of "word3" > - type c-x c-x to swap point and mark > - watch point move from start of "word3" to start of "word2" > > So, doesn't returning 0 in tui_getc_1 set the mark, in accordance with > the active key bindings? > >> I wonder if we should, instead be handling EOF earlier, e.g. in >> readline/input.c, maybe in here we should spot that we read '\0' and >> convert this to EOF? But then what if the user (for some reason) >> legitimately wanted to send \0 to GDB... >> > > Agreed, and I think the scenario mentioned above is an example. > >> So then I start wondering if this is an artefact of switching between >> canonical and non-canonical mode? The EOF arrives in canonical mode, >> which doesn't add \004 to the terminal buffer, then we switch to >> non-canonical mode to read, and by this point its too late. I do wonder >> if the kernel should actually be adding the \004 character to the >> pending characters buffer when we switch between modes... >> > > Apropos, the mode change is done using tcsetattr and it takes an > argument that decides what to do with pending i/o, but that seems more > output oriented, so AFAIU the only input thingy we have available there > is TCSAFLUSH which would discard pending input. > > But regardless, I suspect the problem will be same if the ^D arrives > after switching to non-canonical mode. I think the add-usleep scenario > exposes that. > >> Anyway, having thought about that for a while I did wonder if we really >> need to fix this at all. I mean, right now, the behaviour of GDB is >> that EOF sent to GDB while we're _not_ at a prompt will basically be >> ignored > > Well, my theory is that it's not ignored, but sets the mark, which > mostly looks like it's ignored, unless you do c-x c-x after which you > may find point somewhere in the prompt, I'm guessing. > >>, after your patch the EOF will be acted on once we get back to a >> prompt. Is that a desirable change? >> > > I'd say this is roughly how I expect interactive programs to behave, > yes. I type keystrokes that are processed, if it's busy it'll buffer > those keystrokes and get back to processing them when it stops being > busy. I don't expect the program to suddenly start to process the > keystrokes differently, just because it's busy for a short while. > > So I guess my argument is that the change makes the behaviour more > consistent for the user. > >> So, I wondered if there was a way we could just "fix" the test, that is, >> ensure the Ctrl-D is only sent once the prompt has been displayed and >> readline has put the terminal back into non-canonical mode. Turns out >> that's pretty easy (see the patch below). >> >> With your suggested usleep, I see the eof-exit.exp test fail reliably >> without the patch below, and pass reliably with the patch below. What >> are are your thoughts? >> > > I think that fixing the test-case is a good idea, since we now know > about this issue (and a PR is filed to keep track of it), and there's no > reason to keep this occasional fail around. If we want to change gdb > behaviour, we can just add a test-case. I agree. > > As mentioned before, I'm not confident that this patch is a good > solution, or even that I understand the problem entirely (or that this > is a problem rather than a feature). > > Anyway, test-case patch LGTM. Thanks. I pushed the patch below to fix the test failures you're seeing. Thanks, Andrew --- commit 205d0542821c91e04e0ed174d8862f486a076950 Author: Andrew Burgess Date: Wed Apr 20 14:08:49 2022 +0100 gdb: fix for gdb.base/eof-exit.exp test failures This fix relates to PR gdb/29032, this makes the test more stable by ensuring that the Ctrl-D is only sent once the prompt has been displayed. This issue was also discussed on the mailing list here: https://sourceware.org/pipermail/gdb-patches/2022-April/187670.html The problem identified in the bug report is that sometimes the Ctrl-D (that the test sends to GDB) arrives while GDB is processing a command. When this happens the Ctrl-D is handled differently than if the Ctrl-D is sent while GDB is waiting for input at a prompt. The original intent of the test was that the Ctrl-D be sent while GDB was waiting at a prompt, and that is the case the occurs most often, but, when the Ctrl-D arrives during command processing, then GDB will ignore the Ctrl-D, and the test will fail. This commit ensures the Ctrl-D is always sent while GDB is waiting at a prompt, which makes this test stable. But, that still leaves an open question, what should happen when the Ctrl-D arrives while GDB is processing a command? This commit doesn't attempt to answer that question, which is while bug PR gdb/29032 will not be closed once this commit is merged. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29032 diff --git a/gdb/testsuite/gdb.base/eof-exit.exp b/gdb/testsuite/gdb.base/eof-exit.exp index ad5f33d2f10..e04c38bf8d7 100644 --- a/gdb/testsuite/gdb.base/eof-exit.exp +++ b/gdb/testsuite/gdb.base/eof-exit.exp @@ -25,9 +25,27 @@ proc run_test {} { # # Send a newline character, which will cause GDB to redisplay the # prompt. + # + # We then consume the newline characters, and then make use of + # expect's -notransfer option to ensure that the prompt has been + # displayed, but to leave the prompt in expect's internal buffer. + # This is important as the following test wants to check how GDB + # displays the 'quit' message relative to the prompt, this is much + # easier to do if the prompt is still in expect's buffers. + # + # The other special thing we do here is avoid printing a 'PASS' + # result. The reason for this is so that the GDB output in the + # log file will match what a user should see, this makes it much + # easier to debug issues. Obviously we could print a 'PASS' here + # as the text printed by expect is not considered part of GDB's + # output, so the pattern matching will work just fine... but, the + # log file becomes much harder to read. send_gdb "\n" gdb_test_multiple "" "discard newline" { -re "^\r\n" { + exp_continue + } + -notransfer -re "^\[^\n\]*$::gdb_prompt $" { } }