Re: [PATCH 1/4] native-gdbserver: Clear isremote flag in board info

[Pedro Alves <palves at redhat dot com>]

On 04/06/2016 04:15 AM, Simon Marchi wrote:

> There seems to be some confusion globally in the testsuite about what's
> native and what's not, what's remote and what's not.  It's probably
> because of the overlap in vocabulary between DejaGnu and GDB.

Yeah.  This whole mess, and the confusion it brings in, has been a known
issue for a long time, and I'm very glad someone's doing something about it!

> Because of this confution, a lot of tests in the testsuite don't check
> for the right kind of target remote.  For example, some use [is_remote
> target] (which checks whether the DejaGnu target is remote), when what
> they really want to know is whether GDB is using its remote target (e.g.
> because feature X is only available when GDB debugs natively).  Those
> should use [gdb_is_target_remote] instead.

Note gdb_is_target_remote is a bit "heavy" in that it runs a gdb
command to check what is the target _right now_.  For checks at
the top of testcases that want to bail out early before even
gdb is started, it's cheaper to check:

 [target_info gdb_protocol] == "remote" || [target_info gdb_protocol] == "extended-remote"]

(we should have a proc for that...)

My fault for introducing gdb_is_target_remote and using in several
places, when the "cheaper" test would do.

> Also, some of them require careful analysis/discussion.  I'd still like
> to submit this patch right now anyway, since I don't want to spend
> countless hours fixing everything and then be told that my approach is
> wrong... I'd like to be told right away, if that's the case :).  

I think you're on the right track.

> So, I
> have included a few examples of fixed tests in the following patches,
> but it's by no means comprehensive.
> 

> --- a/gdb/testsuite/boards/native-gdbserver.exp
> +++ b/gdb/testsuite/boards/native-gdbserver.exp
> @@ -36,26 +36,6 @@ set_board_info exit_is_reliable 1
>  # We will be using the standard GDB remote protocol.
>  set_board_info gdb_protocol "remote"
>  
> -proc ${board}_spawn { board cmd } {
> -    global board_info
> +set baseboard [lindex [split $board "/"] 0]
> +set board_info($baseboard,isremote) 0

OOC, don't we need the "global" declarations as
in native-extended-gdbserver.exp?

You're probably already thinking of doing this, but I'll state it
explicitly anyway: it'd be nice to move this isremote frobbing to
a shared .exp file, so that boards that need it can just
source the file.

Thanks,
Pedro Alves