This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/4] native-gdbserver: Clear isremote flag in board info
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Pedro Alves <palves at redhat dot com>, Simon Marchi <simon dot marchi at polymtl dot ca>, <gdb-patches at sourceware dot org>
- Date: Mon, 11 Apr 2016 15:14:39 -0400
- Subject: Re: [PATCH 1/4] native-gdbserver: Clear isremote flag in board info
- Authentication-results: sourceware.org; auth=none
- References: <1459912543-15328-1-git-send-email-simon dot marchi at polymtl dot ca> <570BEF8B dot 70203 at redhat dot com>
On 16-04-11 02:40 PM, Pedro Alves wrote:
> 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.
Ok, that makes sense. I'll investigate in that direction.
>> 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?
Apparently not, since it works :)
I'm not familiar enough with TCL to know that. Are we executing in the body
of a procedure at this point? This file is sourced from some proc in DejaGnu,
so I'd say yes, but this example may suggest otherwise.
> 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.
I wasn't, but it's a good idea. isremote is also handled in an unintuitive way,
since the testsuite only checks if it's defined. So when you set isremote to 0,
it's still considered as "true".