[PATCH] interpreter_exec_cmd (interps.c)

[PATCH] interpreter_exec_cmd (interps.c)

[Nick Roberts]

GDB hangs if it receives the previously legitimate command
server interpreter mi "-var-update *"


GNU gdb 6.3.50.20050125-cvs
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "i586-pc-linux-gnu"...Using host libthread_db library "/lib/libthread_db.so.1".

(gdb) server interpreter mi "-var-update *"
^done,changelist=[]
(gdb) 
error in command: "-var-update *".


The patch below seems to fix it. I don't know what values reason takes as
struct exception in exception.h has no commented description, so this patch is
a bit of a guess and may not be the right fix. The bug is real, however.
There seems to be a further bug, in that once GDB has reached the line in
interps.c, 

error ("error in command: \"%s\".", prules[i]),

,presumably it shouldn't hang.


Nick


*** /home/nick/src/gdb/interps.c.~1.10.~	2005-01-16 15:46:17.000000000 +1300
--- /home/nick/src/gdb/interps.c	2005-01-25 22:47:42.000000000 +1300
***************
*** 399,405 ****
    for (i = 1; i < nrules; i++)
      {
        struct exception e = interp_exec (interp_to_use, prules[i]);
!       if (!e.reason)
  	{
  	  interp_set (old_interp);
  	  interp_set_quiet (interp_to_use, old_quiet);
--- 399,405 ----
    for (i = 1; i < nrules; i++)
      {
        struct exception e = interp_exec (interp_to_use, prules[i]);
!       if (e.reason)
  	{
  	  interp_set (old_interp);
  	  interp_set_quiet (interp_to_use, old_quiet);

Re: [PATCH] interpreter_exec_cmd (interps.c)

[Andrew Cagney]

Nick Roberts wrote:
> GDB hangs if it receives the previously legitimate command
> server interpreter mi "-var-update *"
> 
> 
> GNU gdb 6.3.50.20050125-cvs
> Copyright 2004 Free Software Foundation, Inc.
> GDB is free software, covered by the GNU General Public License, and you are
> welcome to change it and/or distribute copies of it under certain conditions.
> Type "show copying" to see the conditions.
> There is absolutely no warranty for GDB.  Type "show warranty" for details.
> This GDB was configured as "i586-pc-linux-gnu"...Using host libthread_db library "/lib/libthread_db.so.1".
> 
> (gdb) server interpreter mi "-var-update *"
> ^done,changelist=[]
> (gdb) 
> error in command: "-var-update *".
> 
> 
> The patch below seems to fix it. I don't know what values reason takes as
> struct exception in exception.h has no commented description, so this patch is
> a bit of a guess and may not be the right fix. The bug is real, however.
> There seems to be a further bug, in that once GDB has reached the line in
> interps.c, 
> 
> error ("error in command: \"%s\".", prules[i]),
> 
> ,presumably it shouldn't hang.

Hmm, another example is:

	(gdb) interpreter console help foo

Can you create a new minimal test "gdb.base/interpreter.exp" to sanity 
check one of the above (and add anything else that takes your fancy :-)

Be sure to check that the test actually fails without the test (and 
don't forget to mention the system it was tested on).   Consider that 
pre-approved.

> 
> 
> *** /home/nick/src/gdb/interps.c.~1.10.~	2005-01-16 15:46:17.000000000 +1300
> --- /home/nick/src/gdb/interps.c	2005-01-25 22:47:42.000000000 +1300
> ***************
> *** 399,405 ****
>     for (i = 1; i < nrules; i++)
>       {
>         struct exception e = interp_exec (interp_to_use, prules[i]);
> !       if (!e.reason)

Oops!  e.reason < 0 indicates an error (the [very old] description of 
enum return_reason hints at this).  With that tweak, and the testsuite, 
this is approved.

good catch,
Andrew

Re: [PATCH] interpreter_exec_cmd (interps.c)

[Nick Roberts]

 > Hmm, another example is:
 > 
 > 	(gdb) interpreter console help foo
 > 
 > Can you create a new minimal test "gdb.base/interpreter.exp" to sanity 
 > check one of the above (and add anything else that takes your fancy :-)
 > 
 > Be sure to check that the test actually fails without the test (and 
 > don't forget to mention the system it was tested on).   Consider that 
 > pre-approved.

I'll have a go shortly. However, I'll post it here to check my own sanity
first. :-)

Nick

Re: [PATCH] interpreter_exec_cmd (interps.c)

[Nick Roberts]

 > Hmm, another example is:
 > 
 > 	(gdb) interpreter console help foo
 > 

Well it needs to be: interpreter console "help foo"

 > Can you create a new minimal test "gdb.base/interpreter.exp" to sanity 
 > check one of the above (and add anything else that takes your fancy :-)
 > 
 > Be sure to check that the test actually fails without the test (and 
 > don't forget to mention the system it was tested on).   Consider that 
 > pre-approved.

Something like below? It might be a good idea to include more of the output
for -var-update but I couldn't work out how to handle the newlines. I chose
'show version' since is a  two word command (= more careful parsing required).
I can't think of any sensible further tests at the moment but its easy to add
more later.

Nick


# Copyright 2005 Free Software Foundation, Inc.

# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
# 
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
# 
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  

# interp.exp   Test interpreter-exec command

if $tracelevel then {
    strace $tracelevel
}

gdb_start

gdb_test "interpreter-exec mi \"-var-update *\"\n" "\\^done,changelist=[].*"
gdb_test "interpreter-exec console \"show version\"\n" "GNU gdb .*"

gdb_exit

Re: [PATCH] interpreter_exec_cmd (interps.c)

[Andrew Cagney]

Nick Roberts wrote:
>  > Hmm, another example is:
>  > 
>  > 	(gdb) interpreter console help foo
>  > 
> 
> Well it needs to be: interpreter console "help foo"
> 
>  > Can you create a new minimal test "gdb.base/interpreter.exp" to sanity 
>  > check one of the above (and add anything else that takes your fancy :-)
>  > 
>  > Be sure to check that the test actually fails without the test (and 
>  > don't forget to mention the system it was tested on).   Consider that 
>  > pre-approved.
> 
> Something like below? It might be a good idea to include more of the output
> for -var-update but I couldn't work out how to handle the newlines. I chose
> 'show version' since is a  two word command (= more careful parsing required).
> I can't think of any sensible further tests at the moment but its easy to add
> more later.

Yep! Perfect!  Just check it in.

> # Copyright 2005 Free Software Foundation, Inc.
> 
> # This program is free software; you can redistribute it and/or modify
> # it under the terms of the GNU General Public License as published by
> # the Free Software Foundation; either version 2 of the License, or
> # (at your option) any later version.
> # 
> # This program is distributed in the hope that it will be useful,
> # but WITHOUT ANY WARRANTY; without even the implied warranty of
> # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> # GNU General Public License for more details.
> # 
> # You should have received a copy of the GNU General Public License
> # along with this program; if not, write to the Free Software
> # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
> 
> # interp.exp   Test interpreter-exec command
> 
> if $tracelevel then {
>     strace $tracelevel
> }
> 
> gdb_start
> 
> gdb_test "interpreter-exec mi \"-var-update *\"\n" "\\^done,changelist=[].*"
> gdb_test "interpreter-exec console \"show version\"\n" "GNU gdb .*"
> 
> gdb_exit
>