Patchwork [review] Fix "list" command in the TUI

login
register
mail settings
Submitter Simon Marchi (Code Review)
Date Nov. 14, 2019, 11:35 p.m.
Message ID <gerrit.1573774548000.I62013825f6c1afdd568a1c7a8c019b0c881131af@gnutoolchain-gerrit.osci.io>
Download mbox | patch
Permalink /patch/35919/
State New
Headers show

Comments

Simon Marchi (Code Review) - Nov. 14, 2019, 11:35 p.m.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/650
......................................................................

Fix "list" command in the TUI

PR tui/18932 notes that "list" no longer works in the TUI.  At some
point in the past, it switched the TUI source window to show the
specified source; but now this source briefly flashes before the TUI
reverts to showing the current stack frame's source.

This patch fixes this bug by introducing a new observer that notices
when the user selected context has changed.  Then, the existing
before-prompt observer is updated to request the correct update:
either one based on the current stack frame, or one based on the
user's source symtab_and_line.

gdb/ChangeLog
2019-11-14  Tom Tromey  <tom@tromey.com>

	PR tui/18932:
	* tui/tui-hooks.c (tui_refresh_frame_and_register_information):
	Rename parameters.  Handle the not-from-stack-frame case.
	(from_stack, from_source_symtab): New globals.
	(tui_before_prompt, tui_normal_stop): Update.
	(tui_context_changed, tui_symtab_changed): New functions.
	(tui_attach_detach_observers): Attach new observers.

gdb/testsuite/ChangeLog
2019-11-14  Tom Tromey  <tom@tromey.com>

	* gdb.tui/list-before.exp: New file.

Change-Id: I62013825f6c1afdd568a1c7a8c019b0c881131af
---
M gdb/ChangeLog
M gdb/testsuite/ChangeLog
A gdb/testsuite/gdb.tui/list-before.exp
M gdb/tui/tui-hooks.c
4 files changed, 102 insertions(+), 43 deletions(-)

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6e5192c..51d6f49 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,15 @@ 
 2019-11-14  Tom Tromey  <tom@tromey.com>
 
+	PR tui/18932:
+	* tui/tui-hooks.c (tui_refresh_frame_and_register_information):
+	Rename parameters.  Handle the not-from-stack-frame case.
+	(from_stack, from_source_symtab): New globals.
+	(tui_before_prompt, tui_normal_stop): Update.
+	(tui_context_changed, tui_symtab_changed): New functions.
+	(tui_attach_detach_observers): Attach new observers.
+
+2019-11-14  Tom Tromey  <tom@tromey.com>
+
 	* source.c (struct current_source_location) <set, symtab, line>:
 	New methods.
 	<m_symtab, m_line>: Rename.  Now private.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index b79729d..d9f6032 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,9 @@ 
 2019-11-14  Tom Tromey  <tom@tromey.com>
 
+	* gdb.tui/list-before.exp: New file.
+
+2019-11-14  Tom Tromey  <tom@tromey.com>
+
 	* gdb.tui/list.exp: Check for source on initial listing.
 
 2019-11-14  Tom Tromey  <tromey@adacore.com>
diff --git a/gdb/testsuite/gdb.tui/list-before.exp b/gdb/testsuite/gdb.tui/list-before.exp
new file mode 100644
index 0000000..a8c931b
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/list-before.exp
@@ -0,0 +1,34 @@ 
+# Copyright 2019 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/>.
+
+# Ensure that "list" before starting the TUI will affect the view.
+
+load_lib "tuiterm.exp"
+
+standard_testfile tui-layout.c
+
+if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} {
+    return -1
+}
+
+Term::clean_restart 24 80 $testfile
+
+gdb_test "list main"
+
+if {![Term::enter_tui]} {
+    unsupported "TUI not supported"
+}
+
+Term::check_contents "initial source listing" "21 *return 0"
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index 35a9259..44019f6 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -105,53 +105,49 @@ 
   tui_update_all_breakpoint_info (nullptr);
 }
 
-/* Refresh TUI's frame and register information.  This is a hook intended to be
-   used to update the screen after potential frame and register changes.
+/* This is set to true if the next window refresh should come from the
+   current stack frame.  */
 
