[PATCH] gdb: Don't change the uiout to console one when doing gdb.execute in TUI

[PATCH] gdb: Don't change the uiout to console one when doing gdb.execute in TUI

[Marco Barisione]

The implementation of the gdb.execute Python function must have the same
print format for console or MI, which is achieved by temporarily
changing current_uiout to the console interpreter's one.

This was previously done also in TUI mode, but that breaks the output.
For instance:

    (gdb) # This is fine.
    (gdb) backtrace
    #0  main () at foo.c:42
    (gdb) # This produces the same output as backtrace, including ANSI
    (gdb) # control escapes to style it.
    (gdb) python gdb.execute("backtrace")
    #0  main () at foo.c:42
    (gdb) tui enable
    (gdb) # This is still fine.
    (gdb) backtrace
    #0  main () at foo.c:42
    (gdb) # This is not, the output ends up on the wrong line and it's
    (gdb) # not styled.
    (gdb) python gdb.execute("backtrace")#0  main () at foo.c:42

    (gdb)

This fix changes gdb.execute to only change current_uiout while in MI.

gdb/ChangeLog:

	* python/python.c (execute_gdb_command): Change currently_uiout
	only when in MI to avoid breaking TUI mode.

gdb/testsuite/ChangeLog:

	* gdb.python/py-execute-in-tui.c: New test.
	* gdb.python/py-execute-in-tui.exp: New test.
---
 gdb/python/python.c                           | 12 ++-
 gdb/testsuite/gdb.python/py-execute-in-tui.c  | 22 ++++++
 .../gdb.python/py-execute-in-tui.exp          | 74 +++++++++++++++++++
 3 files changed, 104 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-execute-in-tui.c
 create mode 100644 gdb/testsuite/gdb.python/py-execute-in-tui.exp

diff --git a/gdb/python/python.c b/gdb/python/python.c
index 4cea83c3837..881dd1275f4 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -630,10 +630,14 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
 
 	scoped_restore save_uiout = make_scoped_restore (&current_uiout);
 
-	/* Use the console interpreter uiout to have the same print format
-	   for console or MI.  */
-	interp = interp_lookup (current_ui, "console");
-	current_uiout = interp->interp_ui_out ();
+	/* When in MI, use the console interpreter uiout to have the same
+	   print format as console.  It's important the uiout is not
+	   changed in TUI mode, otherwise the output can get garbled.  */
+	if (current_uiout->is_mi_like_p ())
+	  {
+	    interp = interp_lookup (current_ui, INTERP_CONSOLE);
+	    current_uiout = interp->interp_ui_out ();
+	  }
 
 	if (to_string)
 	  to_string_res = execute_control_commands_to_string (lines.get (),
diff --git a/gdb/testsuite/gdb.python/py-execute-in-tui.c b/gdb/testsuite/gdb.python/py-execute-in-tui.c
new file mode 100644
index 00000000000..b642c3d3ac7
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-execute-in-tui.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017-2021 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 ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-execute-in-tui.exp b/gdb/testsuite/gdb.python/py-execute-in-tui.exp
new file mode 100644
index 00000000000..de033aee7d6
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-execute-in-tui.exp
@@ -0,0 +1,74 @@
+# Copyright 2019-2021 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/>.
+
+# Test that gdb.execute produces the correct output when used in TUI
+# mode.
+#
+# To have the same print format whether GDB is using MI or console,
+# GDB must switch current_uiout to INTERP_CONSOLE while doing a
+# gdb.execute in MI.
+# Previously, this was done unconditionally, even in TUI mode, producing
+# garbled and unstyled output.  As the garbled output is difficult to
+# detect in tests, we instead check for the ANSI escape code which styles
+# output.
+
+tuiterm_env
+
+# Prepare GDB by loading (not in TUI mode) a trivial program and stopping
+# at the beginning of main.
+
+standard_testfile .c
+
+if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} {
+  return -1
+}
+
+Term::clean_restart 24 80 $testfile
+
+if ![runto_main] then {
+  fail "can't run to main"
+  return
+}
+
+# A backtrace executed via gdb.execute must produce something like this:
+#     #0  main () at foo.c:42
+# Where the function name is styled.  "\x1b" is the start of an ANSI
+# escape sequence to style the function name.
+
+set gdb_execute_backtrace "python gdb.execute ('backtrace')"
+set expected_backtrace_styled_output ".*#0.*\x1b.*main.*"
+
+gdb_test $gdb_execute_backtrace $expected_backtrace_styled_output \
+  "backtrace from gdb.execute without TUI"
+
+# After entering TUI mode, the behaviour must remain the same.
+
+if {![Term::enter_tui]} {
+  unsupported "TUI not supported"
+  return
+}
+
+# We can't use Term::command and Term::check_contents as that drops the
+# ANSI escape codes.
+send_gdb "${gdb_execute_backtrace}\n"
+set test_name "backtrace from gdb.execute with TUI"
+gdb_expect {
+  -re $expected_backtrace_styled_output {
+    pass $test_name
+  }
+  timeout {
+    fail $test_name
+  }
+}
-- 
2.28.0

