[PATCH 1/6] gdb/testsuite: Better detection of auto-response at y/n prompts

Simon Marchi simon.marchi@polymtl.ca
Wed Jan 9 14:45:00 GMT 2019 

On 2019-01-09 06:24, Andrew Burgess wrote:
> * Simon Marchi <simon.marchi@polymtl.ca> [2019-01-09 00:03:49 -0500]:
> 
>> On 2019-01-07 04:00, Andrew Burgess wrote:
>> > * Simon Marchi <simon.marchi@ericsson.com> [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"
>> 
>> 
>> Thanks, this LGTM.
> 
> Thanks, I pushed this patch.
>> 
>> Btw, what you think about those "fail"s following "perror"s?  It's my
>> understanding that perror throws an error, interrupting the execution 
>> flow,
>> so the fail is not recorded, right?
> 
> That's not what I see.  As an experiment I created the file
> gdb.base/perror-test.exp like this:
> 
>   perror "this is an error"
>   pass "this is a pass"
> 
> With this I see:
> 
>   ERROR: this is an error
> 
>   		=== gdb Summary ===
> 
>   # of unresolved testcases	1
> 
> With the unresolved being instead of the 'PASS'.  Also the return
> value from runtest is non-zero.
> 
> If I change perror-test.exp to this:
> 
>   perror "this is an error"
>   fail "this is a fail"
> 
> Then the results are similar:
> 
>   ERROR: this is an error
> 
>   		=== gdb Summary ===
> 
>   # of unresolved testcases	1
> 
> Again, the unresolved replaces the 'FAIL', and the exit value is
> non-zero.
> 
> Where it gets interesting is if I change perror-test.exp to this:
> 
>   perror "this is an error"
> 
> Then I get this result:
> 
>   ERROR: this is an error
> 
>   		=== gdb Summary ===
> 
> And alarmingly (maybe?) the exit value from runtest is 0, so it looks
> like the test has passed.
> 
> My conclusions then:
> 
>   - perror doesn't interrupt control flow,
>   - perror doesn't itself cause the test script to appear to fail,
>   - perror causes the next pass/fail (at least) to become unresolved,
> 
> Given that the perror calls we're looking at here are in mi_gdb_test,
> the entire pass/fail printing is wrapped up within this method, I
> don't think the unresolved-ness should leak out into the next test,
> so, in this case I think perror/fail is correct.

Ah, this is clearer when reading the documentation :)

https://www.gnu.org/software/dejagnu/manual/perror-procedure.html

It's a bit confusing, since it overrides the following pass/fail 
message.  If you don't know what perror does, it looks like the 
pass/fail is not reached...  Thanks for looking into it.

>> And a note, maybe you are already aware of its existence, but I think 
>> that
>> "make check-read1" could have been useful to reproduce the issue 
>> reliably
>> here.  It forces the read syscall to read 1 byte at the time, so you 
>> are
>> certain to reach a point where -re "\\(y or n\\) " matches.
> 
> I did not know this.  Might be worth testing the whole testsuite
> against this and look for regressions..... I'll add it to my (long)
> list.

At least, it can be a good practice to check the tests we are working 
with it.

Simon

More information about the Gdb-patches mailing list