Re: [PATCH] gdb.mi/mi-info-os.exp: Fix cross-debugger testing

[Pedro Alves <palves@redhat.com>]

On 11/20/2013 08:01 PM, Maciej W. Rozycki wrote:
> On Wed, 20 Nov 2013, Pedro Alves wrote:
> 
>>> Index: gdb-fsf-trunk-quilt/gdb/testsuite/gdb.mi/mi-info-os.exp
>>> ===================================================================
>>> --- gdb-fsf-trunk-quilt.orig/gdb/testsuite/gdb.mi/mi-info-os.exp	2013-11-01 00:52:48.000000000 +0000
>>> +++ gdb-fsf-trunk-quilt/gdb/testsuite/gdb.mi/mi-info-os.exp	2013-11-01 00:56:43.588756235 +0000
>>> @@ -33,6 +33,20 @@ if [mi_gdb_start] {
>>>      continue
>>>  }
>>>  
>>> +# In non-native configurations we need to have a live target.
>>> +if { ![isnative] } {
>>
>> Shouldn't this be [is_remote target] ?  isnative just compares
>> the _build_ and target triplets.
> 
>  Well is_remote merely checks if the target board is not the same as the 
> local (build) machine.  

True, though we have boards that because they behave like a
local machine, set board_info,isremote to 0, e.g.,
testsuite/boards/native-extended-gdbserver.exp.

So, with 

  if { ![isnative] } {

'make check RUNTESTFLAGS="--target_board=native-gdbserver mi-info-os.exp"'

... passes, but looking closer, you'll find that -info-os actually ran
against the default native target, because build and target triplet
were the same, therefore isnative returned false.

With

  if { [is_remote target] } {

instead, testing with the native-gdbserver board correctly runs
the command against the remote target ("target remote").

So for that alone, it looks like a better predicate to me.

> I don't think it matters, it doesn't tell anything 
> about GDB configuration, e.g. GDB might be native after all, running on a 
> remote host that is the same as the target (i.e. no `gdbserver' or 
> suchlike involved, although we don't actually care here whether 
> `gdbserver' is used or not), like in a crossed build of a native 
> toolchain.
> 
>  FWIW I actually scanned our testsuite for various configuration 
> conditions used before making this change and I believe the cases of 
> [is_remote target] I saw are incorrect -- the authors of the pieces of 
> code in question appear to have assumed that this condition translates to 
> the use of the RSP and `gdbserver' while it is not what DejaGNU 
> implements.  

Yeah, it's all a little conflated.

> All it implies is what I noted above -- that is that the 
> target machine is not the same as the build machine (one running `make 
> check' or the equivalent).  That has nothing to do with `gdbserver' or the 
> use of the RSP.
>
>> (And even if host and target triplets matched, we could still
>> have a configuration without a native target)
> 
>  How come?

I have a patch to force disabling the native target, to catch
such testsuite bugs, where when testing against a local gdbserver
by mistake the test actually tests the default run target.
I should post it and see what others think.

>> Otherwise OK.
> 
>  So what is your proposal?  What's the proper way of determining whether 
> this configuration of GDB is native (includes a native target) or not?

There's no such exact test currently, AFAIK.  We can however check whether
this is a "stub-like" target, one where once we connect we find the
program already running, and where "run" means connecting, with:

  if { $use_gdb_stub } {

That wouldn't be perfect, but it's a little closer to what we want.

But, see below.

> 
>  To maintain current coverage we need to complement the check made in 
> target_get_osdata:
> 
>   if (current_target.to_stratum >= process_stratum)
>     t = current_target.beneath;
>   else
>     t = find_default_run_target ("get OS data");
> 
> and do not connect to any target in configurations where 
> find_default_run_target returns success.

Yeah.  I can't think of a way to probe this, with current GDB.

But, WDYT of this alternative below?  We don't want to
unconditionally use mi_run_to_main, as then with native targets
we'd lose the testing of -info-os before the program is running.
But we can do what mi_run_to_main ends up using to connect
to the target instead (mi_gdb_target_load).

---
gdb.mi/mi-info-os.exp: Fix cross-debugger testing

A live target is required for `-info-os' to work in non-native
configurations.

 (gdb)
 Expecting: ^(-info-os[
 ]+)?(.*\^done,OSDataTable=.*[
 ]+[(]gdb[)]
 [ ]*)
- info-os
 ^error,msg="Don't know how to get OS data.  Try \"help target\"."
 (gdb)
 FAIL: gdb.mi/mi-info-os.exp: -info-os

If GDB does have a native configuration included, but we're testing
remote, it'll be worse, as if we're not connected yet, -info-os will
run against the default run target, and pass, falsely giving the
impression the remote bits were exercised.

gdb/testsuite/
2013-11-21  Maciej W. Rozycki  <macro@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	* gdb.mi/mi-info-os.exp: Connect to the target with
        mi_gdb_target_load.
---

 gdb/testsuite/gdb.mi/mi-info-os.exp |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/gdb/testsuite/gdb.mi/mi-info-os.exp b/gdb/testsuite/gdb.mi/mi-info-os.exp
index d410cfb..f062510 100644
--- a/gdb/testsuite/gdb.mi/mi-info-os.exp
+++ b/gdb/testsuite/gdb.mi/mi-info-os.exp
@@ -33,6 +33,26 @@ if [mi_gdb_start] {
     continue
 }
 
+standard_testfile basics.c
+
+if [build_executable "Failed to build $testfile" $testfile $srcfile \
+	debug] {
+    return -1;
+}
+if {[mi_gdb_load $binfile] < 0} {
+    return -1
+}
+# When testing a cross configuration, we need to be sure to first
+# connect to the target.  If we didn't do that, GDB would try running
+# the command against the default run target.  The usual way to do
+# that and covera all target is to run to main, with mi_run_to_main.
+# However, with native configurations, -info-os should work before
+# running any program, so we want to avoid "run".  Using
+# mi_gdb_target_load directly instead achieves this.
+if {[mi_gdb_target_load] < 0} {
+    return -1
+}
+
 # Try the argument-less form that lists all the types in a table.
 
 mi_gdb_test "-info-os" ".*\\^done,OSDataTable=.*" "-info-os"