[Bug tui/31044] New: [gdb/tui] segfault when enabling TUI in window with two lines

[Bug tui/31044] New: [gdb/tui] segfault when enabling TUI in window with two lines

[vries at gcc dot gnu.org]

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

            Bug ID: 31044
           Summary: [gdb/tui] segfault when enabling TUI  in window with
                    two lines
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: tui
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

Say I got a terminal with 3 lines:
...
$ echo $COLUMNS
71
$ echo $LINES
3
...

Doing:
...
$ gdb -q a.out -tui
...
gets me:
...
┌─hello.c─────────────────────────────────────────────────────────────┐
(gdb) 

...

I guess this hits the GIGO case here:
...
  /* Step 3: Resize.  */
  int size_accum = 0;
  const int maximum = m_vertical ? height : width;
  for (int i = 0; i < m_splits.size (); ++i)
    {
      /* If we fall off the bottom, just make allocations overlap.              
         GIGO.  */
...

[ FWIW, we could make the source window unboxed in this case to make more
efficiently use of our screen. ]

Anyway, now reduce window size to two lines:
...
$ echo $LINES
2
...
and try again:
...
Thread 1 "gdb" received signal SIGSEGV, Segmentation fault.
0x0000000000d1e273 in gdb_puts (linebuffer=0x16df2e0
<internal_vproblem(internal_problem*, char const*, int, char const*,
__va_list_tag*)::msg> "Recursive internal problem.\n", stream=0x0) at
/data/vries/gdb/src/gdb/utils.c:1811
1811      stream->puts (linebuffer);
(gdb) bt
#0  0x0000000000d1e273 in gdb_puts (
    linebuffer=0x16df2e0 <internal_vproblem(internal_problem*, char const*,
int, char const*, __va_list_tag*)::msg> "Recursive internal problem.\n",
stream=0x0) at /data/vries/gdb/src/gdb/utils.c:1811
#1  0x0000000000d1b1cd in abort_with_message (
    msg=0x16df2e0 <internal_vproblem(internal_problem*, char const*, int, char
const*, __va_list_tag*)::msg> "Recursive internal problem.\n") at
/data/vries/gdb/src/gdb/utils.c:177
#2  0x0000000000d1b36e in internal_vproblem(internal_problem *, const char *,
int, const char *, typedef __va_list_tag __va_list_tag *) (problem=0x26cb8e0
<internal_error_problem>, file=0x1582e50 "/data/vries/gdb/src/gdb/cli-out.c", 
    line=436, fmt=0x1582e11 "%s: Assertion `%s' failed.", ap=0x7fffffffcde8) at
/data/vries/gdb/src/gdb/utils.c:339
#3  0x0000000000d1b90b in internal_verror (file=0x1582e50
"/data/vries/gdb/src/gdb/cli-out.c", line=436, 
    fmt=0x1582e11 "%s: Assertion `%s' failed.", ap=0x7fffffffcde8) at
/data/vries/gdb/src/gdb/utils.c:476
#4  0x00000000014d5465 in internal_error_loc (file=0x1582e50
"/data/vries/gdb/src/gdb/cli-out.c", line=436, 
    fmt=0x1582e11 "%s: Assertion `%s' failed.") at
/data/vries/gdb/src/gdbsupport/errors.cc:58
#5  0x00000000005ef44a in cli_ui_out::cli_ui_out (this=0x7fffffffcf30,
stream=0x0, flags=...)
    at /data/vries/gdb/src/gdb/cli-out.c:436
#6  0x0000000000cc67f5 in ui_file::vprintf (this=0x0, format=0x16df363 "%s\n",
args=0x7fffffffd008)
    at /data/vries/gdb/src/gdb/ui-file.c:74
#7  0x0000000000d1e50d in gdb_vprintf (stream=0x0, format=0x16df363 "%s\n",
args=0x7fffffffd008)
    at /data/vries/gdb/src/gdb/utils.c:1876
#8  0x0000000000d1e5e4 in gdb_printf (stream=0x0, format=0x16df363 "%s\n") at
/data/vries/gdb/src/gdb/utils.c:1891
#9  0x0000000000d1b53c in internal_vproblem(internal_problem *, const char *,
int, const char *, typedef __va_list_tag __va_list_tag *) (problem=0x26cb8e0
<internal_error_problem>, 
    file=0x16b80c0 "/data/vries/gdb/src/gdb/tui/tui-layout.c", line=90,
fmt=0x16b8020 "%s: Assertion `%s' failed.", 
    ap=0x7fffffffd238) at /data/vries/gdb/src/gdb/utils.c:393
