python: Use console format for output of gdb.execute command

Message ID 1456756452-15880-1-git-send-email-catalin.udma@freescale.com
State New, archived
Headers

Commit Message

Catalin Udma Feb. 29, 2016, 2:34 p.m. UTC
  When gdb is started in MI mode, the output of gdb.execute
command is in MI-format in case when it is executed from python stop
handler while for all other cases the output is in console-format.

To assure consistent output format, this is fixed by using the console
format for all python gdb command executions.

PR python/19743

2016-02-29  Catalin Udma  <catalin.udma@freescale.com>

       PR python/19743
       * python/python.c (execute_gdb_command): Use console uiout
       when executing gdb command.

2016-02-29  Catalin Udma  <catalin.udma@freescale.com>

       PR python/19743
       * gdb.python/py-mi-events-gdb.py: New file.
       * gdb.python/py-mi-events.c: New file.
       * gdb.python/py-mi-events.exp: New file.

Signed-off-by: Catalin Udma <catalin.udma@freescale.com>
---
 gdb/python/python.c                          |   11 ++++
 gdb/testsuite/gdb.python/py-mi-events-gdb.py |   48 +++++++++++++++
 gdb/testsuite/gdb.python/py-mi-events.c      |   23 +++++++
 gdb/testsuite/gdb.python/py-mi-events.exp    |   84 ++++++++++++++++++++++++++
 4 files changed, 166 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-mi-events-gdb.py
 create mode 100644 gdb/testsuite/gdb.python/py-mi-events.c
 create mode 100644 gdb/testsuite/gdb.python/py-mi-events.exp
  

Comments

Phil Muldoon March 2, 2016, 10:20 p.m. UTC | #1
On 29/02/16 14:34, Catalin Udma wrote:
> When gdb is started in MI mode, the output of gdb.execute
> command is in MI-format in case when it is executed from python stop
> handler while for all other cases the output is in console-format.
> 
> To assure consistent output format, this is fixed by using the console
> format for all python gdb command executions.
> 
> PR python/19743

While I have no problems with the patch, is the expectation that if
the interpreter is in MI mode and a command has previously output that
structured output, isn't the inverse the bug? Shouldn't gdb.execute
honor the interpreter instead of forcing one? I'm not too sure on
this, not being an MI expert, or whether some commands in GDB
arbitrarily output one or the other. I'll dig around when I find some
time and see if I can find anything definitive.

The other thought of course is, if this is right, it arguably
introduces an API regression. So we might have to work with some
compatibility mode for existing scripts.

A tricky bug!

Cheers

Phil
  
Catalin-Dan Udma March 3, 2016, 10:25 a.m. UTC | #2
Thank you, Phil, for review.  I added more info inline.

Regards,
Catalin

> -----Original Message-----
> From: Phil Muldoon [mailto:pmuldoon@redhat.com]
> 
> While I have no problems with the patch, is the expectation that if
> the interpreter is in MI mode and a command has previously output that
> structured output, isn't the inverse the bug? Shouldn't gdb.execute
> honor the interpreter instead of forcing one? 

[Catalin Udma]From GDB manual (7.11.50.20160229, chapter 23.2.2.1 Basic Python):
"gdb.execute (command [, from tty [, to string]]) [Function]
	Evaluate command, a string, as a gdb CLI command"

Therefore a CLI command is expected and the  output should be printed in CLI format
Only the MI commands should be printed on MI format.
 Here some examples (gdb without this patch):

"info break" (CLI command) is always shown in CLI format, even interpreter is MI:
1. 
info break
&"info break\n"
~"Num     Type           Disp Enb Address            What\n"
~"1       breakpoint     keep y   0x000000000000abcd \n"

2
python gdb.execute("info break")
&"python gdb.execute(\"info break\")\n"
~"Num     Type           Disp Enb Address            What\n"
~"1       breakpoint     keep y   0x000000000000abcd \n"


Based on the gdb manual definition, gdb.execute cannot directly execute a MI command, e.g:
python gdb.execute("-break-list")
&"python gdb.execute(\"-break-list\")\n"
&"Traceback (most recent call last):\n"
&"  File \"<string>\", line 1, in <module>\n"
&"gdb.error: Undefined command: \"-break-list\".  Try \"help\".\n"
&"Error while executing Python code.\n"
^error,msg="Error while executing Python code."

This can be done only using gdb.execute("interpreter-exec  mi '-break-list'").
We can have a MI output for MI command, however this (e.g interpreter-exec) it is a CLI command.
This patch does not break the above command: using interpreter-exec + MI command, the
output is MI format.


