Re: [PATCH] [gdb/tui] Don't show incorrect source file in source window

[Andrew Burgess <aburgess@redhat.com>]

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

> Consider the test-case sources main.c and foo.c:
> ...
> $ cat main.c
> extern int foo (void);
>
> int
> main (void)
> {
>   return foo ();
> }
> $ cat foo.c
> extern int foo (void);
>
> int
> foo (void)
> {
>   return 0;
> }
> ...
> and main.c compiled with debug info, and foo.c without:
> ...
> $ gcc -g main.c -c
> $ gcc foo.c -c
> $ gcc -g main.o foo.o
> ...
>
> In TUI mode, if we run to foo:
> ...
> $ gdb -q a.out -tui -ex "b foo" -ex run
> ...
> it gets us "[ No Source Available ]":
> ...
> ┌─main.c─────────────────────────────────────────┐
> │                                                │
> │                                                │
> │                                                │
> │            [ No Source Available ]             │
> │                                                │
> │                                                │
> └────────────────────────────────────────────────┘
> (src) In: foo                  L??   PC: 0x400566
> db.so.1".
>
> Breakpoint 1, 0x0000000000400566 in foo ()
> (gdb)
> ...
>
> But after resizing (pressing ctrl-<minus> in the gnome-terminal), we get
> instead the source for main.c:
> ...
> ┌─main.c───────────────────────────────────────────────────┐
> │        3 int                                             │
> │        4 main (void)                                     │
> │        5 {                                               │
> │        6   return foo ();                                │
> │        7 }                                               │
> │                                                          │
> │                                                          │
> └──────────────────────────────────────────────────────────┘
> (src) In: foo                            L??   PC: 0x400566
> db.so.1".
>
> Breakpoint 1, 0x0000000000400566 in foo ()
> (gdb)
> ...
> which is inappropriate because we're stopped in function foo, which is not in
> main.c.
>
> Fix this in tui_source_window_base::rerender.
>
> Tested on x86_64-linux.
>
> Reported-By: Andrew Burgess <aburgess@redhat.com>
>
> PR tui/32614
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32614
> ---
>  gdb/testsuite/gdb.tui/resize-3-foo.c  | 24 ++++++++++
>  gdb/testsuite/gdb.tui/resize-3-main.c | 24 ++++++++++
>  gdb/testsuite/gdb.tui/resize-3.exp    | 63 +++++++++++++++++++++++++++
>  gdb/tui/tui-winsource.c               |  6 +++
>  4 files changed, 117 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.tui/resize-3-foo.c
>  create mode 100644 gdb/testsuite/gdb.tui/resize-3-main.c
>  create mode 100644 gdb/testsuite/gdb.tui/resize-3.exp
>
> diff --git a/gdb/testsuite/gdb.tui/resize-3-foo.c b/gdb/testsuite/gdb.tui/resize-3-foo.c
> new file mode 100644
> index 00000000000..653b24ba92b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.tui/resize-3-foo.c
> @@ -0,0 +1,24 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2025 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/>.  */
> +
> +extern int foo (void);
> +
> +int
> +foo (void)
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.tui/resize-3-main.c b/gdb/testsuite/gdb.tui/resize-3-main.c
> new file mode 100644
> index 00000000000..95fdd3b6ba0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.tui/resize-3-main.c
> @@ -0,0 +1,24 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2025 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/>.  */
> +
> +extern int foo (void);
> +
> +int
> +main (void)
> +{
> +  return foo ();
> +}
> diff --git a/gdb/testsuite/gdb.tui/resize-3.exp b/gdb/testsuite/gdb.tui/resize-3.exp
> new file mode 100644
> index 00000000000..ec022b4a177
> --- /dev/null
> +++ b/gdb/testsuite/gdb.tui/resize-3.exp
> @@ -0,0 +1,63 @@
> +# Copyright 2025 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 TUI resizing while showing "No Source Available".
> +
> +require allow_tui_tests
> +
> +standard_testfile -main.c -foo.c
> +
> +if { [build_executable_from_specs "failed to prepare" \
> +	  $testfile {debug} \
> +	  $srcfile {debug} \
> +	  $srcfile2 {nodebug}] == -1 } {
> +    return -1
> +}
> +
> +tuiterm_env
> +
> +Term::clean_restart 24 80 $testfile
> +
> +# It would be simpler to run directly to foo and then enter TUI, but that
> +# fails to trigger PR32614.  So instead, we first run to main, enter TUI and
> +# then run to foo.
> +if {![runto_main]} {
> +    perror "test suppressed"
> +    return
> +}
> +
> +# Set a breakpoint on foo, easier to do before entering TUI.
> +gdb_breakpoint foo
> +
> +if {![Term::enter_tui]} {
> +    unsupported "TUI not supported"
> +    return
> +}
> +
> +# Continue to foo.
> +Term::command continue
> +
> +with_test_prefix "before resize" {
> +    Term::check_contents "Source window empty" \
> +	"No Source Available"
> +}
> +
> +Term::resize 40 90
> +
> +with_test_prefix "after resize" {
> +    # Regression test for PR32614.
> +    Term::check_contents "Source window empty" \
> +	"No Source Available"
> +}
> diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
> index a5d0c594545..bf4052755df 100644
> --- a/gdb/tui/tui-winsource.c
> +++ b/gdb/tui/tui-winsource.c
> @@ -460,6 +460,12 @@ tui_source_window_base::rerender ()
>        struct gdbarch *gdbarch = get_frame_arch (frame);
>  
>        struct symtab *s = find_pc_line_symtab (get_frame_pc (frame));
> +      if (s == nullptr)
> +	{
> +	  erase_source_content ();
> +	  return;
> +	}

