Skipping tests that use remote protocol

  This patch introduces gdb_using_remote_protocol and
gdb_using_extended_protocol.  These procedures are needed to reliably
determine if tests not supported for one or both of the remote protocols
should be skipped.  An example of the use of gdb_using_remote_protocol in
gdb.base/attach.exp is included in the patch.

We encountered problems here in a scenario where runtest is invoked on
system A, the host is system B, and the target is system A.  is_remote
failed to detect that host != target.

The existing methods of detecting this use procedures that don't work in
all cases.  In gdb.base/attach.exp, for example, it attempts to skip the
test for 'target remote' by checking [is_remote target].  However,
[is_remote target] checks if the target is remote relative to the "build"
system, the system where runtest is executing.  It doesn't check if the
target is remote relative to the host, which may or may not be the same
as "build".

Other procedures are also used that don't really check the right thing.

* isnative checks the build triplet against the target triplet.

* gdb_is_target_remote and target_is_gdbserver don't differentiate between
  remote and extended-remote.  Both require GDB to be running, which makes
  using them to skip a test less efficient than a procedure that uses info
  from the target board config file.

* target_info use_gdb_stub is used in lib/gdb.exp to explicitly determine
  if a target is remote and not extended-remote.

If we reach consensus on this approach I'll follow up with patches to
convert other tests to use these procedures instead of is_remote,
isnative, and so on.

Thanks!
--Don

gdb/testsuite/
2014-12-11  Don Breazeal  <donb@codesourcery.com>

	* gdb.base/attach.exp: Replace call to is_remote with call to
	gdb_using_remote_protocol and if so, call unsupported.
	* lib/gdb.exp (gdb_using_remote_protocol): New proc.
	(gdb_using_extended_protocol): New proc.

---
 gdb/testsuite/gdb.base/attach.exp |  5 +++--
 gdb/testsuite/lib/gdb.exp         | 20 ++++++++++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)
  

Don Breazeal <donb@codesourcery.com> writes:

> Other procedures are also used that don't really check the right thing.
>
> * isnative checks the build triplet against the target triplet.
>
> * gdb_is_target_remote and target_is_gdbserver don't differentiate between
>   remote and extended-remote.  Both require GDB to be running, which makes
>   using them to skip a test less efficient than a procedure that uses info
>   from the target board config file.
>
> * target_info use_gdb_stub is used in lib/gdb.exp to explicitly determine
>   if a target is remote and not extended-remote.
>
> If we reach consensus on this approach I'll follow up with patches to
> convert other tests to use these procedures instead of is_remote,
> isnative, and so on.

Does "![isnative] || [is_remote host] || [target_info exists use_gdb_stub]"
work for you?  It is used in some attach related tests.
  

On 12/11/2014 7:11 PM, Qi, Yao wrote:
> Don Breazeal <donb@codesourcery.com> writes:
> 
>> Other procedures are also used that don't really check the right thing.
>>
>> * isnative checks the build triplet against the target triplet.
>>
>> * gdb_is_target_remote and target_is_gdbserver don't differentiate between
>>   remote and extended-remote.  Both require GDB to be running, which makes
>>   using them to skip a test less efficient than a procedure that uses info
>>   from the target board config file.
>>
>> * target_info use_gdb_stub is used in lib/gdb.exp to explicitly determine
>>   if a target is remote and not extended-remote.
>>
>> If we reach consensus on this approach I'll follow up with patches to
>> convert other tests to use these procedures instead of is_remote,
>> isnative, and so on.
> 
> Does "![isnative] || [is_remote host] || [target_info exists use_gdb_stub]"
> work for you?  It is used in some attach related tests.
> 
Hm. This would work for my attach example, but I don't think that this
check is sufficient for all cases (e.g. gdb.base/catch-syscall.exp,
which we want to skip for both "remote" and "extended-remote" on Linux).

[target_info exists use_gdb_stub] alone would work for the attach tests,
which we want to skip for remote but run for extended-remote.  This
(use_gdb_stub) seems to be equivalent to my new proc
[gdb_using_remote_protocol], meaning "using gdbserver/stub" and protocol
== "remote".  The name use_gdb_stub is misleading, since it is only set
for the remote protocol and not the extended protocol.  Things go wrong
in lib/gdb.exp if you set use_gdb_stub and run extended-mode tests.

If we put aside the fact that we can control the results of is_remote by
setting the variable isremote in the board file, then [isnative] and
[is_remote] don't provide the information we really need.  In the
example above they are checking whether build!=target and build!=host,
respectively.  That doesn't cover all the cases, e.g. if build != target
and build != host, we don't know for sure whether target == host.

We can set isremote in the board files, as in native-gdbserver.exp, to
control what is_remote returns.  But checking if we are using gdbserver
or a stub is not the purpose of is_remote, and trying to use it in
general for that could have negative side-effects (e.g. to gcc tests).

My conclusion from all of this is that we should never use isnative or
is_remote to decide whether to skip tests for remote targets.  The two
new proc's are testing the specific conditions that affect the remote
tests.  We could use [target_info exists use_gdb_stub] in place of
[gdb_using_remote_protocol], but the name may be misleading.

What do you think?  In any case I'd like this discussion to result in a
standard approach for skipping remote tests for each of the relevant cases.

Thanks!
--Don
  

"Breazeal, Don" <donb@codesourcery.com> writes:

> Hm. This would work for my attach example, but I don't think that this
> check is sufficient for all cases (e.g. gdb.base/catch-syscall.exp,
> which we want to skip for both "remote" and "extended-remote" on
> Linux).

I'd like tests automatically detect whether syscall catchpoint is
supported, instead of hard-coded some unsupported targets.  We can
insert a syscall catchpoint and resume the inferior, warning

