Patchwork [4/6] testsuite: Don't use expect_background to reap gdbserver

login
register
mail settings
Submitter Yao Qi
Date April 13, 2015, 11:42 a.m.
Message ID <864mokuuep.fsf@gmail.com>
Download mbox | patch
Permalink /patch/6178/
State New
Headers show

Comments

Yao Qi - April 13, 2015, 11:42 a.m.
Pedro Alves <palves@redhat.com> writes:

> +proc gdb_exit {} {
> +    global gdb_spawn_id server_spawn_id
> +    global gdb_prompt
> +
> +    if {[info exists gdb_spawn_id] && [info exists server_spawn_id]} {
> +	send_gdb "monitor exit\n";
> +	gdb_expect {
> +	    -re "$gdb_prompt $" {
> +		exp_continue
> +	    }
> +	    -i "$server_spawn_id" eof {
> +		wait -i $expect_out(spawn_id)
> +		unset server_spawn_id
> +	    }
> +	}
> +    }

Do we need to catch exception here?

I see the error when I run gdb-sigterm.exp with native-gdbserver
on x86_64-linux.

infrun: prepare_to_wait^M
Cannot execute this command while the target is running.^M
Use the "interrupt" command to stop the target^M
and then try again.^M
gdb.base/gdb-sigterm.exp: expect eof #0: got eof
gdb.base/gdb-sigterm.exp: expect eof #0: stepped 12 times
ERROR OCCURED: : spawn id exp8 not open
    while executing
"expect {
-i exp8 -timeout 10
            -re "$gdb_prompt $" {
                exp_continue
            }
            -i "$server_spawn_id" eof {
                wait -i $expect_out(spawn_id)
                unse..."
    ("uplevel" body line 1)
    invoked from within

In gdb-sigterm.exp, SIGTERM is sent to GDB and it exits.  However,
Dejagnu or tcl doesn't know this.


> +    close_gdbserver
> +
> +    gdbserver_orig_gdb_exit
> +}

This error terminates the whole testing, so the following tests are
not run.

I wrap the send_gdb and gdb_expect statement above by "catch",
testing looks fine, although error messages are still shown in the
console and gdb.log.
Pedro Alves - April 13, 2015, 12:09 p.m.
On 04/13/2015 12:42 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> +proc gdb_exit {} {
>> +    global gdb_spawn_id server_spawn_id
>> +    global gdb_prompt
>> +
>> +    if {[info exists gdb_spawn_id] && [info exists server_spawn_id]} {
>> +	send_gdb "monitor exit\n";
>> +	gdb_expect {
>> +	    -re "$gdb_prompt $" {
>> +		exp_continue
>> +	    }
>> +	    -i "$server_spawn_id" eof {
>> +		wait -i $expect_out(spawn_id)
>> +		unset server_spawn_id
>> +	    }
>> +	}
>> +    }
> 
> Do we need to catch exception here?

Whoops, yes, looks like it.

> I wrap the send_gdb and gdb_expect statement above by "catch",
> testing looks fine, although error messages are still shown in the
> console and gdb.log.

Why not suppress the error message?  I think you just need to pass
a var name as second parameter to "catch".

Thanks,
Pedro Alves
Yao Qi - April 13, 2015, 1:25 p.m.
On 13/04/15 13:09, Pedro Alves wrote:
>> I wrap the send_gdb and gdb_expect statement above by "catch",
>> >testing looks fine, although error messages are still shown in the
>> >console and gdb.log.
> Why not suppress the error message?  I think you just need to pass
> a var name as second parameter to "catch".

I did that, but it is useless.  These messages prefixed with
"ERROR OCCURED:" are printed by DejaGNU, lib/remote.exp:remote_expect,

     if {$code == 1} {
         if {[info exists string]} {send_user "ERROR OCCURED: $errorInfo 
$errorCode $string"}

looks we can't prevent DejaGNU invoking send_user.  If this error is
annoying, we can unset gdb_spawn_id at the end of proc do_test in
gdb-sigterm.exp.
Pedro Alves - April 13, 2015, 1:51 p.m.
On 04/13/2015 02:25 PM, Yao Qi wrote:
> On 13/04/15 13:09, Pedro Alves wrote:
>>> I wrap the send_gdb and gdb_expect statement above by "catch",
>>>> testing looks fine, although error messages are still shown in the
>>>> console and gdb.log.
>> Why not suppress the error message?  I think you just need to pass
>> a var name as second parameter to "catch".
> 
> I did that, but it is useless.  These messages prefixed with
> "ERROR OCCURED:" are printed by DejaGNU, lib/remote.exp:remote_expect,
> 
>      if {$code == 1} {
>          if {[info exists string]} {send_user "ERROR OCCURED: $errorInfo 
> $errorCode $string"}
> 
> looks we can't prevent DejaGNU invoking send_user.

I think we should just call raw "expect" instead then.

> If this error is
> annoying, we can unset gdb_spawn_id at the end of proc do_test in
> gdb-sigterm.exp.

I think also need to call wait too?  There are other eof calls in other
tests too and also under lib/  We'd need to do the same to all of those,
and then, at least, we'd need to make default_gdb_exit not skip the
inotify_log_file code too.  I'm not sure I like that direction.

(BTW, that remote host code in default_gdb_exit looks like should be
given the same treatment.)

Thanks,
Pedro Alves

Patch

diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 53843b8..8d4858a 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -353,15 +353,20 @@  proc gdb_exit {} {
     global gdb_prompt
 
     if {[info exists gdb_spawn_id] && [info exists server_spawn_id]} {
-	send_gdb "monitor exit\n";
-	gdb_expect {
-	    -re "$gdb_prompt $" {
-		exp_continue
-	    }
-	    -i "$server_spawn_id" eof {
-		wait -i $expect_out(spawn_id)
-		unset server_spawn_id
-		unset inferior_spawn_id
+	# GDB may be terminated in an expected way or an unexpected way,
+	# but DejaGNU doesn't know that, so gdb_spawn_id isn't unset.
+	# Catch the exceptions.
+	catch {
+	    send_gdb "monitor exit\n";
+	    gdb_expect {
+		-re "$gdb_prompt $" {
+		    exp_continue
+		}
+		-i "$server_spawn_id" eof {
+		    wait -i $expect_out(spawn_id)
+		    unset server_spawn_id
+		    unset inferior_spawn_id
+		}
 	    }
 	}
     }