[PATCH] [gdb/tui] Fix displaying main after resizing

[PATCH] [gdb/tui] Fix displaying main after resizing

[Tom de Vries]

A TUI src window is displaying either:
- the source for the current frame,
- the source for main, or
- the string "[ No Source Available ]".

Since commit 03893ce67b5 ("[gdb/tui] Fix resizing of terminal to 1 or 2 lines")
we're able to resize the TUI to 1 line without crashing.

I noticed that if TUI is displaying main, and we resize to 1 line (destroying
the src window) and then back to a larger terminal (reconstructing the src
window), the TUI displays "[ No Source Available ]" instead of main.

Fix this by moving a call to tui_display_main from tui_enable to
tui_apply_current_layout.

Tested on x86_64-linux.
---
 gdb/testsuite/gdb.tui/resize-one-line.exp | 50 +++++++++++++++++++++++
 gdb/tui/tui-layout.c                      |  5 ++-
 gdb/tui/tui.c                             |  2 -
 3 files changed, 54 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.tui/resize-one-line.exp

diff --git a/gdb/testsuite/gdb.tui/resize-one-line.exp b/gdb/testsuite/gdb.tui/resize-one-line.exp
new file mode 100644
index 00000000000..5f8847cb94b
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/resize-one-line.exp
@@ -0,0 +1,50 @@
+# 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/>.
+
+# Check that when showing main (because of lack of current frame) in the src
+# window, resizing to 1 line and back doesn't change the contents of the src
+# window.
+
+tuiterm_env
+
+standard_testfile main-one-line.c
+
+if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} {
+    return -1
+}
+
+Term::clean_restart 24 80 $testfile
+
+if {![Term::enter_tui]} {
+    unsupported "TUI not supported"
+    return
+}
+
+# Check that we have a source box containing main.
+Term::check_box "source box" 0 0 80 15
+Term::check_region_contents "src window shows main" 0 0 80 15 "main"
+
+# Resize to a single line.
+Term::resize 1 80 0
+Term::wait_for ""
+
+# Resize back to the previous size.
+Term::resize 24 80 0
+
+# Check that we still have a source box containing main.
+with_test_prefix "after resizing" {
+    gdb_assert { [Term::wait_for_region_contents  0 0 80 15 "main"] } \
+	"src window shows main"
+}
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 85f4991c769..e1507fa25c2 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -111,7 +111,10 @@ tui_apply_current_layout (bool preserve_cmd_win_size_p)
 
   if (gdbarch == nullptr && TUI_DISASM_WIN != nullptr)
     tui_get_begin_asm_address (&gdbarch, &addr);
-  tui_update_source_windows_with_addr (gdbarch, addr);
+  if (addr == 0)
+    tui_display_main ();
+  else
+    tui_update_source_windows_with_addr (gdbarch, addr);
 }
 
 /* See tui-layout.  */
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 941c65c970f..83fe343bd3d 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -495,8 +495,6 @@ tui_enable (void)
 
   if (deprecated_safe_get_selected_frame ())
     tui_show_frame_info (deprecated_safe_get_selected_frame ());