Re: [PATCH] gdb: Don't change the uiout to console one when doing gdb.execute in TUI

[Keith Seitz]

On 5/20/21 8:37 AM, Marco Barisione via Gdb-patches wrote:
> 
> This fix changes gdb.execute to only change current_uiout while in MI.
> 

This seems reasonable to me, but please wait for a maintainer's approval.
I have just a few minor comments.

> gdb/ChangeLog:
> 
> 	* python/python.c (execute_gdb_command): Change currently_uiout
> 	only when in MI to avoid breaking TUI mode.

s/currently/current/ ?

> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.python/py-execute-in-tui.c: New test.
> 	* gdb.python/py-execute-in-tui.exp: New test.
> ---
>  gdb/python/python.c                           | 12 ++-
>  gdb/testsuite/gdb.python/py-execute-in-tui.c  | 22 ++++++
>  .../gdb.python/py-execute-in-tui.exp          | 74 +++++++++++++++++++
>  3 files changed, 104 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.python/py-execute-in-tui.c
>  create mode 100644 gdb/testsuite/gdb.python/py-execute-in-tui.exp
> 
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 4cea83c3837..881dd1275f4 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -630,10 +630,14 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
>  
>  	scoped_restore save_uiout = make_scoped_restore (&current_uiout);
>  
> -	/* Use the console interpreter uiout to have the same print format
> -	   for console or MI.  */
> -	interp = interp_lookup (current_ui, "console");
> -	current_uiout = interp->interp_ui_out ();
> +	/* When in MI, use the console interpreter uiout to have the same
> +	   print format as console.  It's important the uiout is not
> +	   changed in TUI mode, otherwise the output can get garbled.  */

I appreciate the comment!

