diff options
author | Andrew Burgess <andrew.burgess@embecosm.com> | 2021-01-15 10:31:19 +0000 |
---|---|---|
committer | Andrew Burgess <andrew.burgess@embecosm.com> | 2021-02-08 11:56:16 +0000 |
commit | 29db1eb3390cd45066680ef865214588afdc0eca (patch) | |
tree | a4616a120c22a3a8d006263fd4353705b5fea622 /gdb/tui | |
parent | e0c23e11da18b615c382888da8e978f16428e81b (diff) | |
download | binutils-29db1eb3390cd45066680ef865214588afdc0eca.zip binutils-29db1eb3390cd45066680ef865214588afdc0eca.tar.gz binutils-29db1eb3390cd45066680ef865214588afdc0eca.tar.bz2 |
gdb: return true in TuiWindow.is_valid only if TUI is enabled
If the user implements a TUI window in Python, and this window
responds to GDB events and then redraws its window contents then there
is currently an edge case which can lead to problems.
The Python API documentation suggests that calling methods like erase
or write on a TUI window (from Python code) will raise an exception if
the window is not valid.
And the description for is_valid says:
This method returns True when this window is valid. When the user
changes the TUI layout, windows no longer visible in the new layout
will be destroyed. At this point, the gdb.TuiWindow will no longer
be valid, and methods (and attributes) other than is_valid will
throw an exception.
From this I, as a user, would expect that if I did 'tui disable' to
switch back to CLI mode, then the window would no longer be valid.
However, this is not the case.
When the TUI is disabled the windows in the TUI are not deleted, they
are simply hidden. As such, currently, the is_valid method continues
to return true.
This means that if the users Python code does something like:
def event_handler (e):
global tui_window_object
if tui_window_object->is_valid ():
tui_window_object->erase ()
tui_window_object->write ("Hello World")
gdb.events.stop.connect (event_handler)
Then when a stop event arrives GDB will try to draw the TUI window,
even when the TUI is disabled.
This exposes two bugs. First, is_valid should be returning false in
this case, second, if the user forgot to add the is_valid call, then I
believe the erase and write calls should be throwing an
exception (when the TUI is disabled).
The solution to both of these issues is I think bound together, as it
depends on having a working 'is_valid' check.
There's a rogue assert added into tui-layout.c as part of this
commit. While working on this commit I managed to break GDB such that
TUI_CMD_WIN was nullptr, this was causing GDB to abort. I'm leaving
the assert in as it might help people catch issues in the future.
This patch is inspired by the work done here:
https://sourceware.org/pipermail/gdb-patches/2020-December/174338.html
gdb/ChangeLog:
* python/py-tui.c (gdbpy_tui_window) <is_valid>: 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) <is_visible>: 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.
gdb/doc/ChangeLog:
* python.texinfo (TUI Windows In Python): Extend description of
TuiWindow.is_valid.
gdb/testsuite/ChangeLog:
* gdb.python/tui-window-disabled.c: New file.
* gdb.python/tui-window-disabled.exp: New file.
* gdb.python/tui-window-disabled.py: New file.
Diffstat (limited to 'gdb/tui')
-rw-r--r-- | gdb/tui/tui-data.h | 2 | ||||
-rw-r--r-- | gdb/tui/tui-layout.c | 1 | ||||
-rw-r--r-- | gdb/tui/tui.c | 22 |
3 files changed, 17 insertions, 8 deletions
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h index c92c8f9..b4d788d 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 f08d62d..bf9c3ff 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 ce8dab3..af92b2a 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 ()) |