-  else
-    tui_display_main ();
 
   /* Install the TUI specific hooks.  This must be done after the call to
      tui_display_main so that we don't detect the symtab changed event it

base-commit: ad785961c9ee2dbe80f233cf7f6f408e678d2197
-- 
2.35.3

Re: [PATCH] [gdb/tui] Fix displaying main after resizing

[Tom Tromey]

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

Tom> Fix this by moving a call to tui_display_main from tui_enable to
Tom> tui_apply_current_layout.

Tom> -  tui_update_source_windows_with_addr (gdbarch, addr);
Tom> +  if (addr == 0)
Tom> +    tui_display_main ();
Tom> +  else
Tom> +    tui_update_source_windows_with_addr (gdbarch, addr);

This 'if' seems like something the source window ought to know, not the
layout code.

But between this and the register window patch in the same spot, I
wonder if there's some more generic solution available.  Like, if I
write a TUI window in Python, will redoing the layout cause a refresh?
If not -- isn't that a bug?  Or if so -- why can't built-in windows work
the same way?

Tom

Re: [PATCH] [gdb/tui] Fix displaying main after resizing

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 1115 bytes --]

On 11/25/23 04:43, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> Fix this by moving a call to tui_display_main from tui_enable to
> Tom> tui_apply_current_layout.
> 
> Tom> -  tui_update_source_windows_with_addr (gdbarch, addr);
> Tom> +  if (addr == 0)
> Tom> +    tui_display_main ();
> Tom> +  else
> Tom> +    tui_update_source_windows_with_addr (gdbarch, addr);
> 
> This 'if' seems like something the source window ought to know, not the
> layout code.
> 

I've fixed this by moving the logic to tui_source_window_base::rerender.

> But between this and the register window patch in the same spot, I
> wonder if there's some more generic solution available.  Like, if I
> write a TUI window in Python, will redoing the layout cause a refresh?

I think so.

> If not -- isn't that a bug?  Or if so -- why can't built-in windows work
> the same way?

My understanding is that the rerender was simply incomplete, and python 
windows will can implement whatever they like in a rerender, so I 
suppose I see no problem there.

I hope that answers your question.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-tui-Fix-displaying-main-after-resizing.patch --]
[-- Type: text/x-patch, Size: 6972 bytes --]

From 045afd2065366b69b63734669c28360b97417127 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Sat, 25 Nov 2023 10:10:23 +0100
Subject: [PATCH] [gdb/tui] Fix displaying main after resizing

A TUI src window is displaying either:
- the source for the current frame,
- the source for main, or
- the string "[ No Source Available ]".

Since commit 03893ce67b5 ("[gdb/tui] Fix resizing of terminal to 1 or 2 lines")
we're able to resize the TUI to 1 line without crashing.

I noticed that if TUI is displaying main, and we resize to 1 line (destroying
the src window) and then back to a larger terminal (reconstructing the src
window), the TUI displays "[ No Source Available ]" instead of main.

Fix this by moving the responsibility for showing main from tui_enable to
tui_source_window_base::rerender.

Tested on x86_64-linux.
---
 gdb/testsuite/gdb.tui/resize-one-line.exp | 50 +++++++++++++++++++++++
 gdb/tui/tui-layout.c                      | 25 ------------
 gdb/tui/tui-winsource.c                   | 22 +++++++++-
 gdb/tui/tui-winsource.h                   |  5 +++
 gdb/tui/tui.c                             |  5 ---
 5 files changed, 76 insertions(+), 31 deletions(-)
 create mode 100644 gdb/testsuite/gdb.tui/resize-one-line.exp

diff --git a/gdb/testsuite/gdb.tui/resize-one-line.exp b/gdb/testsuite/gdb.tui/resize-one-line.exp
new file mode 100644
index 00000000000..5f8847cb94b
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/resize-one-line.exp
@@ -0,0 +1,50 @@
+# 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/>.
+
+# Check that when showing main (because of lack of current frame) in the src
+# window, resizing to 1 line and back doesn't change the contents of the src
+# window.
+
+tuiterm_env
+
+standard_testfile main-one-line.c
+
+if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} {
+    return -1
+}
+
+Term::clean_restart 24 80 $testfile
+
+if {![Term::enter_tui]} {
+    unsupported "TUI not supported"
+    return
+}
+
+# Check that we have a source box containing main.
+Term::check_box "source box" 0 0 80 15
+Term::check_region_contents "src window shows main" 0 0 80 15 "main"
+
+# Resize to a single line.
+Term::resize 1 80 0
+Term::wait_for ""
+
+# Resize back to the previous size.
+Term::resize 24 80 0
+
+# Check that we still have a source box containing main.
+with_test_prefix "after resizing" {
+    gdb_assert { [Term::wait_for_region_contents  0 0 80 15 "main"] } \
+	"src window shows main"
+}
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 85f4991c769..a4cb5a8b7bf 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -45,8 +45,6 @@
 #include "gdb_curses.h"
 #include "gdbsupport/gdb-safe-ctype.h"
 
-static void extract_display_start_addr (struct gdbarch **, CORE_ADDR *);
-
 /* The layouts.  */
 static std::vector<std::unique_ptr<tui_layout_split>> layouts;
 
