Re: [PATCH][gdb] Make INTERP_TUI's default ui_out the INTERP_CONSOLE ui_out

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Tom de Vries <tdevries@suse.de> [2019-09-05 11:58:22 +0200]:

> Hi,
> 
> When calling the bt command using a user-defined command while logging with
> redirect (escaping '#' using '\' for git commit message convenience):
> ...
> $ gdb -q a.out
> Reading symbols from a.out...
> (gdb) define mybt
> Type commands for definition of "mybt".
> End with a line saying just "end".
> >bt
> >end
> (gdb) set logging redirect on
> (gdb) set logging on
> Redirecting output to gdb.txt.
> Copying debug output to gdb.txt.
> (gdb) start
> (gdb) mybt
> \#0  main () at hello.c:9
> ...
> we get the bt output (the '#0  main () at hello.c:9' above) in the gdb
> output rather than in the log file gdb.txt:
> ...
> $ cat gdb.txt
> Temporary breakpoint 1 at 0x40053b: file hello.c, line 9.
> Starting program: /data/gdb_versions/devel/a.out
> 
> Temporary breakpoint 1, main () at hello.c:9
> 9         printf ("hello\n");
> ...
> 
> If we do the same using "bt" instead of "mybt", the output does end up in the
> log file, and not in the gdb output, as expected.
> 
> Also, if we build gdb with --disable-tui, the problem disappears.
> 
> The problem is caused by:
> - the fact that INTERP_TUI maintains two ui_outs, one for TUI mode disabled
>   (called the default ui_out) and one for TUI mode enabled, combined with
> - the fact that the user-defined commands are interpreted by INTERP_CONSOLE,
>   which has its own ui_out.
> 
> With --disable-tui, the main interpreter is INTERP_CONSOLE, so the logging
> redirect is setup for INTERP_CONSOLE's ui_out, and the redirect has effect
> when interpreting the mybt command.
> 
> With --enable-tui, the main interpreter is INTERP_TUI, so the logging
> redirect is setup for INTERP_TUI's default ui_out, and the redirect has no
> effect when interpreting the mybt command using INTERP_CONSOLE.
> 
> Fix this by making INTERP_TUI's default ui_out the console ui_out.

With this patch applied I see the fixed behaviour you describe at the
CLI, however, if I do:

   (gdb) define mybt
   bt
   end
   (gdb) start
   (gdb) set logging redirect on
   (gdb) tui enable
   (gdb) set logging on
   (gdb) mybt

The I see output appear on both the console and in the log file.  If I
replace mybt with bt then I see the output only go to the log file.
Could this be addressed as part of this patch?

Thanks,
Andrew



> 
> Tested on x86_64-linux.
> 
> OK for trunk?
> 
> Thanks,
> - Tom
> 
> [gdb] Make INTERP_TUI's default ui_out the INTERP_CONSOLE ui_out
> 
> gdb/ChangeLog:
> 
> 2019-09-05  Tom de Vries  <tdevries@suse.de>
> 
> 	PR gdb/24956
> 	* tui/tui-io.c (tui_initialize_io): Make default ui_out the console
> 	ui_out.
> 
> gdb/testsuite/ChangeLog:
> 
> 2019-09-05  Tom de Vries  <tdevries@suse.de>
> 
> 	PR gdb/24956
> 	* gdb.base/ui-redirect.exp: Test output of user-defined command.
> 
> ---
>  gdb/testsuite/gdb.base/ui-redirect.exp | 21 +++++++++++++++++++++
>  gdb/tui/tui-io.c                       |  5 ++++-
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/ui-redirect.exp b/gdb/testsuite/gdb.base/ui-redirect.exp
> index 4507ac51a2..a1a81aecac 100644
> --- a/gdb/testsuite/gdb.base/ui-redirect.exp
> +++ b/gdb/testsuite/gdb.base/ui-redirect.exp
> @@ -47,11 +47,30 @@ if ![runto_main] {
>  gdb_breakpoint "foo"
>  gdb_breakpoint "bar"
>  
> +with_test_prefix "userdefined" {
> +    set test "define userdefined"
> +    gdb_test_multiple $test $test {
> +	-re "End with a line saying just \"end\"\\.\r\n>$" {
> +	    pass $test
> +	}
> +    }
> +
> +    set test "bt"
> +    gdb_test_multiple $test $test {
> +	-re "\r\n>$" {
> +	    pass $test
> +	}
> +    }
> +
> +    gdb_test_no_output "end"
> +}
> +
>  with_test_prefix "logging" {
>      gdb_test_no_output "set logging file /dev/null"
>      gdb_test "set logging on" \
>      "Copying output to /dev/null.*Copying debug output to /dev/null\\."
>      gdb_test "save breakpoints /dev/null" "Saved to file '/dev/null'\\."
> +    gdb_test "userdefined" "#0  main ().*"
>      gdb_test "set logging off" "Done logging to /dev/null\\."
>      gdb_test "help" "List of classes of commands:.*"
>  }
> @@ -61,6 +80,7 @@ with_test_prefix "redirect" {
>      gdb_test "set logging on" \
>      "Redirecting output to /dev/null.*Copying debug output to /dev/null\\."
>      gdb_test_no_output "save breakpoints /dev/null"
> +    gdb_test_no_output "userdefined"
>      gdb_test "set logging off" "Done logging to /dev/null\\."
>      gdb_test "help" "List of classes of commands:.*"
>  }
> @@ -72,6 +92,7 @@ with_test_prefix "redirect while already logging" {
>      gdb_test "set logging redirect on" \
>      ".*warning: Currently logging .*Turn the logging off and on to make the new setting effective.*"
>      gdb_test "save breakpoints /dev/null" "Saved to file '/dev/null'\\."
> +    gdb_test "userdefined" "#0  main ().*"
>      gdb_test "set logging off" "Done logging to /dev/null\\."
>      gdb_test "help" "List of classes of commands:.*"
>      gdb_test_no_output "set logging redirect off"
> diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
> index ee581a2ff6..eab894325a 100644
> --- a/gdb/tui/tui-io.c
> +++ b/gdb/tui/tui-io.c
> @@ -43,6 +43,7 @@
>  #include "gdbsupport/filestuff.h"
>  #include "completer.h"
>  #include "gdb_curses.h"
> +#include "interps.h"
>  #include <map>
>  
>  /* This redefines CTRL if it is not already defined, so it must come
> @@ -870,7 +871,9 @@ tui_initialize_io (void)
>    tui_out = tui_out_new (tui_stdout);
>  
>    /* Create the default UI.  */
> -  tui_old_uiout = cli_out_new (gdb_stdout);
> +  struct interp *interp = interp_lookup (current_ui, "console");
> +  tui_old_uiout = dynamic_cast<cli_ui_out *> (interp->interp_ui_out());
> +  gdb_assert (tui_old_uiout != nullptr);
>  
>  #ifdef TUI_USE_PIPE_FOR_READLINE
>    /* Temporary solution for readline writing to stdout: redirect