#10 0x0000000000d1b90b in internal_verror (file=0x16b80c0
"/data/vries/gdb/src/gdb/tui/tui-layout.c", line=90, 
    fmt=0x16b8020 "%s: Assertion `%s' failed.", ap=0x7fffffffd238) at
/data/vries/gdb/src/gdb/utils.c:476
#11 0x00000000014d5465 in internal_error_loc (file=0x16b80c0
"/data/vries/gdb/src/gdb/tui/tui-layout.c", line=90, 
    fmt=0x16b8020 "%s: Assertion `%s' failed.") at
/data/vries/gdb/src/gdbsupport/errors.cc:58
#12 0x0000000000ca9cc2 in tui_apply_current_layout
(preserve_cmd_win_size_p=false)
--Type <RET> for more, q to quit, c to continue without paging--
    at /data/vries/gdb/src/gdb/tui/tui-layout.c:90
#13 0x0000000000caa05c in tui_set_layout (layout=0x2b58ee0) at
/data/vries/gdb/src/gdb/tui/tui-layout.c:149
#14 0x0000000000caa305 in tui_set_initial_layout () at
/data/vries/gdb/src/gdb/tui/tui-layout.c:225
#15 0x0000000000cc305b in tui_enable () at
/data/vries/gdb/src/gdb/tui/tui.c:465
#16 0x0000000000ca4318 in tui_interp::resume (this=0x2d56890) at
/data/vries/gdb/src/gdb/tui/tui-interp.c:132
#17 0x00000000008a4c49 in interp_set (interp=0x2d56890, top_level=true) at
/data/vries/gdb/src/gdb/interps.c:148
#18 0x00000000008a4e56 in set_top_level_interpreter (name=0x2984950
<interpreter_p[abi:cxx11]+16> "tui")
    at /data/vries/gdb/src/gdb/interps.c:199
#19 0x0000000000921926 in captured_main_1 (context=0x7fffffffda60) at
/data/vries/gdb/src/gdb/main.c:1143
#20 0x000000000092223d in captured_main (data=0x7fffffffda60) at
/data/vries/gdb/src/gdb/main.c:1314
#21 0x00000000009222dc in gdb_main (args=0x7fffffffda60) at
/data/vries/gdb/src/gdb/main.c:1343
#22 0x0000000000418bd2 in main (argc=11, argv=0x7fffffffdb78) at
/data/vries/gdb/src/gdb/gdb.c:39
(gdb) 
...

We segfault while failing this assertion in tui_apply_current_layout:
...
  /* This should always be made visible by a layout.  */
  gdb_assert (TUI_CMD_WIN != nullptr);
...

FWIW, it's the same problem (not just the assertion failure, also the segfault)
when typing "tui enable" on the gdb prompt instead of using -tui.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/31044] [gdb/tui] segfault when enabling TUI in window with two lines

[vries at gcc dot gnu.org]

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

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
Likewise, we run into trouble when we resize from 3 to 2 lines:
...
Thread 1 "gdb" received signal SIGSEGV, Segmentation fault.
__cxxabiv1::__dynamic_cast (src_ptr=0x3177c60, src_type=0x16b7160 <typeinfo for
tui_win_info>, dst_type=0x16bbd10 <typeinfo for tui_source_window_base>,
src2dst=0) at ../../../../libstdc++-v3/libsupc++/dyncast.cc:57
57        const void *whole_ptr =
(gdb) bt
#0  __cxxabiv1::__dynamic_cast (src_ptr=0x3177c60, src_type=0x16b7160 <typeinfo
for tui_win_info>, 
    dst_type=0x16bbd10 <typeinfo for tui_source_window_base>, src2dst=0)
    at ../../../../libstdc++-v3/libsupc++/dyncast.cc:57
#1  0x0000000000cbafd6 in tui_source_window_iterator::advance
(this=0x7ffe269ea2e0)
    at /data/vries/gdb/src/gdb/tui/tui-winsource.h:307
#2  0x0000000000cbaeb1 in
tui_source_window_iterator::tui_source_window_iterator (this=0x7ffe269ea2e0,
it=0x3177c60, 
    end=0x200000006) at /data/vries/gdb/src/gdb/tui/tui-winsource.h:277
#3  0x0000000000cbb041 in tui_source_windows::begin (this=0x7ffe269ea3af)
    at /data/vries/gdb/src/gdb/tui/tui-winsource.h:326
