[testsuite] setting sysroot for target-board

[testsuite] setting sysroot for target-board

[Wei-cheng Wang]

When testing remote cross-target, libc and other shared libraries will not be
compatible with the one on host.

This patch adds support for setting sysroot in target-board files, e.g.,
set_board_info gdb,sysroot "/path/to/target/libc",
so that testsuite can be used for remote cross-target.

--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -44,6 +44,11 @@
 proc gdb_target_cmd { targetname serialport } {
     global gdb_prompt

+    if [target_info exists gdb,sysroot] {
+        set gdb_sysroot [target_info gdb,sysroot]
+        send_gdb "set sysroot $gdb_sysroot\n"
+    }
+
     set serialport_re [string_to_regexp $serialport]
     for {set i 1} {$i <= 3} {incr i} {
        send_gdb "target $targetname $serialport\n"

Re: [testsuite] setting sysroot for target-board

[Tom Tromey]

>>>>> ">" == Wei-cheng Wang <cole945@gmail.com> writes:

>> When testing remote cross-target, libc and other shared libraries
>> will not be compatible with the one on host.

>> This patch adds support for setting sysroot in target-board files, e.g.,
>> set_board_info gdb,sysroot "/path/to/target/libc",
>> so that testsuite can be used for remote cross-target.

I think this looks reasonable.

There is a comment just above the definition of gdb_target_cmd that
needs an update for this new parameter.

This patch also needs a ChangeLog entry.

Tom

Re: [testsuite] setting sysroot for target-board

[Wei-cheng Wang]

Here is the update :-)

2011-05-06  Wei-cheng Wang <cole945@gmail.com>

        * lib/gdbserver-support.exp: Add support for setting sysroot for
        target system.

--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -36,6 +36,10 @@
 #      Port id to use for socket connection.  If not set explicitly,
 #      it will start at "2345" and increment for each use.
 #
+#   set_board_info gdb,sysroot
+#       System root for the target to find shared libraries.
+#       If target and host are different, this should be properly set.
+#

 #
 # gdb_target_cmd
@@ -44,6 +48,11 @@
 proc gdb_target_cmd { targetname serialport } {
     global gdb_prompt

+    if [target_info exists gdb,sysroot] {
+        set gdb_sysroot [target_info gdb,sysroot]
+        send_gdb "set sysroot $gdb_sysroot\n"
+    }
+
     set serialport_re [string_to_regexp $serialport]
     for {set i 1} {$i <= 3} {incr i} {
        send_gdb "target $targetname $serialport\n"

Wei-cheng

On Fri, May 6, 2011 at 1:36 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> ">" == Wei-cheng Wang <cole945@gmail.com> writes:
>
>>> When testing remote cross-target, libc and other shared libraries
>>> will not be compatible with the one on host.
>
>>> This patch adds support for setting sysroot in target-board files, e.g.,
>>> set_board_info gdb,sysroot "/path/to/target/libc",
>>> so that testsuite can be used for remote cross-target.
>
> I think this looks reasonable.
>
> There is a comment just above the definition of gdb_target_cmd that
> needs an update for this new parameter.
>
> This patch also needs a ChangeLog entry.
>
> Tom
>

Re: [testsuite] setting sysroot for target-board

[Pedro Alves]

On Friday 06 May 2011 03:24:08, Wei-cheng Wang wrote:
> Here is the update :-)
> 
> 2011-05-06  Wei-cheng Wang <cole945@gmail.com>
> 
>         * lib/gdbserver-support.exp: Add support for setting sysroot for
>         target system.
> 
> --- a/gdb/testsuite/lib/gdbserver-support.exp
> +++ b/gdb/testsuite/lib/gdbserver-support.exp
> @@ -36,6 +36,10 @@
>  #      Port id to use for socket connection.  If not set explicitly,
>  #      it will start at "2345" and increment for each use.
>  #
> +#   set_board_info gdb,sysroot
> +#       System root for the target to find shared libraries.
> +#       If target and host are different, this should be properly set.
> +#
> 
>  #
>  # gdb_target_cmd
> @@ -44,6 +48,11 @@
>  proc gdb_target_cmd { targetname serialport } {
>      global gdb_prompt

Doing this here assumes you'll only need the sysroot after connecting.
How have you tested this?  It seems you forgot the MI equivalent.
I think a better place would be right after spawning
gdb (default_gdb_start/default_mi_gdb_start).  If since you
already have a board file anyway, you could also override *gdb_start
and do it there instead of adding a new flag.

> 
> +    if [target_info exists gdb,sysroot] {
> +        set gdb_sysroot [target_info gdb,sysroot]
> +        send_gdb "set sysroot $gdb_sysroot\n"

This leaves a gdb prompt in the expect buffer, which is a recipe
for trouble.  I guess it doesn't cause trouble as it, because
the "target foo" command below doesn't happen to have
a '-re ".*$gdb_prompt"' match, but that's fragile.
(actually it may cause trouble afterwards if the -notransfer regex
matches)

> +    }
> 
> Wei-cheng
> 
> On Fri, May 6, 2011 at 1:36 AM, Tom Tromey <tromey@redhat.com> wrote:
> >>>>>> ">" == Wei-cheng Wang <cole945@gmail.com> writes:
> >
> >>> When testing remote cross-target, libc and other shared libraries
> >>> will not be compatible with the one on host.
> >
> >>> This patch adds support for setting sysroot in target-board files, e.g.,
> >>> set_board_info gdb,sysroot "/path/to/target/libc",
> >>> so that testsuite can be used for remote cross-target.
> >
> > I think this looks reasonable.
> >
> > There is a comment just above the definition of gdb_target_cmd that
> > needs an update for this new parameter.
> >
> > This patch also needs a ChangeLog entry.
> >
> > Tom
> >
> 

-- 
Pedro Alves

Re: [testsuite] setting sysroot for target-board

[Wei-cheng Wang]

On Fri, May 6, 2011 at 5:47 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Friday 06 May 2011 03:24:08, Wei-cheng Wang wrote:
> Doing this here assumes you'll only need the sysroot after connecting.
> How have you tested this?  It seems you forgot the MI equivalent.
Yes, I did test it.
I found this issue when I tested gdb/gdbserver with a remote
cross-target. Some thread- and shared library-related test cases
failed due to using the wrong shared libraries for the host, so I
tried to add sysroot support and I’ve tested it again to make sure gdb
will use the right shared libraries for the target.
> I think a better place would be right after spawning
> gdb (default_gdb_start/default_mi_gdb_start).  If since you
> already have a board file anyway, you could also override *gdb_start
> and do it there instead of adding a new flag.
I agree. It’s better to put this in default_gdb_start/default_mi_gdb_start.
Since these two functions are not just a few of lines,
I will suggest adding this support, so it much easier to set sysroot as needed.
And here is the update according to your suggestion. Thanks.
- - - -
2011-05-06  Wei-cheng Wang <cole945@gmail.com>

        * lib/gdb.exp (default_gdb_start): Add support for setting sysroot for
        target system.
        * lib/mi-support.exp (default_mi_gdb_start): Likewise.

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 20e2fb7..c6fa62d 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1349,6 +1349,18 @@ proc default_gdb_start { } {
 	    warning "Couldn't set the width to 0."
 	}
     }
+
+    if [target_info exists gdb,sysroot] {
+        send_gdb "set sysroot [target_info gdb,sysroot]\n"
+        gdb_expect 10 {
+	    -re "$gdb_prompt $" {
+	        verbose "Setting sysroot to [target_info gdb,sysroot]" 2
+	    }
+	    timeout {
+                warning "Couldn't set the sysroot to [target_info gdb,sysroot]"
+	    }
+        }
+    }
     return 0;
 }

diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index e1dbd19..15565bc 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -221,6 +221,17 @@ proc default_mi_gdb_start { args } {
 	    }
     	}
     }