-   REGISTERS_TOO_P controls whether to refresh our register information even
-   if frame information hasn't changed.  */
+static bool from_stack;
+
+/* This is set to true if the next window refresh should come from the
+   current source symtab.  */
+
+static bool from_source_symtab;
+
+/* Refresh TUI's frame and register information.  This is a hook intended to be
+   used to update the screen after potential frame and register changes.  */
 
 static void
-tui_refresh_frame_and_register_information (bool registers_too_p)
+tui_refresh_frame_and_register_information ()
 {
-  struct frame_info *fi;
-  CORE_ADDR pc;
-  int frame_info_changed_p;
-
-  if (!has_stack_frames ())
+  if (!from_stack && !from_source_symtab)
     return;
 
   target_terminal::scoped_restore_terminal_state term_state;
   target_terminal::ours_for_output ();
 
-  fi = get_selected_frame (NULL);
-  /* Ensure that symbols for this frame are read in.  Also, determine
-     the source language of this frame, and switch to it if
-     desired.  */
-  if (get_frame_pc_if_available (fi, &pc))
+  if (from_stack && has_stack_frames ())
     {
-      struct symtab *s;
+      struct frame_info *fi = get_selected_frame (NULL);
 
-      s = find_pc_line_symtab (pc);
-      /* elz: This if here fixes the problem with the pc not being
-	 displayed in the tui asm layout, with no debug symbols.  The
-	 value of s would be 0 here, and select_source_symtab would
-	 abort the command by calling the 'error' function.  */
-      if (s)
-	select_source_symtab (s);
+      /* Display the frame position (even if there is no symbols or
+	 the PC is not known).  */
+      int frame_info_changed_p = tui_show_frame_info (fi);
+
+      /* Refresh the register window if it's visible.  */
+      if (tui_is_window_visible (DATA_WIN)
+	  && (frame_info_changed_p || from_stack))
+	{
+	  tui_refreshing_registers = 1;
+	  TUI_DATA_WIN->check_register_values (fi);
+	  tui_refreshing_registers = 0;
+	}
     }
-
-  /* Display the frame position (even if there is no symbols or the PC
-     is not known).  */
-  frame_info_changed_p = tui_show_frame_info (fi);
-
-  /* Refresh the register window if it's visible.  */
-  if (tui_is_window_visible (DATA_WIN)
-      && (frame_info_changed_p || registers_too_p))
+  else if (!from_stack)
     {
-      tui_refreshing_registers = 1;
-      TUI_DATA_WIN->check_register_values (fi);
-      tui_refreshing_registers = 0;
+      struct symtab_and_line sal = get_current_source_symtab_and_line ();
+      tui_update_source_windows_with_line (sal);
     }
 }
 
@@ -183,12 +179,9 @@ 
 static void
 tui_before_prompt (const char *current_gdb_prompt)
 {
-  /* This refresh is intended to catch changes to the selected frame following
-     a call to "up", "down" or "frame".  As such we don't necessarily want to
-     refresh registers here unless the frame actually changed by one of these
-     commands.  Registers will otherwise be refreshed after a normal stop or by
-     our tui_register_changed_hook.  */
-  tui_refresh_frame_and_register_information (/*registers_too_p=*/false);
+  tui_refresh_frame_and_register_information ();
+  from_stack = false;
+  from_source_symtab = false;
 }
 
 /* Observer for the normal_stop notification.  */
@@ -196,9 +189,23 @@ 
 static void
 tui_normal_stop (struct bpstats *bs, int print_frame)
 {
-  /* This refresh is intended to catch changes to the selected frame and to
-     registers following a normal stop.  */
-  tui_refresh_frame_and_register_information (/*registers_too_p=*/true);
+  from_stack = true;
+}
+
+/* Observer for user_selected_context_changed.  */
+
+static void
+tui_context_changed (user_selected_what ignore)
+{
+  from_stack = true;
+}
+
+/* Observer for current_source_symtab_and_line_changed.  */
+
+static void
+tui_symtab_changed ()
+{
+  from_source_symtab = true;
 }
 
 /* Token associated with observers registered while TUI hooks are
@@ -236,6 +243,10 @@ 
 		    tui_normal_stop, attach);
   attach_or_detach (gdb::observers::register_changed,
 		    tui_register_changed, attach);
+  attach_or_detach (gdb::observers::user_selected_context_changed,
+		    tui_context_changed, attach);
+  attach_or_detach (gdb::observers::current_source_symtab_and_line_changed,
+		    tui_symtab_changed, attach);
 }
 
 /* Install the TUI specific hooks.  */