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

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

[Tom de Vries]

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.

PR tui/31044
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31044
---
 gdb/testsuite/gdb.tui/small-term.exp | 29 ++++++++++++++++++++++++++++
 gdb/tui/tui-command.h                | 12 ++++++++++++
 2 files changed, 41 insertions(+)
 create mode 100644 gdb/testsuite/gdb.tui/small-term.exp

diff --git a/gdb/testsuite/gdb.tui/small-term.exp b/gdb/testsuite/gdb.tui/small-term.exp
new file mode 100644
index 00000000000..c67c5cae1e0
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/small-term.exp
@@ -0,0 +1,29 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test running TUI in a tuiterm with few lines.
+
+tuiterm_env
+
+foreach_with_prefix lines { 1 2 3 4 5 6 7 } {
+    Term::clean_restart $lines 80
+
+    if {![Term::enter_tui]} {
+	unsupported "TUI not supported"
+	return
+    }
+
+    Term::check_region_contents "has prompt" 0 0 80 $lines "$gdb_prompt"
+}
diff --git a/gdb/tui/tui-command.h b/gdb/tui/tui-command.h
index f6842880bb2..e8c96ecee30 100644
--- a/gdb/tui/tui-command.h
+++ b/gdb/tui/tui-command.h
@@ -57,6 +57,18 @@ struct tui_cmd_window : public tui_win_info
     /* The command window can't be made invisible.  */
   }
 
+  /* Compute the minimum height of this window.  */
+  virtual int min_height () const override
+  {
+    int preferred_min = tui_win_info::min_height ();
+    int max = max_height ();
+    /* If there is enough space to accommodate the preferred minimum height,
+       use it.  Otherwise, use as much as possible.  */
+    return (preferred_min <= max
+	    ? preferred_min
+	    : max);
+  }
+
   int start_line = 0;
 
 protected:

base-commit: cf5f570bd002fac5bc820d220d5cf8584cbaed6d
-- 
2.35.3

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

[Tom de Vries]

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.

PR tui/31044
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31044
---
 gdb/testsuite/gdb.tui/resize.exp |  9 +++++++++
 gdb/tui/tui-layout.c             | 22 ++++++++++++++++++++--
 gdb/tui/tui-layout.h             |  6 +++++-
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/resize.exp b/gdb/testsuite/gdb.tui/resize.exp
index 60d5116886b..e15f8a2f507 100644
--- a/gdb/testsuite/gdb.tui/resize.exp
+++ b/gdb/testsuite/gdb.tui/resize.exp
@@ -39,3 +39,12 @@ Term::check_contents "source at startup" "\\|.*21 *return 0"
 
 Term::resize 40 90
 Term::check_box "source box after resize" 0 0 90 26
+
+# Check that resizing to less than 3 lines doesn't cause problems.
+foreach lines { 2 1 } {
+    with_test_prefix lines=$lines {
+	Term::resize $lines 90 0
+	Term::wait_for ""
+	Term::check_region_contents "has prompt" 0 0 90 $lines "$gdb_prompt"
+    }
+}
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 159445dc520..4f8a4bc3bcc 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -452,6 +452,13 @@ tui_layout_window::apply (int x_, int y_, int width_, int height_,
   width = width_;
   height = height_;
   gdb_assert (m_window != nullptr);
+  if (width == 0 || height == 0)
+    {
+      /* The window was dropped, so it's going to be deleted, reset the
+	 soon to be dangling pointer.  */
+      m_window = nullptr;
+      return;
+    }
   m_window->resize (height, width, x, y);
 }
 
@@ -833,6 +840,7 @@ tui_layout_split::apply (int x_, int y_, int width_, int height_,
   int available_size = m_vertical ? height : width;
   int last_index = -1;
   int total_weight = 0;
+  int prev = -1;
   for (int i = 0; i < m_splits.size (); ++i)
     {
       bool cmd_win_already_exists = TUI_CMD_WIN != nullptr;
@@ -864,6 +872,14 @@ tui_layout_split::apply (int x_, int y_, int width_, int height_,
 	  info[i].max_size = info[i].min_size;
 	}
 
+      if (info[i].min_size > info[i].max_size)
+	{
+	  /* There is not enough room for this window, drop it.  */
+	  info[i].min_size = 0;
+	  info[i].max_size = 0;
+	  continue;
+	}
+
       if (info[i].min_size == info[i].max_size)
 	available_size -= info[i].min_size;
       else
@@ -874,10 +890,12 @@ tui_layout_split::apply (int x_, int y_, int width_, int height_,
 
       /* Two adjacent boxed windows will share a border, making a bit
 	 more size available.  */
-      if (i > 0
-	  && m_splits[i - 1].layout->last_edge_has_border_p ()
+      if (prev != -1
+	  && m_splits[prev].layout->last_edge_has_border_p ()
 	  && m_splits[i].layout->first_edge_has_border_p ())
 	info[i].share_box = true;
+
+      prev = i;
     }
 
   /* If last_index is set then we have a window that is not of a fixed
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:
-- 
2.35.3

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

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> +  /* Compute the minimum height of this window.  */
Tom> +  virtual int min_height () const override
Tom> +  {
Tom> +    int preferred_min = tui_win_info::min_height ();
Tom> +    int max = max_height ();
Tom> +    /* If there is enough space to accommodate the preferred minimum height,
Tom> +       use it.  Otherwise, use as much as possible.  */
Tom> +    return (preferred_min <= max
Tom> +	    ? preferred_min
Tom> +	    : max);

I read this and was wondering if this will make the command window
ignore the weight given to it in the layout.

Tom

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

[Tom de Vries]

On 11/13/23 18:36, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> +  /* Compute the minimum height of this window.  */
> Tom> +  virtual int min_height () const override
> Tom> +  {
> Tom> +    int preferred_min = tui_win_info::min_height ();
> Tom> +    int max = max_height ();
> Tom> +    /* If there is enough space to accommodate the preferred minimum height,
> Tom> +       use it.  Otherwise, use as much as possible.  */
> Tom> +    return (preferred_min <= max
> Tom> +	    ? preferred_min
> Tom> +	    : max);
> 
> I read this and was wondering if this will make the command window
> ignore the weight given to it in the layout.


The current minimum height of the command window is 
tui_win_info::min_height () == 3.

A weight can only make the size larger, not smaller than the minimum size.

This patch only has effect for the case that the terminal size is 
smaller than 3, so my understanding is that weight in this case is 
likewise inconsequential.

Thanks,
- Tom