[Bug tui/31522] New: TUI misses highlight after run to main

[Bug tui/31522] New: TUI misses highlight after run to main

[pedro at palves dot net]

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

            Bug ID: 31522
           Summary: TUI misses highlight after run to main
           Product: gdb
           Version: unknown
            Status: NEW
          Severity: normal
          Priority: P2
         Component: tui
          Assignee: unassigned at sourceware dot org
          Reporter: pedro at palves dot net
  Target Milestone: ---

If I run to main with "start", and then enable the TUI, the current line
(stopped at main) isn't highlighted, nor is the current line centered on the
source window.

If I do a step, then the highlight appears.

If I enable the TUI _before_ running to main, then the current line once GDB
stops at main, is highlighted.

This seems like a recent breakage.  At least, I hadn't noticed it before.

Observed on commit 9bec569fda7c, from Mar 21 2024.

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

[Bug tui/31522] TUI misses highlight after run to main

[ssbssa at sourceware dot org]

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

Hannes Domani <ssbssa at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ssbssa at sourceware dot org

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

[Bug tui/31522] TUI misses highlight after run to main

[tromey at sourceware dot org]

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

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tromey at sourceware dot org

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

[Bug tui/31522] TUI misses highlight after run to main

[tromey at sourceware dot org]

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

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |15.1

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

[Bug tui/31522] TUI misses highlight after run to main

[tromey at sourceware dot org]

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

--- Comment #1 from Tom Tromey <tromey at sourceware dot org> ---
Not sure what broke this, but changing tui_enable to pass
the selected frame to tui_show_frame_info fixes it.

This seems reasonable to me, but maybe it is a change if
you exit the TUI and then re-enter it in some scenario.
Personally I'm not too concerned about this.

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

[Bug tui/31522] TUI misses highlight after run to main

[tromey at sourceware dot org]

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

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at sourceware dot org   |tromey at sourceware dot org

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

[Bug tui/31522] TUI misses highlight after run to main

[tromey at sourceware dot org]

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

--- Comment #2 from Tom Tromey <tromey at sourceware dot org> ---
I found out while testing that there is a similar bug
if you "tui enable" and then open a core file.
You can see it in the screen dump from gdb.tui/corefile-run.exp

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

[Bug tui/31522] TUI misses highlight after run to main

[pedro at palves dot net]

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

--- Comment #3 from Pedro Alves <pedro at palves dot net> ---
> Not sure what broke this, but changing tui_enable to pass
> the selected frame to tui_show_frame_info fixes it.

Thanks, I gave that a try, but it still doesn't behave like before.  It shows
the current line at the top of the source window, instead of in the middle. 
That's a bit of an usability regression, as it doesn't even show the line with
the function name.

> This seems reasonable to me, but maybe it is a change if
> you exit the TUI and then re-enter it in some scenario.
> Personally I'm not too concerned about this.

I do that frequently with c-x,a.  I'll be debugging gdb, and stepping/nexting
with the TUI enabled, then decide I'm at the right spot the program that I want
to inspect variables or run a command that I know is going to output many
lines, so I'll switch out of the TUI, issue whatever commands, and then
re-enable the TUI to continue stepping, etc.  I don't know if passing the frame
causes the TUI to lose its selected context/line when it was active or not. 
Seems like not from a quick experimentation.

BTW, this is what I tried following your instructions:

 --- c/gdb/tui/tui.c
 +++ w/gdb/tui/tui.c
 @@ -467,7 +467,7 @@ tui_enable (void)
       tui_set_term_width_to (COLS);
       def_prog_mode ();

 -      tui_show_frame_info (0);
 +      tui_show_frame_info (get_selected_frame (nullptr));

(Of course, this misbehaves if you enable the TUI before starting a process.)

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

[Bug tui/31522] TUI misses highlight after run to main

[tromey at sourceware dot org]

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

--- Comment #4 from Tom Tromey <tromey at sourceware dot org> ---
I used

diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index dea6ffbf954..638f91f5612 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -467,7 +467,7 @@ tui_enable (void)
       tui_set_term_width_to (COLS);
       def_prog_mode ();

