This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

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".

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]