@@ -69,11 +67,6 @@ std::vector<tui_win_info *> tui_windows;
 void
 tui_apply_current_layout (bool preserve_cmd_win_size_p)
 {
-  struct gdbarch *gdbarch;
-  CORE_ADDR addr;
-
-  extract_display_start_addr (&gdbarch, &addr);
-
   for (tui_win_info *win_info : tui_windows)
     win_info->make_visible (false);
 
@@ -108,10 +101,6 @@ tui_apply_current_layout (bool preserve_cmd_win_size_p)
 
   /* Replace the global list of active windows.  */
   tui_windows = std::move (new_tui_windows);
-
-  if (gdbarch == nullptr && TUI_DISASM_WIN != nullptr)
-    tui_get_begin_asm_address (&gdbarch, &addr);
-  tui_update_source_windows_with_addr (gdbarch, addr);
 }
 
 /* See tui-layout.  */
@@ -284,20 +273,6 @@ tui_remove_some_windows ()
   tui_apply_current_layout (true);
 }
 
-static void
-extract_display_start_addr (struct gdbarch **gdbarch_p, CORE_ADDR *addr_p)
-{
-  if (TUI_SRC_WIN != nullptr)
-    TUI_SRC_WIN->display_start_addr (gdbarch_p, addr_p);
-  else if (TUI_DISASM_WIN != nullptr)
-    TUI_DISASM_WIN->display_start_addr (gdbarch_p, addr_p);
-  else
-    {
-      *gdbarch_p = nullptr;
-      *addr_p = 0;
-    }
-}
-
 void
 tui_win_info::resize (int height_, int width_,
 		      int origin_x_, int origin_y_)
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index ea4ca219292..83a3ea37054 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -178,6 +178,18 @@ tui_source_window_base::update_source_window_as_is
 }
 
 
+/* See tui-winsource.h.  */
+void
+tui_source_window_base::update_source_window_with_addr (struct gdbarch *gdbarch,
+							CORE_ADDR addr)
+{
+  struct symtab_and_line sal {};
+  if (addr != 0)
+    sal = find_pc_line (addr, 0);
+
+  update_source_window (gdbarch, sal);
+}
+
 /* Function to ensure that the source and/or disassembly windows
    reflect the input address.  */
 void
@@ -476,7 +488,15 @@ tui_source_window_base::rerender ()
       update_source_window (gdbarch, cursal);
     }
   else
-    erase_source_content ();
+    {
+      CORE_ADDR addr;
+      struct gdbarch *gdbarch;
+      tui_get_begin_asm_address (&gdbarch, &addr);
+      if (addr == 0)
+	erase_source_content ();
+      else
+	update_source_window_with_addr (gdbarch, addr);
+    }
 }
 
 /* See tui-data.h.  */
diff --git a/gdb/tui/tui-winsource.h b/gdb/tui/tui-winsource.h
index c2826165a10..efaca50f034 100644
--- a/gdb/tui/tui-winsource.h
+++ b/gdb/tui/tui-winsource.h
@@ -183,6 +183,11 @@ struct tui_source_window_base : public tui_win_info
   virtual void display_start_addr (struct gdbarch **gdbarch_p,
 				   CORE_ADDR *addr_p) = 0;
 
+  /* Function to ensure that the source or disassembly window
+     reflects the input address.  Single window variant of
+     update_source_windows_with_addr. */
+  void update_source_window_with_addr (struct gdbarch *, CORE_ADDR);
+
 private:
 
   /* Used for horizontal scroll.  */
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 941c65c970f..33aced2e7e3 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -493,11 +493,6 @@ tui_enable (void)
       tui_resize_all ();
     }
 
-  if (deprecated_safe_get_selected_frame ())
-    tui_show_frame_info (deprecated_safe_get_selected_frame ());
-  else
-    tui_display_main ();
-
   /* Install the TUI specific hooks.  This must be done after the call to
      tui_display_main so that we don't detect the symtab changed event it
      can cause.  */

base-commit: c97aab39b81bb8b603678bc664c0de7f534a4bd5
-- 
2.35.3

Re: [PATCH] [gdb/tui] Fix displaying main after resizing

[Tom Tromey]

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

Tom> My understanding is that the rerender was simply incomplete, and
Tom> python windows will can implement whatever they like in a rerender, so
Tom> I suppose I see no problem there.

Tom> I hope that answers your question.

Thanks.  I think this patch is ok.
Approved-By: Tom Tromey <tom@tromey.com>

Tom