> +	if (current_uiout->is_mi_like_p ())
> +	  {
> +	    interp = interp_lookup (current_ui, INTERP_CONSOLE);
> +	    current_uiout = interp->interp_ui_out ();
> +	  }
>  
>  	if (to_string)
>  	  to_string_res = execute_control_commands_to_string (lines.get (),
> diff --git a/gdb/testsuite/gdb.python/py-execute-in-tui.exp b/gdb/testsuite/gdb.python/py-execute-in-tui.exp
> new file mode 100644
> index 00000000000..de033aee7d6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-execute-in-tui.exp
> @@ -0,0 +1,74 @@

I realize this file is copied from another test, however, in the interests
of keeping the code clean(er)...

> +if ![runto_main] then {
> +  fail "can't run to main"
> +  return
> +}

Please maintain a consistent Tcl style here. [Again, I realize this is
copy/paste.] The canonical way to do `if' statements in modern Tcl is

    if {cond} {
    }

[Modern Tcl? Oxymoron? :-)]

> +
> +# A backtrace executed via gdb.execute must produce something like this:
> +#     #0  main () at foo.c:42
> +# Where the function name is styled.  "\x1b" is the start of an ANSI
> +# escape sequence to style the function name.
> +
> +set gdb_execute_backtrace "python gdb.execute ('backtrace')"
> +set expected_backtrace_styled_output ".*#0.*\x1b.*main.*"
> +
> +gdb_test $gdb_execute_backtrace $expected_backtrace_styled_output \
> +  "backtrace from gdb.execute without TUI"
> +
> +# After entering TUI mode, the behaviour must remain the same.
> +
> +if {![Term::enter_tui]} {
> +  unsupported "TUI not supported"
> +  return
> +}
> +
> +# We can't use Term::command and Term::check_contents as that drops the
> +# ANSI escape codes.
> +send_gdb "${gdb_execute_backtrace}\n"
> +set test_name "backtrace from gdb.execute with TUI"
> +gdb_expect {
> +  -re $expected_backtrace_styled_output {
> +    pass $test_name
> +  }
> +  timeout {
> +    fail $test_name
> +  }
> +}
> 

We normally use gdb_test instead of gdb_send/gdb_expect. Is it not possible here?

Keith

Re: [PATCH] gdb: Don't change the uiout to console one when doing gdb.execute in TUI

[Marco Barisione]

On 24 May 2021, at 19:25, Keith Seitz <keiths@redhat.com> wrote:
>> +# We can't use Term::command and Term::check_contents as that drops the
>> +# ANSI escape codes.
>> +send_gdb "${gdb_execute_backtrace}\n"
>> +set test_name "backtrace from gdb.execute with TUI"
>> +gdb_expect {
>> +  -re $expected_backtrace_styled_output {
>> +    pass $test_name
>> +  }
>> +  timeout {
>> +    fail $test_name
>> +  }
>> +}
>> 
> 
> We normally use gdb_test instead of gdb_send/gdb_expect. Is it not possible here?

I couldn’t get it to work in TUI mode and I couldn’t understand why (but
I’m not an expert in TCL or how printing works in TUI mode).  I will try
again as the test code is now simpler than it used to be while I was
developing the test and, if I cannot get gdb_test to work, I will add
a comment.

-- 
Marco Barisione

Re: [PATCH] gdb: Don't change the uiout to console one when doing gdb.execute in TUI

[Keith Seitz]

On 5/25/21 12:47 AM, Marco Barisione wrote:
> On 24 May 2021, at 19:25, Keith Seitz <keiths@redhat.com> wrote:
>>
>> We normally use gdb_test instead of gdb_send/gdb_expect. Is it not possible here?
> 
> I couldn’t get it to work in TUI mode and I couldn’t understand why (but
> I’m not an expert in TCL or how printing works in TUI mode).  I will try
> again as the test code is now simpler than it used to be while I was
> developing the test and, if I cannot get gdb_test to work, I will add
> a comment.

That would be great. Thank you!

Keith

[PATCH v2] gdb: Don't change the uiout to console one when doing gdb.execute in TUI

[Marco Barisione]

The implementation of the gdb.execute Python function must have the same
print format for console or MI, which is achieved by temporarily
changing current_uiout to the console interpreter's one.

This was previously done also in TUI mode, but that breaks the output.
For instance:

    (gdb) # This is fine.
    (gdb) backtrace
    #0  main () at foo.c:42
    (gdb) # This produces the same output as backtrace, including ANSI
    (gdb) # control escapes to style it.
    (gdb) python gdb.execute("backtrace")
    #0  main () at foo.c:42
    (gdb) tui enable
    (gdb) # This is still fine.
    (gdb) backtrace
    #0  main () at foo.c:42
    (gdb) # This is not, the output ends up on the wrong line and it's
    (gdb) # not styled.
    (gdb) python gdb.execute("backtrace")#0  main () at foo.c:42

    (gdb)

This fix changes gdb.execute to only change current_uiout while in MI.

gdb/ChangeLog:

	* python/python.c (execute_gdb_command): Change current_uiout
	only when in MI to avoid breaking TUI mode.

gdb/testsuite/ChangeLog:

	* gdb.python/py-execute-in-tui.c: New test.
	* gdb.python/py-execute-in-tui.exp: New test.
---
 gdb/python/python.c                           | 12 ++-
 gdb/testsuite/gdb.python/py-execute-in-tui.c  | 22 ++++++
 .../gdb.python/py-execute-in-tui.exp          | 76 +++++++++++++++++++
 3 files changed, 106 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-execute-in-tui.c
 create mode 100644 gdb/testsuite/gdb.python/py-execute-in-tui.exp

diff --git a/gdb/python/python.c b/gdb/python/python.c
index 4cea83c3837..881dd1275f4 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -630,10 +630,14 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
 
 	scoped_restore save_uiout = make_scoped_restore (&current_uiout);
 
-	/* Use the console interpreter uiout to have the same print format
-	   for console or MI.  */
-	interp = interp_lookup (current_ui, "console");
-	current_uiout = interp->interp_ui_out ();
+	/* When in MI, use the console interpreter uiout to have the same
+	   print format as console.  It's important the uiout is not
+	   changed in TUI mode, otherwise the output can get garbled.  */
+	if (current_uiout->is_mi_like_p ())
+	  {
+	    interp = interp_lookup (current_ui, INTERP_CONSOLE);
+	    current_uiout = interp->interp_ui_out ();
+	  }
 
 	if (to_string)
 	  to_string_res = execute_control_commands_to_string (lines.get (),
diff --git a/gdb/testsuite/gdb.python/py-execute-in-tui.c b/gdb/testsuite/gdb.python/py-execute-in-tui.c
new file mode 100644
index 00000000000..b642c3d3ac7
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-execute-in-tui.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017-2021 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 ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-execute-in-tui.exp b/gdb/testsuite/gdb.python/py-execute-in-tui.exp
new file mode 100644
index 00000000000..56c5310e75c
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-execute-in-tui.exp
@@ -0,0 +1,76 @@
+# Copyright 2019-2021 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/>.
+
+# Test that gdb.execute produces the correct output when used in TUI
+# mode.
+#
+# To have the same print format whether GDB is using MI or console,
+# GDB must switch current_uiout to INTERP_CONSOLE while doing a
+# gdb.execute in MI.
+# Previously, this was done unconditionally, even in TUI mode, producing
+# garbled and unstyled output.  As the garbled output is difficult to
+# detect in tests, we instead check for the ANSI escape code which styles
+# output.
+
+tuiterm_env
+
+# Prepare GDB by loading (not in TUI mode) a trivial program and stopping
+# at the beginning of main.
+
+standard_testfile .c
+
+if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} {
+  return -1
+}
+
+Term::clean_restart 24 80 $testfile
+
+if {![runto_main]} {
+  fail "can't run to main"
+  return
+}
+
+# A backtrace executed via gdb.execute must produce something like this:
+#     #0  main () at foo.c:42
+# Where the function name is styled.  "\x1b" is the start of an ANSI
+# escape sequence to style the function name.
+
+set gdb_execute_backtrace "python gdb.execute ('backtrace')"
+set expected_backtrace_styled_output ".*#0.*\x1b.*main.*"
+
+gdb_test $gdb_execute_backtrace $expected_backtrace_styled_output \
+  "backtrace from gdb.execute without TUI"
+
+# After entering TUI mode, the behaviour must remain the same.
+
+if {![Term::enter_tui]} {
+  unsupported "TUI not supported"
+  return
+}
+
+# We can't use Term::command and Term::check_contents as that drops the
+# ANSI escape codes.
+# gdb_test doesn't seem to work correctly while in TUI mode, so we use
+# the lower level send_gdb and gdb_expect functions.
+send_gdb "${gdb_execute_backtrace}\n"
+set test_name "backtrace from gdb.execute with TUI"
+gdb_expect {
+  -re $expected_backtrace_styled_output {
+    pass $test_name
+  }
+  timeout {
+    fail $test_name
+  }
+}
-- 
2.28.0