This didn't look quite right to me.  But while trying to find the set of
steps to show why I don't think this change is exactly right ... I ran
into another bug, which means that this isn't wrong right now ... but
might be once the other bug is fixed ... maybe ....

So, what worried me, is that this ::rerender is called for both the SRC
window and the ASM window.  My thinking was that if this change is
fixing things so we call erase_source_content() when we cannot find the
source file, then we're also going to call erase_source_content() for
the ASM window when we cannot find the source file, and that cannot be
right, we should always be able to update the ASM window with the
correct disassembler output.

So I tried the following with an UNPATCHED gdb:

  1. layout src
  2. step into a region where "No Source Available" is shown.
  3. layout asm

My expectation is that I should immediately see the disassembler output
for the current $pc location.  What I actually see is "No Assembly
Available".

Turns out we hit the erase_source_content() call in
tui_source_window_base::update_source_window_as_is because the sal.pc
within that function is 0, and so the set_contents call returns false.

If I then `si` the ASM window immediately updates with the disassembler
output.  This time the update originates from tui_show_frame_info, which
gets the symtab_and_line via a different approach than the ::rerender
function does.

So I wondered, would a better solution be to rewrite at least part of
::rerender based on tui_show_frame_info?

Below is a _rough_ patch which does just this.  There's no commit
message or anything yet.  It passes all the gdb.tui/ tests, including
your new test that you added with this patch.

This also seems to resolve the src -> asm switching issue I detail
above.

I'd love to hear what you think about this approach.

If you think this is the way to go then I'd propose that I write this
up, steal your new test, and add another test to expose the src to asm
switching issue.

But I'm worried that I might be missing something.

Thanks,
Andrew

---

diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index a5d0c594545..b9c26e88a3a 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -454,25 +454,10 @@ tui_source_window_base::rerender ()
     }
   else if (deprecated_safe_get_selected_frame () != NULL)
     {
-      symtab_and_line cursal
-	= get_current_source_symtab_and_line (current_program_space);
       frame_info_ptr frame = deprecated_safe_get_selected_frame ();
-      struct gdbarch *gdbarch = get_frame_arch (frame);
-
-      struct symtab *s = find_pc_line_symtab (get_frame_pc (frame));
-      if (this != tui_src_win ())
-	find_line_pc (s, cursal.line, &cursal.pc);
-
-      /* This centering code is copied from tui_source_window::maybe_update.
-	 It would be nice to do centering more often, and do it in just one
-	 location.  But since this is a regression fix, handle this
-	 conservatively for now.  */
-      int start_line = (cursal.line - ((height - box_size ()) / 2)) + 1;
-      if (start_line <= 0)
-	start_line = 1;
-      cursal.line = start_line;
-
-      update_source_window (gdbarch, cursal);
+      symtab_and_line cursal = find_frame_sal (frame);
+
+      maybe_update (frame, cursal);
     }
   else
     {