-      tui_show_frame_info (0);
+      tui_show_frame_info (deprecated_safe_get_selected_frame ());
       tui_set_initial_layout ();
       tui_set_win_focus_to (TUI_SRC_WIN);
       keypad (TUI_CMD_WIN->handle.get (), TRUE);



Maybe we need to bisect this one now.

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

[Bug tui/31522] TUI misses highlight after run to main

[vries at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vries at gcc dot gnu.org

--- Comment #5 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom Tromey from comment #4)
> Maybe we need to bisect this one now.

Bisects to:
...
commit ee1e9bbb5139d766d70cfa036979cec73a1223b7
Author: Tom de Vries <tdevries@suse.de>
Date:   Fri Dec 8 17:36:35 2023 +0100

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

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

[Bug tui/31522] TUI misses highlight after run to main

[tromey at sourceware dot org]

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

--- Comment #6 from Tom Tromey <tromey at sourceware dot org> ---
Thank you for bisecting that.
I was hoping the commit would be a bit more enlightening than
it turned out to be, like I don't immediately see the connection.

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

[Bug tui/31522] TUI misses highlight after run to main

[vries at gcc dot gnu.org]

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

--- Comment #7 from Tom de Vries <vries at gcc dot gnu.org> ---
I investigated a bit, by setting a breakpoint on
tui_source_window::set_contents, and observing what sal.line is used.

With commit ee1e9bbb513^, we have (for the "tui enable" command):
- sal.line == 6 (set by tui_source_window_base::rerender)
- sal.line == 0 (set by tui_update_source_windows_with_addr)
- sal.line == 2 (set by tui_source_window::maybe_update)

With commit ee1e9bbb513 (as well as with the fix of comment 4), we have:
- sal.line == 6 (set by tui_source_window_base::rerender)

The desired behaviour of centering on a source line is only present in
tui_source_window::maybe_update.

Duplicating that functionality in tui_source_window_base::rerender:
...
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 52c0b5b69a4..c140e9053be 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -482,6 +482,10 @@ tui_source_window_base::rerender ()
       struct symtab *s = find_pc_line_symtab (get_frame_pc (frame));
       if (this != TUI_SRC_WIN)
        find_line_pc (s, cursal.line, &cursal.pc);
+      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);
     }
   else
...
gives us the desired centering.

It passes the TUI tests.

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

[Bug tui/31522] TUI misses highlight after run to main

[pedro at palves dot net]

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

--- Comment #8 from Pedro Alves <pedro at palves dot net> ---
Thanks!  Indeed, I played a bit with that patch, and the centering seems to
work just like it did before, in all the use cases I threw at it.  Tromey's
change is also needed for the highlight.

The old code did:

-  if (deprecated_safe_get_selected_frame ())
-    tui_show_frame_info (deprecated_safe_get_selected_frame ());
-  else
-    tui_display_main ();

and I wondered whether we need that tui_display_main call, but I couldn't spot
a behavior difference without it.  I tried enabling the TUI before starting the
program, with and without using "list foo" to move the current source position
elsewhere.

IMO, it would be nice if we centered in more cases, like after "frame", for
example, or before running the program and you enable the TUI (TUI shows main),
but those are not regressions AFAICT.

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

[Bug tui/31522] TUI misses highlight after run to main

[vries at gcc dot gnu.org]

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

--- Comment #9 from Tom de Vries <vries at gcc dot gnu.org> ---
Created attachment 15440
  --> https://sourceware.org/bugzilla/attachment.cgi?id=15440&action=edit
tentative patch including test-case

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

[Bug tui/31522] TUI misses highlight after run to main

