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

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

[Maciej W. Rozycki]

Hi,

 A live target is required for `-info-os' to work in non-native 
configurations.  Here's a fix for gdb.mi/mi-info-os.exp that removes the 
following failure:

(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

Tested with mips-linux-gnu (crossed from i686-linux-gnu).  OK to apply?

2011-11-01  Maciej W. Rozycki  <macro@codesourcery.com>

	gdb/testsuite/
	* gdb.mi/mi-info-os.exp: Connect to the target in non-native 
	configurations.

  Maciej

gdb-test-mi-info-os.diff
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] } {
+    standard_testfile basics.c
+
+    if [build_executable "Failed to build $testfile" $testfile $srcfile \
+	debug] {
+	return -1;
+    }
+
+    if {[mi_run_to_main] < 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"

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

[Maciej W. Rozycki]

On Fri, 1 Nov 2013, Maciej W. Rozycki wrote:

>  A live target is required for `-info-os' to work in non-native 
> configurations.  Here's a fix for gdb.mi/mi-info-os.exp that removes the 
> following failure:
> 
> (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
> 
> Tested with mips-linux-gnu (crossed from i686-linux-gnu).  OK to apply?
> 
> 2011-11-01  Maciej W. Rozycki  <macro@codesourcery.com>
> 
> 	gdb/testsuite/
> 	* gdb.mi/mi-info-os.exp: Connect to the target in non-native 
> 	configurations.

 Ping: http://sourceware.org/ml/gdb-patches/2013-11/msg00001.html

  Maciej

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

[Pedro Alves]

On 11/01/2013 01:16 AM, Maciej W. Rozycki wrote:

> gdb-test-mi-info-os.diff
> 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.

(And even if host and target triplets matched, we could still
have a configuration without a native target)

Otherwise OK.

> +    standard_testfile basics.c
> +
> +    if [build_executable "Failed to build $testfile" $testfile $srcfile \
> +	debug] {
> +	return -1;
> +    }
> +
> +    if {[mi_run_to_main] < 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"
> 


-- 
Pedro Alves

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

[Maciej W. Rozycki]

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

> 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?

 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.

  Maciej

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

[Pedro Alves]

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"

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

[Pedro Alves]

Hi guys,

Going through my local git branches I found this
one almost forgotten.  FYI, I went ahead and pushed
it in.

-- 
Pedro Alves

On 11/21/2013 11:36 AM, Pedro Alves wrote:

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

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

[Maciej W. Rozycki]

On Thu, 9 Jan 2014, Pedro Alves wrote:

> Going through my local git branches I found this
> one almost forgotten.  FYI, I went ahead and pushed
> it in.

 Thanks.  Somehow I lost your earlier e-mail from my radar, sorry.  The 
change you've committed works for me; I haven't checked a truly native 
configuration though.

  Maciej

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

[Pedro Alves]

On 01/09/2014 11:05 PM, Maciej W. Rozycki wrote:
> On Thu, 9 Jan 2014, Pedro Alves wrote:
> 
>> Going through my local git branches I found this
>> one almost forgotten.  FYI, I went ahead and pushed
>> it in.
> 
>  Thanks.  Somehow I lost your earlier e-mail from my radar, sorry.
> The change you've committed works for me; I haven't checked a truly native
> configuration though.

No worries, thanks for confirming.

-- 
Pedro Alves