+    if [target_info exists gdb,sysroot] {
+    send_gdb "103-gdb-set sysroot [target_info gdb,sysroot]\n"
+        gdb_expect 10 {
+	    -re ".*103-gdb-set sysroot [target_info
gdb,sysroot]\r\n103\\\^done\r\n$mi_gdb_prompt$" {
+	        verbose "Setting sysroot to [target_info gdb,sysroot]" 2
+	    }
+	    timeout {
+	        warning "Couldn't set the sysroot to [target_info gdb,sysroot]."
+	    }
+        }
+    }

     detect_async

Re: [testsuite] setting sysroot for target-board

[Pedro Alves]

On Sunday 08 May 2011 18:01:16, Wei-cheng Wang wrote:
> On Fri, May 6, 2011 at 5:47 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> > On Friday 06 May 2011 03:24:08, Wei-cheng Wang wrote:
> > Doing this here assumes you'll only need the sysroot after connecting.
> > How have you tested this?  It seems you forgot the MI equivalent.
> Yes, I did test it.
> I found this issue when I tested gdb/gdbserver with a remote
> cross-target. Some thread- and shared library-related test cases
> failed due to using the wrong shared libraries for the host, so I
> tried to add sysroot support and I’ve tested it again to make sure gdb
> will use the right shared libraries for the target.
> > I think a better place would be right after spawning
> > gdb (default_gdb_start/default_mi_gdb_start).  If since you
> > already have a board file anyway, you could also override *gdb_start
> > and do it there instead of adding a new flag.
> I agree. It’s better to put this in default_gdb_start/default_mi_gdb_start.
> Since these two functions are not just a few of lines,

