[PATCH] Fix resizing of TUI python windows

[PATCH] Fix resizing of TUI python windows

[Hannes Domani]

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.
---
 gdb/python/py-tui.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index 64f22dbb462..a84e38a0563 100644
--- a/gdb/python/py-tui.c
+++ b/gdb/python/py-tui.c
@@ -102,6 +102,8 @@ class tui_py_window : public tui_win_info
       tui_win_info::refresh_window ();
   }
 
+  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.  */
@@ -232,6 +234,14 @@ tui_py_window::do_scroll_vertical (int num_to_scroll)
     }
 }
 
+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)
 {
-- 
2.35.1

Re: [PATCH] Fix resizing of TUI python windows

[Andrew Burgess]

Hannes Domani <ssbssa@yahoo.de> writes:

> 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.

Yeah, I wonder why we do it that way.  It feels like calling wclear()
might be more efficient.

Still given where we are I think your fix makes sense.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


> ---
>  gdb/python/py-tui.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
> index 64f22dbb462..a84e38a0563 100644
> --- a/gdb/python/py-tui.c
> +++ b/gdb/python/py-tui.c
> @@ -102,6 +102,8 @@ class tui_py_window : public tui_win_info
>        tui_win_info::refresh_window ();
>    }
>  
> +  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.  */
> @@ -232,6 +234,14 @@ tui_py_window::do_scroll_vertical (int num_to_scroll)
>      }
>  }
>  
> +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)
>  {
> -- 
> 2.35.1

Re: [PATCH] Fix resizing of TUI python windows

[Hannes Domani]

 Am Montag, 6. November 2023, 17:56:23 MEZ hat Andrew Burgess <aburgess@redhat.com> Folgendes geschrieben:

> Hannes Domani <ssbssa@yahoo.de> writes:
>
> > 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.
>
> Yeah, I wonder why we do it that way.  It feels like calling wclear()
> might be more efficient.
>
> Still given where we are I think your fix makes sense.
>
> Approved-By: Andrew Burgess <aburgess@redhat.com>
>
> Thanks,
> Andrew

Pushed, thanks.


Regards
Hannes