#4  0x0000000000cbfa58 in tui_update_source_windows_with_addr
(gdbarch=0x3c75fb0, addr=4195607)
    at /data/vries/gdb/src/gdb/tui/tui-winsource.c:190
#5  0x0000000000ca9e51 in tui_apply_current_layout
(preserve_cmd_win_size_p=false)
    at /data/vries/gdb/src/gdb/tui/tui-layout.c:114
#6  0x0000000000cbcc3f in tui_resize_all () at
/data/vries/gdb/src/gdb/tui/tui-win.c:525
#7  0x0000000000cbccee in tui_async_resize_screen (arg=0x0) at
/data/vries/gdb/src/gdb/tui/tui-win.c:562
#8  0x00000000005328da in invoke_async_signal_handlers () at
/data/vries/gdb/src/gdb/async-event.c:234
#9  0x00000000014d5758 in gdb_do_one_event (mstimeout=-1) at
/data/vries/gdb/src/gdbsupport/event-loop.cc:199
#10 0x00000000009208d1 in start_event_loop () at
/data/vries/gdb/src/gdb/main.c:407
#11 0x0000000000920a31 in captured_command_loop () at
/data/vries/gdb/src/gdb/main.c:471
#12 0x0000000000922242 in captured_main (data=0x7ffe269ea700) at
/data/vries/gdb/src/gdb/main.c:1324
#13 0x00000000009222dc in gdb_main (args=0x7ffe269ea700) at
/data/vries/gdb/src/gdb/main.c:1343
#14 0x0000000000418bd2 in main (argc=11, argv=0x7ffe269ea818) at
/data/vries/gdb/src/gdb/gdb.c:39
(gdb) 
...

What happens is that tui_windows and new_tui_windows contain the same windows:
...
gdb) p tui_windows   
$14 = std::vector of length 3, capacity 4 = {0x424fc60, 0x4d466d0, 0x4d467b0}
(gdb) p new_tui_windows
$15 = std::vector of length 3, capacity 4 = {0x424fc60, 0x4d466d0, 0x4d467b0}
...
but now the source window is no longer visible, so this in this loop:
...
  /* Now delete any window that was not re-applied.  */
  tui_win_info *focus = tui_win_with_focus ();
  for (tui_win_info *win_info : tui_windows)
    {
      if (!win_info->is_visible ())
        {
          if (focus == win_info)
            tui_set_win_focus_to (new_tui_windows[0]);
          delete win_info;
        }
    }
...
we delete the source window without deleting it from new_tui_windows.

In fact, we also set the focus to the deleted window.

The assumption seems to be that new_tui_windows contains only visible windows. 
This makes sure that that's the case:
...
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 159445dc520..2826207ac0c 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -93,6 +93,11 @@ tui_apply_current_layout (bool preserve_cmd_win_size_p)
   /* Get the new list of currently visible windows.  */
   std::vector<tui_win_info *> new_tui_windows;
   applied_layout->get_windows (&new_tui_windows);
+  for (auto it = new_tui_windows.begin (); it != new_tui_windows.end (); )
+    if ((*it)->is_visible ())
+      it++;
+    else
+      it = new_tui_windows.erase (it);

   /* Now delete any window that was not re-applied.  */
   tui_win_info *focus = tui_win_with_focus ();
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/31044] [gdb/tui] segfault when enabling TUI in window with two lines

[vries at gcc dot gnu.org]

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

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #1)
> Likewise, we run into trouble when we resize from 3 to 2 lines:

The root cause seems to be that this call to newwin returns nullptr:
...
│  >   168    handle.reset (newwin (height, width, y, x));
...
because y == -1:
...
(gdb) p height
$33 = 3
(gdb) p width 
$34 = 68
(gdb) p y
$35 = -1
(gdb) p x
$36 = 0
(...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/31044] [gdb/tui] segfault when enabling TUI in window with two lines

[vries at gcc dot gnu.org]

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

