From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 0ED223858D20 for ; Wed, 31 May 2023 15:49:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0ED223858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id B03B21FD84; Wed, 31 May 2023 15:49:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1685548157; h=from:from:reply-to: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=If9bigbHV9zpClyVLOyaCIVNWfM5M8LM9/z+MgIEzoU=; b=dCqoCs/UbQgiZ3jNwbYuVkMdkBVbVdV+T+vXyNkz1yeh7d4e8yTOhbx1srTMcfxgG1ogFN qnw7jD3tzhRrDhDMfwsSKjE5qyJGof/WviIk6bTe95n+L6W65DuXu2OAgTBIBIxI/ysm+W PnFA4lnSsAv+MDCO3n/6KtbcGaWwesg= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1685548157; h=from:from:reply-to: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=If9bigbHV9zpClyVLOyaCIVNWfM5M8LM9/z+MgIEzoU=; b=AFKDRONNofQbkjyIaKhS5DV/taWytzggfJ+3VKSlYTrIfgecRU82SDX2kOO3CWXAqLKu+d /PXy214Vuajjd1Ag== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 96F39138E8; Wed, 31 May 2023 15:49:17 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id Sx6ZI31sd2THVAAAMHmgww (envelope-from ); Wed, 31 May 2023 15:49:17 +0000 Message-ID: <564b8e8c-dd01-575f-f59d-d599673af54b@suse.de> Date: Wed, 31 May 2023 17:49:18 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH] [gdb/testsuite] Fix gdb.tui/wrap-line.exp Content-Language: en-US To: Andrew Burgess , gdb-patches@sourceware.org Cc: Simon Marchi References: <20230518061046.17837-1-tdevries@suse.de> <87ilcm83tt.fsf@redhat.com> <87bki253w9.fsf@redhat.com> From: Tom de Vries In-Reply-To: <87bki253w9.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 5/30/23 15:45, Andrew Burgess wrote: > Tom de Vries writes: > >> On 5/21/23 10:51, Andrew Burgess wrote: >>> Tom de Vries via Gdb-patches writes: >>> >>>> PR testsuite/30458 reports the following FAIL: >>>> ... >>>> PASS: gdb.tui/wrap-line.exp: width-auto-detected: cli: wrap >>>> ^CQuit >>>> (gdb) WARNING: timeout in accept_gdb_output >>>> Screen Dump (size 50 columns x 24 rows, cursor at column 6, row 3): >>>> 0 Quit >>>> 1 (gdb) 7890123456789012345678901234567890123456789 >>>> 2 W^CQuit >>>> 3 (gdb) >>>> ... >>>> FAIL: gdb.tui/wrap-line.exp: width-auto-detected: cli: prompt after wrap >>>> ... >>>> >>>> The problem is that the regexp doesn't account for the ^C: >>>> ... >>>> gdb_assert { [Term::wait_for "^WQuit"] } "prompt after wrap" >>>> ... >>>> >>>> Fix this by updating the regexp, and likewise in another place in the >>>> test-case where we use ^C. >>> >>> Do we know why we sometimes manage to see '^C'? I guess it's a timing >>> thing, but right now I'm at a loss for how we manage to see it. It >>> appears that we print the wrapping line, that ends with 'W', and then >>> wait for this to be displayed. >>> >>> At this point GDB should be in a stable state waiting in the >>> event-loop. >>> >>> When we send \003 this should trigger an event, which should trigger >>> async_request_quit, which should result in the 'Quit' exception being >>> thrown, caught, and printed. >>> >>> I think the '^C' must be getting printed from tui_redisplay_readline >>> maybe, but I can't see how that gets triggered with \003 in the input >>> buffer. >>> >>> I only ask because if we understand why '^C' is sometimes printed then >>> we might be able to decide if this should always be printed, or never be >>> printed, and change GDB accordingly... >>> >> >> Hi Andrew, >> >> yes, that's a good question. >> >> [ Note that it's a TUI test-case, but the FAIL we're observing is in the >> cli part, before activating TUI, so tui_redisplay_readline has nothing >> to do with the FAIL. ] >> >> I've added an assert in rl_echo_signal_char and managed to trigger it to >> generate a core file corresponding to the FAIL condition (more details >> in the PR). >> >> My guess at what happens is the following: >> - we send a W to gdb >> - readline handles this, and echoes it >> - after readline echoing it, expect notices this and we send a ^C to gdb >> - at this point readline is still in the code handling the W, and >> handles the ^C by echoing it. >> >> Usually, at this point we're already back in gdb and handle the ^C >> without echoing it. > > Thanks for the breakdown. I agree with your assessment. If I apply this > patch: > > ## START ## > > diff --git a/readline/readline/readline.c b/readline/readline/readline.c > index 0e33587f234..e5825a0a9b0 100644 > --- a/readline/readline/readline.c > +++ b/readline/readline/readline.c > @@ -678,6 +678,9 @@ readline_internal_charloop (void) > else if (rl_mark_active_p ()) > rl_deactivate_mark (); > > + if (getenv ("RL_CHAR_DELAY") != NULL) > + sleep (1); > + > _rl_internal_char_cleanup (); > > #if defined (READLINE_CALLBACKS) > > ## END ## > > Then run GDB with the RL_CHAR_DELAY environment variable set, it is now > possible to type a character and quickly hit Ctrl-C in order to always > see the '^C' displayed. > > Given the following assumptions: > > An application using readline in callback mode will spend most of its > time outside of the readline code, and will therefore mostly have its > own signal handlers installed. > > And, the documentation for the readline function rl_echo_signal_char > says: > > If an application wishes to install its own signal handlers, but > still have readline display characters that generate signals, > calling this function with SIG set to 'SIGINT', 'SIGQUIT', or > 'SIGTSTP' will display the character generating that signal. > > I wonder if the single call to 'rl_echo_signal_char' which can be found > in readline/readline/signals.c should be wrapped in an `#if` such that > this call is disabled when readline is used in callback mode? Like this > patch: > > ## START ## > > diff --git a/readline/readline/signals.c b/readline/readline/signals.c > index 8fedc370a1a..f10534c6872 100644 > --- a/readline/readline/signals.c > +++ b/readline/readline/signals.c > @@ -271,7 +271,9 @@ _rl_handle_signal (int sig) > sigprocmask (SIG_BLOCK, &set, &oset); > #endif > > +#if !defined (READLINE_CALLBACKS) > rl_echo_signal_char (sig); > +#endif > rl_cleanup_after_signal (); > > /* At this point, the application's signal handler, if any, is the > > ## END ## > > My reasoning would be that, when using in callback mode, it is up to the > application's signal handler to ensure that rl_echo_signal_char is > called if the application actually wants '^C' to be printed. If must be > doing this or mostly '^C' would not (currently) be printed. > > If we hit this race condition then the application is now going to print > a double '^C^C' string, which is also a bug. > > And if the applications signal handler doesn't cause rl_echo_signal_char > to be called (like GDB) then it feels weird that in this one corner case > we do end up calling it. > > In conclusion, I think I am arguing that what we have here is a readline > bug. > > I'm happy to present this on the readline mailing list, but I wanted to > discuss this with you first -- to see if I've convinced you? You did :) The bit of documentation you quoted suggests to me that it's unintended behaviour, thanks for digging that up. Thanks, - Tom