You'd override "gdb_start", which is already a hook, not default_gdb_start
itself:

proc gdb_start { } {
    default_gdb_start
}

You can also override big functions in tcl and rename/call
the overridden version, so that the fact that the functions
are big is not a problem, though that's a bit hackish.

> I will suggest adding this support, so it much easier to set sysroot as needed.

Agreed.

> And here is the update according to your suggestion. Thanks.

Thanks.  The tab/space/indenting looked a bit off, not sure
that's just patch/email tricking me, but otherwise looks
good to me.


> - - - -
> 2011-05-06  Wei-cheng Wang <cole945@gmail.com>
> 
>         * lib/gdb.exp (default_gdb_start): Add support for setting sysroot for
>         target system.
>         * lib/mi-support.exp (default_mi_gdb_start): Likewise.
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 20e2fb7..c6fa62d 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -1349,6 +1349,18 @@ proc default_gdb_start { } {
>  	    warning "Couldn't set the width to 0."
>  	}
>      }
> +
> +    if [target_info exists gdb,sysroot] {
> +        send_gdb "set sysroot [target_info gdb,sysroot]\n"
> +        gdb_expect 10 {
> +	    -re "$gdb_prompt $" {
> +	        verbose "Setting sysroot to [target_info gdb,sysroot]" 2
> +	    }
> +	    timeout {
> +                warning "Couldn't set the sysroot to [target_info gdb,sysroot]"
> +	    }
> +        }
> +    }
>      return 0;
>  }
> 
> diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
> index e1dbd19..15565bc 100644
> --- a/gdb/testsuite/lib/mi-support.exp
> +++ b/gdb/testsuite/lib/mi-support.exp
> @@ -221,6 +221,17 @@ proc default_mi_gdb_start { args } {
>  	    }
>      	}
>      }
> +    if [target_info exists gdb,sysroot] {
> +    send_gdb "103-gdb-set sysroot [target_info gdb,sysroot]\n"
> +        gdb_expect 10 {
> +	    -re ".*103-gdb-set sysroot [target_info
> gdb,sysroot]\r\n103\\\^done\r\n$mi_gdb_prompt$" {
> +	        verbose "Setting sysroot to [target_info gdb,sysroot]" 2
> +	    }
> +	    timeout {
> +	        warning "Couldn't set the sysroot to [target_info gdb,sysroot]."
> +	    }
> +        }
> +    }
> 
>      detect_async
> 

-- 
Pedro Alves