>I'm not too sure on
> this, not being an MI expert, or whether some commands in GDB
> arbitrarily output one or the other. I'll dig around when I find some
> time and see if I can find anything definitive.
> 
> The other thought of course is, if this is right, it arguably
> introduces an API regression. So we might have to work with some
> compatibility mode for existing scripts.

[Catalin Udma] I expect to have no regression with the existing scripts. But I'm happy to 
take into consideration any use case I have missed and update the patch accordingly. 
By the way, I ran the gdb dejagnu testsuite and found no regression with this patch.

> 
> A tricky bug!
> 
> Cheers
> 
> Phil
  
Pedro Alves March 29, 2016, 4:39 p.m. UTC | #3
Hi Catalin,

On 02/29/2016 02:34 PM, Catalin Udma wrote:
> When gdb is started in MI mode, the output of gdb.execute
> command is in MI-format in case when it is executed from python stop
> handler while for all other cases the output is in console-format.
> 
> To assure consistent output format, this is fixed by using the console
> format for all python gdb command executions.

This seems like the right thing to do.  See comments below.

> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 7202105..8a987f9 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -658,10 +658,18 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
>         /* Copy the argument text in case the command modifies it.  */
>         char *copy = xstrdup (arg);
>         struct cleanup *cleanup = make_cleanup (xfree, copy);
> +      struct ui_out *saved_uiout;
> +      struct interp *interp;
>   
>         make_cleanup_restore_integer (&interpreter_async);
>         interpreter_async = 0;
>   
> +      /* Use the console interpreter uiout to have the same print format
> +	for console or MI.  */
> +      interp = interp_lookup ("console");
> +      saved_uiout = current_uiout;
> +      current_uiout = interp_ui_out (interp);
> +
>         prevent_dont_repeat ();
>         if (to_string)
>   	result = execute_command_to_string (copy, from_tty);
> @@ -671,6 +679,9 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
>   	  execute_command (copy, from_tty);
>   	}
>   
> +      /* Restore the saved uiout.  */
> +      current_uiout = saved_uiout;

This needs to be restored with a cleanup instead.

> +
>         do_cleanups (cleanup);
>       }
>     CATCH (except, RETURN_MASK_ALL)
> diff --git a/gdb/testsuite/gdb.python/py-mi-events-gdb.py b/gdb/testsuite/gdb.python/py-mi-events-gdb.py
> new file mode 100644
> index 0000000..9d0c070
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-mi-events-gdb.py


> +# This file is part of the GDB testsuite. It tests python printing
> +# to string from event handlers.

Double-space after period.

> +
> +
> +import gdb
> +
> +
> +def signal_stop_handler (event):
> +    """Stop event handler"""
> +    if (isinstance (event, gdb.StopEvent)):
> +

OOC, why is this one an isinstance check, while below you
have an assert?

> +def continue_handler (event):
> +    """Continue event handler"""
> +    assert (isinstance (event, gdb.ContinueEvent))

> +int
> +main (void)
> +{
> +  while (1);

Please avoid tests that run forever:

 https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Don.27t_write_tests_that_run_forever

Thanks,
Pedro Alves
  
Catalin-Dan Udma March 31, 2016, 10:42 a.m. UTC | #4
Hi,
I'll fix the comments in patch v2.
Please see my comments inline.

Thank you,
Catalin

> >   	}
> >
> > +      /* Restore the saved uiout.  */
> > +      current_uiout = saved_uiout;
> 
> This needs to be restored with a cleanup instead.
 [Udma Catalin-Dan] I did not find a cleanup used for struct ui_out. For this purpose
I'll add a new cleanup handler for it in gdb/utils.c and may be re-used in other locations.
  

Patch

diff --git a/gdb/python/python.c b/gdb/python/python.c
index 7202105..8a987f9 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -658,10 +658,18 @@  execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
       /* Copy the argument text in case the command modifies it.  */
       char *copy = xstrdup (arg);
       struct cleanup *cleanup = make_cleanup (xfree, copy);
+      struct ui_out *saved_uiout;
+      struct interp *interp;
 
       make_cleanup_restore_integer (&interpreter_async);
       interpreter_async = 0;
 
+      /* Use the console interpreter uiout to have the same print format
+	for console or MI.  */
+      interp = interp_lookup ("console");
+      saved_uiout = current_uiout;
+      current_uiout = interp_ui_out (interp);
+
       prevent_dont_repeat ();
       if (to_string)
 	result = execute_command_to_string (copy, from_tty);
