]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
gdb/tui: use wrefresh if output is not surpressed
authorAndrew Burgess <aburgess@redhat.com>
Wed, 5 Feb 2025 20:12:03 +0000 (20:12 +0000)
committerAndrew Burgess <aburgess@redhat.com>
Sat, 8 Feb 2025 19:43:42 +0000 (19:43 +0000)
Recent work in the TUI has improved GDB's use of the curses
wnoutrefresh and doupdate mechanism, which improves performance by
batching together updates and then doing a single set of writes to the
screen when doupdate is finally called.

The tui_batch_rendering type is a RAII class which, in its destructor,
calls doupdate to send the batched updates to the screen.

However, if there is no tui_batch_rendering active on the call stack
then any wnoutrefresh calls will remain batched but undisplayed until
the next time doupdate happens to be called.

This problem can be seen in PR gdb/32623.  When an inferior is started
the 'Starting program' message is not immediately displayed to the
user.

The 'Starting program' message originates from run_command_1 in
infcmd.c, the message is sent to the current_uiout, which will be the
TUI ui_out.  After the message is sent, ui_out::flush() is called,
here's the backtrace when that happens:

  #0  tui_file::flush (this=0x36e4ab0) at ../../src/gdb/tui/tui-file.c:42
  #1  0x0000000001004f4b in pager_file::flush (this=0x36d35f0) at ../../src/gdb/utils.c:1531
  #2  0x0000000001004f71 in gdb_flush (stream=0x36d35f0) at ../../src/gdb/utils.c:1539
  #3  0x00000000006975ab in cli_ui_out::do_flush (this=0x35a50b0) at ../../src/gdb/cli-out.c:250
  #4  0x00000000009fd1f9 in ui_out::flush (this=0x35a50b0) at ../../src/gdb/ui-out.h:263
  #5  0x00000000009f56ad in run_command_1 (args=0x0, from_tty=1, run_how=RUN_NORMAL) at ../../src/gdb/infcmd.c:449
  #6  0x00000000009f599a in run_command (args=0x0, from_tty=1) at ../../src/gdb/infcmd.c:511

And if we check out tui_file::flush (tui-file.c) we can see that this
just calls tui_win_info::refresh_window(), which in turn, just uses
wnoutrefresh to batch any pending output.

The problem is that, in the above backtrace, there is no
tui_batch_rendering active, and so there will be no doupdate call to
flush the output to the screen.

We could add a tui_batch_rendering into tui_file::flush.  And
tui_file::write.  And tui_file::puts .....

... but that all seems a bit unnecessary.  Instead, I propose that
tui_win_info::refresh_window() should be changed.  If suppress_output
is true (i.e. a tui_batch_rendering is active) then we should continue
to call wnoutrefresh().  But if suppress_output is false, meaning that
no tui_batch_rendering is in place, then we should call wrefresh(),
which immediately writes the output to the screen.

Testing but PR gdb/32623 was a little involved.  We need to 'run' the
inferior and check for the 'Starting program' message.  But DejaGNUU
can only check for the message once it knows the message should have
appeared.  But, as the bug is that output is not displayed, we don't
have any output hints that the inferior is started yet...

In the end, I have the inferior create a file in the test's output
directory.  Now DejaGNU can send the 'run' command, and wait for the
file to appear.  Once that happens, we know that the 'Starting
program' message should have appeared.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32623

Approved-By: Tom Tromey <tom@tromey.com>
gdb/testsuite/gdb.tui/flush-after-run.c [new file with mode: 0644]
gdb/testsuite/gdb.tui/flush-after-run.exp [new file with mode: 0644]
gdb/tui/tui-wingeneral.c

diff --git a/gdb/testsuite/gdb.tui/flush-after-run.c b/gdb/testsuite/gdb.tui/flush-after-run.c
new file mode 100644 (file)
index 0000000..f2dfded
--- /dev/null
@@ -0,0 +1,54 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2025 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/>.  */
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <stdio.h>
+
+int
+main (int argc, char *argv[])
+{
+  /* Don't let this test stay alive forever.  */
+  alarm (300);
+
+  if (argc != 2)
+    abort ();
+
+  /* Check the file doesn't already exist.  */
+  const char *filename = argv[1];
+  struct stat buf;
+  if (stat (filename, &buf) == 0 || errno != ENOENT)
+    abort ();
+
+  /* Create the file, and write something into it.  */
+  FILE *out = fopen (filename, "w");
+  if (out == NULL)
+    abort ();
+
+  fprintf (out, "Hello World\n");
+
+  if (fclose (out) != 0)
+    abort ();
+
+  /* Spin until the marker file is deleted.  */
+  while (stat (filename, &buf) == 0)
+    sleep (1);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.tui/flush-after-run.exp b/gdb/testsuite/gdb.tui/flush-after-run.exp
new file mode 100644 (file)
index 0000000..c222b19
--- /dev/null
@@ -0,0 +1,66 @@
+# Copyright 2025 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/>.
+
+# Check that the 'Starting program' message is correctly flushed to
+# the TUI terminal as soon as it is available.
+
+require allow_tui_tests
+require target_can_use_run_cmd
+
+tuiterm_env
+
+standard_testfile
+
+if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} {
+    return -1
+}
+
+Term::clean_restart 24 80 $testfile
+
+if {![Term::enter_tui]} {
+    unsupported "TUI not supported"
+    return
+}
+
+# Pick a name for a marker file, and ensure it doesn't exist.
+set marker_file [standard_output_file "marker"]
+file delete $marker_file
+
+# Run the inferior, which will create MARKER_FILE.
+send_gdb "run \"$marker_file\"\n"
+
+# Spin until MARKER_FILE appears.
+while { ! [file exists $marker_file] } {
+    sleep 1
+}
+
+# We now know that the inferior has started, and that the 'Starting
+# program: ' string should have been printed to the terminal.  Don't
+# use Term::wait_for here as there will be no prompt after the
+# 'Starting program' message.
+gdb_assert {[Term::wait_for_region_contents 0 16 80 7 "Starting program: "]} \
+    "starting program message has appeared"
+
+# Delete MARKER_FILE.  This will cause the inferior to exit.
+file delete $marker_file
+
+# Now wait for the prompt, and check that the inferior exited message
+# appeared.
+gdb_assert {[Term::wait_for ""]} \
+    "wait for prompt after inferior exits"
+Term::check_region_contents \
+    "check for inferior exited message" \
+    0 16 80 8 \
+    "\\\[Inferior $decimal \[^\r\n\]+ exited normally\\\]"
index 369d1529f94a54547a56d1661fcf976e82c8f9b3..56e7abd8f2c3748e06334ed0314e042a04a1c4ab 100644 (file)
@@ -56,7 +56,12 @@ void
 tui_win_info::refresh_window ()
 {
   if (handle != NULL)
-    wnoutrefresh (handle.get ());
+    {
+      if (suppress_output)
+       wnoutrefresh (handle.get ());
+      else
+       wrefresh (handle.get ());
+    }
 }
 
 /* Draw a border around the window.  */