Make INTERP_TUI's default ui_out the INTERP_CONSOLE ui_out

Message ID 20190905095815.GA27717@delia
State New, archived
Headers

Commit Message

Tom de Vries Sept. 5, 2019, 9:58 a.m. UTC
  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.

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(-)
  

Comments

Tom de Vries Sept. 24, 2019, 3:24 p.m. UTC | #1
On 05-09-19 11:58, Tom de Vries wrote:
> 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.
> 
> Tested on x86_64-linux.
> 
> OK for trunk?
> 

Ping.

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
>
  
Andrew Burgess Sept. 24, 2019, 5:31 p.m. UTC | #2
* 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
  
Tom de Vries Sept. 24, 2019, 11:20 p.m. UTC | #3
On 24-09-19 19:31, Andrew Burgess wrote:
> 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.

I can't reproduce this. Are you sure you removed the gdb.txt file before
starting gdb?

Thanks,
- Tom
  
Andrew Burgess Sept. 25, 2019, 8:36 a.m. UTC | #4
* Tom de Vries <tdevries@suse.de> [2019-09-25 01:20:22 +0200]:

> On 24-09-19 19:31, Andrew Burgess wrote:
> > 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.
> 
> I can't reproduce this. Are you sure you removed the gdb.txt file before
> starting gdb?

You're absolutely right, nothing goes to the log for `mybt`, this was
a silly mistake on my side.  Apologies.  However...

With tui enabled and logging on and redirect on, the 'bt' __does__
write to the log, while 'mybt' writes to the screen.  This is the same
bug you're fixing for non-tui mode, correct?  Any solution should
ideally address both cases, or at least be accompanied with an
explanation for why these problems are distinct and should be solved
separately.

Thanks,
Andrew


> 
> Thanks,
> - Tom
  

Patch

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