From patchwork Mon Jan 7 09:00:21 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 30984 Received: (qmail 35047 invoked by alias); 7 Jan 2019 09:01:00 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 25205 invoked by uid 89); 7 Jan 2019 09:00:43 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=fills, window, Window, deleting X-HELO: mail-wm1-f66.google.com Received: from mail-wm1-f66.google.com (HELO mail-wm1-f66.google.com) (209.85.128.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 07 Jan 2019 09:00:40 +0000 Received: by mail-wm1-f66.google.com with SMTP id a62so109772wmh.4 for ; Mon, 07 Jan 2019 01:00:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=T7wCpzEMyjK5Kxf79Ul25wvEs1XVSjKhNjqEP/4BS3E=; b=PvHgEmSFE4imBH0riXc1LW+fNJLOrteoqFzUU8cXr0eQHxmWd4kinSGLKjqUoCezS0 /GzvwhSJCvgmB7pkxK4D82vPGoZMj41+9ONVG6dEZ6qY1U/BJupurpoT0iPjmxCA57F3 u9zHTvocW0Th8Qoidie6GPEw6yfBt18JAcL1p/b/P7Kw8b4FWov0ItiAgmOCg/iXRIpi TEhU5dmHhW08nIz5H81hP6oDHDi+tbSr6en/hjmF1ejOvi0IQRzob7gtXEGgx6oyul0T 9MInHvweqIpIv2DYI8WDGVWN3y8+/u/k8NctPa/KtULbdRp4iznr2MrdAod3+OgydlwW ggfw== Return-Path: Received: from localhost (host86-172-198-47.range86-172.btcentralplus.com. [86.172.198.47]) by smtp.gmail.com with ESMTPSA id c10sm47105825wrw.49.2019.01.07.01.00.22 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 07 Jan 2019 01:00:23 -0800 (PST) Date: Mon, 7 Jan 2019 09:00:21 +0000 From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: Re: [PATCH 1/6] gdb/testsuite: Better detection of auto-response at y/n prompts Message-ID: <20190107090021.GR3456@embecosm.com> References: <9f15f4ed7b5f22de0b0606fe363bce6689b11a60.1546382416.git.andrew.burgess@embecosm.com> <0988cac0-b039-d3af-4e2e-95528aebb545@ericsson.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <0988cac0-b039-d3af-4e2e-95528aebb545@ericsson.com> X-Fortune: Green light in A.M. for new projects. Red light in P.M. for traffic tickets. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes * Simon Marchi [2019-01-02 21:48:06 +0000]: > On 2019-01-01 5:45 p.m., Andrew Burgess wrote: > > I noticed that when running this test: > > > > make check-gdb RUNTESTFLAGS="--target_board=native-gdbserver gdb.mi/mi-break.exp" > > > > I would occasionally see some UNRESOLVED test results like this: > > > > (gdb) > > PASS: gdb.mi/mi-break.exp: mi-mode=separate: breakpoint at main > > Expecting: ^(kill[ > > ]+)?(.*[ > > ]+[(]gdb[)] > > [ ]*) > > kill > > &"kill\n" > > ~"Kill the program being debugged? (y or n) [answered Y; input not from terminal]\n" > > =thread-group-exited,id="i1" > > ERROR: Got interactive prompt. > > UNRESOLVED: gdb.mi/mi-break.exp: mi-mode=separate: > > > > The problem appears to be that the expect buffer fills up to include > > the '(y or n)' prompt without including the following lines. > > > > The pattern supplied by the outer test script is looking for the > > following lines. As the following lines are not present then expect > > matches on the interactive prompt case rather than the case for the > > user supplied pattern. > > > > The problem with this is that we are not really at an interactive > > prompt, GDB is providing an answer for us and then moving on. When I > > examine a successful run of the test the output from GDB is identical, > > the only difference is where expect happens to buffer the output from > > GDB. > > > > This patch introduces a second check inside the 'y or n' prompt case, > > which gives expect a chance to refill its buffers and catches the > > 'answered Y; input ...' text. > > > > This second check is on a short 1 second timeout, I'm currently > > assuming that the auto-answer text will either already be in expect, > > or is waiting to be read in. If after 1 second the auto-answer text > > is not seen then we assume that GDB really is waiting at an > > interactive prompt. > > > > With this patch in place I can now leave the following loop running > > indefinitely, where before it would fail usually after ~10 > > iterations. > > For me it fails consistently the way you describe. > > > diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp > > index d193592a843..48ea45d62c7 100644 > > --- a/gdb/testsuite/lib/mi-support.exp > > +++ b/gdb/testsuite/lib/mi-support.exp > > @@ -834,9 +834,24 @@ proc mi_gdb_test { args } { > > fail "$message" > > } > > -re "\\(y or n\\) " { > > - send_gdb "n\n" > > - perror "Got interactive prompt." > > - fail "$message" > > + # If the expect buffer just happens to fill up to the 'y > > + # or n' prompt then we can end up in this case, even > > + # though GDB will automatically provide a response for us. > > + # We give expect another chance here to look for the auto > > + # answer text before declaring a fail. > > + set auto_response_seen 0 > > + gdb_expect 1 { > > + -re ".answered Y; input not from terminal." { > > + set auto_response_seen 1 > > + } > > + } > > + if { ! $auto_response_seen } { > > + send_gdb "n\n" > > + perror "Got interactive prompt." > > + fail "$message" > > + } else { > > + exp_continue > > + } > > } > > eof { > > perror "Process no longer exists" > > Do we need this stanza at all? I understand that it can save some time (avoid having > to wait for the timeout) if the MI interpreter suddenly starts asking interactive > "y or n" questions (which it should not do), but since it's causing this kind of trouble, > maybe we can just get rid of it? I don't see any real reason to keep it. I tried deleting it and no tests seem to regress, and the previously unstable test is now stable. Is this OK to apply? Thanks, Andrew --- gdb/testsuite: Remove interactive prompt case from mi_gdb_test I noticed that when running this test: make check-gdb RUNTESTFLAGS="--target_board=native-gdbserver gdb.mi/mi-break.exp" I would occasionally see some UNRESOLVED test results like this: (gdb) PASS: gdb.mi/mi-break.exp: mi-mode=separate: breakpoint at main Expecting: ^(kill[ ]+)?(.*[ ]+[(]gdb[)] [ ]*) kill &"kill\n" ~"Kill the program being debugged? (y or n) [answered Y; input not from terminal]\n" =thread-group-exited,id="i1" ERROR: Got interactive prompt. UNRESOLVED: gdb.mi/mi-break.exp: mi-mode=separate: The problem appears to be that the expect buffer fills up to include the '(y or n)' prompt without including the following lines. The pattern supplied by the outer test script is looking for the following lines. As the following lines are not present then expect matches on the interactive prompt case rather than the case for the user supplied pattern. The problem with this is that we are not really at an interactive prompt, GDB is providing an answer for us and then moving on. When I examine a successful run of the test the output from GDB is identical, the only difference is where expect happens to buffer the output from GDB. This patch remove all special handling of the interactive prompt case. This means that if we ever break GDB and start seeing an unexpected interactive prompt then tests will rely on a timeout to fail, instead of having dedicated interactive prompt detection, but this solves the problem that an auto-answered prompt looks very similar to an interactive prompt. With this patch in place I can now leave the following loop running indefinitely, where before it would fail usually after ~10 iterations. while make check-gdb RUNTESTFLAGS="--target_board=native-gdbserver gdb.mi/mi-break.exp"; \ do /bin/true; \ done gdb/testsuite/ChangeLog: * lib/mi-support.exp (mi_gdb_test): Remove interactive prompt case. --- gdb/testsuite/ChangeLog | 5 +++++ gdb/testsuite/lib/mi-support.exp | 5 ----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp index d193592a843..a58c4f6e119 100644 --- a/gdb/testsuite/lib/mi-support.exp +++ b/gdb/testsuite/lib/mi-support.exp @@ -832,11 +832,6 @@ proc mi_gdb_test { args } { send_gdb "\n" perror "Window too small." fail "$message" - } - -re "\\(y or n\\) " { - send_gdb "n\n" - perror "Got interactive prompt." - fail "$message" } eof { perror "Process no longer exists"