--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
The resizing problem seem to be fixed by:
...
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 159445dc520..ace92fec52b 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -453,6 +453,12 @@ tui_layout_window::apply (int x_, int y_, int width_, int
height_,
   height = height_;
   gdb_assert (m_window != nullptr);
   m_window->resize (height, width, x, y);
+  if (!m_window->is_visible ())
+    {
+      /* The window is not visible, so it's going to be deleted, reset the
+        soon to be dangling pointer.  */
+      m_window = nullptr;
+    }
 }

 /* See tui-layout.h.  */
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index a6d34851bef..4d29c0570ac 100644
--- a/gdb/tui/tui-layout.h
+++ b/gdb/tui/tui-layout.h
@@ -189,7 +189,11 @@ class tui_layout_window : public tui_layout_base
   /* See tui_layout_base::get_windows.  */
   void get_windows (std::vector<tui_win_info *> *windows) override
   {
-    windows->push_back (m_window);
+    if (m_window != nullptr && m_window->is_visible ())
+      {
+       /* Only get visible windows.  */
+       windows->push_back (m_window);
+      }
   }

 protected:
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/31044] [gdb/tui] segfault when enabling TUI in window with two lines

[vries at gcc dot gnu.org]

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

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
Submitted patch series here:
https://sourceware.org/pipermail/gdb-patches/2023-November/203928.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/31044] [gdb/tui] segfault when enabling TUI in window with two lines

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <vries@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6697fa28bb6ee610323ea10cb815bac85c6d8184

commit 6697fa28bb6ee610323ea10cb815bac85c6d8184
Author: Tom de Vries <tdevries@suse.de>
Date:   Wed Nov 22 19:07:47 2023 +0100

    [gdb/tui] Allow command window of 1 or 2 lines

    When starting TUI in a terminal with 2 lines (likewise with 1 line):
    ...
    $ echo $LINES
    2
    $ gdb -q -tui
    ...
    we run into this assert in tui_apply_current_layout:
    ...
      /* This should always be made visible by a layout.  */
      gdb_assert (TUI_CMD_WIN != nullptr);
    ...

    The problem is that for the command window:
    - the minimum height is 3 (the default), but
    - the maximum height is only 2 because there are only 2 lines.

    This discrepancy eventually leads to a call to newwin in make_window with:
    ...
    (gdb) p height
    $1 = 3
    (gdb) p width
    $2 = 66
    (gdb) p y
    $3 = -1
    (gdb) p x
    $4 = 0
    (gdb)
    ...
    which results in a nullptr, which eventually triggers the assert.

    The easiest way to fix this is to change the minimum height of the command
    window to 1.  However, that would also change behaviour for the case that
the
    screen size is 3 lines or more.  For instance, in gdb.tui/winheight.exp the
    number of lines in the terminal is 24, and the test-case checks that the
user
    cannot increase the source window height to the point that the command
window
    height would be less than 3.

    Fix this by calculating the minimum height of the command window as
follows:
    - the default (3) if max_height () allows it, and
    - max_height () otherwise.

    Tested on x86_64-linux.

    Approved-By: Tom Tromey <tom@tromey.com>

    PR tui/31044
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31044

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/31044] [gdb/tui] segfault when enabling TUI in window with two lines

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <vries@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=03893ce67b5d1af649d59f7e63b3959ce0ac0709

commit 03893ce67b5d1af649d59f7e63b3959ce0ac0709
Author: Tom de Vries <tdevries@suse.de>
Date:   Wed Nov 22 19:07:47 2023 +0100

    [gdb/tui] Fix resizing of terminal to 1 or 2 lines

    When starting TUI in a terminal with 3 lines:
    ...
    $ echo $LINES
    3
    $ gdb -q -tui
    ...
    and resizing the terminal to 2 lines we run into a segfault.

    The problem is that for the source window:
    - the minimum height is 3 (the default), but
    - the maximum height is only 2 because there are only 2 lines.

    This discrepancy eventually leads to a call to newwin in make_window with:
    ...
    (gdb) p height
    $1 = 3
    (gdb) p width
    $2 = 56
    (gdb) p y
    $3 = -1
    (gdb) p x
    $4 = 0
    ...
    which results in a nullptr.

    This violates the assumption here in tui_apply_current_layout:
    ....
      /* Get the new list of currently visible windows.  */
      std::vector<tui_win_info *> new_tui_windows;
      applied_layout->get_windows (&new_tui_windows);
    ...
    that get_windows only returns visible windows, which leads to tui_windows
    holding a dangling pointer, which results in the segfault.

    Fix this by:
    - making sure get_windows only returns visible windows, and
    - detecting the situation and dropping windows from the layout if
      there's no room for them.

    Tested on x86_64-linux.

    Approved-By: Tom Tromey <tom@tromey.com>

    PR tui/31044
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31044

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tui/31044] [gdb/tui] segfault when enabling TUI in window with two lines

[vries at gcc dot gnu.org]

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

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |15.1
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Tom de Vries <vries at gcc dot gnu.org> ---
Fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.