"warning: Error inserting catchpoint 2: Your system does not support this type"

can tell us that syscall cachtpoint isn't supported.  It is similar for
detecting tracepoint support.

>
> [target_info exists use_gdb_stub] alone would work for the attach
> tests,

I am afraid not.  attach tests should be skipped on remote host testing
(build != host, host == target) too, because test program is spawned on
build, and the corresponding pid (on build) is used for gdb (on host) to
attach.

> which we want to skip for remote but run for extended-remote.  This
> (use_gdb_stub) seems to be equivalent to my new proc
> [gdb_using_remote_protocol], meaning "using gdbserver/stub" and protocol
> == "remote".  The name use_gdb_stub is misleading, since it is only set
> for the remote protocol and not the extended protocol.  Things go wrong
> in lib/gdb.exp if you set use_gdb_stub and run extended-mode tests.
>
> If we put aside the fact that we can control the results of is_remote by
> setting the variable isremote in the board file, then [isnative] and
> [is_remote] don't provide the information we really need.  In the
> example above they are checking whether build!=target and build!=host,
> respectively.  That doesn't cover all the cases, e.g. if build != target
> and build != host, we don't know for sure whether target == host.
>
> We can set isremote in the board files, as in native-gdbserver.exp, to
> control what is_remote returns.  But checking if we are using gdbserver
> or a stub is not the purpose of is_remote, and trying to use it in
> general for that could have negative side-effects (e.g. to gcc tests).
>
> My conclusion from all of this is that we should never use isnative or
> is_remote to decide whether to skip tests for remote targets.  The two
> new proc's are testing the specific conditions that affect the remote
> tests.  We could use [target_info exists use_gdb_stub] in place of
> [gdb_using_remote_protocol], but the name may be misleading.
>
> What do you think?  In any case I'd like this discussion to result in a
> standard approach for skipping remote tests for each of the relevant cases.

Some tests have to be skipped on remote targets, and even on other
targets.  I'd like to list all of them (tests having troubles running on
remote target), and then figure out how to fix them.  We can use a
standard approach if necessary.
  

On 12/15/2014 3:58 AM, Qi, Yao wrote:
> "Breazeal, Don" <donb@codesourcery.com> writes:
> 

>> [target_info exists use_gdb_stub] alone would work for the attach
>> tests,
> 
> I am afraid not.  attach tests should be skipped on remote host testing
> (build != host, host == target) too, because test program is spawned on
> build, and the corresponding pid (on build) is used for gdb (on host) to
> attach.
> 
Ah.  Right.

> Some tests have to be skipped on remote targets, and even on other
> targets.  I'd like to list all of them (tests having troubles running on
> remote target), and then figure out how to fix them.  We can use a
> standard approach if necessary.
> 
OK.  I will start with all of the various attach tests for now.

--Don
  

I see now we're discussing the same things here:

 https://sourceware.org/ml/gdb-patches/2014-12/msg00507.html

On 12/15/2014 11:58 AM, Yao Qi wrote:

>> [target_info exists use_gdb_stub] alone would work for the attach
>> tests,
> 
> I am afraid not.  attach tests should be skipped on remote host testing
> (build != host, host == target) too, because test program is spawned on
> build, and the corresponding pid (on build) is used for gdb (on host) to
> attach.

As I mentioned in the other email, in that scenario, build != target too,
so an is_remote target check already causes it to be skipped.

In the (build != host, build == target) scenario, the program
is spawned on the target, and although gdb runs on a different
machine from where the target programs run, the PID that is
passed to gdb is a target pid (e.g., debugging with extended-remote).
So in that scenario, the test should be able to run.  But if we
checked "is_remote host", it wouldn't.

>> which we want to skip for remote but run for extended-remote.  This
>> (use_gdb_stub) seems to be equivalent to my new proc
>> [gdb_using_remote_protocol], meaning "using gdbserver/stub" and protocol
>> == "remote".  

It means "use stub-like mechanisms".  IOW, "when gdb connects,
the program is already running, because the debug agent is really
a piece of code (stub) that runs inside the target program".  GDB used
to support a bunch more remote protocols that we've been dropping
over the years.  And then GDBserver/"target remote" debugging behaves
mostly like a real stub, so nowadays use_gdb_stub is it's mostly
equivalent to "remote", though the former is more generic.

>> The name use_gdb_stub is misleading, since it is only set
>> for the remote protocol and not the extended protocol.  Things go wrong
>> in lib/gdb.exp if you set use_gdb_stub and run extended-mode tests.

See above.  The extended protocol does not behave like a stub.

>>
>> If we put aside the fact that we can control the results of is_remote by
>> setting the variable isremote in the board file, then [isnative] and
>> [is_remote] don't provide the information we really need.  In the
>> example above they are checking whether build!=target and build!=host,
>> respectively.  That doesn't cover all the cases, e.g. if build != target
>> and build != host, we don't know for sure whether target == host.

True, but what are the cases where that matters?

>> We can set isremote in the board files, as in native-gdbserver.exp, to
>> control what is_remote returns.  But checking if we are using gdbserver
>> or a stub is not the purpose of is_remote, and trying to use it in
>> general for that could have negative side-effects (e.g. to gcc tests).

Agreed.

>>
>> My conclusion from all of this is that we should never use isnative or
>> is_remote to decide whether to skip tests for remote targets.  The two
>> new proc's are testing the specific conditions that affect the remote
>> tests.  We could use [target_info exists use_gdb_stub] in place of
>> [gdb_using_remote_protocol], but the name may be misleading.
>>
>> What do you think?  In any case I'd like this discussion to result in a
>> standard approach for skipping remote tests for each of the relevant cases.

See https://sourceware.org/ml/gdb-patches/2014-12/msg00507.html.

Thanks,
Pedro Alves