From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 2uq3Naf1X2I2ZQAAWB0awg (envelope-from ) for ; Wed, 20 Apr 2022 07:59:35 -0400 Received: by simark.ca (Postfix, from userid 112) id C80AB1E004; Wed, 20 Apr 2022 07:59:35 -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=Ubyx6I0i; 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=-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.6 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 1DE1C1E002 for ; Wed, 20 Apr 2022 07:59:34 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6F513385782D for ; Wed, 20 Apr 2022 11:59:33 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6F513385782D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1650455973; bh=d10g3gTN2khbtgs2oEcPZklkY+j0qSVclIKfZcJLBdg=; 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=Ubyx6I0iNKlXxuLh5EkK4mua39CK+SiMfUdiU8uIdBhhkRXbEe77CHU5YsS7K9GyP xhJ+bOhYUEa3qBEJEzss/17OVsGwZwqvoDIgeyyIa6+2EXevolpWRRmFaV/u9BufPO QY8g4uAivSJVlx+QjhggVlFrmB+IcKCZ/SGOwqpM= 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 DADFF3858C53 for ; Wed, 20 Apr 2022 11:59:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DADFF3858C53 Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-218-DLX0k_RcP7WDPwCENDX9mA-1; Wed, 20 Apr 2022 07:59:12 -0400 X-MC-Unique: DLX0k_RcP7WDPwCENDX9mA-1 Received: by mail-wm1-f70.google.com with SMTP id p31-20020a05600c1d9f00b0038ed0964a90so866885wms.4 for ; Wed, 20 Apr 2022 04:59:12 -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=d10g3gTN2khbtgs2oEcPZklkY+j0qSVclIKfZcJLBdg=; b=a4/sW6GkjFMMMPJhB02gUIy6TbiBbKa58x72GYBixtrSxzYEiyfw3a4fJfYSs72qp8 yf12KQQjJ7CVSldYTXK5hcUSw4epIwf3W7lyeFWp6yFXbjP8XJadQ7XW+92ifym/+oBR cTg0IqRkZjSr/5mr7z1TtP/WcpkngJgK8fK2U32GaKCdT3WfmdE5eByMgdrhGHIelKD9 PCy631sMSpGXHnsfxofaPOIwn3GayO3Y94udXMCfNDSWlMVEGgNNgIR6IeXrXm+a/vcf MtB8M4MLlB5Uq4iwR3vu/54fWum4o3V7SYcEjXKNWCPhw7zoeqT8doXAjkaaVG96ybA+ 0g+A== X-Gm-Message-State: AOAM530wzry+d82hlQeW2203rOJepkuT2rWUh4+0lVZiRjxYl90zHwVq jFXQXgFs8OQYW6jeNg3atLc6UA6xIM53QuLXnw/fswkfueO20s40w6/dijXgMgxNX7tSg8vE+7A Bayrq3Dx23UYsxmuHaZ7B6A== X-Received: by 2002:a05:600c:3494:b0:390:8a95:1b95 with SMTP id a20-20020a05600c349400b003908a951b95mr3285487wmq.15.1650455950759; Wed, 20 Apr 2022 04:59:10 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwj/YvMxfTre3HA8zAc5l0e3eFGn4NJ33k0YgC0Wd6LqhJSCs7LAUaK1cs+ZxEybtXMLB6BKA== X-Received: by 2002:a05:600c:3494:b0:390:8a95:1b95 with SMTP id a20-20020a05600c349400b003908a951b95mr3285464wmq.15.1650455950442; Wed, 20 Apr 2022 04:59:10 -0700 (PDT) Received: from localhost (host81-136-113-48.range81-136.btcentralplus.com. [81.136.113.48]) by smtp.gmail.com with ESMTPSA id y11-20020a056000168b00b0020a919422ccsm9692186wrd.109.2022.04.20.04.59.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Apr 2022 04:59:10 -0700 (PDT) To: Tom de Vries , gdb-patches@sourceware.org Subject: Re: [RFC][gdb] Handle ^D when terminal is not prepped In-Reply-To: <20220408153656.GA32258@delia> References: <20220408153656.GA32258@delia> Date: Wed, 20 Apr 2022 12:59:09 +0100 Message-ID: <87y200q836.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: 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: > 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 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... 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... 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, after your patch the EOF will be acted on once we get back to a prompt. Is that a desirable change? 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? Thanks, Andrew --- diff --git a/gdb/testsuite/gdb.base/eof-exit.exp b/gdb/testsuite/gdb.base/eof-exit.exp index 2d9530ccebe..c88aced9f35 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 $" { } }