binutils-gdb/gdb/python/py-tui.c

637 lines
16 KiB
C
Raw Permalink Normal View History

/* TUI windows implemented in Python
Copyright (C) 2020-2024 Free Software Foundation, Inc.
This file is part of GDB.
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 "arch-utils.h"
#include "python-internal.h"
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
#include "gdbsupport/intrusive_list.h"
#ifdef TUI
/* Note that Python's public headers may define HAVE_NCURSES_H, so if
we unconditionally include this (outside the #ifdef above), then we
can get a compile error when ncurses is not in fact installed. See
PR tui/25597; or the upstream Python bug
https://bugs.python.org/issue20768. */
#include "gdb_curses.h"
#include "tui/tui-data.h"
#include "tui/tui-io.h"
#include "tui/tui-layout.h"
#include "tui/tui-wingeneral.h"
#include "tui/tui-winsource.h"
class tui_py_window;
/* A PyObject representing a TUI window. */
struct gdbpy_tui_window
{
PyObject_HEAD
/* The TUI window, or nullptr if the window has been deleted. */
tui_py_window *window;
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.
2021-01-15 10:31:19 +00:00
/* Return true if this object is valid. */
bool is_valid () const;
};
extern PyTypeObject gdbpy_tui_window_object_type
CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("gdbpy_tui_window");
/* A TUI window written in Python. */
class tui_py_window : public tui_win_info
{
public:
tui_py_window (const char *name, gdbpy_ref<gdbpy_tui_window> wrapper)
: m_name (name),
m_wrapper (std::move (wrapper))
{
m_wrapper->window = this;
}
~tui_py_window ();
DISABLE_COPY_AND_ASSIGN (tui_py_window);
/* Set the "user window" to the indicated reference. The user
window is the object returned the by user-defined window
constructor. */
void set_user_window (gdbpy_ref<> &&user_window)
{
m_window = std::move (user_window);
}
const char *name () const override
{
return m_name.c_str ();
}
void rerender () override;
void do_scroll_vertical (int num_to_scroll) override;
void do_scroll_horizontal (int num_to_scroll) override;
void refresh_window () override
{
if (m_inner_window != nullptr)
{
wnoutrefresh (handle.get ());
touchwin (m_inner_window.get ());
tui_wrefresh (m_inner_window.get ());
}
else
tui_win_info::refresh_window ();
}
Fix resizing of TUI python windows When resizing from a big to small terminal size, and you have a TUI python window that would then be outside of the new size, valgrind shows this error: ==3389== Invalid read of size 1 ==3389== at 0xC3DFEE: wnoutrefresh (lib_refresh.c:167) ==3389== by 0xC3E3C9: wrefresh (lib_refresh.c:63) ==3389== by 0xA9766C: tui_unhighlight_win(tui_win_info*) (tui-wingeneral.c:134) ==3389== by 0x98921C: tui_py_window::rerender() (py-tui.c:183) ==3389== by 0xA8C23C: tui_layout_split::apply(int, int, int, int, bool) (tui-layout.c:1030) ==3389== by 0xA8C2A2: tui_layout_split::apply(int, int, int, int, bool) (tui-layout.c:1033) ==3389== by 0xA8C23C: tui_layout_split::apply(int, int, int, int, bool) (tui-layout.c:1030) ==3389== by 0xA8B1F8: tui_apply_current_layout(bool) (tui-layout.c:81) ==3389== by 0xA95CDB: tui_resize_all() (tui-win.c:525) ==3389== by 0xA95D1E: tui_async_resize_screen(void*) (tui-win.c:562) ==3389== by 0x6B855D: invoke_async_signal_handlers() (async-event.c:234) ==3389== by 0xC0CEF8: gdb_do_one_event(int) (event-loop.cc:199) ==3389== Address 0x115cc214 is 1,332 bytes inside a block of size 2,240 free'd ==3389== at 0x4A0A430: free (vg_replace_malloc.c:446) ==3389== by 0xC3CF7D: _nc_freewin (lib_newwin.c:121) ==3389== by 0xA8B1C6: tui_apply_current_layout(bool) (tui-layout.c:78) ==3389== by 0xA95CDB: tui_resize_all() (tui-win.c:525) ==3389== by 0xA95D1E: tui_async_resize_screen(void*) (tui-win.c:562) ==3389== by 0x6B855D: invoke_async_signal_handlers() (async-event.c:234) ==3389== by 0xC0CEF8: gdb_do_one_event(int) (event-loop.cc:199) ==3389== by 0x8E40E9: captured_command_loop() (main.c:407) ==3389== by 0x8E5E54: gdb_main(captured_main_args*) (main.c:1324) ==3389== by 0x62AC04: main (gdb.c:39) It's because tui_py_window::m_inner_window still has the outside coordinates, and wnoutrefresh then does an out-of-bounds access. Fix this by resetting m_inner_window on every resize, it will anyways be recreated in the next rerender call. Approved-By: Andrew Burgess <aburgess@redhat.com>
2023-11-06 18:32:41 +01:00
void resize (int height, int width, int origin_x, int origin_y) override;
void click (int mouse_x, int mouse_y, int mouse_button) override;
/* Erase and re-box the window. */
void erase ()
{
if (is_visible () && m_inner_window != nullptr)
{
werase (m_inner_window.get ());
check_and_display_highlight_if_needed ();
}
}
/* Write STR to the window. FULL_WINDOW is true to erase the window
contents beforehand. */
void output (const char *str, bool full_window);
/* A helper function to compute the viewport width. */
int viewport_width () const
{
return std::max (0, width - 2);
}
/* A helper function to compute the viewport height. */
int viewport_height () const
{
return std::max (0, height - 2);
}
private:
/* The name of this window. */
std::string m_name;
/* We make our own inner window, so that it is easy to print without
overwriting the border. */
std::unique_ptr<WINDOW, curses_deleter> m_inner_window;
/* The underlying Python window object. */
gdbpy_ref<> m_window;
/* The Python wrapper for this object. */
gdbpy_ref<gdbpy_tui_window> m_wrapper;
};
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.
2021-01-15 10:31:19 +00:00
/* 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;
/* This can be null if the user-provided Python construction
function failed. */
if (m_window != nullptr
&& PyObject_HasAttrString (m_window.get (), "close"))
{
gdbpy_ref<> result = gdbpy_call_method (m_window, "close");
if (result == nullptr)
gdbpy_print_stack ();
}
/* Unlink. */
m_wrapper->window = nullptr;
/* Explicitly free the Python references. We have to do this
manually because we need to hold the GIL while doing so. */
m_wrapper.reset (nullptr);
m_window.reset (nullptr);
}
void
tui_py_window::rerender ()
{
tui_win_info::rerender ();
gdbpy_enter enter_py;
int h = viewport_height ();
int w = viewport_width ();
if (h == 0 || w == 0)
{
/* The window would be too small, so just remove the
contents. */
m_inner_window.reset (nullptr);
return;
}
m_inner_window.reset (newwin (h, w, y + 1, x + 1));
if (PyObject_HasAttrString (m_window.get (), "render"))
{
gdbpy_ref<> result = gdbpy_call_method (m_window, "render");
if (result == nullptr)
gdbpy_print_stack ();
}
}
void
tui_py_window::do_scroll_horizontal (int num_to_scroll)
{
gdbpy_enter enter_py;
if (PyObject_HasAttrString (m_window.get (), "hscroll"))
{
gdbpy_ref<> result = gdbpy_call_method (m_window, "hscroll",
num_to_scroll);
if (result == nullptr)
gdbpy_print_stack ();
}
}
void
tui_py_window::do_scroll_vertical (int num_to_scroll)
{
gdbpy_enter enter_py;
if (PyObject_HasAttrString (m_window.get (), "vscroll"))
{
gdbpy_ref<> result = gdbpy_call_method (m_window, "vscroll",
num_to_scroll);
if (result == nullptr)
gdbpy_print_stack ();
}
}
Fix resizing of TUI python windows When resizing from a big to small terminal size, and you have a TUI python window that would then be outside of the new size, valgrind shows this error: ==3389== Invalid read of size 1 ==3389== at 0xC3DFEE: wnoutrefresh (lib_refresh.c:167) ==3389== by 0xC3E3C9: wrefresh (lib_refresh.c:63) ==3389== by 0xA9766C: tui_unhighlight_win(tui_win_info*) (tui-wingeneral.c:134) ==3389== by 0x98921C: tui_py_window::rerender() (py-tui.c:183) ==3389== by 0xA8C23C: tui_layout_split::apply(int, int, int, int, bool) (tui-layout.c:1030) ==3389== by 0xA8C2A2: tui_layout_split::apply(int, int, int, int, bool) (tui-layout.c:1033) ==3389== by 0xA8C23C: tui_layout_split::apply(int, int, int, int, bool) (tui-layout.c:1030) ==3389== by 0xA8B1F8: tui_apply_current_layout(bool) (tui-layout.c:81) ==3389== by 0xA95CDB: tui_resize_all() (tui-win.c:525) ==3389== by 0xA95D1E: tui_async_resize_screen(void*) (tui-win.c:562) ==3389== by 0x6B855D: invoke_async_signal_handlers() (async-event.c:234) ==3389== by 0xC0CEF8: gdb_do_one_event(int) (event-loop.cc:199) ==3389== Address 0x115cc214 is 1,332 bytes inside a block of size 2,240 free'd ==3389== at 0x4A0A430: free (vg_replace_malloc.c:446) ==3389== by 0xC3CF7D: _nc_freewin (lib_newwin.c:121) ==3389== by 0xA8B1C6: tui_apply_current_layout(bool) (tui-layout.c:78) ==3389== by 0xA95CDB: tui_resize_all() (tui-win.c:525) ==3389== by 0xA95D1E: tui_async_resize_screen(void*) (tui-win.c:562) ==3389== by 0x6B855D: invoke_async_signal_handlers() (async-event.c:234) ==3389== by 0xC0CEF8: gdb_do_one_event(int) (event-loop.cc:199) ==3389== by 0x8E40E9: captured_command_loop() (main.c:407) ==3389== by 0x8E5E54: gdb_main(captured_main_args*) (main.c:1324) ==3389== by 0x62AC04: main (gdb.c:39) It's because tui_py_window::m_inner_window still has the outside coordinates, and wnoutrefresh then does an out-of-bounds access. Fix this by resetting m_inner_window on every resize, it will anyways be recreated in the next rerender call. Approved-By: Andrew Burgess <aburgess@redhat.com>
2023-11-06 18:32:41 +01:00
void
tui_py_window::resize (int height_, int width_, int origin_x_, int origin_y_)
{
m_inner_window.reset (nullptr);
tui_win_info::resize (height_, width_, origin_x_, origin_y_);
}
void
tui_py_window::click (int mouse_x, int mouse_y, int mouse_button)
{
gdbpy_enter enter_py;
if (PyObject_HasAttrString (m_window.get (), "click"))
{
gdbpy_ref<> result = gdbpy_call_method (m_window, "click",
mouse_x, mouse_y, mouse_button);
if (result == nullptr)
gdbpy_print_stack ();
}
}
void
tui_py_window::output (const char *text, bool full_window)
{
if (m_inner_window != nullptr)
{
if (full_window)
werase (m_inner_window.get ());
tui_puts (text, m_inner_window.get ());
if (full_window)
check_and_display_highlight_if_needed ();
else
tui_wrefresh (m_inner_window.get ());
}
}
/* A callable that is used to create a TUI window. It wraps the
user-supplied window constructor. */
class gdbpy_tui_window_maker
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
: public intrusive_list_node<gdbpy_tui_window_maker>
{
public:
explicit gdbpy_tui_window_maker (gdbpy_ref<> &&constr)
: m_constr (std::move (constr))
{
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
m_window_maker_list.push_back (*this);
}
~gdbpy_tui_window_maker ();
gdbpy_tui_window_maker (gdbpy_tui_window_maker &&other) noexcept
: m_constr (std::move (other.m_constr))
{
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
m_window_maker_list.push_back (*this);
}
gdbpy_tui_window_maker (const gdbpy_tui_window_maker &other)
{
gdbpy_enter enter_py;
m_constr = other.m_constr;
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
m_window_maker_list.push_back (*this);
}
gdbpy_tui_window_maker &operator= (gdbpy_tui_window_maker &&other)
{
m_constr = std::move (other.m_constr);
return *this;
}
gdbpy_tui_window_maker &operator= (const gdbpy_tui_window_maker &other)
{
gdbpy_enter enter_py;
m_constr = other.m_constr;
return *this;
}
tui_win_info *operator() (const char *name);
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
/* Reset the m_constr field of all gdbpy_tui_window_maker objects back to
nullptr, this will allow the Python object referenced to be
deallocated. This function is intended to be called when GDB is
shutting down the Python interpreter to allow all Python objects to be
deallocated and cleaned up. */
static void
invalidate_all ()
{
gdbpy_enter enter_py;
for (gdbpy_tui_window_maker &f : m_window_maker_list)
f.m_constr.reset (nullptr);
}
private:
/* A constructor that is called to make a TUI window. */
gdbpy_ref<> m_constr;
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
/* A global list of all gdbpy_tui_window_maker objects. */
static intrusive_list<gdbpy_tui_window_maker> m_window_maker_list;
};
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
/* See comment in class declaration above. */
intrusive_list<gdbpy_tui_window_maker>
gdbpy_tui_window_maker::m_window_maker_list;
gdbpy_tui_window_maker::~gdbpy_tui_window_maker ()
{
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
/* Remove this gdbpy_tui_window_maker from the global list. */
[gdb/tui] Fix assert in ~gdbpy_tui_window_maker In gdb/tui/tui-layout.c, we have: ... static window_types_map known_window_types; ... and in gdb/python/py-tui.c: ... /* A global list of all gdbpy_tui_window_maker objects. */ static intrusive_list<gdbpy_tui_window_maker> m_window_maker_list; }; /* See comment in class declaration above. */ intrusive_list<gdbpy_tui_window_maker> gdbpy_tui_window_maker::m_window_maker_list; ... With a gdb build with -O0 or -O2, the static destructor calling order seems to be: - first gdb/tui/tui-layout.c, - then gdb/python/py-tui.c. So when running test-case gdb.python/tui-window-factory.exp, we see the following order of events: - the destructor for known_window_types is called, which triggers calling the destructor for the only element E of m_window_maker_list. The destructor destroys E, and also removes E from m_window_maker_list, leaving it empty. - the destructor for m_window_maker_list is called. It's empty, so it's a nop. However, when building gdb with -O2 -flto=auto, the static destructor calling order seems to be reversed. Instead, we have these events: - the destructor for m_window_maker_list is called. This doesn't destroy it's only element E, but it does make m_window_maker_list empty. - the destructor for known_window_types is called, which triggers calling the destructor for E. An attempt is done to remove E from m_window_maker_list, but we run into an assertion failure, because the list is empty. Fix this by checking is_linked () before attempting to remove from m_window_maker_list, similar to how things were addressed in commit 995a34b1772 ("Guard against frame.c destructors running before frame-info.c's"). Tested on x86_64-linux. PR tui/30646 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30646
2023-07-26 12:29:28 +02:00
if (is_linked ())
m_window_maker_list.erase (m_window_maker_list.iterator_to (*this));
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
if (m_constr != nullptr)
{
gdbpy_enter enter_py;
m_constr.reset (nullptr);
}
}
tui_win_info *
gdbpy_tui_window_maker::operator() (const char *win_name)
{
gdbpy_enter enter_py;
gdbpy_ref<gdbpy_tui_window> wrapper
(PyObject_New (gdbpy_tui_window, &gdbpy_tui_window_object_type));
if (wrapper == nullptr)
{
gdbpy_print_stack ();
return nullptr;
}
std::unique_ptr<tui_py_window> window
(new tui_py_window (win_name, wrapper));
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
/* There's only two ways that m_constr can be reset back to nullptr,
first when the parent gdbpy_tui_window_maker object is deleted, in
which case it should be impossible to call this method, or second, as
a result of a gdbpy_tui_window_maker::invalidate_all call, but this is
only called when GDB's Python interpreter is being shut down, after
which, this method should not be called. */
gdb_assert (m_constr != nullptr);
gdbpy_ref<> user_window
(PyObject_CallFunctionObjArgs (m_constr.get (),
(PyObject *) wrapper.get (),
nullptr));
if (user_window == nullptr)
{
gdbpy_print_stack ();
return nullptr;
}
window->set_user_window (std::move (user_window));
/* Window is now owned by the TUI. */
return window.release ();
}
/* Implement "gdb.register_window_type". */
PyObject *
gdbpy_register_tui_window (PyObject *self, PyObject *args, PyObject *kw)
{
static const char *keywords[] = { "name", "constructor", nullptr };
const char *name;
PyObject *cons_obj;
if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "sO", keywords,
&name, &cons_obj))
return nullptr;
try
{
gdbpy_tui_window_maker constr (gdbpy_ref<>::new_reference (cons_obj));
tui_register_window (name, constr);
}
catch (const gdb_exception &except)
{
gdbpy_convert_exception (except);
return nullptr;
}
Py_RETURN_NONE;
}
/* Require that "Window" be a valid window. */
#define REQUIRE_WINDOW(Window) \
do { \
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.
2021-01-15 10:31:19 +00:00
if (!(Window)->is_valid ()) \
gdb, gdbserver, gdbsupport: fix leading space vs tabs issues Many spots incorrectly use only spaces for indentation (for example, there are a lot of spots in ada-lang.c). I've always found it awkward when I needed to edit one of these spots: do I keep the original wrong indentation, or do I fix it? What if the lines around it are also wrong, do I fix them too? I probably don't want to fix them in the same patch, to avoid adding noise to my patch. So I propose to fix as much as possible once and for all (hopefully). One typical counter argument for this is that it makes code archeology more difficult, because git-blame will show this commit as the last change for these lines. My counter counter argument is: when git-blaming, you often need to do "blame the file at the parent commit" anyway, to go past some other refactor that touched the line you are interested in, but is not the change you are looking for. So you already need a somewhat efficient way to do this. Using some interactive tool, rather than plain git-blame, makes this trivial. For example, I use "tig blame <file>", where going back past the commit that changed the currently selected line is one keystroke. It looks like Magit in Emacs does it too (though I've never used it). Web viewers of Github and Gitlab do it too. My point is that it won't really make archeology more difficult. The other typical counter argument is that it will cause conflicts with existing patches. That's true... but it's a one time cost, and those are not conflicts that are difficult to resolve. I have also tried "git rebase --ignore-whitespace", it seems to work well. Although that will re-introduce the faulty indentation, so one needs to take care of fixing the indentation in the patch after that (which is easy). gdb/ChangeLog: * aarch64-linux-tdep.c: Fix indentation. * aarch64-ravenscar-thread.c: Fix indentation. * aarch64-tdep.c: Fix indentation. * aarch64-tdep.h: Fix indentation. * ada-lang.c: Fix indentation. * ada-lang.h: Fix indentation. * ada-tasks.c: Fix indentation. * ada-typeprint.c: Fix indentation. * ada-valprint.c: Fix indentation. * ada-varobj.c: Fix indentation. * addrmap.c: Fix indentation. * addrmap.h: Fix indentation. * agent.c: Fix indentation. * aix-thread.c: Fix indentation. * alpha-bsd-nat.c: Fix indentation. * alpha-linux-tdep.c: Fix indentation. * alpha-mdebug-tdep.c: Fix indentation. * alpha-nbsd-tdep.c: Fix indentation. * alpha-obsd-tdep.c: Fix indentation. * alpha-tdep.c: Fix indentation. * amd64-bsd-nat.c: Fix indentation. * amd64-darwin-tdep.c: Fix indentation. * amd64-linux-nat.c: Fix indentation. * amd64-linux-tdep.c: Fix indentation. * amd64-nat.c: Fix indentation. * amd64-obsd-tdep.c: Fix indentation. * amd64-tdep.c: Fix indentation. * amd64-windows-tdep.c: Fix indentation. * annotate.c: Fix indentation. * arc-tdep.c: Fix indentation. * arch-utils.c: Fix indentation. * arch/arm-get-next-pcs.c: Fix indentation. * arch/arm.c: Fix indentation. * arm-linux-nat.c: Fix indentation. * arm-linux-tdep.c: Fix indentation. * arm-nbsd-tdep.c: Fix indentation. * arm-pikeos-tdep.c: Fix indentation. * arm-tdep.c: Fix indentation. * arm-tdep.h: Fix indentation. * arm-wince-tdep.c: Fix indentation. * auto-load.c: Fix indentation. * auxv.c: Fix indentation. * avr-tdep.c: Fix indentation. * ax-gdb.c: Fix indentation. * ax-general.c: Fix indentation. * bfin-linux-tdep.c: Fix indentation. * block.c: Fix indentation. * block.h: Fix indentation. * blockframe.c: Fix indentation. * bpf-tdep.c: Fix indentation. * break-catch-sig.c: Fix indentation. * break-catch-syscall.c: Fix indentation. * break-catch-throw.c: Fix indentation. * breakpoint.c: Fix indentation. * breakpoint.h: Fix indentation. * bsd-uthread.c: Fix indentation. * btrace.c: Fix indentation. * build-id.c: Fix indentation. * buildsym-legacy.h: Fix indentation. * buildsym.c: Fix indentation. * c-typeprint.c: Fix indentation. * c-valprint.c: Fix indentation. * c-varobj.c: Fix indentation. * charset.c: Fix indentation. * cli/cli-cmds.c: Fix indentation. * cli/cli-decode.c: Fix indentation. * cli/cli-decode.h: Fix indentation. * cli/cli-script.c: Fix indentation. * cli/cli-setshow.c: Fix indentation. * coff-pe-read.c: Fix indentation. * coffread.c: Fix indentation. * compile/compile-cplus-types.c: Fix indentation. * compile/compile-object-load.c: Fix indentation. * compile/compile-object-run.c: Fix indentation. * completer.c: Fix indentation. * corefile.c: Fix indentation. * corelow.c: Fix indentation. * cp-abi.h: Fix indentation. * cp-namespace.c: Fix indentation. * cp-support.c: Fix indentation. * cp-valprint.c: Fix indentation. * cris-linux-tdep.c: Fix indentation. * cris-tdep.c: Fix indentation. * darwin-nat-info.c: Fix indentation. * darwin-nat.c: Fix indentation. * darwin-nat.h: Fix indentation. * dbxread.c: Fix indentation. * dcache.c: Fix indentation. * disasm.c: Fix indentation. * dtrace-probe.c: Fix indentation. * dwarf2/abbrev.c: Fix indentation. * dwarf2/attribute.c: Fix indentation. * dwarf2/expr.c: Fix indentation. * dwarf2/frame.c: Fix indentation. * dwarf2/index-cache.c: Fix indentation. * dwarf2/index-write.c: Fix indentation. * dwarf2/line-header.c: Fix indentation. * dwarf2/loc.c: Fix indentation. * dwarf2/macro.c: Fix indentation. * dwarf2/read.c: Fix indentation. * dwarf2/read.h: Fix indentation. * elfread.c: Fix indentation. * eval.c: Fix indentation. * event-top.c: Fix indentation. * exec.c: Fix indentation. * exec.h: Fix indentation. * expprint.c: Fix indentation. * f-lang.c: Fix indentation. * f-typeprint.c: Fix indentation. * f-valprint.c: Fix indentation. * fbsd-nat.c: Fix indentation. * fbsd-tdep.c: Fix indentation. * findvar.c: Fix indentation. * fork-child.c: Fix indentation. * frame-unwind.c: Fix indentation. * frame-unwind.h: Fix indentation. * frame.c: Fix indentation. * frv-linux-tdep.c: Fix indentation. * frv-tdep.c: Fix indentation. * frv-tdep.h: Fix indentation. * ft32-tdep.c: Fix indentation. * gcore.c: Fix indentation. * gdb_bfd.c: Fix indentation. * gdbarch.sh: Fix indentation. * gdbarch.c: Re-generate * gdbarch.h: Re-generate. * gdbcore.h: Fix indentation. * gdbthread.h: Fix indentation. * gdbtypes.c: Fix indentation. * gdbtypes.h: Fix indentation. * glibc-tdep.c: Fix indentation. * gnu-nat.c: Fix indentation. * gnu-nat.h: Fix indentation. * gnu-v2-abi.c: Fix indentation. * gnu-v3-abi.c: Fix indentation. * go32-nat.c: Fix indentation. * guile/guile-internal.h: Fix indentation. * guile/scm-cmd.c: Fix indentation. * guile/scm-frame.c: Fix indentation. * guile/scm-iterator.c: Fix indentation. * guile/scm-math.c: Fix indentation. * guile/scm-ports.c: Fix indentation. * guile/scm-pretty-print.c: Fix indentation. * guile/scm-value.c: Fix indentation. * h8300-tdep.c: Fix indentation. * hppa-linux-nat.c: Fix indentation. * hppa-linux-tdep.c: Fix indentation. * hppa-nbsd-nat.c: Fix indentation. * hppa-nbsd-tdep.c: Fix indentation. * hppa-obsd-nat.c: Fix indentation. * hppa-tdep.c: Fix indentation. * hppa-tdep.h: Fix indentation. * i386-bsd-nat.c: Fix indentation. * i386-darwin-nat.c: Fix indentation. * i386-darwin-tdep.c: Fix indentation. * i386-dicos-tdep.c: Fix indentation. * i386-gnu-nat.c: Fix indentation. * i386-linux-nat.c: Fix indentation. * i386-linux-tdep.c: Fix indentation. * i386-nto-tdep.c: Fix indentation. * i386-obsd-tdep.c: Fix indentation. * i386-sol2-nat.c: Fix indentation. * i386-tdep.c: Fix indentation. * i386-tdep.h: Fix indentation. * i386-windows-tdep.c: Fix indentation. * i387-tdep.c: Fix indentation. * i387-tdep.h: Fix indentation. * ia64-libunwind-tdep.c: Fix indentation. * ia64-libunwind-tdep.h: Fix indentation. * ia64-linux-nat.c: Fix indentation. * ia64-linux-tdep.c: Fix indentation. * ia64-tdep.c: Fix indentation. * ia64-tdep.h: Fix indentation. * ia64-vms-tdep.c: Fix indentation. * infcall.c: Fix indentation. * infcmd.c: Fix indentation. * inferior.c: Fix indentation. * infrun.c: Fix indentation. * iq2000-tdep.c: Fix indentation. * language.c: Fix indentation. * linespec.c: Fix indentation. * linux-fork.c: Fix indentation. * linux-nat.c: Fix indentation. * linux-tdep.c: Fix indentation. * linux-thread-db.c: Fix indentation. * lm32-tdep.c: Fix indentation. * m2-lang.c: Fix indentation. * m2-typeprint.c: Fix indentation. * m2-valprint.c: Fix indentation. * m32c-tdep.c: Fix indentation. * m32r-linux-tdep.c: Fix indentation. * m32r-tdep.c: Fix indentation. * m68hc11-tdep.c: Fix indentation. * m68k-bsd-nat.c: Fix indentation. * m68k-linux-nat.c: Fix indentation. * m68k-linux-tdep.c: Fix indentation. * m68k-tdep.c: Fix indentation. * machoread.c: Fix indentation. * macrocmd.c: Fix indentation. * macroexp.c: Fix indentation. * macroscope.c: Fix indentation. * macrotab.c: Fix indentation. * macrotab.h: Fix indentation. * main.c: Fix indentation. * mdebugread.c: Fix indentation. * mep-tdep.c: Fix indentation. * mi/mi-cmd-catch.c: Fix indentation. * mi/mi-cmd-disas.c: Fix indentation. * mi/mi-cmd-env.c: Fix indentation. * mi/mi-cmd-stack.c: Fix indentation. * mi/mi-cmd-var.c: Fix indentation. * mi/mi-cmds.c: Fix indentation. * mi/mi-main.c: Fix indentation. * mi/mi-parse.c: Fix indentation. * microblaze-tdep.c: Fix indentation. * minidebug.c: Fix indentation. * minsyms.c: Fix indentation. * mips-linux-nat.c: Fix indentation. * mips-linux-tdep.c: Fix indentation. * mips-nbsd-tdep.c: Fix indentation. * mips-tdep.c: Fix indentation. * mn10300-linux-tdep.c: Fix indentation. * mn10300-tdep.c: Fix indentation. * moxie-tdep.c: Fix indentation. * msp430-tdep.c: Fix indentation. * namespace.h: Fix indentation. * nat/fork-inferior.c: Fix indentation. * nat/gdb_ptrace.h: Fix indentation. * nat/linux-namespaces.c: Fix indentation. * nat/linux-osdata.c: Fix indentation. * nat/netbsd-nat.c: Fix indentation. * nat/x86-dregs.c: Fix indentation. * nbsd-nat.c: Fix indentation. * nbsd-tdep.c: Fix indentation. * nios2-linux-tdep.c: Fix indentation. * nios2-tdep.c: Fix indentation. * nto-procfs.c: Fix indentation. * nto-tdep.c: Fix indentation. * objfiles.c: Fix indentation. * objfiles.h: Fix indentation. * opencl-lang.c: Fix indentation. * or1k-tdep.c: Fix indentation. * osabi.c: Fix indentation. * osabi.h: Fix indentation. * osdata.c: Fix indentation. * p-lang.c: Fix indentation. * p-typeprint.c: Fix indentation. * p-valprint.c: Fix indentation. * parse.c: Fix indentation. * ppc-linux-nat.c: Fix indentation. * ppc-linux-tdep.c: Fix indentation. * ppc-nbsd-nat.c: Fix indentation. * ppc-nbsd-tdep.c: Fix indentation. * ppc-obsd-nat.c: Fix indentation. * ppc-ravenscar-thread.c: Fix indentation. * ppc-sysv-tdep.c: Fix indentation. * ppc64-tdep.c: Fix indentation. * printcmd.c: Fix indentation. * proc-api.c: Fix indentation. * producer.c: Fix indentation. * producer.h: Fix indentation. * prologue-value.c: Fix indentation. * prologue-value.h: Fix indentation. * psymtab.c: Fix indentation. * python/py-arch.c: Fix indentation. * python/py-bpevent.c: Fix indentation. * python/py-event.c: Fix indentation. * python/py-event.h: Fix indentation. * python/py-finishbreakpoint.c: Fix indentation. * python/py-frame.c: Fix indentation. * python/py-framefilter.c: Fix indentation. * python/py-inferior.c: Fix indentation. * python/py-infthread.c: Fix indentation. * python/py-objfile.c: Fix indentation. * python/py-prettyprint.c: Fix indentation. * python/py-registers.c: Fix indentation. * python/py-signalevent.c: Fix indentation. * python/py-stopevent.c: Fix indentation. * python/py-stopevent.h: Fix indentation. * python/py-threadevent.c: Fix indentation. * python/py-tui.c: Fix indentation. * python/py-unwind.c: Fix indentation. * python/py-value.c: Fix indentation. * python/py-xmethods.c: Fix indentation. * python/python-internal.h: Fix indentation. * python/python.c: Fix indentation. * ravenscar-thread.c: Fix indentation. * record-btrace.c: Fix indentation. * record-full.c: Fix indentation. * record.c: Fix indentation. * reggroups.c: Fix indentation. * regset.h: Fix indentation. * remote-fileio.c: Fix indentation. * remote.c: Fix indentation. * reverse.c: Fix indentation. * riscv-linux-tdep.c: Fix indentation. * riscv-ravenscar-thread.c: Fix indentation. * riscv-tdep.c: Fix indentation. * rl78-tdep.c: Fix indentation. * rs6000-aix-tdep.c: Fix indentation. * rs6000-lynx178-tdep.c: Fix indentation. * rs6000-nat.c: Fix indentation. * rs6000-tdep.c: Fix indentation. * rust-lang.c: Fix indentation. * rx-tdep.c: Fix indentation. * s12z-tdep.c: Fix indentation. * s390-linux-tdep.c: Fix indentation. * score-tdep.c: Fix indentation. * ser-base.c: Fix indentation. * ser-mingw.c: Fix indentation. * ser-uds.c: Fix indentation. * ser-unix.c: Fix indentation. * serial.c: Fix indentation. * sh-linux-tdep.c: Fix indentation. * sh-nbsd-tdep.c: Fix indentation. * sh-tdep.c: Fix indentation. * skip.c: Fix indentation. * sol-thread.c: Fix indentation. * solib-aix.c: Fix indentation. * solib-darwin.c: Fix indentation. * solib-frv.c: Fix indentation. * solib-svr4.c: Fix indentation. * solib.c: Fix indentation. * source.c: Fix indentation. * sparc-linux-tdep.c: Fix indentation. * sparc-nbsd-tdep.c: Fix indentation. * sparc-obsd-tdep.c: Fix indentation. * sparc-ravenscar-thread.c: Fix indentation. * sparc-tdep.c: Fix indentation. * sparc64-linux-tdep.c: Fix indentation. * sparc64-nbsd-tdep.c: Fix indentation. * sparc64-obsd-tdep.c: Fix indentation. * sparc64-tdep.c: Fix indentation. * stabsread.c: Fix indentation. * stack.c: Fix indentation. * stap-probe.c: Fix indentation. * stubs/ia64vms-stub.c: Fix indentation. * stubs/m32r-stub.c: Fix indentation. * stubs/m68k-stub.c: Fix indentation. * stubs/sh-stub.c: Fix indentation. * stubs/sparc-stub.c: Fix indentation. * symfile-mem.c: Fix indentation. * symfile.c: Fix indentation. * symfile.h: Fix indentation. * symmisc.c: Fix indentation. * symtab.c: Fix indentation. * symtab.h: Fix indentation. * target-float.c: Fix indentation. * target.c: Fix indentation. * target.h: Fix indentation. * tic6x-tdep.c: Fix indentation. * tilegx-linux-tdep.c: Fix indentation. * tilegx-tdep.c: Fix indentation. * top.c: Fix indentation. * tracefile-tfile.c: Fix indentation. * tracepoint.c: Fix indentation. * tui/tui-disasm.c: Fix indentation. * tui/tui-io.c: Fix indentation. * tui/tui-regs.c: Fix indentation. * tui/tui-stack.c: Fix indentation. * tui/tui-win.c: Fix indentation. * tui/tui-winsource.c: Fix indentation. * tui/tui.c: Fix indentation. * typeprint.c: Fix indentation. * ui-out.h: Fix indentation. * unittests/copy_bitwise-selftests.c: Fix indentation. * unittests/memory-map-selftests.c: Fix indentation. * utils.c: Fix indentation. * v850-tdep.c: Fix indentation. * valarith.c: Fix indentation. * valops.c: Fix indentation. * valprint.c: Fix indentation. * valprint.h: Fix indentation. * value.c: Fix indentation. * value.h: Fix indentation. * varobj.c: Fix indentation. * vax-tdep.c: Fix indentation. * windows-nat.c: Fix indentation. * windows-tdep.c: Fix indentation. * xcoffread.c: Fix indentation. * xml-syscall.c: Fix indentation. * xml-tdesc.c: Fix indentation. * xstormy16-tdep.c: Fix indentation. * xtensa-config.c: Fix indentation. * xtensa-linux-nat.c: Fix indentation. * xtensa-linux-tdep.c: Fix indentation. * xtensa-tdep.c: Fix indentation. gdbserver/ChangeLog: * ax.cc: Fix indentation. * dll.cc: Fix indentation. * inferiors.h: Fix indentation. * linux-low.cc: Fix indentation. * linux-nios2-low.cc: Fix indentation. * linux-ppc-ipa.cc: Fix indentation. * linux-ppc-low.cc: Fix indentation. * linux-x86-low.cc: Fix indentation. * linux-xtensa-low.cc: Fix indentation. * regcache.cc: Fix indentation. * server.cc: Fix indentation. * tracepoint.cc: Fix indentation. gdbsupport/ChangeLog: * common-exceptions.h: Fix indentation. * event-loop.cc: Fix indentation. * fileio.cc: Fix indentation. * filestuff.cc: Fix indentation. * gdb-dlfcn.cc: Fix indentation. * gdb_string_view.h: Fix indentation. * job-control.cc: Fix indentation. * signals.cc: Fix indentation. Change-Id: I4bad7ae6be0fbe14168b8ebafb98ffe14964a695
2020-11-02 10:26:14 -05:00
return PyErr_Format (PyExc_RuntimeError, \
_("TUI window is invalid.")); \
} while (0)
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.
2021-01-15 10:31:19 +00:00
/* 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 *
gdbpy_tui_is_valid (PyObject *self, PyObject *args)
{
gdbpy_tui_window *win = (gdbpy_tui_window *) self;
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.
2021-01-15 10:31:19 +00:00
if (win->is_valid ())
Py_RETURN_TRUE;
Py_RETURN_FALSE;
}
/* Python function that erases the TUI window. */
static PyObject *
gdbpy_tui_erase (PyObject *self, PyObject *args)
{
gdbpy_tui_window *win = (gdbpy_tui_window *) self;
REQUIRE_WINDOW (win);
win->window->erase ();
Py_RETURN_NONE;
}
/* Python function that writes some text to a TUI window. */
static PyObject *
gdbpy_tui_write (PyObject *self, PyObject *args, PyObject *kw)
{
static const char *keywords[] = { "string", "full_window", nullptr };
gdbpy_tui_window *win = (gdbpy_tui_window *) self;
const char *text;
int full_window = 0;
if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords,
&text, &full_window))
return nullptr;
REQUIRE_WINDOW (win);
win->window->output (text, full_window);
Py_RETURN_NONE;
}
/* Return the width of the TUI window. */
static PyObject *
gdbpy_tui_width (PyObject *self, void *closure)
{
gdbpy_tui_window *win = (gdbpy_tui_window *) self;
REQUIRE_WINDOW (win);
gdbpy_ref<> result
= gdb_py_object_from_longest (win->window->viewport_width ());
return result.release ();
}
/* Return the height of the TUI window. */
static PyObject *
gdbpy_tui_height (PyObject *self, void *closure)
{
gdbpy_tui_window *win = (gdbpy_tui_window *) self;
REQUIRE_WINDOW (win);
gdbpy_ref<> result
= gdb_py_object_from_longest (win->window->viewport_height ());
return result.release ();
}
/* Return the title of the TUI window. */
static PyObject *
gdbpy_tui_title (PyObject *self, void *closure)
{
gdbpy_tui_window *win = (gdbpy_tui_window *) self;
REQUIRE_WINDOW (win);
return host_string_to_python_string (win->window->title ().c_str ()).release ();
}
/* Set the title of the TUI window. */
static int
gdbpy_tui_set_title (PyObject *self, PyObject *newvalue, void *closure)
{
gdbpy_tui_window *win = (gdbpy_tui_window *) self;
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.
2021-01-15 10:31:19 +00:00
REQUIRE_WINDOW_FOR_SETTER (win);
if (newvalue == nullptr)
{
PyErr_Format (PyExc_TypeError, _("Cannot delete \"title\" attribute."));
return -1;
}
gdb::unique_xmalloc_ptr<char> value
= python_string_to_host_string (newvalue);
if (value == nullptr)
return -1;
win->window->set_title (value.get ());
return 0;
}
static gdb_PyGetSetDef tui_object_getset[] =
{
{ "width", gdbpy_tui_width, NULL, "Width of the window.", NULL },
{ "height", gdbpy_tui_height, NULL, "Height of the window.", NULL },
{ "title", gdbpy_tui_title, gdbpy_tui_set_title, "Title of the window.",
NULL },
{ NULL } /* Sentinel */
};
static PyMethodDef tui_object_methods[] =
{
{ "is_valid", gdbpy_tui_is_valid, METH_NOARGS,
"is_valid () -> Boolean\n\
Return true if this TUI window is valid, false if not." },
{ "erase", gdbpy_tui_erase, METH_NOARGS,
"Erase the TUI window." },
{ "write", (PyCFunction) gdbpy_tui_write, METH_VARARGS | METH_KEYWORDS,
"Append a string to the TUI window." },
{ NULL } /* Sentinel. */
};
PyTypeObject gdbpy_tui_window_object_type =
{
PyVarObject_HEAD_INIT (NULL, 0)
"gdb.TuiWindow", /*tp_name*/
sizeof (gdbpy_tui_window), /*tp_basicsize*/
0, /*tp_itemsize*/
0, /*tp_dealloc*/
0, /*tp_print*/
0, /*tp_getattr*/
0, /*tp_setattr*/
0, /*tp_compare*/
0, /*tp_repr*/
0, /*tp_as_number*/
0, /*tp_as_sequence*/
0, /*tp_as_mapping*/
0, /*tp_hash */
0, /*tp_call*/
0, /*tp_str*/
0, /*tp_getattro*/
0, /*tp_setattro */
0, /*tp_as_buffer*/
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /*tp_flags*/
"GDB TUI window object", /* tp_doc */
0, /* tp_traverse */
0, /* tp_clear */
0, /* tp_richcompare */
0, /* tp_weaklistoffset */
0, /* tp_iter */
0, /* tp_iternext */
tui_object_methods, /* tp_methods */
0, /* tp_members */
tui_object_getset, /* tp_getset */
0, /* tp_base */
0, /* tp_dict */
0, /* tp_descr_get */
0, /* tp_descr_set */
0, /* tp_dictoffset */
0, /* tp_init */
0, /* tp_alloc */
};
#endif /* TUI */
/* Initialize this module. */
gdb/python: add mechanism to manage Python initialization functions Currently, when we add a new python sub-system to GDB, e.g. py-inferior.c, we end up having to create a new function like gdbpy_initialize_inferior, which then has to be called from the function do_start_initialization in python.c. In some cases (py-micmd.c and py-tui.c), we have two functions gdbpy_initialize_*, and gdbpy_finalize_*, with the second being called from finalize_python which is also in python.c. This commit proposes a mechanism to manage these initialization and finalization calls, this means that adding a new Python subsystem will no longer require changes to python.c or python-internal.h, instead, the initialization and finalization functions will be registered directly from the sub-system file, e.g. py-inferior.c, or py-micmd.c. The initialization and finalization functions are managed through a new class gdbpy_initialize_file in python-internal.h. This class contains a single global vector of all the initialization and finalization functions. In each Python sub-system we create a new gdbpy_initialize_file object, the object constructor takes care of registering the two callback functions. Now from python.c we can call static functions on the gdbpy_initialize_file class which take care of walking the callback list and invoking each callback in turn. To slightly simplify the Python sub-system files I added a new macro GDBPY_INITIALIZE_FILE, which hides the need to create an object. We can now just do this: GDBPY_INITIALIZE_FILE (gdbpy_initialize_registers); One possible problem with this change is that there is now no guaranteed ordering of how the various sub-systems are initialized (or finalized). To try and avoid dependencies creeping in I have added a use of the environment variable GDB_REVERSE_INIT_FUNCTIONS, this is the same environment variable used in the generated init.c file. Just like with init.c, when this environment variable is set we reverse the list of Python initialization (and finalization) functions. As there is already a test that starts GDB with the environment variable set then this should offer some level of protection against dependencies creeping in - though for full protection I guess we'd need to run all gdb.python/*.exp tests with the variable set. I have tested this patch with the environment variable set, and saw no regressions, so I think we are fine right now. One other change of note was for gdbpy_initialize_gdb_readline, this function previously returned void. In order to make this function have the correct signature I've updated its return type to int, and we now return 0 to indicate success. All of the other initialize (and finalize) functions have been made static within their respective sub-system files. There should be no user visible changes after this commit.
2022-09-16 16:08:17 +01:00
static int CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION
gdbpy_initialize_tui ()
{
#ifdef TUI
gdbpy_tui_window_object_type.tp_new = PyType_GenericNew;
if (PyType_Ready (&gdbpy_tui_window_object_type) < 0)
return -1;
#endif /* TUI */
return 0;
}
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
/* Finalize this module. */
gdb/python: add mechanism to manage Python initialization functions Currently, when we add a new python sub-system to GDB, e.g. py-inferior.c, we end up having to create a new function like gdbpy_initialize_inferior, which then has to be called from the function do_start_initialization in python.c. In some cases (py-micmd.c and py-tui.c), we have two functions gdbpy_initialize_*, and gdbpy_finalize_*, with the second being called from finalize_python which is also in python.c. This commit proposes a mechanism to manage these initialization and finalization calls, this means that adding a new Python subsystem will no longer require changes to python.c or python-internal.h, instead, the initialization and finalization functions will be registered directly from the sub-system file, e.g. py-inferior.c, or py-micmd.c. The initialization and finalization functions are managed through a new class gdbpy_initialize_file in python-internal.h. This class contains a single global vector of all the initialization and finalization functions. In each Python sub-system we create a new gdbpy_initialize_file object, the object constructor takes care of registering the two callback functions. Now from python.c we can call static functions on the gdbpy_initialize_file class which take care of walking the callback list and invoking each callback in turn. To slightly simplify the Python sub-system files I added a new macro GDBPY_INITIALIZE_FILE, which hides the need to create an object. We can now just do this: GDBPY_INITIALIZE_FILE (gdbpy_initialize_registers); One possible problem with this change is that there is now no guaranteed ordering of how the various sub-systems are initialized (or finalized). To try and avoid dependencies creeping in I have added a use of the environment variable GDB_REVERSE_INIT_FUNCTIONS, this is the same environment variable used in the generated init.c file. Just like with init.c, when this environment variable is set we reverse the list of Python initialization (and finalization) functions. As there is already a test that starts GDB with the environment variable set then this should offer some level of protection against dependencies creeping in - though for full protection I guess we'd need to run all gdb.python/*.exp tests with the variable set. I have tested this patch with the environment variable set, and saw no regressions, so I think we are fine right now. One other change of note was for gdbpy_initialize_gdb_readline, this function previously returned void. In order to make this function have the correct signature I've updated its return type to int, and we now return 0 to indicate success. All of the other initialize (and finalize) functions have been made static within their respective sub-system files. There should be no user visible changes after this commit.
2022-09-16 16:08:17 +01:00
static void
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
gdbpy_finalize_tui ()
{
#ifdef TUI
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
gdbpy_tui_window_maker::invalidate_all ();
#endif /* TUI */
gdb/python: deallocate tui window factories at Python shut down The previous commit relied on spotting when a Python defined TUI window factory was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment, they are only deleted when one window factory replaces another. Consider this example Python script: class TestWindowFactory: def __init__(self, msg): self.msg = msg print("Entering TestWindowFactory.__init__: %s" % self.msg) def __call__(self, tui_win): print("Entering TestWindowFactory.__call__: %s" % self.msg) return TestWindow(tui_win, self.msg) def __del__(self): print("Entering TestWindowFactory.__del__: %s" % self.msg) gdb.register_window_type("test_window", TestWindowFactory("A")) gdb.register_window_type("test_window", TestWindowFactory("B")) And this GDB session: (gdb) source tui.py Entering TestWindowFactory.__init__: A Entering TestWindowFactory.__init__: B Entering TestWindowFactory.__del__: B (gdb) quit Notice that when the 'B' window replaces the 'A' window we see the 'A' object being deleted. But, when Python is shut down (after the 'quit') the 'B' object is never deleted. Instead, GDB retains a reference to the window factory object, which forces the Python object to remain live even after the Python interpreter itself has been shut down. The references themselves are held in a dynamically allocated std::unordered_map (in tui/tui-layout.c) which is never deallocated, thus the underlying Python references are never decremented to zero, and so GDB never tries to delete these Python objects. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects (the objects that implement the TUI window factory callback for Python defined TUI windows), are now linked together into a global list using the intrusive list mechanism. When GDB shuts down the Python interpreter we can now walk this global list and release the reference that is held to the underlying Python object. By releasing this reference the Python object will now be deleted. I've added a new assert in gdbpy_tui_window_maker::operator(), this will catch the case where we somehow end up in here after having reset the reference to the underlying Python object. I don't think this should ever happen though as we only clear the references when shutting down the Python interpreter, and the ::operator() function is only called when trying to apply a new TUI layout - something that shouldn't happen while GDB itself is shutting down. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. Reviewed-By: Tom Tromey <tom@tromey.com>
2023-01-12 15:47:17 +00:00
}
gdb/python: add mechanism to manage Python initialization functions Currently, when we add a new python sub-system to GDB, e.g. py-inferior.c, we end up having to create a new function like gdbpy_initialize_inferior, which then has to be called from the function do_start_initialization in python.c. In some cases (py-micmd.c and py-tui.c), we have two functions gdbpy_initialize_*, and gdbpy_finalize_*, with the second being called from finalize_python which is also in python.c. This commit proposes a mechanism to manage these initialization and finalization calls, this means that adding a new Python subsystem will no longer require changes to python.c or python-internal.h, instead, the initialization and finalization functions will be registered directly from the sub-system file, e.g. py-inferior.c, or py-micmd.c. The initialization and finalization functions are managed through a new class gdbpy_initialize_file in python-internal.h. This class contains a single global vector of all the initialization and finalization functions. In each Python sub-system we create a new gdbpy_initialize_file object, the object constructor takes care of registering the two callback functions. Now from python.c we can call static functions on the gdbpy_initialize_file class which take care of walking the callback list and invoking each callback in turn. To slightly simplify the Python sub-system files I added a new macro GDBPY_INITIALIZE_FILE, which hides the need to create an object. We can now just do this: GDBPY_INITIALIZE_FILE (gdbpy_initialize_registers); One possible problem with this change is that there is now no guaranteed ordering of how the various sub-systems are initialized (or finalized). To try and avoid dependencies creeping in I have added a use of the environment variable GDB_REVERSE_INIT_FUNCTIONS, this is the same environment variable used in the generated init.c file. Just like with init.c, when this environment variable is set we reverse the list of Python initialization (and finalization) functions. As there is already a test that starts GDB with the environment variable set then this should offer some level of protection against dependencies creeping in - though for full protection I guess we'd need to run all gdb.python/*.exp tests with the variable set. I have tested this patch with the environment variable set, and saw no regressions, so I think we are fine right now. One other change of note was for gdbpy_initialize_gdb_readline, this function previously returned void. In order to make this function have the correct signature I've updated its return type to int, and we now return 0 to indicate success. All of the other initialize (and finalize) functions have been made static within their respective sub-system files. There should be no user visible changes after this commit.
2022-09-16 16:08:17 +01:00
GDBPY_INITIALIZE_FILE (gdbpy_initialize_tui, gdbpy_finalize_tui);