[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gdb-patches
Subject:    [PATCH] Improve MI inferior output check / mi-console.exp
From:       lgustavo () codesourcery ! com (Luis Machado)
Date:       2013-10-30 12:30:00
Message-ID: 5270FBD1.7040908 () codesourcery ! com
[Download RAW message or body]

On 10/29/2013 09:09 AM, Pedro Alves wrote:
> On 10/28/2013 08:43 PM, Luis Machado wrote:
> 
> > *sigh*, that is true. I think fiddling with these checks may not be ideal.
> > 
> > What about this simpler version that just checks for both patterns
> > without other changes?
> 
> > 2013-10-28  Iain Sandoe  <iain@codesourcery.com>
> > 	    Luis Machado  <lgustavo@codesourcery.com>
> > 
> > 	gdb/testsuite
> > 	* gdb.mi/mi-console.exp: Check for an additional inferior
> > 	output pattern.
> 
> I suggest:
> 
> 	* gdb.mi/mi-console.exp: Check for semihosted inferior
> 	output pattern.
> 

Done.

> > 
> > diff --git a/gdb/testsuite/gdb.mi/mi-console.exp \
> > b/gdb/testsuite/gdb.mi/mi-console.exp index b7643e7..c9a1a84 100644
> > --- a/gdb/testsuite/gdb.mi/mi-console.exp
> > +++ b/gdb/testsuite/gdb.mi/mi-console.exp
> > @@ -36,11 +36,20 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" \
> > "${binfile}" executable {deb 
> > mi_run_to_main
> > 
> > +# The output we get from the target depends on how it is hosted.
> > +# If we are local, we just see the executable's output.
> > +# If we are remote (and the configuration supports semihosting),
> > +# we see the target responses encapsulated by gdb's annotation.
> > +# Therefore we consider two patterns here.
> 
> Not only remote, but the sim, or any other semihosted configuration.
> I think it'd be good to make the comment and variable names reflect
> that, and be more precise, making it a easier for future
> maintainers.  So:
> 
> # The output we get from the target depends on how it is hosted.  If
> # we are semihosted (e.g., the sim or a remote target that supports
> # the File I/O remote protocol extension), we see the target I/O
> # encapsulated in MI target output stream records.  If debugging with
> # a native target, the inferior's I/O streams are connected directly
> # to a PTY we create for the inferior (notice separate-inferior-tty
> # above), and we just see the program's output unadorned.  If
> # debugging with a remote target that doesn't support semihosting,
> # we'll see nothing.
> 
> set semihosted_output "@\"H\"\r\n@\"e\"\r\n@\"l\"\r\n@\"l\"\r\n@\"o\"\r\n@\" \
> \"\r\n@\"\\\\\\\\\"\r\n@\"\\\\\"\"\r\n@\"!\"\r\n@\"\\\\r\"\r\n@\"\\\\n\"\r\n" set \
> output "Hello \\\\\"!\[\r\n\]+" set both_patterns "($semihosted_output|$output)"
> 
> 
> This is OK with that change, though I'd prefer if $semihosted_output
> was computed from $output...
> 

The following patch accomplishes that and tweaks things a little further.

First, i removed the misleading test description. It does not seem to be 
aligned with what the test does nowadays. It is not restricted to 
remote/sim targets anymore.

I've put back the mi-support.exp bit since that is required for remote 
targets, otherwise we will try to read from the spawn id.

I'm explicitly checking for "remote" and "extended-remote" gdb_protocol 
values here since neither "isremote" nor "use_gdb_stub" seem to have a 
consistent meaning. Extended-remote should be both "use_gdb_stub" and 
"isremote", but it isn't.

Hopefully this version also addresses the gdb-simulator vs rsp-simulator 
confusion.

I regression tested this with native gdb (full passes), native-gdbserver 
(4 passes and 1 unsupported), native-extended-gdbserver (4 passes and 1 
unsupported) and also in our semihosted-enabled target (full passes).

WDYT?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mi_console_v2.diff
Type: text/x-patch
Size: 4192 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20131030/5be83342/attachment.bin>



[prev in list] [next in list] [prev in thread] [next in thread]