[vries at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from Tom de Vries <vries at gcc dot gnu.org> ---
https://sourceware.org/pipermail/gdb-patches/2024-March/207641.html

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

[Bug tui/31522] TUI misses highlight after run to main

[vries at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |---
             Status|RESOLVED                    |REOPENED

--- Comment #11 from Tom de Vries <vries at gcc dot gnu.org> ---
Oops, didn't mean to close it.

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

[Bug tui/31522] TUI misses highlight after run to main

[vries at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |NEW

--- Comment #12 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Pedro Alves from comment #8)
> Thanks!  Indeed, I played a bit with that patch, and the centering seems to
> work just like it did before, in all the use cases I threw at it.  Tromey's
> change is also needed for the highlight.
> 

Thanks for trying it out (maybe consider adding a tested-by tag to the
submitted patch?).

> The old code did:
> 
> -  if (deprecated_safe_get_selected_frame ())
> -    tui_show_frame_info (deprecated_safe_get_selected_frame ());
> -  else
> -    tui_display_main ();
> 
> and I wondered whether we need that tui_display_main call, but I couldn't
> spot a behavior difference without it.  I tried enabling the TUI before
> starting the program, with and without using "list foo" to move the current
> source position elsewhere.
> 

Ok, understood.

> IMO, it would be nice if we centered in more cases, like after "frame", for
> example, or before running the program and you enable the TUI (TUI shows
> main), but those are not regressions AFAICT.

Agreed.  I played a bit with 'gdb -q a.out -tui' vs 'gdb -q -a.out -ex "tui
enable"', where there's also a difference in behaviour.

Anyway, since this is a regression fix, I went with this minimal fix, and added
a comment at the fix that we could try to improve the fix.

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

[Bug tui/31522] TUI misses highlight after run to main

[pedro at palves dot net]

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

--- Comment #13 from Pedro Alves <pedro at palves dot net> ---
Filed PR tui/31570 for those.

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

[Bug tui/31522] TUI misses highlight after run to main

[tromey at sourceware dot org]

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

--- Comment #14 from Tom Tromey <tromey at sourceware dot org> ---
(In reply to Pedro Alves from comment #3)

> > This seems reasonable to me, but maybe it is a change if
> > you exit the TUI and then re-enter it in some scenario.
> > Personally I'm not too concerned about this.
> 
> I do that frequently with c-x,a.  I'll be debugging gdb, and
> stepping/nexting with the TUI enabled, then decide I'm at the right spot the
> program that I want to inspect variables or run a command [...]

What I meant here is that I was wondering if there are cases
where "tui disable" + "tui enable" would preserve the state of
the source window, even if there were changes (like stepping,
whatever) in between.

I tend to think that it is fine for "tui enable" to just recenter
the selected frame's line -- but is that ok with you?

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

[Bug tui/31522] TUI misses highlight after run to main

[cvs-commit at gcc dot gnu.org]

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

--- Comment #15 from Sourceware Commits <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=b35013e29f3bcf9028aa22291f378010420322fe

commit b35013e29f3bcf9028aa22291f378010420322fe
Author: Tom de Vries <tdevries@suse.de>
Date:   Tue Apr 2 16:09:10 2024 +0200

    [gdb/tui] Fix centering and highlighting of current line

    After starting TUI like this with a hello world a.out:
    ...
    $ gdb -q a.out -ex start -ex "tui enable"
    ...
    we get:
    ...
   
ââhello.câââââââââââââââââââââââââââââââ
    â        5 {                           â
    â        6   printf ("hello\n");       â
    â        7                             â
    â        8   return 0;                 â
    â        9 }                           â
    â                                      â
   
ââââââââââââââââââââââââââââââââââââââââ
    ...

    This is a regression since commit ee1e9bbb513 ("[gdb/tui] Fix displaying
main
    after resizing"), before which we had instead:
    ...
   
ââhello.câââââââââââââââââââââââââââââââ
    â        4 main (void)                 â
    â        5 {                           â
    â  >     6 [7m  printf ("hello\n");[0m       â
    â        7                             â
    â        8   return 0;                 â
    â        9 }                           â
   
ââââââââââââââââââââââââââââââââââââââââ
    ...

    In other words, the problems are:
    - the active line (source line 6) is no longer highlighted, and
    - the active line is not vertically centered (screen line 2 out 6 instead
of
      screen line 3 out of 6).

    Fix these problems respectively by:
    - in tui_enable, instead of "tui_show_frame_info (0)" using
      'tui_show_frame_info (deprecated_safe_get_selected_frame ())", and
    - in tui_source_window_base::rerender, adding centering functionality.

    Tested on aarch64-linux.

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

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

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

[Bug tui/31522] TUI misses highlight after run to main

[vries at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
            Version|unknown                     |HEAD
             Status|NEW                         |RESOLVED

--- Comment #16 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.