@@ -671,6 +679,9 @@  execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
 	  execute_command (copy, from_tty);
 	}
 
+      /* Restore the saved uiout.  */
+      current_uiout = saved_uiout;
+
       do_cleanups (cleanup);
     }
   CATCH (except, RETURN_MASK_ALL)
diff --git a/gdb/testsuite/gdb.python/py-mi-events-gdb.py b/gdb/testsuite/gdb.python/py-mi-events-gdb.py
new file mode 100644
index 0000000..9d0c070
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-mi-events-gdb.py
@@ -0,0 +1,48 @@ 
+# Copyright (C) 2016 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 3 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, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite. It tests python printing 
+# to string from event handlers.
+
+
+import gdb
+
+
+def signal_stop_handler (event):
+    """Stop event handler"""
+    if (isinstance (event, gdb.StopEvent)):
+        print ("stop_handler")
+        print gdb.execute("info break", False, True)
+
+
+def continue_handler (event):
+    """Continue event handler"""
+    assert (isinstance (event, gdb.ContinueEvent))
+    print ("continue_handler")
+    print gdb.execute("info break", False, True)
+
+
+class test_events (gdb.Command):
+    """Test events."""
+
+    def __init__ (self):
+        gdb.Command.__init__ (self, "test-events", gdb.COMMAND_STACK)
+
+    def invoke (self, arg, from_tty):
+        gdb.events.stop.connect (signal_stop_handler)
+        gdb.events.cont.connect (continue_handler)
+        print ("Event testers registered.")
+
+test_events ()
diff --git a/gdb/testsuite/gdb.python/py-mi-events.c b/gdb/testsuite/gdb.python/py-mi-events.c
new file mode 100644
index 0000000..3299e58
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-mi-events.c
@@ -0,0 +1,23 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 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 3 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, see  <http://www.gnu.org/licenses/>.  */
+
+int
+main (void)
+{
+  while (1);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-mi-events.exp b/gdb/testsuite/gdb.python/py-mi-events.exp
new file mode 100644
index 0000000..4c78b9c
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-mi-events.exp
@@ -0,0 +1,84 @@ 
+# Copyright (C) 2008-2016 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 3 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, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite.  It tests PR 19743.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi2"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile
+set pyfile ${testfile}-gdb.py
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested ${testfile}.exp
+    return -1
+}
+
+if { [mi_skip_python_tests] } { continue }
+
+set remote_python_file [gdb_remote_download host ${srcdir}/${subdir}/${pyfile}]
+
+mi_delete_breakpoints
+mi_gdb_reinitialize_dir $srcdir/$subdir
+mi_gdb_test "set auto-load safe-path ${remote_python_file}" \
+    {.*\^done} \
+    "set safe-path"
+
+if [is_remote host] {
+    set filename ${testfile}
+    remote_download host ${binfile} ${filename}
+} else {
+    set filename ${binfile}
+}
+
+mi_gdb_test "-file-exec-and-symbols ${filename}"  ".*\\^done" "file-exec-and-symbols operation"
+mi_run_to_main
+
+
+# register the python event handlers with test-events command
+mi_gdb_test "test-events" \
+    ".*~\"Event testers registered.*\\^done" \
+    "register events"
+
+
+# set a breakpoint to while (1) loop
+mi_gdb_test "break ${srcfile}:[gdb_get_line_number "while (1)"]" \
+    ".*Breakpoint $decimal at 0x\[0-9a-fA-F\]+: file .*${srcfile}.*\\\.*\\^done" \
+    "set the breakpoint"
+
+
+# resume the program
+mi_send_resuming_command "exec-continue" "continue"
+
+
+# test the python event handlers execution. The following checks are performed:
+# - python continue handler is executed
+# - the continue handler prints "info breakpoints" output in console format
+# - breakpoint is hit and python stop handler is executed
+# - the stop handler prints "info breakpoints" output in console format
+mi_gdb_test "" ".*continue_handler.*
+.*Num.*Type.*Disp.*Enb.*Address.*\
+.*$decimal.*breakpoint.*keep.*y.* 0x\[0-9a-fA-F\]+.*${srcfile}.*
+.*stop_handler.*
+.*Num.*Type.*Disp.*Enb.*Address.*\
+.*$decimal.*breakpoint.*keep.*y.* 0x\[0-9a-fA-F\]+.*${srcfile}.*" \
+"check python continue and stop handlers"
+
+mi_gdb_exit