diff --git a/gdb/ChangeLog b/gdb/ChangeLog index b7b4909f84b..912ca6b9444 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,17 @@ +2021-02-08 Andrew Burgess + + * python/py-tui.c (gdbpy_tui_window) : New member + function. + (REQUIRE_WINDOW): Call is_valid member function. + (REQUIRE_WINDOW_FOR_SETTER): New define. + (gdbpy_tui_is_valid): Call is_valid member function. + (gdbpy_tui_set_title): Call REQUIRE_WINDOW_FOR_SETTER instead. + * tui/tui-data.h (struct tui_win_info) : Check + tui_active too. + * tui/tui-layout.c (tui_apply_current_layout): Add an assert. + * tui/tui.c (tui_enable): Move setting of tui_active earlier in + the function. + 2021-02-08 Andrew Burgess * python/py-tui.c (gdbpy_tui_set_title): Check that the new value diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog index 7b21966d3d4..bc84fdc6ee6 100644 --- a/gdb/doc/ChangeLog +++ b/gdb/doc/ChangeLog @@ -1,3 +1,8 @@ +2021-02-08 Andrew Burgess + + * python.texinfo (TUI Windows In Python): Extend description of + TuiWindow.is_valid. + 2021-02-02 Lancelot SIX * gdb.texinfo (Inferiors Connections and Programs): Document the diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index 1a5209ae4bd..ba3d2f92a43 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -5848,6 +5848,11 @@ user changes the TUI layout, windows no longer visible in the new layout will be destroyed. At this point, the @code{gdb.TuiWindow} will no longer be valid, and methods (and attributes) other than @code{is_valid} will throw an exception. + +When the TUI is disabled using @code{tui disable} (@pxref{TUI +Commands,,tui disable}) the window is hidden rather than destroyed, +but @code{is_valid} will still return @code{False} and other methods +(and attributes) will still throw an exception. @end defun @defvar TuiWindow.width diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c index 73b73f33d4a..72e9c0d5e2b 100644 --- a/gdb/python/py-tui.c +++ b/gdb/python/py-tui.c @@ -47,6 +47,9 @@ struct gdbpy_tui_window /* The TUI window, or nullptr if the window has been deleted. */ tui_py_window *window; + + /* Return true if this object is valid. */ + bool is_valid () const; }; extern PyTypeObject gdbpy_tui_window_object_type @@ -137,6 +140,14 @@ private: gdbpy_ref m_wrapper; }; +/* See gdbpy_tui_window declaration above. */ + +bool +gdbpy_tui_window::is_valid () const +{ + return window != nullptr && tui_active; +} + tui_py_window::~tui_py_window () { gdbpy_enter enter_py (get_current_arch (), current_language); @@ -344,11 +355,23 @@ gdbpy_register_tui_window (PyObject *self, PyObject *args, PyObject *kw) #define REQUIRE_WINDOW(Window) \ do { \ - if ((Window)->window == nullptr) \ + if (!(Window)->is_valid ()) \ return PyErr_Format (PyExc_RuntimeError, \ _("TUI window is invalid.")); \ } while (0) +/* Require that "Window" be a valid window. */ + +#define REQUIRE_WINDOW_FOR_SETTER(Window) \ + do { \ + if (!(Window)->is_valid ()) \ + { \ + PyErr_Format (PyExc_RuntimeError, \ + _("TUI window is invalid.")); \ + return -1; \ + } \ + } while (0) + /* Python function which checks the validity of a TUI window object. */ static PyObject * @@ -356,7 +379,7 @@ gdbpy_tui_is_valid (PyObject *self, PyObject *args) { gdbpy_tui_window *win = (gdbpy_tui_window *) self; - if (win->window != nullptr) + if (win->is_valid ()) Py_RETURN_TRUE; Py_RETURN_FALSE; } @@ -428,11 +451,7 @@ gdbpy_tui_set_title (PyObject *self, PyObject *newvalue, void *closure) { gdbpy_tui_window *win = (gdbpy_tui_window *) self; - if (win->window == nullptr) - { - PyErr_Format (PyExc_RuntimeError, _("TUI window is invalid.")); - return -1; - } + REQUIRE_WINDOW_FOR_SETTER (win); if (newvalue == nullptr) { diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index faee2c42637..3e622adb4d0 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2021-02-08 Andrew Burgess + + * gdb.python/tui-window-disabled.c: New file. + * gdb.python/tui-window-disabled.exp: New file. + * gdb.python/tui-window-disabled.py: New file. + 2021-02-08 Andrew Burgess * gdb.python/tui-window.exp: Add new tests. diff --git a/gdb/testsuite/gdb.python/tui-window-disabled.c b/gdb/testsuite/gdb.python/tui-window-disabled.c new file mode 100644 index 00000000000..898c5361ca3 --- /dev/null +++ b/gdb/testsuite/gdb.python/tui-window-disabled.c @@ -0,0 +1,43 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 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 . */ + +#include "../lib/attributes.h" + +volatile int val; + +void __attribute__((noinline)) ATTRIBUTE_NOCLONE +func (int i) +{ + val = i; +} + +int +main () +{ + func (0); + func (1); + func (2); + func (3); + func (4); + func (5); + func (6); + func (7); + func (8); + func (9); + + return 0; +} diff --git a/gdb/testsuite/gdb.python/tui-window-disabled.exp b/gdb/testsuite/gdb.python/tui-window-disabled.exp new file mode 100644 index 00000000000..af1fa0cde63 --- /dev/null +++ b/gdb/testsuite/gdb.python/tui-window-disabled.exp @@ -0,0 +1,189 @@ +# Copyright (C) 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 . + +# Create a TUI window in Python that responds to GDB event. Each +# event will trigger the TUI window to redraw itself. +# +# This test is checking how GDB behaves if the user first displays a +# Python based tui window, and then does 'tui disable'. At one point +# it was possible that GDB would try to redraw the tui window even +# though the tui should be disabled. + +load_lib gdb-python.exp +tuiterm_env + +standard_testfile + +if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} { + return -1 +} + +# Copy the Python script to where the tests are being run. +set remote_python_file [gdb_remote_download host \ + ${srcdir}/${subdir}/${testfile}.py] + +proc clean_restart_and_setup { prefix } { + global testfile + global remote_python_file + + with_test_prefix $prefix { + + Term::clean_restart 24 80 $testfile + + # Skip all tests if Python scripting is not enabled. + if { [skip_python_tests] } { return 0 } + + # Now source the python script. + gdb_test_no_output "source ${remote_python_file}" \ + "source ${testfile}.py" + + # Create a new layout making use of our new event window. + gdb_test_no_output "tui new-layout test events 1 cmd 1" + + # Disable source code highlighting. + gdb_test_no_output "set style sources off" + + if {![runto_main]} { + perror "test suppressed" + return + } + } + + return 1 +} + +# Run the test. CLEANUP_PROPERLY is either true or false. This is +# used to set a flag in the Python code which controls whether the +# Python TUI window cleans up properly or not. +# +# When the Python window does not cleanup properly then it retains a +# cyclic reference to itself, this means that it is still possible for +# the object to try and redraw itself even when the tui is disabled. +proc run_test { cleanup_properly } { + + if { ![clean_restart_and_setup "initial restart"] } { + unsupported "couldn't restart GDB" + return + } + + if { $cleanup_properly } { + gdb_test_no_output "python cleanup_properly = True" + } else { + gdb_test_no_output "python cleanup_properly = False" + } + + if {![Term::enter_tui]} { + unsupported "TUI not supported" + return + } + + Term::command "layout test" + + # Confirm that the events box is initially empty, then perform two + # actions that will add two events to the window. + Term::check_box_contents "no events yet" 0 0 80 16 "" + Term::command "next" + Term::check_box_contents "single stop event" 0 0 80 16 "stop" + Term::command "next" + Term::check_box_contents "two stop events" 0 0 80 16 \ + "stop\[^\n\]+\nstop" + + # Now disable the tui, we should end up back at a standard GDB prompt. + Term::command "tui disable" + + # Do something just so we know that the CLI is working. + gdb_test "print 1 + 1 + 1" " = 3" + + # Now perform an action that would trigger an event. At one point + # there was a bug where the TUI window might try to redraw itself. + # This is why we use GDB_TEST_MULTIPLE here, so we can spot the tui + # window title and know that things have gone wrong. + gdb_test_multiple "next" "next at cli" { + -re -wrap "func \\(3\\);" { + pass $gdb_test_name + } + + -re "This Is The Event Window" { + fail $gdb_test_name + } + } + + # Do something just so we know that the CLI is still working. This + # also serves to drain the expect buffer if the above test failed. + gdb_test "print 2 + 2 + 2" " = 6" + + # Now tell the Python code not to check the window is valid before + # calling rerended. The result is the Python code will try to draw to + # the screen. This should throw a Python exception. + gdb_test_no_output "python perform_valid_check = False" + set exception_pattern "\r\nPython Exception\[^\n\r\]+TUI window is invalid\[^\n\r\]+" + gdb_test_multiple "next" "next at cli, with an exception" { + -re -wrap "func \\(4\\);${exception_pattern}" { + pass $gdb_test_name + } + + -re "This Is The Event Window" { + fail $gdb_test_name + } + } + + # Do something just so we know that the CLI is still working. This + # also serves to drain the expect buffer if the above test failed. + gdb_test "print 3 + 3 + 3" " = 9" + + # Set 'update_title' to True. The Python script will now try to set + # the window title when an event occurs (instead of trying to redraw + # the window). As the window is still not displayed this will again + # through an exception. + gdb_test_no_output "python update_title = True" + gdb_test_multiple "next" "next at cli, with an exception for setting the title" { + -re -wrap "func \\(5\\);${exception_pattern}" { + pass $gdb_test_name + } + + -re "This Is The Event Window" { + fail $gdb_test_name + } + } + + # We need to perform a restart here as the TUI library we use for + # testing doesn't seem to handle output in the command window + # correctly, and gets really upset as we approach the bottom of + # the screen. + # + # Restart GDB, enable tui mode, select the new layout. Then + # disable tui and re-enable again. + if { ![clean_restart_and_setup "second restart"] } { + unsupported "couldn't restart GDB" + return + } + + with_test_prefix "enter tui again" { + if {![Term::enter_tui]} { + unsupported "TUI not supported" + return + } + } + + Term::command "layout test" + Term::command "tui disable" + send_gdb "tui enable\n" + Term::check_box "check for python window" 0 0 80 16 +} + +# Run the tests in both cleanup modes. +foreach_with_prefix cleanup_properly { True False } { + run_test $cleanup_properly +} diff --git a/gdb/testsuite/gdb.python/tui-window-disabled.py b/gdb/testsuite/gdb.python/tui-window-disabled.py new file mode 100644 index 00000000000..0b3c076f7e9 --- /dev/null +++ b/gdb/testsuite/gdb.python/tui-window-disabled.py @@ -0,0 +1,89 @@ +# Copyright (C) 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 . + +# A TUI window implemented in Python that responds to, and displays, +# stop and exit events. + +import gdb + +# When an event arrives we ask the window to redraw itself. We should +# only do this if the window is valid. When this flag is true we +# perform the is_valid check. When this flag is false +perform_valid_check = True +update_title = False +cleanup_properly = False + +# A global place into which we can write the window title. +titles_at_the_close = {} + +class EventWindow: + def __init__ (self, win): + self._win = win + self._count = 0 + win.title = "This Is The Event Window" + self._stop_listener = lambda e : self._event ('stop', e) + gdb.events.stop.connect (self._stop_listener) + self._exit_listener = lambda e : self._event ('exit', e) + gdb.events.exited.connect (self._exit_listener) + self._events = [] + + # Ensure we can erase and write to the window from the + # constructor, the window should be valid by this point. + self._win.erase () + self._win.write ("Hello world...") + + def close (self): + global cleanup_properly + global titles_at_the_close + + # Ensure that window properties can be read within the close method. + titles_at_the_close[self._win.title] = dict (width=self._win.width, + height=self._win.height) + + # The following calls are pretty pointless, but this ensures + # that we can erase and write to a window from the close + # method, the last moment a window should be valid. + self._win.erase () + self._win.write ("Goodbye cruel world...") + + if cleanup_properly: + # Disconnect the listeners and delete the lambda functions. + # This removes cyclic references to SELF, and so alows SELF to + # be deleted. + gdb.events.stop.disconnect (self._stop_listener) + gdb.events.exited.disconnect (self._exit_listener) + self._stop_listener = None + self._exit_listener = None + + def _event (self, type, event): + global perform_valid_check + global update_title + + self._count += 1 + self._events.insert (0, type) + if not perform_valid_check or self._win.is_valid (): + if update_title: + self._win.title = "This Is The Event Window (" + str (self._count) + ")" + else: + self.render () + + def render (self): + self._win.erase () + w = self._win.width + h = self._win.height + for i in range (min (h, len (self._events))): + self._win.write (self._events[i] + "\n") + +gdb.register_window_type("events", EventWindow) diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h index c92c8f9d364..b4d788dd0a4 100644 --- a/gdb/tui/tui-data.h +++ b/gdb/tui/tui-data.h @@ -96,7 +96,7 @@ public: /* Return true if this window is visible. */ bool is_visible () const { - return handle != nullptr; + return handle != nullptr && tui_active; } /* Return true if this window can accept the focus. */ diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c index f08d62d681f..bf9c3ff4ddc 100644 --- a/gdb/tui/tui-layout.c +++ b/gdb/tui/tui-layout.c @@ -86,6 +86,7 @@ tui_apply_current_layout () tui_win_list[win_type] = nullptr; /* This should always be made visible by a layout. */ + gdb_assert (TUI_CMD_WIN != nullptr); gdb_assert (TUI_CMD_WIN->is_visible ()); /* Get the new list of currently visible windows. */ diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c index ce8dab3c642..af92b2a8042 100644 --- a/gdb/tui/tui.c +++ b/gdb/tui/tui.c @@ -420,6 +420,12 @@ tui_enable (void) } #endif + /* We must mark the tui sub-system active before trying to setup the + current layout as tui windows defined by an extension language + rely on this flag being true in order to know that the window + they are creating is currently valid. */ + tui_active = true; + cbreak (); noecho (); /* timeout (1); */ @@ -439,19 +445,21 @@ tui_enable (void) } else { - /* Save the current gdb setting of the terminal. - Curses will restore this state when endwin() is called. */ - def_shell_mode (); - clearok (stdscr, TRUE); - } + /* Save the current gdb setting of the terminal. + Curses will restore this state when endwin() is called. */ + def_shell_mode (); + clearok (stdscr, TRUE); + + tui_active = true; + } + + gdb_assert (tui_active); if (tui_update_variables ()) tui_rehighlight_all (); tui_setup_io (1); - tui_active = true; - /* Resize windows before anything might display/refresh a window. */ if (tui_win_resized ())