[PATCH][gdb] Only force INTERP_CONSOLE ui_out for breakpoint commands in MI mode

[Tom de Vries <tdevries@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]

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

On 01-10-19 14:59, Tom de Vries wrote:
> On 25-09-19 10:36, Andrew Burgess wrote:
>> * 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?
> 
> From user perspective, yes.
> 
>> 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.
>>
> 
> Ideally yes, agreed.
> 
> This tentative patch fixes both cases, by limiting the fix of commit
> 3a87ae656c28 "Use console uiout when executing breakpoint commands" to
> the case for which the corresponding problem was reported: INTERP_MI. [
> Adding CC simark ]
> 
> At this point I don't understand yet why special-casing INTERP_MI would
> be the correct thing to do here.

I've investigated a bit further the problem that commit 3a87ae656c28
fixes, and constructed a hopefully convincing rationale.

OK for trunk?

Thanks,
- Tom


[-- Attachment #2: 0001-gdb-Only-force-INTERP_CONSOLE-ui_out-for-breakpoint-commands-in-MI-mode.patch --]
[-- Type: text/x-patch, Size: 5455 bytes --]

[gdb] Only force INTERP_CONSOLE ui_out for breakpoint commands in MI mode

The problem reported in PR mi/25055 is that the output of the backtrace
command, when executed as breakpoint command does not show when executing
using the MI interpreter:
...
$ gdb a.out
Reading symbols from a.out...
(gdb) break main
Breakpoint 1 at 0x4003c0: file test.c, line 19.
(gdb) commands
Type commands for breakpoint(s) 1, one per line.
End with a line saying just "end".
>bt
>end
(gdb) interpreter-exec mi "-exec-run"
^done

Breakpoint 1, main () at test.c:19
19        return foo (4);
(gdb)
...

Interestingly, the function print_frame is called twice during -exec-run:
- once during tui_on_normal_stop where the ui_out is temporarily set to
  tui->interp_ui_out (), resulting in the part after the comma in
  "Breakpoint 1, main () at test.c:19"
- once during execute_control_command, where the ui_out is the default for the
  current interpreter: mi_ui_out, which ignores calls to output text.

The commit 3a87ae656c2 "Use console uiout when executing breakpoint commands"
fixes the problem by temporarily switching to the ui_out of INTERP_CONSOLE in
execute_control_command.

This however caused a regression in redirection (escaping '#' using '\' for
git commit message convenience):
...
$ rm -f gdb.txt; gdb a.out
Reading symbols from a.out...
(gdb) break main
Breakpoint 1 at 0x4003c0: file test.c, line 19.
(gdb) commands
Type commands for breakpoint(s) 1, one per line.
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) run
\#0  main () at test.c:19
(gdb) q
A debugging session is active.

        Inferior 1 [process 22428] will be killed.

Quit anyway? (y or n) y
$ cat gdb.txt
Starting program: /data/gdb_versions/devel/a.out

Breakpoint 1, main () at test.c:19
19        return foo (4);
...

The problem is that the '#0  main () at test.c:19' ends up in the gdb output
output rather than in gdb.txt.  This is due to the fact that the redirect is
setup for the current ui_out (which is tui->interp_ui_out ()), while the
backtrace output is printed to the INTERP_CONSOLE ui_out.

Fix this by limiting switching to INTERP_CONSOLE ui_out to when INTERP_MI is
active.

Tested on x86_64-linux.

gdb/ChangeLog:

2019-10-02  Tom de Vries  <tdevries@suse.de>

	PR gdb/24956
	* cli/cli-script.c (execute_control_command): Only switch to
	INTERP_CONSOLE's ui_out when INTERP_MI is active.

gdb/testsuite/ChangeLog:

2019-10-02  Tom de Vries  <tdevries@suse.de>

	PR gdb/24956
	* gdb.base/ui-redirect.exp: Test output of user-defined command.

---
 gdb/cli/cli-script.c                   |  3 +++
 gdb/testsuite/gdb.base/ui-redirect.exp | 21 +++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 4fc9c70259c..706876a8fee 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -697,6 +697,9 @@ execute_control_command_1 (struct command_line *cmd, int from_tty)
 enum command_control_type
 execute_control_command (struct command_line *cmd, int from_tty)
 {
+  if (!current_interp_named_p (INTERP_MI))
+    return execute_control_command_1 (cmd, from_tty);
+
   /* Make sure we use the console uiout.  It's possible that we are executing
      breakpoint commands while running the MI interpreter.  */
   interp *console = interp_lookup (current_ui, INTERP_CONSOLE);
diff --git a/gdb/testsuite/gdb.base/ui-redirect.exp b/gdb/testsuite/gdb.base/ui-redirect.exp
index efcac14b7cc..932c74af4b5 100644
--- a/gdb/testsuite/gdb.base/ui-redirect.exp
+++ b/gdb/testsuite/gdb.base/ui-redirect.exp
@@ -57,6 +57,24 @@ set cmds "$cmds\n"
 set outdir [standard_output_file {}]
 set cmds_file "$outdir/cmds.txt"
 
+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" \
@@ -64,6 +82,7 @@ with_test_prefix "logging" {
     gdb_test "save breakpoints $cmds_file" "Saved to file '$cmds_file'\\." \
 	"save breakpoints cmds.txt"
     cmp_file_string "$cmds_file" "$cmds" "cmds.txt"
+    gdb_test "userdefined" "#0  main ().*"
     gdb_test "set logging off" "Done logging to /dev/null\\."
     gdb_test "help" "List of classes of commands:.*"
 }
@@ -74,6 +93,7 @@ with_test_prefix "redirect" {
     "Redirecting output to /dev/null.*Copying debug output to /dev/null\\."
     gdb_test_no_output "save breakpoints $cmds_file" "save breakpoints cmds.txt"
     cmp_file_string "$cmds_file" "$cmds" "cmds.txt"
+    gdb_test_no_output "userdefined"
     gdb_test "set logging off" "Done logging to /dev/null\\."
     gdb_test "help" "List of classes of commands:.*"
 }
@@ -87,6 +107,7 @@ with_test_prefix "redirect while already logging" {
     gdb_test "save breakpoints $cmds_file" "Saved to file '$cmds_file'\\." \
 	"save breakpoints cmds.txt"
     cmp_file_string "$cmds_file" "$cmds" "cmds.txt"
+    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"