[PATCH 00/15] Mixed bag of TUI tests and fixes

[PATCH 00/15] Mixed bag of TUI tests and fixes

[Andrew Burgess]

This series started as a passing observation that an existing TUI
tests was no longer working as expected.

Then I noticed a gap in the testing...

... which exposed a bug ...

... and things spiralled from there :/

Thanks,
Andrew

---

Andrew Burgess (15):
  gdb/testsuite: extend gdb.tui/tui-layout.exp test script
  gdb/testsuite: update gdb.tui/tui-disasm-long-lines.exp
  gdb/testsuite: update gdb.tui/tui-nl-filtered-output.exp
  gdb/testsuite/tui: more testing of the 'focus' command
  gdb/tui: convert if/error to an assert
  gdb/tui: better filtering of tab completion results for focus command
  gdb/testsuite: fix line feed scrolling in tuiterm.exp
  gdb/tui: improve errors from tui focus command
  gdb/tui: disable tui mode when an assert triggers
  gdb/tui: make m_horizontal_offset private
  gdb/tui: rewrite of tui_source_window_base to handle very long lines
  gdb/tui: avoid extra refresh_window on horizontal scroll
  gdb/tui: avoid extra refresh_window on vertical scroll
  gdb/tui: more debug output
  gdb/tui: make use of a scoped_restore

 gdb/event-top.c                               |   8 +
 gdb/testsuite/gdb.tui/completion.exp          |  18 +-
 .../gdb.tui/tui-disasm-long-lines.exp         |  15 +-
 gdb/testsuite/gdb.tui/tui-focus.c             |  22 ++
 gdb/testsuite/gdb.tui/tui-focus.exp           | 137 +++++++++++
 gdb/testsuite/gdb.tui/tui-layout.exp          |  57 ++++-
 .../gdb.tui/tui-nl-filtered-output.exp        |  41 ++--
 gdb/testsuite/lib/tuiterm.exp                 |  18 +-
 gdb/tui/tui-io.c                              |  10 +-
 gdb/tui/tui-layout.c                          |  11 +-
 gdb/tui/tui-layout.h                          |  45 ++++
 gdb/tui/tui-win.c                             |  74 +++++-
 gdb/tui/tui-winsource.c                       | 216 ++++++++++++++++--
 gdb/tui/tui-winsource.h                       |  69 +++++-
 gdb/tui/tui.h                                 |   3 +
 gdb/utils.c                                   |   7 +-
 16 files changed, 656 insertions(+), 95 deletions(-)
 create mode 100644 gdb/testsuite/gdb.tui/tui-focus.c
 create mode 100644 gdb/testsuite/gdb.tui/tui-focus.exp


base-commit: dcd469925324f430e8cf656c7f63538dd7a01d08
-- 
2.25.4

[PATCH 01/15] gdb/testsuite: extend gdb.tui/tui-layout.exp test script

[Andrew Burgess]

In passing I noticed that the gdb.tui/tui-layout.exp test script was a
little strange, it tests the layout command multiple times, but never
sets up our ANSI terminal emulator, so every layout command fails with
a message about the terminal lacking the required abilities.

It turns out that this was caused by this commit:

  commit 9162a27c5f5828240b53379d735679e2a69a9f41
  Date:   Tue Nov 13 11:59:03 2018 -0700

      Change gdb test suite's TERM setting

This was when we changed the testsuite to set the TERM environment
variable to "dumb" by default.

After this, any tui test that didn't set the terminal mode back to
'ansi' would fail to activate tui mode.

For the tui-layout.exp test it just so happens that the test patterns
are generic enough that the test continued to pass, even after this
change.

In this commit I have updated the test so we now check the layout
command both with a 'dumb' terminal and with the 'ansi' terminal.
When testing with the 'ansi' terminal, I have some limited validation
that GDB correctly entered tui mode.

I figured that it is probably worth having at least one test in the
test suite that deliberately tries to enter tui mode in a dumb
terminal, it would be sad if we one day managed to break GDB such that
this caused a crash, and never noticed.
---
 gdb/testsuite/gdb.tui/tui-layout.exp | 57 ++++++++++++++++++++++------
 1 file changed, 46 insertions(+), 11 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/tui-layout.exp b/gdb/testsuite/gdb.tui/tui-layout.exp
index 97734cf7b68..e69760193c5 100644
--- a/gdb/testsuite/gdb.tui/tui-layout.exp
+++ b/gdb/testsuite/gdb.tui/tui-layout.exp
@@ -16,6 +16,8 @@
 # Minimal testcase that just checks that the various "layout $foo"
 # commands do not cause gdb to crash.
 
+tuiterm_env
+
 standard_testfile
 
 if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile}]} {
@@ -26,13 +28,33 @@ if {[skip_tui_tests]} {
     return
 }
 
-# Test one layout command.  EXECUTION indicates whether to activate
-# the layout with or without execution.
+# Run one test of the 'layout' command, selecting LAYOUT_NAME.
+#
+# TERMINAL should be either 'dumb' or 'ansi'.  When TERMINAL is 'dumb'
+# then GDB is started in a terminal that does not support tui mode, in
+# this case the layout command is expected to fail.
+#
+# When TERMINAL is 'ansi' then GDB is started using our emulated ANSI
+# terminal, and the layout command is expected to succeed.
+#
+# When EXECUTION is true then a call to runto_main is used, otherwise
+# this call is skipped and the inferior is left in whatever state it
+# happens to be in after a call to clean_restart.
 
-proc test_layout {layout execution} {
+proc test_layout_or_focus {layout_name terminal execution} {
     global binfile gdb_prompt
 
-    clean_restart $binfile
+    set dumb_terminal [string equal $terminal "dumb"]
+
+    if {$dumb_terminal} {
+	clean_restart $binfile
+    } else {
+	Term::clean_restart 24 80 $binfile
+	if {![Term::prepare_for_tui]} {
+	    unsupported "TUI not supported"
+	    return
+	}
+    }
 
     if {$execution} {
 	if {![runto_main]} {
@@ -40,16 +62,29 @@ proc test_layout {layout execution} {
 	}
     }
 
-    set test "layout command"
-    gdb_test_multiple "layout $layout" $test {
-	-re "$gdb_prompt $" {
-	    pass $test
+    if {$dumb_terminal} {
+	gdb_test "layout $layout_name" \
+	    "Cannot enable the TUI: terminal doesn't support cursor addressing \\\[TERM=dumb\\\]"
+    } else {
+	Term::command_no_prompt_prefix "layout $layout_name"
+	if {$layout_name == "asm"} {
+	    Term::check_box "asm box" 0 0 80 15
+	} elseif {$layout_name == "reg"} {
+	    Term::check_box "reg box" 0 0 80 8
+	    Term::check_box "src box" 0 7 80 8
+	} elseif {$layout_name == "src"} {
+	    Term::check_box "src box" 0 0 80 15
+	} elseif {$layout_name == "split"} {
+	    Term::check_box "src box" 0 0 80 8
+	    Term::check_box "asm box" 0 7 80 8
 	}
     }
 }
 
-foreach_with_prefix execution {0 1} {
-    foreach_with_prefix layout {"asm" "reg" "src" "split"} {
-	test_layout $layout $execution
+foreach_with_prefix terminal {ansi dumb} {
+    foreach_with_prefix execution {false true} {
+	foreach_with_prefix layout {"asm" "reg" "src" "split"} {
+	    test_layout_or_focus $layout $terminal $execution
+	}
     }
 }
-- 
2.25.4

Re: [PATCH 01/15] gdb/testsuite: extend gdb.tui/tui-layout.exp test script

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> In passing I noticed that the gdb.tui/tui-layout.exp test script was a
> little strange, it tests the layout command multiple times, but never
> sets up our ANSI terminal emulator, so every layout command fails with
> a message about the terminal lacking the required abilities.
>
> It turns out that this was caused by this commit:
>
>   commit 9162a27c5f5828240b53379d735679e2a69a9f41
>   Date:   Tue Nov 13 11:59:03 2018 -0700
>
>       Change gdb test suite's TERM setting
>
> This was when we changed the testsuite to set the TERM environment
> variable to "dumb" by default.
>
> After this, any tui test that didn't set the terminal mode back to
> 'ansi' would fail to activate tui mode.
>
> For the tui-layout.exp test it just so happens that the test patterns
> are generic enough that the test continued to pass, even after this
> change.
>
> In this commit I have updated the test so we now check the layout
> command both with a 'dumb' terminal and with the 'ansi' terminal.
> When testing with the 'ansi' terminal, I have some limited validation
> that GDB correctly entered tui mode.
>
> I figured that it is probably worth having at least one test in the
> test suite that deliberately tries to enter tui mode in a dumb
> terminal, it would be sad if we one day managed to break GDB such that
> this caused a crash, and never noticed.

I've gone ahead and pushed this patch.  The final version I pushed is
included below.

Thanks,
Andrew

---

commit b3b0595ff66d8597637c415f74d43dd459a18cb3
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Fri Nov 25 18:19:16 2022 +0000

    gdb/testsuite: extend gdb.tui/tui-layout.exp test script
    
    In passing I noticed that the gdb.tui/tui-layout.exp test script was a
    little strange, it tests the layout command multiple times, but never
    sets up our ANSI terminal emulator, so every layout command fails with
    a message about the terminal lacking the required abilities.
    
    It turns out that this was caused by this commit:
    
      commit 9162a27c5f5828240b53379d735679e2a69a9f41
      Date:   Tue Nov 13 11:59:03 2018 -0700
    
          Change gdb test suite's TERM setting
    
    This was when we changed the testsuite to set the TERM environment
    variable to "dumb" by default.
    
    After this, any tui test that didn't set the terminal mode back to
    'ansi' would fail to activate tui mode.
    
    For the tui-layout.exp test it just so happens that the test patterns
    are generic enough that the test continued to pass, even after this
    change.
    
    In this commit I have updated the test so we now check the layout
    command both with a 'dumb' terminal and with the 'ansi' terminal.
    When testing with the 'ansi' terminal, I have some limited validation
    that GDB correctly entered tui mode.
    
    I figured that it is probably worth having at least one test in the
    test suite that deliberately tries to enter tui mode in a dumb
    terminal, it would be sad if we one day managed to break GDB such that
    this caused a crash, and never noticed.

diff --git a/gdb/testsuite/gdb.tui/tui-layout.exp b/gdb/testsuite/gdb.tui/tui-layout.exp
index 2c502191d11..c7fdb1b0c0a 100644
--- a/gdb/testsuite/gdb.tui/tui-layout.exp
+++ b/gdb/testsuite/gdb.tui/tui-layout.exp
@@ -18,19 +18,41 @@
 
 require allow_tui_tests
 
+tuiterm_env
+
 standard_testfile
 
 if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile}]} {
     return -1
 }
 
-# Test one layout command.  EXECUTION indicates whether to activate
-# the layout with or without execution.
+# Run one test of the 'layout' command, selecting LAYOUT_NAME.
+#
+# TERMINAL should be either 'dumb' or 'ansi'.  When TERMINAL is 'dumb'
+# then GDB is started in a terminal that does not support tui mode, in
+# this case the layout command is expected to fail.
+#
+# When TERMINAL is 'ansi' then GDB is started using our emulated ANSI
+# terminal, and the layout command is expected to succeed.
+#
+# When EXECUTION is true then a call to runto_main is used, otherwise
+# this call is skipped and the inferior is left in whatever state it
+# happens to be in after a call to clean_restart.
 
-proc test_layout {layout execution} {
+proc test_layout_or_focus {layout_name terminal execution} {
     global binfile gdb_prompt
 
-    clean_restart $binfile
+    set dumb_terminal [string equal $terminal "dumb"]
+
+    if {$dumb_terminal} {
+	clean_restart $binfile
+    } else {
+	Term::clean_restart 24 80 $binfile
+	if {![Term::prepare_for_tui]} {
+	    unsupported "TUI not supported"
+	    return
+	}
+    }
 
     if {$execution} {
 	if {![runto_main]} {
@@ -38,16 +60,29 @@ proc test_layout {layout execution} {
 	}
     }
 
-    set test "layout command"
-    gdb_test_multiple "layout $layout" $test {
-	-re "$gdb_prompt $" {
-	    pass $test
+    if {$dumb_terminal} {
+	gdb_test "layout $layout_name" \
+	    "Cannot enable the TUI: terminal doesn't support cursor addressing \\\[TERM=dumb\\\]"
+    } else {
+	Term::command_no_prompt_prefix "layout $layout_name"
+	if {$layout_name == "asm"} {
+	    Term::check_box "asm box" 0 0 80 15
+	} elseif {$layout_name == "reg"} {
+	    Term::check_box "reg box" 0 0 80 8
+	    Term::check_box "src box" 0 7 80 8
+	} elseif {$layout_name == "src"} {
+	    Term::check_box "src box" 0 0 80 15
+	} elseif {$layout_name == "split"} {
+	    Term::check_box "src box" 0 0 80 8
+	    Term::check_box "asm box" 0 7 80 8
 	}
     }
 }
 
-foreach_with_prefix execution {0 1} {
-    foreach_with_prefix layout {"asm" "reg" "src" "split"} {
-	test_layout $layout $execution
+foreach_with_prefix terminal {ansi dumb} {
+    foreach_with_prefix execution {false true} {
+	foreach_with_prefix layout {"asm" "reg" "src" "split"} {
+	    test_layout_or_focus $layout $terminal $execution
+	}
     }
 }

[PATCH 02/15] gdb/testsuite: update gdb.tui/tui-disasm-long-lines.exp

[Andrew Burgess]

Following on from the previous commit, in this commit I am updating
the test script gdb.tui/tui-disasm-long-lines.exp to take account of
the changes in commit:

  commit 9162a27c5f5828240b53379d735679e2a69a9f41
  Date:   Tue Nov 13 11:59:03 2018 -0700

      Change gdb test suite's TERM setting

In the above commit the TERM environment variable was changed to be
'dumb' by default, which means that tests, that previously activated
tui mode, no longer do unless TERM is set to 'ansi'.

As the gdb.tui/tui-disasm-long-lines.exp script didn't do this, the
test stopped working.  As the expect patterns in this script were
pretty generic no tests actually started failing, and we never
noticed.

In this commit I update the script to use Term::clean_restart, which
correctly sets TERM to 'ansi'.  I've also added a check that the asm
box does appear on the screen, which should indicate that tui mode has
correctly activated.

However, I also notice that GDB doesn't appear to fully work
correctly.  The test should display the disassembly for the test
program, but it doesn't.

The test is trying to disassemble some code that (deliberately) uses a
very long symbol name, this eventually results in GDB entering
tui_source_window_base::show_source_content and trying to allocate an
ncurses pad in order to hold the current page of disassembler output.

Unfortunately, due to the very long line, the call to newpad fails,
meaning that tui_source_window_base::m_pad is nullptr.  Luckily non of
the following calls appear to crash when passed a nullptr, however,
all the output that is written to the pad is lost, which is why we
don't see any assembly code written to the screen.

As the test history indicates that the script was originally checking
for a crash in GDB when the long identifier was encountered, I think
there is value in just leaving the test as it is for now, I'll see if
I can come up with a solution to the long line problem in a later
commit.
---
 .../gdb.tui/tui-disasm-long-lines.exp         | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp b/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp
index 0ad91308ca4..acc4c54063f 100644
--- a/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp
+++ b/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp
@@ -16,6 +16,8 @@
 # Test that the logic for displaying the TUI disassembly window
 # handles very long lines.
 
+tuiterm_env
+
 standard_testfile
 
 set ccopts {debug quiet}
@@ -31,13 +33,16 @@ if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "$binfile" \
     }
 }
 
-clean_restart "$binfile"
-
-if {[skip_tui_tests]} {
-    # TUI support is disabled.  Check for error message.
-    gdb_test "layout asm" "Undefined command: \"layout\".  Try \"help\"."
+Term::clean_restart 24 80 $binfile
+if {![Term::prepare_for_tui]} {
+    unsupported "TUI not supported"
     return
 }
 
-# Just check the command does not cause gdb to crash.
-gdb_test "layout asm"
+# Just check the command does not cause gdb to crash.  It is worth
+# noting that the asm window does infact fail to correctly display the
+# disassembler output at this point, but initially we are just
+# checking that GDB doesn't crash, fixing the asm display will come
+# later.
+Term::command_no_prompt_prefix "layout asm"
+Term::check_box "asm box" 0 0 80 15
-- 
2.25.4

Re: [PATCH 02/15] gdb/testsuite: update gdb.tui/tui-disasm-long-lines.exp

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> Following on from the previous commit, in this commit I am updating
> the test script gdb.tui/tui-disasm-long-lines.exp to take account of
> the changes in commit:
>
>   commit 9162a27c5f5828240b53379d735679e2a69a9f41
>   Date:   Tue Nov 13 11:59:03 2018 -0700
>
>       Change gdb test suite's TERM setting
>
> In the above commit the TERM environment variable was changed to be
> 'dumb' by default, which means that tests, that previously activated
> tui mode, no longer do unless TERM is set to 'ansi'.
>
> As the gdb.tui/tui-disasm-long-lines.exp script didn't do this, the
> test stopped working.  As the expect patterns in this script were
> pretty generic no tests actually started failing, and we never
> noticed.
>
> In this commit I update the script to use Term::clean_restart, which
> correctly sets TERM to 'ansi'.  I've also added a check that the asm
> box does appear on the screen, which should indicate that tui mode has
> correctly activated.
>
> However, I also notice that GDB doesn't appear to fully work
> correctly.  The test should display the disassembly for the test
> program, but it doesn't.
>
> The test is trying to disassemble some code that (deliberately) uses a
> very long symbol name, this eventually results in GDB entering
> tui_source_window_base::show_source_content and trying to allocate an
> ncurses pad in order to hold the current page of disassembler output.
>
> Unfortunately, due to the very long line, the call to newpad fails,
> meaning that tui_source_window_base::m_pad is nullptr.  Luckily non of
> the following calls appear to crash when passed a nullptr, however,
> all the output that is written to the pad is lost, which is why we
> don't see any assembly code written to the screen.
>
> As the test history indicates that the script was originally checking
> for a crash in GDB when the long identifier was encountered, I think
> there is value in just leaving the test as it is for now, I'll see if
> I can come up with a solution to the long line problem in a later
> commit.

I've gone ahead and pushed this patch.  The final version that I pushed
is included below.

Thanks,
Andrew

---

commit 843a1a4f735e77147710b307db29a1b5f4e1c707
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Dec 20 16:25:33 2022 +0000

    gdb/testsuite: update gdb.tui/tui-disasm-long-lines.exp
    
    Following on from the previous commit, in this commit I am updating
    the test script gdb.tui/tui-disasm-long-lines.exp to take account of
    the changes in commit:
    
      commit 9162a27c5f5828240b53379d735679e2a69a9f41
      Date:   Tue Nov 13 11:59:03 2018 -0700
    
          Change gdb test suite's TERM setting
    
    In the above commit the TERM environment variable was changed to be
    'dumb' by default, which means that tests, that previously activated
    tui mode, no longer do unless TERM is set to 'ansi'.
    
    As the gdb.tui/tui-disasm-long-lines.exp script didn't do this, the
    test stopped working.  As the expect patterns in this script were
    pretty generic no tests actually started failing, and we never
    noticed.
    
    In this commit I update the script to use Term::clean_restart, which
    correctly sets TERM to 'ansi'.  I've also added a check that the asm
    box does appear on the screen, which should indicate that tui mode has
    correctly activated.
    
    However, I also notice that GDB doesn't appear to fully work
    correctly.  The test should display the disassembly for the test
    program, but it doesn't.
    
    The test is trying to disassemble some code that (deliberately) uses a
    very long symbol name, this eventually results in GDB entering
    tui_source_window_base::show_source_content and trying to allocate an
    ncurses pad in order to hold the current page of disassembler output.
    
    Unfortunately, due to the very long line, the call to newpad fails,
    meaning that tui_source_window_base::m_pad is nullptr.  Luckily non of
    the following calls appear to crash when passed a nullptr, however,
    all the output that is written to the pad is lost, which is why we
    don't see any assembly code written to the screen.
    
    As the test history indicates that the script was originally checking
    for a crash in GDB when the long identifier was encountered, I think
    there is value in just leaving the test as it is for now, I have a fix
    for the issue of the newpad call failing, which I'll post in a follow
    up commit later.

diff --git a/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp b/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp
index 7e3cff3950b..ae19f2e69cb 100644
--- a/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp
+++ b/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp
@@ -16,6 +16,10 @@
 # Test that the logic for displaying the TUI disassembly window
 # handles very long lines.
 
+require allow_tui_tests
+
+tuiterm_env
+
 standard_testfile
 
 set ccopts {debug quiet}
@@ -31,13 +35,16 @@ if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "$binfile" \
     }
 }
 
-clean_restart "$binfile"
-
-if {![allow_tui_tests]} {
-    # TUI support is disabled.  Check for error message.
-    gdb_test "layout asm" "Undefined command: \"layout\".  Try \"help\"."
+Term::clean_restart 24 80 $binfile
+if {![Term::prepare_for_tui]} {
+    unsupported "TUI not supported"
     return
 }
 
-# Just check the command does not cause gdb to crash.
-gdb_test "layout asm"
+# Just check the command does not cause gdb to crash.  It is worth
+# noting that the asm window does infact fail to correctly display the
+# disassembler output at this point, but initially we are just
+# checking that GDB doesn't crash, fixing the asm display will come
+# later.
+Term::command_no_prompt_prefix "layout asm"
+Term::check_box "asm box" 0 0 80 15

[PATCH 03/15] gdb/testsuite: update gdb.tui/tui-nl-filtered-output.exp

[Andrew Burgess]

Following on from the previous commit, in this commit I am updating
the test script gdb.tui/tui-nl-filtered-output.exp to take account of
the changes in commit:

  commit 9162a27c5f5828240b53379d735679e2a69a9f41
  Date:   Tue Nov 13 11:59:03 2018 -0700

      Change gdb test suite's TERM setting

In the above commit the TERM environment variable was changed to be
'dumb' by default, which means that tests, that previously activated
tui mode, no longer do unless TERM is set to 'ansi'.

As the gdb.tui/tui-nl-filtered-output.exp script didn't do this, the
test stopped working.  As the expect patterns in this script were
pretty generic no tests actually started failing, and we never
noticed.

In this commit I update the test script to correctly activate our
terminal emulator, the test continues to pass after this update, but
now we are testing in tui mode.
---
 .../gdb.tui/tui-nl-filtered-output.exp        | 41 ++++++++-----------
 1 file changed, 16 insertions(+), 25 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/tui-nl-filtered-output.exp b/gdb/testsuite/gdb.tui/tui-nl-filtered-output.exp
index 5a2a4205bb5..b3d80a36a6f 100644
--- a/gdb/testsuite/gdb.tui/tui-nl-filtered-output.exp
+++ b/gdb/testsuite/gdb.tui/tui-nl-filtered-output.exp
@@ -30,28 +30,19 @@
 #
 #  (gdb)
 
-gdb_exit
-gdb_start
-
-if {[skip_tui_tests]} {
-    return
-}
-
-# Enable the TUI.
-
-set test "tui enable"
-gdb_test_multiple "tui enable" $test {
-    -re "$gdb_prompt $" {
-	pass $test
-    }
-}
-
-# Make sure filtering/pagination is enabled, but make the window high
-# enough that we don't paginate in practice.
-gdb_test_no_output "set pagination on"
-gdb_test_no_output "set height 2000"
-
-gdb_test \
-    {printf "hello\nworld\n"} \
-    "hello\r\nworld" \
-    "correct line breaks"
+tuiterm_env
+
+# Setup and enter TUI mode.
+Term::clean_restart 24 80
+Term::enter_tui
+
+# Send the command, and check the output is correctly split over
+# multiple lines.
+Term::command \
+    {printf "hello\nworld\n"}
+Term::check_region_contents "check printf output" \
+    0 16 80 4 [multi_line \
+		   "$gdb_prompt printf \"hello\\\\nworld\\\\n\"\\s+" \
+		   "hello\\s+" \
+		   "world\\s+" \
+		   "$gdb_prompt\\s+"]
-- 
2.25.4

Re: [PATCH 03/15] gdb/testsuite: update gdb.tui/tui-nl-filtered-output.exp

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> Following on from the previous commit, in this commit I am updating
> the test script gdb.tui/tui-nl-filtered-output.exp to take account of
> the changes in commit:
>
>   commit 9162a27c5f5828240b53379d735679e2a69a9f41
>   Date:   Tue Nov 13 11:59:03 2018 -0700
>
>       Change gdb test suite's TERM setting
>
> In the above commit the TERM environment variable was changed to be
> 'dumb' by default, which means that tests, that previously activated
> tui mode, no longer do unless TERM is set to 'ansi'.
>
> As the gdb.tui/tui-nl-filtered-output.exp script didn't do this, the
> test stopped working.  As the expect patterns in this script were
> pretty generic no tests actually started failing, and we never
> noticed.
>
> In this commit I update the test script to correctly activate our
> terminal emulator, the test continues to pass after this update, but
> now we are testing in tui mode.

I've gone ahead and pushed this patch.  The final version that I pushed
is included below.

Thanks,
Andrew

---

commit 3602634035f6a346ac043a7f01442a7290b9f588
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Dec 22 10:24:22 2022 +0000

    gdb/testsuite: update gdb.tui/tui-nl-filtered-output.exp
    
    Following on from the previous commit, in this commit I am updating
    the test script gdb.tui/tui-nl-filtered-output.exp to take account of
    the changes in commit:
    
      commit 9162a27c5f5828240b53379d735679e2a69a9f41
      Date:   Tue Nov 13 11:59:03 2018 -0700
    
          Change gdb test suite's TERM setting
    
    In the above commit the TERM environment variable was changed to be
    'dumb' by default, which means that tests, that previously activated
    tui mode, no longer do unless TERM is set to 'ansi'.
    
    As the gdb.tui/tui-nl-filtered-output.exp script didn't do this, the
    test stopped working.  As the expect patterns in this script were
    pretty generic no tests actually started failing, and we never
    noticed.
    
    In this commit I update the test script to correctly activate our
    terminal emulator, the test continues to pass after this update, but
    now we are testing in tui mode.

diff --git a/gdb/testsuite/gdb.tui/tui-nl-filtered-output.exp b/gdb/testsuite/gdb.tui/tui-nl-filtered-output.exp
index b7ea67cf723..5b1d6b7d9ba 100644
--- a/gdb/testsuite/gdb.tui/tui-nl-filtered-output.exp
+++ b/gdb/testsuite/gdb.tui/tui-nl-filtered-output.exp
@@ -32,24 +32,19 @@
 
 require allow_tui_tests
 
-gdb_exit
-gdb_start
-
-# Enable the TUI.
-
-set test "tui enable"
-gdb_test_multiple "tui enable" $test {
-    -re "$gdb_prompt $" {
-	pass $test
-    }
-}
-
-# Make sure filtering/pagination is enabled, but make the window high
-# enough that we don't paginate in practice.
-gdb_test_no_output "set pagination on"
-gdb_test_no_output "set height 2000"
-
-gdb_test \
-    {printf "hello\nworld\n"} \
-    "hello\r\nworld" \
-    "correct line breaks"
+tuiterm_env
+
+# Setup and enter TUI mode.
+Term::clean_restart 24 80
+Term::enter_tui
+
+# Send the command, and check the output is correctly split over
+# multiple lines.
+Term::command \
+    {printf "hello\nworld\n"}
+Term::check_region_contents "check printf output" \
+    0 16 80 4 [multi_line \
+		   "$gdb_prompt printf \"hello\\\\nworld\\\\n\"\\s+" \
+		   "hello\\s+" \
+		   "world\\s+" \
+		   "$gdb_prompt\\s+"]

[PATCH 04/15] gdb/testsuite/tui: more testing of the 'focus' command

[Andrew Burgess]

I noticed that we didn't have much testing of the tui 'focus' command,
so I started adding some.  This exposed a bug in GDB, we are able to
focus windows that should not be focusable, e.g. the 'status' window.

This is harmless until we then do 'focus next' or 'focus prev', along
this code path we assert that the currently focused window is
focusable, which obviously, is no longer true, so GDB fails with an
assertion error.

The fix is simple; add some code to the tui_set_focus_command function
that checks if the selected window is focusable.  If it is not then an
error is thrown.
---
 gdb/testsuite/gdb.tui/tui-focus.c   | 22 ++++++++++
 gdb/testsuite/gdb.tui/tui-focus.exp | 66 +++++++++++++++++++++++++++++
 gdb/tui/tui-win.c                   |  3 ++
 3 files changed, 91 insertions(+)
 create mode 100644 gdb/testsuite/gdb.tui/tui-focus.c
 create mode 100644 gdb/testsuite/gdb.tui/tui-focus.exp

diff --git a/gdb/testsuite/gdb.tui/tui-focus.c b/gdb/testsuite/gdb.tui/tui-focus.c
new file mode 100644
index 00000000000..3a264f239ed
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/tui-focus.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   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/>.  */
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.tui/tui-focus.exp b/gdb/testsuite/gdb.tui/tui-focus.exp
new file mode 100644
index 00000000000..156ced44e05
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/tui-focus.exp
@@ -0,0 +1,66 @@
+# 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/>.
+
+# Minimal testcase that just checks that the various "layout $foo"
+# commands do not cause gdb to crash.
+
+tuiterm_env
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile}]} {
+    return -1
+}
+
+if {[skip_tui_tests]} {
+    return
+}
+
+# Run a series of tests based on various test specifications.
+#
+# Each test specification is a tuple where the first item is the name of a
+# window, and the second item is a boolean indicating if we expect that
+# window to be present in the default (src) layout.
+##foreach spec {{src true} {cmd true} {status true} {regs false} {asm false}} {}
+foreach spec {{status false}} {
+    lassign $spec window valid_p
+    with_test_prefix "window=$window" {
+
+	Term::clean_restart 24 80 $binfile
+	if {![Term::prepare_for_tui]} {
+	    unsupported "TUI not supported"
+	    return
+	}
+
+	Term::command_no_prompt_prefix "focus $window"
+
+	if {$valid_p} {
+	    Term::check_region_contents "check focus message" 0 16 80 1 \
+		"^Focus set to $window window\\.\\s*"
+	} else {
+	    if {$window == "status"} {
+		Term::check_region_contents "check focus error" 0 16 80 1 \
+		    "^Window \"$window\" cannot be focused\\s*"
+	    } else {
+		Term::check_region_contents "check focus error" 0 16 80 1 \
+		    "^Unrecognized window name \"$window\"\\s*"
+	    }
+	}
+
+	Term::check_box "check src box" 0 0 80 15
+
+	Term::command "focus prev"
+    }
+}
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 58372005ff8..9c088899817 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -723,6 +723,9 @@ tui_set_focus_command (const char *arg, int from_tty)
   if (!win_info->is_visible ())
     error (_("Window \"%s\" is not visible"), arg);
 
+  if (!win_info->can_focus ())
+    error (_("Window \"%s\" cannot be focused"), arg);
+
   tui_set_win_focus_to (win_info);
   gdb_printf (_("Focus set to %s window.\n"),
 	      tui_win_with_focus ()->name ());
-- 
2.25.4

Re: [PATCH 04/15] gdb/testsuite/tui: more testing of the 'focus' command

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> I noticed that we didn't have much testing of the tui 'focus' command,
> so I started adding some.  This exposed a bug in GDB, we are able to
> focus windows that should not be focusable, e.g. the 'status' window.
>
> This is harmless until we then do 'focus next' or 'focus prev', along
> this code path we assert that the currently focused window is
> focusable, which obviously, is no longer true, so GDB fails with an
> assertion error.
>
> The fix is simple; add some code to the tui_set_focus_command function
> that checks if the selected window is focusable.  If it is not then an
> error is thrown.

I noticed that there was some code commented out in this patch which was
then uncommented in a later patch in the original series.  I've fixed
that so that everything is correctly included in this patch.

With that fix I've gone ahead and pushed this change.  The final version
that I pushed is included below.

Thanks,
Andrew

---

commit 24f3aded1d42f515527e2de7e8e9e26f0b77c932
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Dec 20 15:01:29 2022 +0000

    gdb/testsuite/tui: more testing of the 'focus' command
    
    I noticed that we didn't have many tests of the tui 'focus' command,
    so I started adding some.  This exposed a bug in GDB; we are able to
    focus windows that should not be focusable, e.g. the 'status' window.
    
    This is harmless until we then do 'focus next' or 'focus prev', along
    this code path we assert that the currently focused window is
    focusable, which obviously, is not always true, so GDB fails with an
    assertion error.
    
    The fix is simple; add a check to the tui_set_focus_command function
    to ensure that the selected window is focusable.  If it is not then an
    error is thrown.  The new tests I've added cover this case.

diff --git a/gdb/testsuite/gdb.tui/tui-focus.c b/gdb/testsuite/gdb.tui/tui-focus.c
new file mode 100644
index 00000000000..3a264f239ed
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/tui-focus.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   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/>.  */
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.tui/tui-focus.exp b/gdb/testsuite/gdb.tui/tui-focus.exp
new file mode 100644
index 00000000000..9e2a1cd0935
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/tui-focus.exp
@@ -0,0 +1,68 @@
+# 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/>.
+
+# Minimal testcase that just checks that the various "layout $foo"
+# commands do not cause gdb to crash.
+
+require allow_tui_tests
+
+tuiterm_env
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile}]} {
+    return -1
+}
+
+# Run a series of tests based on various test specifications.
+#
+# Each test specification is a tuple where the first item is the name of a
+# window, and the second item is a boolean indicating if we expect that
+# window to be present in the default (src) layout.
+foreach spec {{src true} {cmd true} {status true} {regs false} {asm false}} {
+    lassign $spec window valid_p
+    with_test_prefix "window=$window" {
+
+	Term::clean_restart 24 80 $binfile
+	if {![Term::prepare_for_tui]} {
+	    unsupported "TUI not supported"
+	    return
+	}
+
+	Term::command_no_prompt_prefix "focus $window"
+
+	if {$valid_p} {
+	    # The 'status' window is special, it's present in the
+	    # default (src) layout, but is not focusable.
+	    if {$window == "status"} {
+		Term::check_region_contents "check focus error" 0 16 80 1 \
+		    "^Window \"$window\" cannot be focused\\s*"
+	    } else {
+		Term::check_region_contents "check focus message" 0 16 80 1 \
+		    "^Focus set to $window window\\.\\s*"
+	    }
+	} else {
+	    Term::check_region_contents "check focus error" 0 16 80 1 \
+		"^Unrecognized window name \"$window\"\\s*"
+	}
+
+	Term::check_box "check src box" 0 0 80 15
+
+	# At one point the following 'focus prev' command would trigger a
+	# crash in GDB, GDB was allowing users to set focus to the 'status'
+	# window, and 'focus prev' would then trigger an assert.
+	Term::command "focus prev"
+    }
+}
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 58372005ff8..9c088899817 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -723,6 +723,9 @@ tui_set_focus_command (const char *arg, int from_tty)
   if (!win_info->is_visible ())
     error (_("Window \"%s\" is not visible"), arg);
 
+  if (!win_info->can_focus ())
+    error (_("Window \"%s\" cannot be focused"), arg);
+
   tui_set_win_focus_to (win_info);
   gdb_printf (_("Focus set to %s window.\n"),
 	      tui_win_with_focus ()->name ());

[PATCH 05/15] gdb/tui: convert if/error to an assert

[Andrew Burgess]

While working on the previous commit, I realised that there was an
error in tui_set_focus_command that could never be triggered.

Since the big tui rewrite (adding dynamic layouts) it is no longer
true that there is a tui_win_info object for every window at all
times.  We now only create a tui_win_info object for a particular
window, when the window is part of the current layout.  As a result,
if we have a tui_win_info pointer, then the window must be visible
inside tui_set_focus_command (this function calls tui_enable as its
first action, which makes the current layout visible).

The gdb.tui/tui-focus.exp test script exercises this area of code, and
doesn't trigger the assert, nor do any of our other existing tui
tests.
---
 gdb/tui/tui-win.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 9c088899817..4fc8e7a4503 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -720,8 +720,11 @@ tui_set_focus_command (const char *arg, int from_tty)
 
   if (win_info == NULL)
     error (_("Unrecognized window name \"%s\""), arg);
-  if (!win_info->is_visible ())
-    error (_("Window \"%s\" is not visible"), arg);
+
+  /* If a window is part of the current layout then it will have a
+     tui_win_info associated with it and be visible, otherwise, there will
+     be no tui_win_info and the above error will have been raised.  */
+  gdb_assert (win_info->is_visible ());
 
   if (!win_info->can_focus ())
     error (_("Window \"%s\" cannot be focused"), arg);
-- 
2.25.4

Re: [PATCH 05/15] gdb/tui: convert if/error to an assert

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> While working on the previous commit, I realised that there was an
> error in tui_set_focus_command that could never be triggered.
>
> Since the big tui rewrite (adding dynamic layouts) it is no longer
> true that there is a tui_win_info object for every window at all
> times.  We now only create a tui_win_info object for a particular
> window, when the window is part of the current layout.  As a result,
> if we have a tui_win_info pointer, then the window must be visible
> inside tui_set_focus_command (this function calls tui_enable as its
> first action, which makes the current layout visible).
>
> The gdb.tui/tui-focus.exp test script exercises this area of code, and
> doesn't trigger the assert, nor do any of our other existing tui
> tests.

I've gone ahead and pushed this patch.  The final version is included
below.

Thanks,
Andrew

---

commit 6789344ab22f7fbce94a31297b994f85685b90c6
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Dec 22 12:43:38 2022 +0000

    gdb/tui: convert if/error to an assert
    
    While working on the previous commit, I realised that there was an
    error in tui_set_focus_command that could never be triggered.
    
    Since the big tui rewrite (adding dynamic layouts) it is no longer
    true that there is a tui_win_info object for every window at all
    times.  We now only create a tui_win_info object for a particular
    window, when the window is part of the current layout.  As a result,
    if we have a tui_win_info pointer, then the window must be visible
    inside tui_set_focus_command (this function calls tui_enable as its
    first action, which makes the current layout visible).
    
    The gdb.tui/tui-focus.exp test script exercises this area of code, and
    doesn't trigger the assert, nor do any of our other existing tui
    tests.

diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 9c088899817..4fc8e7a4503 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -720,8 +720,11 @@ tui_set_focus_command (const char *arg, int from_tty)
 
   if (win_info == NULL)
     error (_("Unrecognized window name \"%s\""), arg);
-  if (!win_info->is_visible ())
-    error (_("Window \"%s\" is not visible"), arg);
+
+  /* If a window is part of the current layout then it will have a
+     tui_win_info associated with it and be visible, otherwise, there will
+     be no tui_win_info and the above error will have been raised.  */
+  gdb_assert (win_info->is_visible ());
 
   if (!win_info->can_focus ())
     error (_("Window \"%s\" cannot be focused"), arg);

[PATCH 06/15] gdb/tui: better filtering of tab completion results for focus command

[Andrew Burgess]

While working on the previous couple of commits, I noticed that the
'focus' command would happily suggest 'status' as a possible focus
completion, even though the 'status' window is non-focusable, and,
after the previous couple of commits, trying to focus the status
window will result in an error.

This commit improves the tab-completion results for the focus command
so that the status window is not included.
---
 gdb/testsuite/gdb.tui/completion.exp | 18 ++++++++++++++++--
 gdb/testsuite/gdb.tui/tui-focus.exp  |  4 +---
 gdb/tui/tui-win.c                    | 24 +++++++++++++++++-------
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/completion.exp b/gdb/testsuite/gdb.tui/completion.exp
index 00956488c9f..1d7de65abb7 100644
--- a/gdb/testsuite/gdb.tui/completion.exp
+++ b/gdb/testsuite/gdb.tui/completion.exp
@@ -13,8 +13,9 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-gdb_exit
-gdb_start
+tuiterm_env
+
+clean_restart
 
 if {[skip_tui_tests] || [target_info exists gdb,nointerrupts]} {
     return
@@ -54,3 +55,16 @@ with_test_prefix "completion of layout names" {
 with_test_prefix "completion of focus command" {
     test_tab_completion "focus" "cmd *next *prev *src *"
 }
+
+# Now run some completion tests when TUI mode is enabled.
+Term::clean_restart 24 80
+if {![Term::prepare_for_tui]} {
+    unsupported "TUI not supported"
+    return
+}
+
+Term::command "layout src"
+Term::command "complete focus "
+Term::dump_screen
+Term::check_region_contents "check focus completions" 0 17 80 5 \
+    "focus cmd\\s*focus next\\s*focus prev\\s*focus src\\s*$gdb_prompt"
diff --git a/gdb/testsuite/gdb.tui/tui-focus.exp b/gdb/testsuite/gdb.tui/tui-focus.exp
index 156ced44e05..0563aacd0a2 100644
--- a/gdb/testsuite/gdb.tui/tui-focus.exp
+++ b/gdb/testsuite/gdb.tui/tui-focus.exp
@@ -33,8 +33,7 @@ if {[skip_tui_tests]} {
 # Each test specification is a tuple where the first item is the name of a
 # window, and the second item is a boolean indicating if we expect that
 # window to be present in the default (src) layout.
-##foreach spec {{src true} {cmd true} {status true} {regs false} {asm false}} {}
-foreach spec {{status false}} {
+foreach spec {{src true} {cmd true} {status false} {regs false} {asm false}} {
     lassign $spec window valid_p
     with_test_prefix "window=$window" {
 
@@ -60,7 +59,6 @@ foreach spec {{status false}} {
 	}
 
 	Term::check_box "check src box" 0 0 80 15
-
 	Term::command "focus prev"
     }
 }
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 4fc8e7a4503..492a5191025 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -362,13 +362,19 @@ show_tui_resize_message (struct ui_file *file, int from_tty,
 \f
 
 /* Generic window name completion function.  Complete window name pointed
-   to by TEXT and WORD.  If INCLUDE_NEXT_PREV_P is true then the special
-   window names 'next' and 'prev' will also be considered as possible
-   completions of the window name.  */
+   to by TEXT and WORD.
+
+   If EXCLUDE_CANNOT_FOCUS_P is true, then windows that can't take focus
+   will be excluded from the completions, otherwise they will be included.
+
+   If INCLUDE_NEXT_PREV_P is true then the special window names 'next' and
+   'prev' will also be considered as possible completions of the window
+   name.  This is independent of EXCLUDE_CANNOT_FOCUS_P.  */
 
 static void
 window_name_completer (completion_tracker &tracker,
-		       int include_next_prev_p,
+		       bool include_next_prev_p,
+		       bool exclude_cannot_focus_p,
 		       const char *text, const char *word)
 {
   std::vector<const char *> completion_name_vec;
@@ -377,10 +383,14 @@ window_name_completer (completion_tracker &tracker,
     {
       const char *completion_name = NULL;
 
-      /* We can't focus on an invisible window.  */
+      /* Don't include an invisible window.  */
       if (!win_info->is_visible ())
 	continue;
 
+      /* If requested, exclude windows that can't be focused.  */
+      if (exclude_cannot_focus_p && !win_info->can_focus ())
+	continue;
+
       completion_name = win_info->name ();
       gdb_assert (completion_name != NULL);
       completion_name_vec.push_back (completion_name);
@@ -415,7 +425,7 @@ focus_completer (struct cmd_list_element *ignore,
 		 completion_tracker &tracker,
 		 const char *text, const char *word)
 {
-  window_name_completer (tracker, 1, text, word);
+  window_name_completer (tracker, true, true, text, word);
 }
 
 /* Complete possible window names for winheight command.  TEXT is the
@@ -432,7 +442,7 @@ winheight_completer (struct cmd_list_element *ignore,
   if (word != text)
     return;
 
-  window_name_completer (tracker, 0, text, word);
+  window_name_completer (tracker, false, false, text, word);
 }
 
 /* Update gdb's knowledge of the terminal size.  */
-- 
2.25.4

Re: [PATCH 06/15] gdb/tui: better filtering of tab completion results for focus command

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> While working on the previous couple of commits, I noticed that the
> 'focus' command would happily suggest 'status' as a possible focus
> completion, even though the 'status' window is non-focusable, and,
> after the previous couple of commits, trying to focus the status
> window will result in an error.
>
> This commit improves the tab-completion results for the focus command
> so that the status window is not included.

I've gone ahead and pushed this change.  The final version that I pushed
is included below.

Thanks,
Andrew

---

commit 6db98f026e4dcd7ca9d069542a7bbb3f4539482b
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Dec 20 15:45:32 2022 +0000

    gdb/tui: better filtering of tab completion results for focus command
    
    While working on the previous couple of commits, I noticed that the
    'focus' command would happily suggest 'status' as a possible focus
    completion, even though the 'status' window is non-focusable, and,
    after the previous couple of commits, trying to focus the status
    window will result in an error.
    
    This commit improves the tab-completion results for the focus command
    so that the status window is not included.

diff --git a/gdb/testsuite/gdb.tui/completion.exp b/gdb/testsuite/gdb.tui/completion.exp
index a3b3114a472..97d0f806a81 100644
--- a/gdb/testsuite/gdb.tui/completion.exp
+++ b/gdb/testsuite/gdb.tui/completion.exp
@@ -15,8 +15,9 @@
 
 require allow_tui_tests
 
-gdb_exit
-gdb_start
+tuiterm_env
+
+clean_restart
 
 if {[target_info exists gdb,nointerrupts]} {
     return
@@ -56,3 +57,16 @@ with_test_prefix "completion of layout names" {
 with_test_prefix "completion of focus command" {
     test_tab_completion "focus" "cmd *next *prev *src *"
 }
+
+# Now run some completion tests when TUI mode is enabled.
+Term::clean_restart 24 80
+if {![Term::prepare_for_tui]} {
+    unsupported "TUI not supported"
+    return
+}
+
+Term::command "layout src"
+Term::command "complete focus "
+Term::dump_screen
+Term::check_region_contents "check focus completions" 0 17 80 5 \
+    "focus cmd\\s*focus next\\s*focus prev\\s*focus src\\s*$gdb_prompt"
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 4fc8e7a4503..492a5191025 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -362,13 +362,19 @@ show_tui_resize_message (struct ui_file *file, int from_tty,
 \f
 
 /* Generic window name completion function.  Complete window name pointed
-   to by TEXT and WORD.  If INCLUDE_NEXT_PREV_P is true then the special
-   window names 'next' and 'prev' will also be considered as possible
-   completions of the window name.  */
+   to by TEXT and WORD.
+
+   If EXCLUDE_CANNOT_FOCUS_P is true, then windows that can't take focus
+   will be excluded from the completions, otherwise they will be included.
+
+   If INCLUDE_NEXT_PREV_P is true then the special window names 'next' and
+   'prev' will also be considered as possible completions of the window
+   name.  This is independent of EXCLUDE_CANNOT_FOCUS_P.  */
 
 static void
 window_name_completer (completion_tracker &tracker,
-		       int include_next_prev_p,
+		       bool include_next_prev_p,
+		       bool exclude_cannot_focus_p,
 		       const char *text, const char *word)
 {
   std::vector<const char *> completion_name_vec;
@@ -377,10 +383,14 @@ window_name_completer (completion_tracker &tracker,
     {
       const char *completion_name = NULL;
 
-      /* We can't focus on an invisible window.  */
+      /* Don't include an invisible window.  */
       if (!win_info->is_visible ())
 	continue;
 
+      /* If requested, exclude windows that can't be focused.  */
+      if (exclude_cannot_focus_p && !win_info->can_focus ())
+	continue;
+
       completion_name = win_info->name ();
       gdb_assert (completion_name != NULL);
       completion_name_vec.push_back (completion_name);
@@ -415,7 +425,7 @@ focus_completer (struct cmd_list_element *ignore,
 		 completion_tracker &tracker,
 		 const char *text, const char *word)
 {
-  window_name_completer (tracker, 1, text, word);
+  window_name_completer (tracker, true, true, text, word);
 }
 
 /* Complete possible window names for winheight command.  TEXT is the
@@ -432,7 +442,7 @@ winheight_completer (struct cmd_list_element *ignore,
   if (word != text)
     return;
 
-  window_name_completer (tracker, 0, text, word);
+  window_name_completer (tracker, false, false, text, word);
 }
 
 /* Update gdb's knowledge of the terminal size.  */

[PATCH 07/15] gdb/testsuite: fix line feed scrolling in tuiterm.exp

[Andrew Burgess]

In a following commit I managed to trigger the line feed scrolling
case in tuiterm.exp.  This case is currently unhandled, and this
commit fills in this missing functionality.

The implementation is pretty simple, just scroll all the content up
one line at a time until the cursor is back on the screen (a single
line of scroll is all that should be needed).

This change is untested in this commit, but is required by the next
commit.
---
 gdb/testsuite/lib/tuiterm.exp | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index 591c4ca9c4c..22fc9ab2369 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -99,10 +99,24 @@ namespace eval Term {
 	_log_cur "Line feed" {
 	    variable _cur_row
 	    variable _rows
+	    variable _cols
+	    variable _chars
 
 	    incr _cur_row 1
-	    if {$_cur_row >= $_rows} {
-		error "FIXME scroll"
+	    while {$_cur_row >= $_rows} {
+		# Scroll the display contents.  We scroll one line at
+		# a time here; as _cur_row was only increased by one,
+		# a single line scroll should be enough to put the
+		# cursor back on the screen.  But we wrap the
+		# scrolling inside a while loop just to be on the safe
+		# side.
+		for {set y 0} {$y < [expr $_rows - 1]} {incr y} {
+		    set next_y [expr $y + 1]
+		    for {set x 0} {$x < $_cols} {incr x} {
+			set _chars($x,$y) $_chars($x,$next_y)
+		    }
+		    incr _cur_row -1
+		}
 	    }
 	}
     }
-- 
2.25.4

[PATCH 08/15] gdb/tui: improve errors from tui focus command

[Andrew Burgess]

This commit improves (I think) the errors from the tui focus command.
There are a number of errors that can be triggered by the focus
command, they include:

  (1) Window name "NAME" is ambiguous

  (2) Unrecognized window name "NAME"

  (3) Window "NAME" cannot be focused

Error (1) is triggered when the user gives a partial window name, and
the name matches multiple windows in the current layout.  It is worth
noting that the ambiguity must be within the current layout, if the
partial name matches one window in the current layout, and one window
not currently displayed, then this is not ambiguous, and focus will
shift to the selected window in the current layout.

This error was not previous being tested, but in this commit I make
use of the Python API to trigger this error.

Error (3) is simple enough, and was already being tested.  This is
triggered by something like 'focus status'.  The named window needs to
be present in the current layout, and non-focusable in order to
trigger the error.

Error (2) is what I'd like to improve in this commit.  This error
triggers if the name the user gives doesn't match any window in the
current layout.  Even if GDB does know about the window, but the
window isn't in the current layout, then GDB will say it doesn't
recognize the window name.

In this commit I propose to change this.  I plan to split this error
into three different errors.  These will be:

  (a) Unrecognized window name "NAME"

  (b) No windows matching "NAME" in the current layout

  (c) Window "NAME" is not in the current layout

Error (a) is the same as before, but will now only trigger if GDB
doesn't know about window NAME at all.  If the window is known, but
not in the current layout then one of the other errors will trigger.

Error (b) will trigger if NAME is ambiguous for multiple windows that
are not in the current layout.  If NAME identifies a single window in
the current layout then that window will continue to be selected, just
as it currently is.  Only in the case where NAME doesn't identify a
window in the current layout do we then check all the other known
windows, if NAME matches multiple of these, then (b) is triggered.

Finally, error (c) is used when NAME uniquely identifies a single
window that is not in the current layout.

The hope with these new errors is that the user will have a better
understanding of what went wrong.  Instead of GDB claiming to not know
about a window, the mention of the current layout will hint to the
user that they should first switch layouts.

There are tests included for all the new errors.
---
 gdb/testsuite/gdb.tui/tui-focus.exp | 81 +++++++++++++++++++++++++++--
 gdb/tui/tui-layout.c                | 11 +++-
 gdb/tui/tui-layout.h                | 45 ++++++++++++++++
 gdb/tui/tui-win.c                   | 40 +++++++++++++-
 4 files changed, 170 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/tui-focus.exp b/gdb/testsuite/gdb.tui/tui-focus.exp
index 0563aacd0a2..4ad2e5327c7 100644
--- a/gdb/testsuite/gdb.tui/tui-focus.exp
+++ b/gdb/testsuite/gdb.tui/tui-focus.exp
@@ -13,10 +13,10 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# Minimal testcase that just checks that the various "layout $foo"
-# commands do not cause gdb to crash.
+# Testcase that just checks the tui 'focus' command works as expected.
 
 tuiterm_env
+load_lib gdb-python.exp
 
 standard_testfile
 
@@ -33,7 +33,8 @@ if {[skip_tui_tests]} {
 # Each test specification is a tuple where the first item is the name of a
 # window, and the second item is a boolean indicating if we expect that
 # window to be present in the default (src) layout.
-foreach spec {{src true} {cmd true} {status false} {regs false} {asm false}} {
+foreach spec {{src true} {cmd true} {status false} {regs false} \
+		  {asm false} {unknown false}} {
     lassign $spec window valid_p
     with_test_prefix "window=$window" {
 
@@ -52,9 +53,12 @@ foreach spec {{src true} {cmd true} {status false} {regs false} {asm false}} {
 	    if {$window == "status"} {
 		Term::check_region_contents "check focus error" 0 16 80 1 \
 		    "^Window \"$window\" cannot be focused\\s*"
-	    } else {
+	    } elseif {$window == "unknown"} {
 		Term::check_region_contents "check focus error" 0 16 80 1 \
 		    "^Unrecognized window name \"$window\"\\s*"
+	    } else {
+		Term::check_region_contents "check focus error" 0 16 80 1 \
+		    "^Window \"$window\" is not in the current layout\\s*"
 	    }
 	}
 
@@ -62,3 +66,72 @@ foreach spec {{src true} {cmd true} {status false} {regs false} {asm false}} {
 	Term::command "focus prev"
     }
 }
+
+# Use the Python TUI API to exercise some of the ambigous window name
+# handling parts of the 'focus' command.
+Term::clean_restart 24 80 $binfile
+if {![skip_python_tests]} {
+    # Create a very simple tui window.
+    gdb_py_test_silent_cmd \
+	[multi_line_input \
+	     "python" \
+	     "class TestWindow:" \
+	     "    def __init__(self, win):" \
+	     "        pass" \
+	     "" \
+	     "    def render(self):" \
+	     "        pass" \
+	     "end"] \
+	"setup dummy window class" \
+	true
+
+    # Register the window with a set of similar names.
+    gdb_test_no_output "python gdb.register_window_type(\"test1\", TestWindow)"
+    gdb_test_no_output "python gdb.register_window_type(\"test2\", TestWindow)"
+    gdb_test_no_output "python gdb.register_window_type(\"test3\", TestWindow)"
+
+    # Create a layout containing just one of the above windows.
+    gdb_test_no_output "tui new-layout example1 test2 1 status 1 cmd 1"
+
+    # Create a layout containing two of the above windows.
+    gdb_test_no_output "tui new-layout example2 test1 1 test2 1 status 1 cmd 1"
+
+    if {![Term::prepare_for_tui]} {
+	unsupported "TUI not supported"
+	return
+    }
+
+    # Try to focus using an ambiguous, partial window name.  This
+    # should fail as the default layout (src) doesn't include any
+    # windows matching this name.
+    Term::command_no_prompt_prefix "focus test"
+    Term::check_region_contents "check no matching window focus message" \
+	0 16 80 1 \
+	"^No windows matching \"test\" in the current layout\\s*"
+
+    # Now select a layout that includes a single window that matches
+    # the ambiguous, partial name 'test', and disable tui mode.
+    Term::command "layout example1"
+    send_gdb "tui disable\n"
+
+    # Reactivate tui mode and try to set focus using the ambiguous,
+    # partial window name.  This should succeed though, as, within the
+    # current layout, the partial name is not actually ambiguous.
+    send_gdb "focus test\n"
+    gdb_assert [Term::wait_for_region_contents 0 19 80 1 \
+		    "^Focus set to test2 window\\.\\s*"] \
+	"check test2 focus message"
+
+    # Now select a layout that includes two windows that matches the
+    # ambiguous, partial name 'test', and disable tui mode.
+    Term::command "layout example2"
+    send_gdb "tui disable\n"
+
+    # Reactivate tui mode and try to set focus using the ambiguous,
+    # partial window name.  This will fail as now the layout includes
+    # multiple windows that match 'test'.
+    send_gdb "focus test\n"
+    gdb_assert [Term::wait_for_region_contents 0 22 80 1 \
+		    "^Window name \"test\" is ambiguous\\s*"] \
+	"check ambiguous focus message"
+}
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 27abee02087..447a20cb614 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -29,7 +29,6 @@
 #include "cli/cli-decode.h"
 #include "cli/cli-utils.h"
 #include <ctype.h>
-#include <unordered_map>
 #include <unordered_set>
 
 #include "tui/tui.h"
@@ -353,6 +352,16 @@ make_standard_window (const char *)
 
 static std::unordered_map<std::string, window_factory> *known_window_types;
 
+/* See tui-layout.h.  */
+
+all_known_window_names_range
+all_known_window_names ()
+{
+  auto begin = all_known_window_names_iterator (known_window_types->begin ());
+  auto end = all_known_window_names_iterator (known_window_types->end ());
+  return all_known_window_names_range (begin, end);
+}
+
 /* Helper function that returns a TUI window, given its name.  */
 
 static tui_win_info *
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index 206f1117445..9ce3ef39507 100644
--- a/gdb/tui/tui-layout.h
+++ b/gdb/tui/tui-layout.h
@@ -26,6 +26,9 @@
 
 #include "tui/tui.h"
 #include "tui/tui-data.h"
+#include "gdbsupport/iterator-range.h"
+
+#include <unordered_map>
 
 /* Values that can be returned when handling a request to adjust a
    window's size.  */
@@ -364,4 +367,46 @@ typedef std::function<tui_win_info * (const char *name)> window_factory;
 
 extern void tui_register_window (const char *name, window_factory &&factory);
 
+/* An iterator class for known tui window names.  This is just a wrapper
+   around an iterator of the underlying data structure that holds the
+   known tui windows.  This iterator only reveals the window names.  */
+
+struct all_known_window_names_iterator
+{
+  using known_window_types_iterator
+    = std::unordered_map<std::string, window_factory>::iterator;
+
+  all_known_window_names_iterator (known_window_types_iterator &&iter)
+    : m_iter (std::move (iter))
+  { /* Nothing.  */ }
+
+  all_known_window_names_iterator &operator++ ()
+  {
+    ++m_iter;
+    return *this;
+  }
+
+  const std::string &operator* () const
+  { return (*m_iter).first; }
+
+  bool operator!= (const all_known_window_names_iterator &other) const
+  { return m_iter != other.m_iter; }
+
+private:
+
+  /* The underlying iterator.  */
+  known_window_types_iterator m_iter;
+};
+
+/* A range adapter that makes it possible to iterate over the names of all
+   known tui windows.  */
+
+using all_known_window_names_range
+  = iterator_range<all_known_window_names_iterator>;
+
+/* Return a range that can be used to walk over the name of all known tui
+   windows in a range-for loop.  */
+
+extern all_known_window_names_range all_known_window_names ();
+
 #endif /* TUI_TUI_LAYOUT_H */
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 492a5191025..008189eb99b 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -728,8 +728,44 @@ tui_set_focus_command (const char *arg, int from_tty)
   else
     win_info = tui_partial_win_by_name (arg);
 
-  if (win_info == NULL)
-    error (_("Unrecognized window name \"%s\""), arg);
+  if (win_info == nullptr)
+    {
+      /* When WIN_INFO is nullptr this can either mean that the window name
+	 is unknown to GDB, or that the window is not in the current
+	 layout.  To try and help the user, give a different error
+	 depending on which of these is the case.  */
+      std::string matching_window_name;
+      bool is_ambiguous = false;
+
+      for (const std::string &name : all_known_window_names ())
+	{
+	  /* Look through all windows in the current layout, if the window
+	     is in the current layout then we're not interested is it.  */
+	  for (tui_win_info *item : all_tui_windows ())
+	    if (item->name () == name)
+	      continue;
+
+	  if (startswith (name, arg))
+	    {
+	      if (matching_window_name.empty ())
+		matching_window_name = name;
+	      else
+		is_ambiguous = true;
+	    }
+	};
+
+      if (!matching_window_name.empty ())
+	{
+	  if (is_ambiguous)
+	    error (_("No windows matching \"%s\" in the current layout"),
+		   arg);
+	  else
+	    error (_("Window \"%s\" is not in the current layout"),
+		   matching_window_name.c_str ());
+	}
+      else
+	error (_("Unrecognized window name \"%s\""), arg);
+    }
 
   /* If a window is part of the current layout then it will have a
      tui_win_info associated with it and be visible, otherwise, there will
-- 
2.25.4

[PATCH 09/15] gdb/tui: disable tui mode when an assert triggers

[Andrew Burgess]

When an assert triggers in tui mode the output is not great, the
internal backtrace that is generated is printed directly to the file
descriptor for gdb_stderr, and, as a result, does not currently format
itself correctly - the output uses only '\n' at the end of each line,
and so, when the terminal is in raw mode, the cursor does not return
to the start of each line after the '\n'.

This is mostly fixable, we could update bt-utils.c to use '\r\n'
instead of just '\n', and this would fix most of the problems.  The
one we can't easily fix is if/when GDB is built to use execinfo
instead of libbacktrace, in this case we use backtrace_symbols_fd to
print the symbols, and this function only uses '\n' as the line
terminator.  Fixing this would require switching to backtrace_symbols,
but that API uses malloc, which is something we're trying to
avoid (this code is called when GDB hits an error, so ideally we don't
want to rely on malloc).

However, the execinfo code is only used when libbacktrace is not
available (or the user specifically disables libbacktrace) so maybe we
can ignore that problem...

... but there is another problem.  When the backtrace is printed in
raw mode, it is possible that the backtrace fills the screen.  With
the terminal in raw mode we don't have the ability to scroll back,
which means we loose some of the backtrace, which isn't ideal.

In this commit I propose that we should disable tui mode whenever we
handle a fatal signal, or when we hit the internal error code
path (e.g. when an assert triggers).  With this done then we don't
need to update the bt-utils.c code, and the execinfo version of the
code (using backtrace_symbols_fd) works just fine.  We also get the
ability to scroll back to view the error message and all of the
backtrace, assuming the users terminal supports scrolling back.

The only downside I see with this change is if the tui_disable call
itself causes an error for some reason, or, if we handle a single at a
time when it is not safe to call tui_disable, in these cases the extra
tui_disable call might cause GDB to loose the original error.

However, I think (just from personal experience) that the above two
issues are pretty rare and the benefits from this change far out
weighs the possible drawbacks.
---
 gdb/event-top.c | 8 ++++++++
 gdb/utils.c     | 7 ++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 4a46e1b9346..14984707df1 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -48,6 +48,10 @@
 #include "readline/readline.h"
 #include "readline/history.h"
 
+#ifdef TUI
+#include "tui/tui.h"
+#endif
+
 /* readline defines this.  */
 #undef savestring
 
@@ -940,6 +944,10 @@ unblock_signal (int sig)
 static void ATTRIBUTE_NORETURN
 handle_fatal_signal (int sig)
 {
+#ifdef TUI
+  tui_disable ();
+#endif
+
 #ifdef GDB_PRINT_INTERNAL_BACKTRACE
   const auto sig_write = [] (const char *msg) -> void
   {
diff --git a/gdb/utils.c b/gdb/utils.c
index 734c5bf7f70..95adbe58e4a 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -29,7 +29,8 @@
 #endif /* HAVE_SYS_RESOURCE_H */
 
 #ifdef TUI
-#include "tui/tui.h"		/* For tui_get_command_dimension.   */
+/* For tui_get_command_dimension and tui_disable.   */
+#include "tui/tui.h"
 #endif
 
 #ifdef __GO32__
@@ -354,6 +355,10 @@ internal_vproblem (struct internal_problem *problem,
       }
   }
 
+#ifdef TUI
+  tui_disable ();
+#endif
+
   /* Create a string containing the full error/warning message.  Need
      to call query with this full string, as otherwize the reason
      (error/warning) and question become separated.  Format using a
-- 
2.25.4

[PATCH 10/15] gdb/tui: make m_horizontal_offset private

[Andrew Burgess]

I noticed that tui_source_window_base::m_horizontal_offset was
protected, but could be made private, so lets do that.

This makes more sense in the context of a later commit where I plan to
add another member variable that is similar to m_horizontal_offset.
The new member variable could also be private.

So I had to choose, place the new member variable next to
m_horizontal_offset making it protected, but grouping similar
variables together, or make m_horizontal_offset private, and then add
the new variable as private too.

I chose to make m_horizontal_offset private, which is this commit.

There should be no user visible changes after this commit.
---
 gdb/tui/tui-winsource.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/tui/tui-winsource.h b/gdb/tui/tui-winsource.h
index 7d6d64d35c4..bf0ca96c09b 100644
--- a/gdb/tui/tui-winsource.h
+++ b/gdb/tui/tui-winsource.h
@@ -124,8 +124,7 @@ struct tui_source_window_base : public tui_win_info
   /* Redraw the complete line of a source or disassembly window.  */
   void show_source_line (int lineno);
 
-  /* Used for horizontal scroll.  */
-  int m_horizontal_offset = 0;
+  /* Where to start generating content from.  */
   struct tui_line_or_address m_start_line_or_addr;
 
   /* Architecture associated with code at this location.  */
@@ -179,6 +178,9 @@ struct tui_source_window_base : public tui_win_info
 
 private:
 
+  /* Used for horizontal scroll.  */
+  int m_horizontal_offset = 0;
+
   void show_source_content ();
 
   /* Called when the user "set style enabled" setting is changed.  */
-- 
2.25.4

[PATCH 11/15] gdb/tui: rewrite of tui_source_window_base to handle very long lines

[Andrew Burgess]

This commit addresses an issue that is exposed by the test script
gdb.tui/tui-disasm-long-lines.exp, that is, tui_source_window_base
does not handle very long lines.

The problem can be traced back to the newpad call in
tui_source_window_base::show_source_content, this is where we allocate
a backing pad to hold the window content.

Unfortunately, there appears to be a limit to the size of pad that can
be allocated, and the gdb.tui/tui-disasm-long-lines.exp test goes
beyond this limit.  As a consequence the newpad call fails and returns
nullptr.

It just so happens that the reset of the tui_source_window_base code
can handle the pad being nullptr (this happens anyway when the window
is first created, so we already depend on nullptr handling), so all
that happens is the source window displays no content.

... well, sort of ... something weird does happen in the command
window, we seem to see a whole bunch of blank lines.  I've not
bothered to track down exactly what's happening there, but it's some
consequence of GDB attempting to write content to a WINDOW* that is
nullptr.

Before explaining my solution, I'll outline how things currently work:

Consider we have the following window content to display:

  aaaaaaaaaa
  bbbbbbbbbbbbbbbbbbbb
  ccccccccccccccc

the longest line here is 20 characters.  If our display window is 10
characters wide, then we will create a pad that is 20 characters wide,
and then copy the lines of content into the pad:

  .--------------------.
  |aaaaaaaaaa          |
  |bbbbbbbbbbbbbbbbbbbb|
  |ccccccccccccccc     |
  .--------------------.

Now we will copy a 10 character wide view into this pad to the
display, our display will then see:

  .----------.
  |aaaaaaaaaa|
  |bbbbbbbbbb|
  |cccccccccc|
  .----------.

As the user scrolls left and right we adjust m_horizontal_offset and
use this to select which part of the pad is copied onto the display.

The benefit of this is that we only need to copy the content to the
pad once, which includes processing the ansi escape sequences, and
then the user can scroll left and right as much as they want
relatively cheaply.

The problem then, is that if the longest content line is very long,
then we try to allocate a very large pad, which can fail.

What I propose is that we allow both the pad and the display view to
scroll.  Once we allow this, then it becomes possible to allocate a
pad that is smaller than the longest display line.  We then copy part
of the content into the pad.  As the user scrolls the view left and
right GDB will continue to copy content from the pad just as it does
right now.  But, when the user scrolls to the edge of the pad, GDB
will copy a new block of content into the pad, and then update the
view as normal.  This all works fine so long as the maximum pad size
is larger than the current window size - which seems a reasonable
restriction, if ncurses can't support a pad of a given size it seems
likely it will not support a display window of that size either.

If we return to our example above, but this time we assume that the
maximum pad size is 15 characters, then initially the pad would be
loaded like this:

  .---------------.
  |aaaaaaaaaa     |
  |bbbbbbbbbbbbbbb|
  |ccccccccccccccc|
  .---------------.

Notice that the last 5 characters from the 'b' line are no longer
included in the pad.  There is still enough content though to fill the
10 character wide display, just as we did before.

The pad contents remain unchanged until the user scrolls the display
right to this point:

  .----------.
  |aaaaa     |
  |bbbbbbbbbb|
  |cccccccccc|
  .----------.

Now, when the user scrolls right once more GDB spots that the user has
reached the end of the pad, and the pad contents are reloaded, like
this:

  .---------------.
  |aaaaa          |
  |bbbbbbbbbbbbbbb|
  |cccccccccc     |
  .---------------.

The display can now be updated from the pad again just like normal.

With this change in place the gdb.tui/tui-disasm-long-lines.exp test
now correctly loads the assembler code, and we can scroll around as
expected.

Most of the changes are pretty mundane, just updating to match the
above.  One interesting change though is the new member function
tui_source_window_base::puts_to_pad_with_skip.  This replaces direct
calls to tui_puts when copying content to the pad.

The content strings contain ansi escape sequences.  When these strings
are written to the pad these escape sequences are translated into
ncurses attribute setting calls.

Now however, we sometimes only write a partial string to the pad,
skipping some of the leading content.  Imagine then that we have a
content line like this:

  "\033[31mABCDEFGHIJKLM\033[0m"

Now the escape sequences in this content mean that the actual
content (the 'ABCDEFGHIJKLM') will have a red foreground color.

If we want to copy this to the pad, but skip the first 3 characters,
then what we expect is to have the pad contain 'DEFGHIJKLM', but this
text should still have a red foreground color.

It is this problem that puts_to_pad_with_skip solves.  This function
skips some number of printable characters, but processes all the
escape sequences.  This means that when we do start printing the
actual content the content will have the expected attributes.
/
---
 .../gdb.tui/tui-disasm-long-lines.exp         |   6 +-
 gdb/tui/tui-winsource.c                       | 177 ++++++++++++++++--
 gdb/tui/tui-winsource.h                       |  54 +++++-
 3 files changed, 215 insertions(+), 22 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp b/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp
index acc4c54063f..345f7d21109 100644
--- a/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp
+++ b/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp
@@ -39,10 +39,6 @@ if {![Term::prepare_for_tui]} {
     return
 }
 
-# Just check the command does not cause gdb to crash.  It is worth
-# noting that the asm window does infact fail to correctly display the
-# disassembler output at this point, but initially we are just
-# checking that GDB doesn't crash, fixing the asm display will come
-# later.
 Term::command_no_prompt_prefix "layout asm"
 Term::check_box "asm box" 0 0 80 15
+Term::check_box_contents "check asm box contents" 0 0 80 15 "<main>"
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 87099ac26f5..6e22638ec74 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -170,6 +170,7 @@ tui_source_window_base::update_source_window_as_is
     erase_source_content ();
   else
     {
+      validate_scroll_offsets ();
       update_breakpoint_info (nullptr, false);
       show_source_content ();
       update_exec_info ();
@@ -231,6 +232,67 @@ tui_source_window_base::do_erase_source_content (const char *str)
     }
 }
 
+/* See tui-winsource.h.  */
+
+void
+tui_source_window_base::puts_to_pad_with_skip (const char *string, int skip)
+{
+  gdb_assert (m_pad.get () != nullptr);
+  WINDOW *w = m_pad.get ();
+
+  while (skip > 0)
+    {
+      const char *next = strpbrk (string, "\033");
+
+      /* Print the plain text prefix.  */
+      size_t n_chars = next == nullptr ? strlen (string) : next - string;
+      if (n_chars > 0)
+	{
+	  if (skip > 0)
+	    {
+	      if (skip < n_chars)
+		{
+		  string += skip;
+		  n_chars -= skip;
+		  skip = 0;
+		}
+	      else
+		{
+		  skip -= n_chars;
+		  string += n_chars;
+		  n_chars = 0;
+		}
+	    }
+
+	  if (n_chars > 0)
+	    {
+	      std::string copy (string, n_chars);
+	      tui_puts (copy.c_str (), w);
+	    }
+	}
+
+      /* We finished.  */
+      if (next == nullptr)
+	break;
+
+      gdb_assert (*next == '\033');
+
+      int n_read;
+      if (skip_ansi_escape (next, &n_read))
+	{
+	  std::string copy (next, n_read);
+	  tui_puts (copy.c_str (), w);
+	  next += n_read;
+	}
+      else
+	gdb_assert_not_reached ("unhandled escape");
+
+      string = next;
+    }
+
+  if (*string != '\0')
+    tui_puts (string, w);
+}
 
 /* Redraw the complete line of a source or disassembly window.  */
 void
@@ -243,7 +305,8 @@ tui_source_window_base::show_source_line (int lineno)
     tui_set_reverse_mode (m_pad.get (), true);
 
   wmove (m_pad.get (), lineno, 0);
-  tui_puts (line->line.c_str (), m_pad.get ());
+  puts_to_pad_with_skip (line->line.c_str (), m_pad_offset);
+
   if (line->is_exec_point)
     tui_set_reverse_mode (m_pad.get (), false);
 }
@@ -257,13 +320,25 @@ tui_source_window_base::refresh_window ()
      the screen, potentially creating a flicker.  */
   wnoutrefresh (handle.get ());
 
-  int pad_width = std::max (m_max_length, width);
-  int left_margin = 1 + TUI_EXECINFO_SIZE + extra_margin ();
-  int view_width = width - left_margin - 1;
-  int pad_x = std::min (pad_width - view_width, m_horizontal_offset);
-  /* Ensure that an equal number of scrolls will work if the user
-     scrolled beyond where we clip.  */
-  m_horizontal_offset = pad_x;
+  int pad_width = getmaxx (m_pad.get ());
+  int left_margin = this->left_margin ();
+  int view_width = this->view_width ();
+  int content_width = m_max_length;
+  int pad_x = m_horizontal_offset - m_pad_offset;
+
+  gdb_assert (m_pad_offset >= 0);
+  gdb_assert (m_horizontal_offset + view_width
+	      <= std::max (content_width, view_width));
+  gdb_assert (pad_x >= 0);
+  gdb_assert (m_horizontal_offset >= 0);
+
+  /* This function can be called before the pad has been allocated, this
+     should only occur during the initial startup.  In this case the first
+     condition in the following asserts will not be true, but the nullptr
+     check will.  */
+  gdb_assert (pad_width > 0 || m_pad.get () == nullptr);
+  gdb_assert (pad_x + view_width <= pad_width || m_pad.get () == nullptr);
+
   prefresh (m_pad.get (), 0, pad_x, y + 1, x + left_margin,
 	    y + m_content.size (), x + left_margin + view_width - 1);
 }
@@ -275,11 +350,51 @@ tui_source_window_base::show_source_content ()
 
   check_and_display_highlight_if_needed ();
 
-  int pad_width = std::max (m_max_length, width);
-  if (m_pad == nullptr || pad_width > getmaxx (m_pad.get ())
-      || m_content.size () > getmaxy (m_pad.get ()))
-    m_pad.reset (newpad (m_content.size (), pad_width));
+  /* The pad should be at least as wide as the window, but ideally, as wide
+     as the content, however, for some very wide content this might not be
+     possible.  */
+  int required_pad_width = std::max (m_max_length, width);
+  int required_pad_height = m_content.size ();
+
+  /* If the required pad width is wider than the previously requested pad
+     width, then we might want to grow the pad.  */
+  if (required_pad_width > m_pad_requested_width
+      || required_pad_height > getmaxy (m_pad.get ()))
+    {
+      /* The current pad width.  */
+      int pad_width = m_pad == nullptr ? 0 : getmaxx (m_pad.get ());
+
+      gdb_assert (pad_width <= m_pad_requested_width);
+
+      /* If the current pad width is smaller than the previously requested
+	 pad width, then this means we previously failed to allocate a
+	 bigger pad.  There's no point asking again, so we'll just make so
+	 with the pad we currently have.  */
+      if (pad_width == m_pad_requested_width
+	  || required_pad_height > getmaxy (m_pad.get ()))
+	{
+	  pad_width = required_pad_width;
+
+	  do
+	    {
+	      /* Try to allocate a new pad.  */
+	      m_pad.reset (newpad (required_pad_height, pad_width));
+
+	      if (m_pad == nullptr)
+		{
+		  int reduced_width = std::max (pad_width / 2, width);
+		  if (reduced_width == pad_width)
+		    error (_("failed to setup source window"));
+		  pad_width = reduced_width;
+		}
+	    }
+	  while (m_pad == nullptr);
+	}
+
+      m_pad_requested_width = required_pad_width;
+    }
 
+  gdb_assert (m_pad != nullptr);
   werase (m_pad.get ());
   for (int lineno = 0; lineno < m_content.size (); lineno++)
     show_source_line (lineno);
@@ -370,6 +485,35 @@ tui_source_window_base::refill ()
   update_source_window_as_is (m_gdbarch, sal);
 }
 
+/* See tui-winsource.h.  */
+
+bool
+tui_source_window_base::validate_scroll_offsets ()
+{
+  int original_pad_offset = m_pad_offset;
+
+  if (m_horizontal_offset < 0)
+    m_horizontal_offset = 0;
+
+  int content_width = m_max_length;
+  int pad_width = getmaxx (m_pad.get ());
+  int view_width = this->view_width ();
+
+  if (m_horizontal_offset + view_width > content_width)
+    m_horizontal_offset = std::max (content_width - view_width, 0);
+
+  if ((m_horizontal_offset + view_width) > (m_pad_offset + pad_width))
+    {
+      m_pad_offset = std::min (m_horizontal_offset, content_width - pad_width);
+      m_pad_offset = std::max (m_pad_offset, 0);
+    }
+  else if (m_horizontal_offset < m_pad_offset)
+    m_pad_offset = std::max (m_horizontal_offset + view_width - pad_width, 0);
+
+  gdb_assert (m_pad_offset >= 0);
+  return (original_pad_offset != m_pad_offset);
+}
+
 /* Scroll the source forward or backward horizontally.  */
 
 void
@@ -377,10 +521,11 @@ tui_source_window_base::do_scroll_horizontal (int num_to_scroll)
 {
   if (!m_content.empty ())
     {
-      int offset = m_horizontal_offset + num_to_scroll;
-      if (offset < 0)
-	offset = 0;
-      m_horizontal_offset = offset;
+      m_horizontal_offset += num_to_scroll;
+
+      if (validate_scroll_offsets ())
+	show_source_content ();
+
       refresh_window ();
     }
 }
diff --git a/gdb/tui/tui-winsource.h b/gdb/tui/tui-winsource.h
index bf0ca96c09b..2762afff010 100644
--- a/gdb/tui/tui-winsource.h
+++ b/gdb/tui/tui-winsource.h
@@ -181,16 +181,68 @@ struct tui_source_window_base : public tui_win_info
   /* Used for horizontal scroll.  */
   int m_horizontal_offset = 0;
 
+  /* Check that the current values of M_HORIZONTAL_OFFSET and M_PAD_OFFSET
+     make sense given the current M_MAX_LENGTH (content width), WIDTH
+     (window size), and window margins.  After calling this function
+     M_HORIZONTAL_OFFSET and M_PAD_OFFSET might have been adjusted to
+     reduce unnecessary whitespace on the right side of the window.
+
+     If M_PAD_OFFSET is adjusted then this function returns true
+     indicating that the pad contents need to be reloaded by calling
+     show_source_content.  If M_PAD_OFFSET is not adjusted then this
+     function returns false, the window contents might still need
+     redrawing if M_HORIZONTAL_OFFSET was adjusted, but right now, this
+     function is only called in contexts where the window is going to be
+     redrawn anyway.  */
+  bool validate_scroll_offsets ();
+
+  /* Return the size of the left margin space, this is the space used to
+     display things like breakpoint markers.  */
+  int left_margin () const
+  { return 1 + TUI_EXECINFO_SIZE + extra_margin (); }
+
+  /* Return the width of the area that is available for window content.
+     This is the window width minus the borders and the left margin, which
+     is used for displaying things like breakpoint markers.  */
+  int view_width () const
+  { return width - left_margin () - 1; }
+
   void show_source_content ();
 
+  /* Write STRING to the window M_PAD, but skip the first SKIP printable
+     characters.  Any escape sequences within the first SKIP characters are
+     still processed though.  This means if we have this string:
+
+     "\033[31mABCDEFGHIJKLM\033[0m"
+
+     and call this function with a skip value of 3, then we effectively
+     write this string to M_PAD:
+
+     "\033[31mDEFGHIJKLM\033[0m"
+
+     the initial escape that sets the color will still be applied.  */
+  void puts_to_pad_with_skip (const char *string, int skip);
+
   /* Called when the user "set style enabled" setting is changed.  */
   void style_changed ();
 
   /* A token used to register and unregister an observer.  */
   gdb::observers::token m_observable;
 
-  /* Pad used to display fixme mumble  */
+  /* Pad to hold some, or all, of the window contents.  Content is then
+     copied from this pad to the screen as the user scrolls horizontally,
+     this avoids the need to recalculate the screen contents each time the
+     user does a horizontal scroll.  */
   std::unique_ptr<WINDOW, curses_deleter> m_pad;
+
+  /* When M_PAD was allocated, this holds the width that was initially
+     asked for.  If we ask for a very large pad then the allocation may
+     fail, and we might instead allocate a narrower pad.  */
+  int m_pad_requested_width = 0;
+
+  /* If M_PAD is not as wide as the content (so less than M_MAX_LENGTH)
+     then this value indicates the offset at which the pad contents begin.  */
+  int m_pad_offset = 0;
 };
 
 
-- 
2.25.4

[PATCH 12/15] gdb/tui: avoid extra refresh_window on horizontal scroll

[Andrew Burgess]

While working on the previous patches I noticed that in some cases I
was seeing two calls to tui_source_window_base::refresh_window when
scrolling the window horizontally.

The two calls would trigger in for the tui-disasm-long-lines.exp test
when the pad needed to be refilled.  The two called both come from
tui_source_window_base::show_source_content.  The first call is nested
within check_and_display_highlight_if_needed, while the second call is
done directly at the end of show_source_content.

The check_and_display_highlight_if_needed is being used to draw the
window box to the window, this is needed here because
show_source_content is what gets called when the window needs
updating, e.g. after a resize.  We could potentially do the boxing in
refresh_window, but then we'd be doing it each time we scroll, even
though the box doesn't need changing in this case.

However, we can move the check_and_display_highlight_if_needed to be
the last thing done in show_source_content, this means that we can
rely on the refresh_window call within it to be our single refresh
call.

There should be no user visible changes after this commit.
---
 gdb/tui/tui-winsource.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 6e22638ec74..50efa80576f 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -348,8 +348,6 @@ tui_source_window_base::show_source_content ()
 {
   gdb_assert (!m_content.empty ());
 
-  check_and_display_highlight_if_needed ();
-
   /* The pad should be at least as wide as the window, but ideally, as wide
      as the content, however, for some very wide content this might not be
      possible.  */
@@ -399,7 +397,11 @@ tui_source_window_base::show_source_content ()
   for (int lineno = 0; lineno < m_content.size (); lineno++)
     show_source_line (lineno);
 
-  refresh_window ();
+  /* Calling check_and_display_highlight_if_needed will call refresh_window
+     (so long as the current window can be boxed), which will ensure that
+     the newly loaded window content is copied to the screen.  */
+  gdb_assert (can_box ());
+  check_and_display_highlight_if_needed ();
 }
 
 tui_source_window_base::tui_source_window_base ()
-- 
2.25.4

[PATCH 13/15] gdb/tui: avoid extra refresh_window on vertical scroll

[Andrew Burgess]

While working on the previous couple of patches I noticed that when I
scroll the src and asm windows vertically, I get two refresh_window
calls.

The two calls can be traced back to
tui_source_window_base::update_source_window_as_is, in here we call
show_source_content, which calls refresh_window, and then
update_exec_info, which also calls refresh_window.

In this commit I propose making the refresh_window call in
update_exec_info optional.  In update_source_window_as_is I'll then
call update_exec_info before calling show_source_content, and pass a
flag to update_exec_info to defer the refresh.

There are places where update_exec_info is used without any subsequent
refresh_window call (e.g. when a breakpoint is updated), so
update_exec_info does not to call refresh_window in some cases, which
is why I'm using a flag to control the refresh.

With this changes I'm now only seeing a single refresh_window call for
each vertical scroll.

There should be no user visible changes after this commit.
---
 gdb/tui/tui-winsource.c | 12 ++++++------
 gdb/tui/tui-winsource.h |  9 ++++++++-
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 50efa80576f..b5b6079a909 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -172,8 +172,8 @@ tui_source_window_base::update_source_window_as_is
     {
       validate_scroll_offsets ();
       update_breakpoint_info (nullptr, false);
+      update_exec_info (false);
       show_source_content ();
-      update_exec_info ();
     }
 }
 
@@ -636,11 +636,10 @@ tui_source_window_base::update_breakpoint_info
   return need_refresh;
 }
 
-/* Function to initialize the content of the execution info window,
-   based upon the input window which is either the source or
-   disassembly window.  */
+/* See tui-winsource.h.  */
+
 void
-tui_source_window_base::update_exec_info ()
+tui_source_window_base::update_exec_info (bool refresh_p)
 {
   update_breakpoint_info (nullptr, true);
   for (int i = 0; i < m_content.size (); i++)
@@ -668,5 +667,6 @@ tui_source_window_base::update_exec_info ()
 
       show_line_number (i);
     }
-  refresh_window ();
+  if (refresh_p)
+    refresh_window ();
 }
diff --git a/gdb/tui/tui-winsource.h b/gdb/tui/tui-winsource.h
index 2762afff010..7370ae95d8b 100644
--- a/gdb/tui/tui-winsource.h
+++ b/gdb/tui/tui-winsource.h
@@ -148,7 +148,14 @@ struct tui_source_window_base : public tui_win_info
 
   virtual bool location_matches_p (struct bp_location *loc, int line_no) = 0;
 
-  void update_exec_info ();
+  /* Fill in the left margin of the current window with execution indicator
+     information, e.g. breakpoint indicators, and line numbers.  When
+     REFRESH_P is true this function will call refresh_window to ensure
+     updates are written to the screen, otherwise the refresh is skipped,
+     which will leave the on screen contents out of date.  When passing
+     false for REFRESH_P you should be planning to call refresh_window
+     yourself.  */
+  void update_exec_info (bool refresh_p = true);
 
   /* Update the window to display the given location.  Does nothing if
      the location is already displayed.  */
-- 
2.25.4

[PATCH 14/15] gdb/tui: more debug output

[Andrew Burgess]

Add some additional debug output that I've found really useful while
working on the previous set of patches.

Unless tui debug is turned on, then there should be no user visible
changes with this commit.
---
 gdb/tui/tui-winsource.c | 21 +++++++++++++++++++++
 gdb/tui/tui.h           |  3 +++
 2 files changed, 24 insertions(+)

diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index b5b6079a909..52a0f7af00f 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -316,6 +316,8 @@ tui_source_window_base::show_source_line (int lineno)
 void
 tui_source_window_base::refresh_window ()
 {
+  TUI_SCOPED_DEBUG_START_END ("window `%s`", name ());
+
   /* tui_win_info::refresh_window would draw the empty background window to
      the screen, potentially creating a flicker.  */
   wnoutrefresh (handle.get ());
@@ -326,6 +328,12 @@ tui_source_window_base::refresh_window ()
   int content_width = m_max_length;
   int pad_x = m_horizontal_offset - m_pad_offset;
 
+  tui_debug_printf ("pad_width = %d, left_margin = %d, view_width = %d",
+		    pad_width, left_margin, view_width);
+  tui_debug_printf ("content_width = %d, pad_x = %d, m_horizontal_offset = %d",
+		    content_width, pad_x, m_horizontal_offset);
+  tui_debug_printf ("m_pad_offset = %d", m_pad_offset);
+
   gdb_assert (m_pad_offset >= 0);
   gdb_assert (m_horizontal_offset + view_width
 	      <= std::max (content_width, view_width));
@@ -346,6 +354,8 @@ tui_source_window_base::refresh_window ()
 void
 tui_source_window_base::show_source_content ()
 {
+  TUI_SCOPED_DEBUG_START_END ("window `%s`", name ());
+
   gdb_assert (!m_content.empty ());
 
   /* The pad should be at least as wide as the window, but ideally, as wide
@@ -390,6 +400,8 @@ tui_source_window_base::show_source_content ()
 	}
 
       m_pad_requested_width = required_pad_width;
+      tui_debug_printf ("requested width %d, allocated width %d",
+			required_pad_width, getmaxx (m_pad.get ()));
     }
 
   gdb_assert (m_pad != nullptr);
@@ -431,6 +443,8 @@ tui_source_window_base::update_tab_width ()
 void
 tui_source_window_base::rerender ()
 {
+  TUI_SCOPED_DEBUG_START_END ("window `%s`", name ());
+
   if (!m_content.empty ())
     {
       struct symtab_and_line cursal
@@ -492,6 +506,8 @@ tui_source_window_base::refill ()
 bool
 tui_source_window_base::validate_scroll_offsets ()
 {
+  TUI_SCOPED_DEBUG_START_END ("window `%s`", name ());
+
   int original_pad_offset = m_pad_offset;
 
   if (m_horizontal_offset < 0)
@@ -501,6 +517,11 @@ tui_source_window_base::validate_scroll_offsets ()
   int pad_width = getmaxx (m_pad.get ());
   int view_width = this->view_width ();
 
+  tui_debug_printf ("pad_width = %d, view_width = %d, content_width = %d",
+		    pad_width, view_width, content_width);
+  tui_debug_printf ("original_pad_offset = %d, m_horizontal_offset = %d",
+		    original_pad_offset, m_horizontal_offset);
+
   if (m_horizontal_offset + view_width > content_width)
     m_horizontal_offset = std::max (content_width - view_width, 0);
 
diff --git a/gdb/tui/tui.h b/gdb/tui/tui.h
index a9ecd589a70..ca30e7cc65e 100644
--- a/gdb/tui/tui.h
+++ b/gdb/tui/tui.h
@@ -36,6 +36,9 @@ extern bool debug_tui;
 #define TUI_SCOPED_DEBUG_ENTER_EXIT \
   scoped_debug_enter_exit (debug_tui, "tui")
 
+#define TUI_SCOPED_DEBUG_START_END(fmt, ...) \
+  scoped_debug_start_end (debug_tui, "tui", fmt, ##__VA_ARGS__)
+
 struct ui_file;
 
 /* Types of error returns.  */
-- 
2.25.4

[PATCH 15/15] gdb/tui: make use of a scoped_restore

[Andrew Burgess]

Make use of a scoped_restore object in tui_mld_read_key instead of
doing a manual save/restore.

I don't think the existing code can throw an exception, so this is
just a cleanup rather than a bug fix.

There should be no user visible changes after this commit.
---
 gdb/tui/tui-io.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 2f39e34df2f..a17cf38a1f2 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -773,14 +773,10 @@ tui_mld_getc (FILE *fp)
 static int
 tui_mld_read_key (const struct match_list_displayer *displayer)
 {
-  rl_getc_func_t *prev = rl_getc_function;
-  int c;
-
   /* We can't use tui_getc as we need NEWLINE to not get emitted.  */
-  rl_getc_function = tui_mld_getc;
-  c = rl_read_key ();
-  rl_getc_function = prev;
-  return c;
+  scoped_restore restore_getc_function
+    = make_scoped_restore (&rl_getc_function, tui_mld_getc);
+  return rl_read_key ();
 }
 
 /* TUI version of rl_completion_display_matches_hook.
-- 
2.25.4

Re: [PATCH 15/15] gdb/tui: make use of a scoped_restore

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> Make use of a scoped_restore object in tui_mld_read_key instead of
> doing a manual save/restore.
>
> I don't think the existing code can throw an exception, so this is
> just a cleanup rather than a bug fix.
>
> There should be no user visible changes after this commit.

I've gone ahead and pushed this patch.

Thanks,
Andrew

> ---
>  gdb/tui/tui-io.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
> index 2f39e34df2f..a17cf38a1f2 100644
> --- a/gdb/tui/tui-io.c
> +++ b/gdb/tui/tui-io.c
> @@ -773,14 +773,10 @@ tui_mld_getc (FILE *fp)
>  static int
>  tui_mld_read_key (const struct match_list_displayer *displayer)
>  {
> -  rl_getc_func_t *prev = rl_getc_function;
> -  int c;
> -
>    /* We can't use tui_getc as we need NEWLINE to not get emitted.  */
> -  rl_getc_function = tui_mld_getc;
> -  c = rl_read_key ();
> -  rl_getc_function = prev;
> -  return c;
> +  scoped_restore restore_getc_function
> +    = make_scoped_restore (&rl_getc_function, tui_mld_getc);
> +  return rl_read_key ();
>  }
>  
>  /* TUI version of rl_completion_display_matches_hook.
> -- 
> 2.25.4

[PATCHv2 0/8] Mixed bag of TUI tests and fixes

[Andrew Burgess]

Changes since v1:

  - I've pushed 7 of the original patches.  These were either smaller
    testsuite only patches, or small, obvious GDB fixes,

  - I've rebased the remaining patches onto HEAD of upstream.

Thanks,
Andrew

---

Andrew Burgess (8):
  gdb/testsuite: fix line feed scrolling in tuiterm.exp
  gdb/tui: improve errors from tui focus command
  gdb/tui: disable tui mode when an assert triggers
  gdb/tui: make m_horizontal_offset private
  gdb/tui: rewrite of tui_source_window_base to handle very long lines
  gdb/tui: avoid extra refresh_window on horizontal scroll
  gdb/tui: avoid extra refresh_window on vertical scroll
  gdb/tui: more debug output

 gdb/event-top.c                               |   8 +
 .../gdb.tui/tui-disasm-long-lines.exp         |   6 +-
 gdb/testsuite/gdb.tui/tui-focus.exp           |  85 ++++++-
 gdb/testsuite/lib/tuiterm.exp                 |  18 +-
 gdb/tui/tui-layout.c                          |  15 +-
 gdb/tui/tui-layout.h                          |  49 ++++
 gdb/tui/tui-win.c                             |  40 +++-
 gdb/tui/tui-winsource.c                       | 216 ++++++++++++++++--
 gdb/tui/tui-winsource.h                       |  69 +++++-
 gdb/tui/tui.h                                 |   3 +
 gdb/utils.c                                   |   7 +-
 11 files changed, 470 insertions(+), 46 deletions(-)


base-commit: 2e10cefd83b6a5b0b3745da1134d35a4924db6c5
-- 
2.25.4

[PATCHv2 1/8] gdb/testsuite: fix line feed scrolling in tuiterm.exp

[Andrew Burgess]

In a following commit I managed to trigger the line feed scrolling
case in tuiterm.exp.  This case is currently unhandled, and this
commit fills in this missing functionality.

The implementation is pretty simple, just scroll all the content up
one line at a time until the cursor is back on the screen (a single
line of scroll is all that should be needed).

This change is untested in this commit, but is required by the next
commit.
---
 gdb/testsuite/lib/tuiterm.exp | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index c38ccbbdbd7..8a3f7a48acc 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -99,10 +99,24 @@ namespace eval Term {
 	_log_cur "Line feed" {
 	    variable _cur_row
 	    variable _rows
+	    variable _cols
+	    variable _chars
 
 	    incr _cur_row 1
-	    if {$_cur_row >= $_rows} {
-		error "FIXME scroll"
+	    while {$_cur_row >= $_rows} {
+		# Scroll the display contents.  We scroll one line at
+		# a time here; as _cur_row was only increased by one,
+		# a single line scroll should be enough to put the
+		# cursor back on the screen.  But we wrap the
+		# scrolling inside a while loop just to be on the safe
+		# side.
+		for {set y 0} {$y < [expr $_rows - 1]} {incr y} {
+		    set next_y [expr $y + 1]
+		    for {set x 0} {$x < $_cols} {incr x} {
+			set _chars($x,$y) $_chars($x,$next_y)
+		    }
+		    incr _cur_row -1
+		}
 	    }
 	}
     }
-- 
2.25.4

Re: [PATCHv2 1/8] gdb/testsuite: fix line feed scrolling in tuiterm.exp

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> In a following commit I managed to trigger the line feed scrolling
Andrew> case in tuiterm.exp.  This case is currently unhandled, and this
Andrew> commit fills in this missing functionality.

Andrew> The implementation is pretty simple, just scroll all the content up
Andrew> one line at a time until the cursor is back on the screen (a single
Andrew> line of scroll is all that should be needed).

Andrew> This change is untested in this commit, but is required by the next
Andrew> commit.

Looks good to me.  Thanks for doing this.  The terminal code just
implements the subset of escape sequences I happened to run into while
writing tests -- there's no reason other than laziness that some are
missing.

Tom

[PATCHv2 2/8] gdb/tui: improve errors from tui focus command

[Andrew Burgess]

This commit improves (I think) the errors from the tui focus command.
There are a number of errors that can be triggered by the focus
command, they include:

  (1) Window name "NAME" is ambiguous

  (2) Unrecognized window name "NAME"

  (3) Window "NAME" cannot be focused

Error (1) is triggered when the user gives a partial window name, and
the name matches multiple windows in the current layout.

It is worth noting that the ambiguity must be within the current
layout; if the partial name matches one window in the current layout,
and one or more windows not in the current layout, then this is not
ambiguous, and focus will shift to the matching window in the current
layout.

This error was not previous being tested, but in this commit I make
use of the Python API to trigger and test this error.

Error (3) is simple enough, and was already being tested.  This is
triggered by something like 'focus status'.  The named window needs to
be present in the current layout, and non-focusable in order to
trigger the error.

Error (2) is what I'd like to improve in this commit.  This error
triggers if the name the user gives doesn't match any window in the
current layout.  Even if GDB does know about the window, but the
window isn't in the current layout, then GDB will say it doesn't
recognize the window name.

In this commit I propose to to split this error into three different
errors.  These will be:

  (a) Unrecognized window name "NAME"

  (b) No windows matching "NAME" in the current layout

  (c) Window "NAME" is not in the current layout

Error (a) is the same as before, but will now only trigger if GDB
doesn't know about window NAME at all.  If the window is known, but
not in the current layout then one of the other errors will trigger.

Error (b) will trigger if NAME is ambiguous for multiple windows that
are not in the current layout.  If NAME identifies a single window in
the current layout then that window will continue to be selected, just
as it currently is.  Only in the case where NAME doesn't identify a
window in the current layout do we then check all the other known
windows, if NAME matches multiple of these, then (b) is triggered.

Finally, error (c) is used when NAME uniquely identifies a single
window that is not in the current layout.

The hope with these new errors is that the user will have a better
understanding of what went wrong.  Instead of GDB claiming to not know
about a window, the mention of the current layout will hint to the
user that they should first switch layouts.

There are tests included for all the new errors.
---
 gdb/testsuite/gdb.tui/tui-focus.exp | 85 +++++++++++++++++++++++++++--
 gdb/tui/tui-layout.c                | 15 ++++-
 gdb/tui/tui-layout.h                | 49 +++++++++++++++++
 gdb/tui/tui-win.c                   | 40 +++++++++++++-
 4 files changed, 179 insertions(+), 10 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/tui-focus.exp b/gdb/testsuite/gdb.tui/tui-focus.exp
index 9e2a1cd0935..72f80523af3 100644
--- a/gdb/testsuite/gdb.tui/tui-focus.exp
+++ b/gdb/testsuite/gdb.tui/tui-focus.exp
@@ -13,12 +13,12 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# Minimal testcase that just checks that the various "layout $foo"
-# commands do not cause gdb to crash.
+# Testcase that just checks the tui 'focus' command works as expected.
 
 require allow_tui_tests
 
 tuiterm_env
+load_lib gdb-python.exp
 
 standard_testfile
 
@@ -31,7 +31,8 @@ if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile}]} {
 # Each test specification is a tuple where the first item is the name of a
 # window, and the second item is a boolean indicating if we expect that
 # window to be present in the default (src) layout.
-foreach spec {{src true} {cmd true} {status true} {regs false} {asm false}} {
+foreach spec {{src true} {cmd true} {status true} {regs false} \
+		  {asm false} {unknown false}} {
     lassign $spec window valid_p
     with_test_prefix "window=$window" {
 
@@ -54,8 +55,13 @@ foreach spec {{src true} {cmd true} {status true} {regs false} {asm false}} {
 		    "^Focus set to $window window\\.\\s*"
 	    }
 	} else {
-	    Term::check_region_contents "check focus error" 0 16 80 1 \
-		"^Unrecognized window name \"$window\"\\s*"
+	    if {$window == "unknown"} {
+		Term::check_region_contents "check focus error" 0 16 80 1 \
+		    "^Unrecognized window name \"$window\"\\s*"
+	    } else {
+		Term::check_region_contents "check focus error" 0 16 80 1 \
+		    "^Window \"$window\" is not in the current layout\\s*"
+	    }
 	}
 
 	Term::check_box "check src box" 0 0 80 15
@@ -66,3 +72,72 @@ foreach spec {{src true} {cmd true} {status true} {regs false} {asm false}} {
 	Term::command "focus prev"
     }
 }
+
+# Use the Python TUI API to exercise some of the ambigous window name
+# handling parts of the 'focus' command.
+Term::clean_restart 24 80 $binfile
+if {[allow_python_tests]} {
+    # Create a very simple tui window.
+    gdb_py_test_silent_cmd \
+	[multi_line_input \
+	     "python" \
+	     "class TestWindow:" \
+	     "    def __init__(self, win):" \
+	     "        pass" \
+	     "" \
+	     "    def render(self):" \
+	     "        pass" \
+	     "end"] \
+	"setup dummy window class" \
+	true
+
+    # Register the window with a set of similar names.
+    gdb_test_no_output "python gdb.register_window_type(\"test1\", TestWindow)"
+    gdb_test_no_output "python gdb.register_window_type(\"test2\", TestWindow)"
+    gdb_test_no_output "python gdb.register_window_type(\"test3\", TestWindow)"
+
+    # Create a layout containing just one of the above windows.
+    gdb_test_no_output "tui new-layout example1 test2 1 status 1 cmd 1"
+
+    # Create a layout containing two of the above windows.
+    gdb_test_no_output "tui new-layout example2 test1 1 test2 1 status 1 cmd 1"
+
+    if {![Term::prepare_for_tui]} {
+	unsupported "TUI not supported"
+	return
+    }
+
+    # Try to focus using an ambiguous, partial window name.  This
+    # should fail as the default layout (src) doesn't include any
+    # windows matching this name.
+    Term::command_no_prompt_prefix "focus test"
+    Term::check_region_contents "check no matching window focus message" \
+	0 16 80 1 \
+	"^No windows matching \"test\" in the current layout\\s*"
+
+    # Now select a layout that includes a single window that matches
+    # the ambiguous, partial name 'test', and disable tui mode.
+    Term::command "layout example1"
+    send_gdb "tui disable\n"
+
+    # Reactivate tui mode and try to set focus using the ambiguous,
+    # partial window name.  This should succeed though, as, within the
+    # current layout, the partial name is not actually ambiguous.
+    send_gdb "focus test\n"
+    gdb_assert [Term::wait_for_region_contents 0 19 80 1 \
+		    "^Focus set to test2 window\\.\\s*"] \
+	"check test2 focus message"
+
+    # Now select a layout that includes two windows that matches the
+    # ambiguous, partial name 'test', and disable tui mode.
+    Term::command "layout example2"
+    send_gdb "tui disable\n"
+
+    # Reactivate tui mode and try to set focus using the ambiguous,
+    # partial window name.  This will fail as now the layout includes
+    # multiple windows that match 'test'.
+    send_gdb "focus test\n"
+    gdb_assert [Term::wait_for_region_contents 0 22 80 1 \
+		    "^Window name \"test\" is ambiguous\\s*"] \
+	"check ambiguous focus message"
+}
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 27abee02087..ecdcd4884a7 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -29,7 +29,6 @@
 #include "cli/cli-decode.h"
 #include "cli/cli-utils.h"
 #include <ctype.h>
-#include <unordered_map>
 #include <unordered_set>
 
 #include "tui/tui.h"
@@ -351,7 +350,17 @@ make_standard_window (const char *)
    shut down, causing crashes if any window destruction requires
    running Python code.  */
 
-static std::unordered_map<std::string, window_factory> *known_window_types;
+static window_types_map *known_window_types;
+
+/* See tui-layout.h.  */
+
+known_window_names_range
+all_known_window_names ()
+{
+  auto begin = known_window_names_iterator (known_window_types->begin ());
+  auto end = known_window_names_iterator (known_window_types->end ());
+  return known_window_names_range (begin, end);
+}
 
 /* Helper function that returns a TUI window, given its name.  */
 
@@ -377,7 +386,7 @@ tui_get_window_by_name (const std::string &name)
 static void
 initialize_known_windows ()
 {
-  known_window_types = new std::unordered_map<std::string, window_factory>;
+  known_window_types = new window_types_map;
 
   known_window_types->emplace (SRC_NAME,
 			       make_standard_window<SRC_WIN,
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index 206f1117445..ff354fb7c2f 100644
--- a/gdb/tui/tui-layout.h
+++ b/gdb/tui/tui-layout.h
@@ -26,6 +26,9 @@
 
 #include "tui/tui.h"
 #include "tui/tui-data.h"
+#include "gdbsupport/iterator-range.h"
+
+#include <unordered_map>
 
 /* Values that can be returned when handling a request to adjust a
    window's size.  */
@@ -358,10 +361,56 @@ extern void tui_adjust_window_width (struct tui_win_info *win,
 
 typedef std::function<tui_win_info * (const char *name)> window_factory;
 
+/* The type for a data structure that maps a window name to that window's
+   factory function.  */
+typedef std::unordered_map<std::string, window_factory> window_types_map;
+
 /* Register a new TUI window type.  NAME is the name of the window
    type.  FACTORY is a function that can be called to instantiate the
    window.  */
 
 extern void tui_register_window (const char *name, window_factory &&factory);
 
+/* An iterator class that exposes just the window names from the
+   known_window_types map in tui-layout.c.  This is just a wrapper around
+   an iterator of the underlying known_window_types map, but this just
+   exposes the window names.  */
+
+struct known_window_names_iterator
+{
+  using known_window_types_iterator = window_types_map::iterator;
+
+  known_window_names_iterator (known_window_types_iterator &&iter)
+    : m_iter (std::move (iter))
+  { /* Nothing.  */ }
+
+  known_window_names_iterator &operator++ ()
+  {
+    ++m_iter;
+    return *this;
+  }
+
+  const std::string &operator* () const
+  { return (*m_iter).first; }
+
+  bool operator!= (const known_window_names_iterator &other) const
+  { return m_iter != other.m_iter; }
+
+private:
+
+  /* The underlying iterator.  */
+  known_window_types_iterator m_iter;
+};
+
+/* A range adapter that makes it possible to iterate over the names of all
+   known tui windows.  */
+
+using known_window_names_range
+  = iterator_range<known_window_names_iterator>;
+
+/* Return a range that can be used to walk over the name of all known tui
+   windows in a range-for loop.  */
+
+extern known_window_names_range all_known_window_names ();
+
 #endif /* TUI_TUI_LAYOUT_H */
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 492a5191025..008189eb99b 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -728,8 +728,44 @@ tui_set_focus_command (const char *arg, int from_tty)
   else
     win_info = tui_partial_win_by_name (arg);
 
-  if (win_info == NULL)
-    error (_("Unrecognized window name \"%s\""), arg);
+  if (win_info == nullptr)
+    {
+      /* When WIN_INFO is nullptr this can either mean that the window name
+	 is unknown to GDB, or that the window is not in the current
+	 layout.  To try and help the user, give a different error
+	 depending on which of these is the case.  */
+      std::string matching_window_name;
+      bool is_ambiguous = false;
+
+      for (const std::string &name : all_known_window_names ())
+	{
+	  /* Look through all windows in the current layout, if the window
+	     is in the current layout then we're not interested is it.  */
+	  for (tui_win_info *item : all_tui_windows ())
+	    if (item->name () == name)
+	      continue;
+
+	  if (startswith (name, arg))
+	    {
+	      if (matching_window_name.empty ())
+		matching_window_name = name;
+	      else
+		is_ambiguous = true;
+	    }
+	};
+
+      if (!matching_window_name.empty ())
+	{
+	  if (is_ambiguous)
+	    error (_("No windows matching \"%s\" in the current layout"),
+		   arg);
+	  else
+	    error (_("Window \"%s\" is not in the current layout"),
+		   matching_window_name.c_str ());
+	}
+      else
+	error (_("Unrecognized window name \"%s\""), arg);
+    }
 
   /* If a window is part of the current layout then it will have a
      tui_win_info associated with it and be visible, otherwise, there will
-- 
2.25.4

[PATCHv2 3/8] gdb/tui: disable tui mode when an assert triggers

[Andrew Burgess]

When an assert triggers in tui mode the output is not great, the
internal backtrace that is generated is printed directly to the file
descriptor for gdb_stderr, and, as a result, does not currently format
itself correctly - the output uses only '\n' at the end of each line,
and so, when the terminal is in raw mode, the cursor does not return
to the start of each line after the '\n'.

This is mostly fixable, we could update bt-utils.c to use '\r\n'
instead of just '\n', and this would fix most of the problems.  The
one we can't easily fix is if/when GDB is built to use execinfo
instead of libbacktrace, in this case we use backtrace_symbols_fd to
print the symbols, and this function only uses '\n' as the line
terminator.  Fixing this would require switching to backtrace_symbols,
but that API uses malloc, which is something we're trying to
avoid (this code is called when GDB hits an error, so ideally we don't
want to rely on malloc).

However, the execinfo code is only used when libbacktrace is not
available (or the user specifically disables libbacktrace) so maybe we
can ignore that problem...

... but there is another problem.  When the backtrace is printed in
raw mode, it is possible that the backtrace fills the screen.  With
the terminal in raw mode we don't have the ability to scroll back,
which means we loose some of the backtrace, which isn't ideal.

In this commit I propose that we should disable tui mode whenever we
handle a fatal signal, or when we hit the internal error code
path (e.g. when an assert triggers).  With this done then we don't
need to update the bt-utils.c code, and the execinfo version of the
code (using backtrace_symbols_fd) works just fine.  We also get the
ability to scroll back to view the error message and all of the
backtrace, assuming the users terminal supports scrolling back.

The only downside I see with this change is if the tui_disable call
itself causes an error for some reason, or, if we handle a single at a
time when it is not safe to call tui_disable, in these cases the extra
tui_disable call might cause GDB to loose the original error.

However, I think (just from personal experience) that the above two
issues are pretty rare and the benefits from this change far out
weighs the possible drawbacks.
---
 gdb/event-top.c | 8 ++++++++
 gdb/utils.c     | 7 ++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 4a46e1b9346..14984707df1 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -48,6 +48,10 @@
 #include "readline/readline.h"
 #include "readline/history.h"
 
+#ifdef TUI
+#include "tui/tui.h"
+#endif
+
 /* readline defines this.  */
 #undef savestring
 
@@ -940,6 +944,10 @@ unblock_signal (int sig)
 static void ATTRIBUTE_NORETURN
 handle_fatal_signal (int sig)
 {
+#ifdef TUI
+  tui_disable ();
+#endif
+
 #ifdef GDB_PRINT_INTERNAL_BACKTRACE
   const auto sig_write = [] (const char *msg) -> void
   {
diff --git a/gdb/utils.c b/gdb/utils.c
index 734c5bf7f70..95adbe58e4a 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -29,7 +29,8 @@
 #endif /* HAVE_SYS_RESOURCE_H */
 
 #ifdef TUI
-#include "tui/tui.h"		/* For tui_get_command_dimension.   */
+/* For tui_get_command_dimension and tui_disable.   */
+#include "tui/tui.h"
 #endif
 
 #ifdef __GO32__
@@ -354,6 +355,10 @@ internal_vproblem (struct internal_problem *problem,
       }
   }
 
+#ifdef TUI
+  tui_disable ();
+#endif
+
   /* Create a string containing the full error/warning message.  Need
      to call query with this full string, as otherwize the reason
      (error/warning) and question become separated.  Format using a
-- 
2.25.4

[PATCHv2 4/8] gdb/tui: make m_horizontal_offset private

[Andrew Burgess]

I noticed that tui_source_window_base::m_horizontal_offset was
protected, but could be made private, so lets do that.

This makes more sense in the context of a later commit where I plan to
add another member variable that is similar to m_horizontal_offset.
The new member variable could also be private.

So I had to choose, place the new member variable next to
m_horizontal_offset making it protected, but grouping similar
variables together, or make m_horizontal_offset private, and then add
the new variable as private too.

I chose to make m_horizontal_offset private, which is this commit.

There should be no user visible changes after this commit.
---
 gdb/tui/tui-winsource.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/tui/tui-winsource.h b/gdb/tui/tui-winsource.h
index 7d6d64d35c4..bf0ca96c09b 100644
--- a/gdb/tui/tui-winsource.h
+++ b/gdb/tui/tui-winsource.h
@@ -124,8 +124,7 @@ struct tui_source_window_base : public tui_win_info
   /* Redraw the complete line of a source or disassembly window.  */
   void show_source_line (int lineno);
 
-  /* Used for horizontal scroll.  */
-  int m_horizontal_offset = 0;
+  /* Where to start generating content from.  */
   struct tui_line_or_address m_start_line_or_addr;
 
   /* Architecture associated with code at this location.  */
@@ -179,6 +178,9 @@ struct tui_source_window_base : public tui_win_info
 
 private:
 
+  /* Used for horizontal scroll.  */
+  int m_horizontal_offset = 0;
+
   void show_source_content ();
 
   /* Called when the user "set style enabled" setting is changed.  */
-- 
2.25.4

[PATCHv2 5/8] gdb/tui: rewrite of tui_source_window_base to handle very long lines

[Andrew Burgess]

This commit addresses an issue that is exposed by the test script
gdb.tui/tui-disasm-long-lines.exp, that is, tui_source_window_base
does not handle very long lines.

The problem can be traced back to the newpad call in
tui_source_window_base::show_source_content, this is where we allocate
a backing pad to hold the window content.

Unfortunately, there appears to be a limit to the size of pad that can
be allocated, and the gdb.tui/tui-disasm-long-lines.exp test goes
beyond this limit.  As a consequence the newpad call fails and returns
nullptr.

It just so happens that the reset of the tui_source_window_base code
can handle the pad being nullptr (this happens anyway when the window
is first created, so we already depend on nullptr handling), so all
that happens is the source window displays no content.

... well, sort of ... something weird does happen in the command
window, we seem to see a whole bunch of blank lines.  I've not
bothered to track down exactly what's happening there, but it's some
consequence of GDB attempting to write content to a WINDOW* that is
nullptr.

Before explaining my solution, I'll outline how things currently work:

Consider we have the following window content to display:

  aaaaaaaaaa
  bbbbbbbbbbbbbbbbbbbb
  ccccccccccccccc

the longest line here is 20 characters.  If our display window is 10
characters wide, then we will create a pad that is 20 characters wide,
and then copy the lines of content into the pad:

  .--------------------.
  |aaaaaaaaaa          |
  |bbbbbbbbbbbbbbbbbbbb|
  |ccccccccccccccc     |
  .--------------------.

Now we will copy a 10 character wide view into this pad to the
display, our display will then see:

  .----------.
  |aaaaaaaaaa|
  |bbbbbbbbbb|
  |cccccccccc|
  .----------.

As the user scrolls left and right we adjust m_horizontal_offset and
use this to select which part of the pad is copied onto the display.

The benefit of this is that we only need to copy the content to the
pad once, which includes processing the ansi escape sequences, and
then the user can scroll left and right as much as they want
relatively cheaply.

The problem then, is that if the longest content line is very long,
then we try to allocate a very large pad, which can fail.

What I propose is that we allow both the pad and the display view to
scroll.  Once we allow this, then it becomes possible to allocate a
pad that is smaller than the longest display line.  We then copy part
of the content into the pad.  As the user scrolls the view left and
right GDB will continue to copy content from the pad just as it does
right now.  But, when the user scrolls to the edge of the pad, GDB
will copy a new block of content into the pad, and then update the
view as normal.  This all works fine so long as the maximum pad size
is larger than the current window size - which seems a reasonable
restriction, if ncurses can't support a pad of a given size it seems
likely it will not support a display window of that size either.

If we return to our example above, but this time we assume that the
maximum pad size is 15 characters, then initially the pad would be
loaded like this:

  .---------------.
  |aaaaaaaaaa     |
  |bbbbbbbbbbbbbbb|
  |ccccccccccccccc|
  .---------------.

Notice that the last 5 characters from the 'b' line are no longer
included in the pad.  There is still enough content though to fill the
10 character wide display, just as we did before.

The pad contents remain unchanged until the user scrolls the display
right to this point:

  .----------.
  |aaaaa     |
  |bbbbbbbbbb|
  |cccccccccc|
  .----------.

Now, when the user scrolls right once more GDB spots that the user has
reached the end of the pad, and the pad contents are reloaded, like
this:

  .---------------.
  |aaaaa          |
  |bbbbbbbbbbbbbbb|
  |cccccccccc     |
  .---------------.

The display can now be updated from the pad again just like normal.

With this change in place the gdb.tui/tui-disasm-long-lines.exp test
now correctly loads the assembler code, and we can scroll around as
expected.

Most of the changes are pretty mundane, just updating to match the
above.  One interesting change though is the new member function
tui_source_window_base::puts_to_pad_with_skip.  This replaces direct
calls to tui_puts when copying content to the pad.

The content strings contain ansi escape sequences.  When these strings
are written to the pad these escape sequences are translated into
ncurses attribute setting calls.

Now however, we sometimes only write a partial string to the pad,
skipping some of the leading content.  Imagine then that we have a
content line like this:

  "\033[31mABCDEFGHIJKLM\033[0m"

Now the escape sequences in this content mean that the actual
content (the 'ABCDEFGHIJKLM') will have a red foreground color.

If we want to copy this to the pad, but skip the first 3 characters,
then what we expect is to have the pad contain 'DEFGHIJKLM', but this
text should still have a red foreground color.

It is this problem that puts_to_pad_with_skip solves.  This function
skips some number of printable characters, but processes all the
escape sequences.  This means that when we do start printing the
actual content the content will have the expected attributes.
/
---
 .../gdb.tui/tui-disasm-long-lines.exp         |   6 +-
 gdb/tui/tui-winsource.c                       | 177 ++++++++++++++++--
 gdb/tui/tui-winsource.h                       |  54 +++++-
 3 files changed, 215 insertions(+), 22 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp b/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp
index ae19f2e69cb..5d395cd604b 100644
--- a/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp
+++ b/gdb/testsuite/gdb.tui/tui-disasm-long-lines.exp
@@ -41,10 +41,6 @@ if {![Term::prepare_for_tui]} {
     return
 }
 
-# Just check the command does not cause gdb to crash.  It is worth
-# noting that the asm window does infact fail to correctly display the
-# disassembler output at this point, but initially we are just
-# checking that GDB doesn't crash, fixing the asm display will come
-# later.
 Term::command_no_prompt_prefix "layout asm"
 Term::check_box "asm box" 0 0 80 15
+Term::check_box_contents "check asm box contents" 0 0 80 15 "<main>"
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 87099ac26f5..6e22638ec74 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -170,6 +170,7 @@ tui_source_window_base::update_source_window_as_is
     erase_source_content ();
   else
     {
+      validate_scroll_offsets ();
       update_breakpoint_info (nullptr, false);
       show_source_content ();
       update_exec_info ();
@@ -231,6 +232,67 @@ tui_source_window_base::do_erase_source_content (const char *str)
     }
 }
 
+/* See tui-winsource.h.  */
+
+void
+tui_source_window_base::puts_to_pad_with_skip (const char *string, int skip)
+{
+  gdb_assert (m_pad.get () != nullptr);
+  WINDOW *w = m_pad.get ();
+
+  while (skip > 0)
+    {
+      const char *next = strpbrk (string, "\033");
+
+      /* Print the plain text prefix.  */
+      size_t n_chars = next == nullptr ? strlen (string) : next - string;
+      if (n_chars > 0)
+	{
+	  if (skip > 0)
+	    {
+	      if (skip < n_chars)
+		{
+		  string += skip;
+		  n_chars -= skip;
+		  skip = 0;
+		}
+	      else
+		{
+		  skip -= n_chars;
+		  string += n_chars;
+		  n_chars = 0;
+		}
+	    }
+
+	  if (n_chars > 0)
+	    {
+	      std::string copy (string, n_chars);
+	      tui_puts (copy.c_str (), w);
+	    }
+	}
+
+      /* We finished.  */
+      if (next == nullptr)
+	break;
+
+      gdb_assert (*next == '\033');
+
+      int n_read;
+      if (skip_ansi_escape (next, &n_read))
+	{
+	  std::string copy (next, n_read);
+	  tui_puts (copy.c_str (), w);
+	  next += n_read;
+	}
+      else
+	gdb_assert_not_reached ("unhandled escape");
+
+      string = next;
+    }
+
+  if (*string != '\0')
+    tui_puts (string, w);
+}
 
 /* Redraw the complete line of a source or disassembly window.  */
 void
@@ -243,7 +305,8 @@ tui_source_window_base::show_source_line (int lineno)
     tui_set_reverse_mode (m_pad.get (), true);
 
   wmove (m_pad.get (), lineno, 0);
-  tui_puts (line->line.c_str (), m_pad.get ());
+  puts_to_pad_with_skip (line->line.c_str (), m_pad_offset);
+
   if (line->is_exec_point)
     tui_set_reverse_mode (m_pad.get (), false);
 }
@@ -257,13 +320,25 @@ tui_source_window_base::refresh_window ()
      the screen, potentially creating a flicker.  */
   wnoutrefresh (handle.get ());
 
-  int pad_width = std::max (m_max_length, width);
-  int left_margin = 1 + TUI_EXECINFO_SIZE + extra_margin ();
-  int view_width = width - left_margin - 1;
-  int pad_x = std::min (pad_width - view_width, m_horizontal_offset);
-  /* Ensure that an equal number of scrolls will work if the user
-     scrolled beyond where we clip.  */
-  m_horizontal_offset = pad_x;
+  int pad_width = getmaxx (m_pad.get ());
+  int left_margin = this->left_margin ();
+  int view_width = this->view_width ();
+  int content_width = m_max_length;
+  int pad_x = m_horizontal_offset - m_pad_offset;
+
+  gdb_assert (m_pad_offset >= 0);
+  gdb_assert (m_horizontal_offset + view_width
+	      <= std::max (content_width, view_width));
+  gdb_assert (pad_x >= 0);
+  gdb_assert (m_horizontal_offset >= 0);
+
+  /* This function can be called before the pad has been allocated, this
+     should only occur during the initial startup.  In this case the first
+     condition in the following asserts will not be true, but the nullptr
+     check will.  */
+  gdb_assert (pad_width > 0 || m_pad.get () == nullptr);
+  gdb_assert (pad_x + view_width <= pad_width || m_pad.get () == nullptr);
+
   prefresh (m_pad.get (), 0, pad_x, y + 1, x + left_margin,
 	    y + m_content.size (), x + left_margin + view_width - 1);
 }
@@ -275,11 +350,51 @@ tui_source_window_base::show_source_content ()
 
   check_and_display_highlight_if_needed ();
 
-  int pad_width = std::max (m_max_length, width);
-  if (m_pad == nullptr || pad_width > getmaxx (m_pad.get ())
-      || m_content.size () > getmaxy (m_pad.get ()))
-    m_pad.reset (newpad (m_content.size (), pad_width));
+  /* The pad should be at least as wide as the window, but ideally, as wide
+     as the content, however, for some very wide content this might not be
+     possible.  */
+  int required_pad_width = std::max (m_max_length, width);
+  int required_pad_height = m_content.size ();
+
+  /* If the required pad width is wider than the previously requested pad
+     width, then we might want to grow the pad.  */
+  if (required_pad_width > m_pad_requested_width
+      || required_pad_height > getmaxy (m_pad.get ()))
+    {
+      /* The current pad width.  */
+      int pad_width = m_pad == nullptr ? 0 : getmaxx (m_pad.get ());
+
+      gdb_assert (pad_width <= m_pad_requested_width);
+
+      /* If the current pad width is smaller than the previously requested
+	 pad width, then this means we previously failed to allocate a
+	 bigger pad.  There's no point asking again, so we'll just make so
+	 with the pad we currently have.  */
+      if (pad_width == m_pad_requested_width
+	  || required_pad_height > getmaxy (m_pad.get ()))
+	{
+	  pad_width = required_pad_width;
+
+	  do
+	    {
+	      /* Try to allocate a new pad.  */
+	      m_pad.reset (newpad (required_pad_height, pad_width));
+
+	      if (m_pad == nullptr)
+		{
+		  int reduced_width = std::max (pad_width / 2, width);
+		  if (reduced_width == pad_width)
+		    error (_("failed to setup source window"));
+		  pad_width = reduced_width;
+		}
+	    }
+	  while (m_pad == nullptr);
+	}
+
+      m_pad_requested_width = required_pad_width;
+    }
 
+  gdb_assert (m_pad != nullptr);
   werase (m_pad.get ());
   for (int lineno = 0; lineno < m_content.size (); lineno++)
     show_source_line (lineno);
@@ -370,6 +485,35 @@ tui_source_window_base::refill ()
   update_source_window_as_is (m_gdbarch, sal);
 }
 
+/* See tui-winsource.h.  */
+
+bool
+tui_source_window_base::validate_scroll_offsets ()
+{
+  int original_pad_offset = m_pad_offset;
+
+  if (m_horizontal_offset < 0)
+    m_horizontal_offset = 0;
+
+  int content_width = m_max_length;
+  int pad_width = getmaxx (m_pad.get ());
+  int view_width = this->view_width ();
+
+  if (m_horizontal_offset + view_width > content_width)
+    m_horizontal_offset = std::max (content_width - view_width, 0);
+
+  if ((m_horizontal_offset + view_width) > (m_pad_offset + pad_width))
+    {
+      m_pad_offset = std::min (m_horizontal_offset, content_width - pad_width);
+      m_pad_offset = std::max (m_pad_offset, 0);
+    }
+  else if (m_horizontal_offset < m_pad_offset)
+    m_pad_offset = std::max (m_horizontal_offset + view_width - pad_width, 0);
+
+  gdb_assert (m_pad_offset >= 0);
+  return (original_pad_offset != m_pad_offset);
+}
+
 /* Scroll the source forward or backward horizontally.  */
 
 void
@@ -377,10 +521,11 @@ tui_source_window_base::do_scroll_horizontal (int num_to_scroll)
 {
   if (!m_content.empty ())
     {
-      int offset = m_horizontal_offset + num_to_scroll;
-      if (offset < 0)
-	offset = 0;
-      m_horizontal_offset = offset;
+      m_horizontal_offset += num_to_scroll;
+
+      if (validate_scroll_offsets ())
+	show_source_content ();
+
       refresh_window ();
     }
 }
diff --git a/gdb/tui/tui-winsource.h b/gdb/tui/tui-winsource.h
index bf0ca96c09b..2762afff010 100644
--- a/gdb/tui/tui-winsource.h
+++ b/gdb/tui/tui-winsource.h
@@ -181,16 +181,68 @@ struct tui_source_window_base : public tui_win_info
   /* Used for horizontal scroll.  */
   int m_horizontal_offset = 0;
 
+  /* Check that the current values of M_HORIZONTAL_OFFSET and M_PAD_OFFSET
+     make sense given the current M_MAX_LENGTH (content width), WIDTH
+     (window size), and window margins.  After calling this function
+     M_HORIZONTAL_OFFSET and M_PAD_OFFSET might have been adjusted to
+     reduce unnecessary whitespace on the right side of the window.
+
+     If M_PAD_OFFSET is adjusted then this function returns true
+     indicating that the pad contents need to be reloaded by calling
+     show_source_content.  If M_PAD_OFFSET is not adjusted then this
+     function returns false, the window contents might still need
+     redrawing if M_HORIZONTAL_OFFSET was adjusted, but right now, this
+     function is only called in contexts where the window is going to be
+     redrawn anyway.  */
+  bool validate_scroll_offsets ();
+
+  /* Return the size of the left margin space, this is the space used to
+     display things like breakpoint markers.  */
+  int left_margin () const
+  { return 1 + TUI_EXECINFO_SIZE + extra_margin (); }
+
+  /* Return the width of the area that is available for window content.
+     This is the window width minus the borders and the left margin, which
+     is used for displaying things like breakpoint markers.  */
+  int view_width () const
+  { return width - left_margin () - 1; }
+
   void show_source_content ();
 
+  /* Write STRING to the window M_PAD, but skip the first SKIP printable
+     characters.  Any escape sequences within the first SKIP characters are
+     still processed though.  This means if we have this string:
+
+     "\033[31mABCDEFGHIJKLM\033[0m"
+
+     and call this function with a skip value of 3, then we effectively
+     write this string to M_PAD:
+
+     "\033[31mDEFGHIJKLM\033[0m"
+
+     the initial escape that sets the color will still be applied.  */
+  void puts_to_pad_with_skip (const char *string, int skip);
+
   /* Called when the user "set style enabled" setting is changed.  */
   void style_changed ();
 
   /* A token used to register and unregister an observer.  */
   gdb::observers::token m_observable;
 
-  /* Pad used to display fixme mumble  */
+  /* Pad to hold some, or all, of the window contents.  Content is then
+     copied from this pad to the screen as the user scrolls horizontally,
+     this avoids the need to recalculate the screen contents each time the
+     user does a horizontal scroll.  */
   std::unique_ptr<WINDOW, curses_deleter> m_pad;
+
+  /* When M_PAD was allocated, this holds the width that was initially
+     asked for.  If we ask for a very large pad then the allocation may
+     fail, and we might instead allocate a narrower pad.  */
+  int m_pad_requested_width = 0;
+
+  /* If M_PAD is not as wide as the content (so less than M_MAX_LENGTH)
+     then this value indicates the offset at which the pad contents begin.  */
+  int m_pad_offset = 0;
 };
 
 
-- 
2.25.4

[PATCHv2 6/8] gdb/tui: avoid extra refresh_window on horizontal scroll

[Andrew Burgess]

While working on the previous patches I noticed that in some cases I
was seeing two calls to tui_source_window_base::refresh_window when
scrolling the window horizontally.

The two calls would trigger in for the tui-disasm-long-lines.exp test
when the pad needed to be refilled.  The two called both come from
tui_source_window_base::show_source_content.  The first call is nested
within check_and_display_highlight_if_needed, while the second call is
done directly at the end of show_source_content.

The check_and_display_highlight_if_needed is being used to draw the
window box to the window, this is needed here because
show_source_content is what gets called when the window needs
updating, e.g. after a resize.  We could potentially do the boxing in
refresh_window, but then we'd be doing it each time we scroll, even
though the box doesn't need changing in this case.

However, we can move the check_and_display_highlight_if_needed to be
the last thing done in show_source_content, this means that we can
rely on the refresh_window call within it to be our single refresh
call.

There should be no user visible changes after this commit.
---
 gdb/tui/tui-winsource.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 6e22638ec74..50efa80576f 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -348,8 +348,6 @@ tui_source_window_base::show_source_content ()
 {
   gdb_assert (!m_content.empty ());
 
-  check_and_display_highlight_if_needed ();
-
   /* The pad should be at least as wide as the window, but ideally, as wide
      as the content, however, for some very wide content this might not be
      possible.  */
@@ -399,7 +397,11 @@ tui_source_window_base::show_source_content ()
   for (int lineno = 0; lineno < m_content.size (); lineno++)
     show_source_line (lineno);
 
-  refresh_window ();
+  /* Calling check_and_display_highlight_if_needed will call refresh_window
+     (so long as the current window can be boxed), which will ensure that
+     the newly loaded window content is copied to the screen.  */
+  gdb_assert (can_box ());
+  check_and_display_highlight_if_needed ();
 }
 
 tui_source_window_base::tui_source_window_base ()
-- 
2.25.4

Re: [PATCHv2 6/8] gdb/tui: avoid extra refresh_window on horizontal scroll

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> However, we can move the check_and_display_highlight_if_needed to be
Andrew> the last thing done in show_source_content, this means that we can
Andrew> rely on the refresh_window call within it to be our single refresh
Andrew> call.

Thanks for doing this.
I think there was still a bit of leftover spaghetti in this area even
after all the refactorings; nice to see some of it going away.

Tom

[PATCHv2 7/8] gdb/tui: avoid extra refresh_window on vertical scroll

[Andrew Burgess]

While working on the previous couple of patches I noticed that when I
scroll the src and asm windows vertically, I get two refresh_window
calls.

The two calls can be traced back to
tui_source_window_base::update_source_window_as_is, in here we call
show_source_content, which calls refresh_window, and then
update_exec_info, which also calls refresh_window.

In this commit I propose making the refresh_window call in
update_exec_info optional.  In update_source_window_as_is I'll then
call update_exec_info before calling show_source_content, and pass a
flag to update_exec_info to defer the refresh.

There are places where update_exec_info is used without any subsequent
refresh_window call (e.g. when a breakpoint is updated), so
update_exec_info does not to call refresh_window in some cases, which
is why I'm using a flag to control the refresh.

With this changes I'm now only seeing a single refresh_window call for
each vertical scroll.

There should be no user visible changes after this commit.
---
 gdb/tui/tui-winsource.c | 12 ++++++------
 gdb/tui/tui-winsource.h |  9 ++++++++-
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 50efa80576f..b5b6079a909 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -172,8 +172,8 @@ tui_source_window_base::update_source_window_as_is
     {
       validate_scroll_offsets ();
       update_breakpoint_info (nullptr, false);
+      update_exec_info (false);
       show_source_content ();
-      update_exec_info ();
     }
 }
 
@@ -636,11 +636,10 @@ tui_source_window_base::update_breakpoint_info
   return need_refresh;
 }
 
-/* Function to initialize the content of the execution info window,
-   based upon the input window which is either the source or
-   disassembly window.  */
+/* See tui-winsource.h.  */
+
 void
-tui_source_window_base::update_exec_info ()
+tui_source_window_base::update_exec_info (bool refresh_p)
 {
   update_breakpoint_info (nullptr, true);
   for (int i = 0; i < m_content.size (); i++)
@@ -668,5 +667,6 @@ tui_source_window_base::update_exec_info ()
 
       show_line_number (i);
     }
-  refresh_window ();
+  if (refresh_p)
+    refresh_window ();
 }
diff --git a/gdb/tui/tui-winsource.h b/gdb/tui/tui-winsource.h
index 2762afff010..7370ae95d8b 100644
--- a/gdb/tui/tui-winsource.h
+++ b/gdb/tui/tui-winsource.h
@@ -148,7 +148,14 @@ struct tui_source_window_base : public tui_win_info
 
   virtual bool location_matches_p (struct bp_location *loc, int line_no) = 0;
 
-  void update_exec_info ();
+  /* Fill in the left margin of the current window with execution indicator
+     information, e.g. breakpoint indicators, and line numbers.  When
+     REFRESH_P is true this function will call refresh_window to ensure
+     updates are written to the screen, otherwise the refresh is skipped,
+     which will leave the on screen contents out of date.  When passing
+     false for REFRESH_P you should be planning to call refresh_window
+     yourself.  */
+  void update_exec_info (bool refresh_p = true);
 
   /* Update the window to display the given location.  Does nothing if
      the location is already displayed.  */
-- 
2.25.4

[PATCHv2 8/8] gdb/tui: more debug output

[Andrew Burgess]

Add some additional debug output that I've found really useful while
working on the previous set of patches.

Unless tui debug is turned on, then there should be no user visible
changes with this commit.
---
 gdb/tui/tui-winsource.c | 21 +++++++++++++++++++++
 gdb/tui/tui.h           |  3 +++
 2 files changed, 24 insertions(+)

diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index b5b6079a909..52a0f7af00f 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -316,6 +316,8 @@ tui_source_window_base::show_source_line (int lineno)
 void
 tui_source_window_base::refresh_window ()
 {
+  TUI_SCOPED_DEBUG_START_END ("window `%s`", name ());
+
   /* tui_win_info::refresh_window would draw the empty background window to
      the screen, potentially creating a flicker.  */
   wnoutrefresh (handle.get ());
@@ -326,6 +328,12 @@ tui_source_window_base::refresh_window ()
   int content_width = m_max_length;
   int pad_x = m_horizontal_offset - m_pad_offset;
 
+  tui_debug_printf ("pad_width = %d, left_margin = %d, view_width = %d",
+		    pad_width, left_margin, view_width);
+  tui_debug_printf ("content_width = %d, pad_x = %d, m_horizontal_offset = %d",
+		    content_width, pad_x, m_horizontal_offset);
+  tui_debug_printf ("m_pad_offset = %d", m_pad_offset);
+
   gdb_assert (m_pad_offset >= 0);
   gdb_assert (m_horizontal_offset + view_width
 	      <= std::max (content_width, view_width));
@@ -346,6 +354,8 @@ tui_source_window_base::refresh_window ()
 void
 tui_source_window_base::show_source_content ()
 {
+  TUI_SCOPED_DEBUG_START_END ("window `%s`", name ());
+
   gdb_assert (!m_content.empty ());
 
   /* The pad should be at least as wide as the window, but ideally, as wide
@@ -390,6 +400,8 @@ tui_source_window_base::show_source_content ()
 	}
 
       m_pad_requested_width = required_pad_width;
+      tui_debug_printf ("requested width %d, allocated width %d",
+			required_pad_width, getmaxx (m_pad.get ()));
     }
 
   gdb_assert (m_pad != nullptr);
@@ -431,6 +443,8 @@ tui_source_window_base::update_tab_width ()
 void
 tui_source_window_base::rerender ()
 {
+  TUI_SCOPED_DEBUG_START_END ("window `%s`", name ());
+
   if (!m_content.empty ())
     {
       struct symtab_and_line cursal
@@ -492,6 +506,8 @@ tui_source_window_base::refill ()
 bool
 tui_source_window_base::validate_scroll_offsets ()
 {
+  TUI_SCOPED_DEBUG_START_END ("window `%s`", name ());
+
   int original_pad_offset = m_pad_offset;
 
   if (m_horizontal_offset < 0)
@@ -501,6 +517,11 @@ tui_source_window_base::validate_scroll_offsets ()
   int pad_width = getmaxx (m_pad.get ());
   int view_width = this->view_width ();
 
+  tui_debug_printf ("pad_width = %d, view_width = %d, content_width = %d",
+		    pad_width, view_width, content_width);
+  tui_debug_printf ("original_pad_offset = %d, m_horizontal_offset = %d",
+		    original_pad_offset, m_horizontal_offset);
+
   if (m_horizontal_offset + view_width > content_width)
     m_horizontal_offset = std::max (content_width - view_width, 0);
 
diff --git a/gdb/tui/tui.h b/gdb/tui/tui.h
index a9ecd589a70..ca30e7cc65e 100644
--- a/gdb/tui/tui.h
+++ b/gdb/tui/tui.h
@@ -36,6 +36,9 @@ extern bool debug_tui;
 #define TUI_SCOPED_DEBUG_ENTER_EXIT \
   scoped_debug_enter_exit (debug_tui, "tui")
 
+#define TUI_SCOPED_DEBUG_START_END(fmt, ...) \
+  scoped_debug_start_end (debug_tui, "tui", fmt, ##__VA_ARGS__)
+
 struct ui_file;
 
 /* Types of error returns.  */
-- 
2.25.4

Re: [PATCHv2 0/8] Mixed bag of TUI tests and fixes

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> Changes since v1:
Andrew>   - I've pushed 7 of the original patches.  These were either smaller
Andrew>     testsuite only patches, or small, obvious GDB fixes,

Andrew>   - I've rebased the remaining patches onto HEAD of upstream.

I skimmed through these and the overall direction all seems fine, no
real comments from me.  Thank you for doing this.  I didn't read the
patches in *too* much detail but I think you should still go ahead and
check them in.

Tom

Re: [PATCHv2 0/8] Mixed bag of TUI tests and fixes

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> Changes since v1:
> Andrew>   - I've pushed 7 of the original patches.  These were either smaller
> Andrew>     testsuite only patches, or small, obvious GDB fixes,
>
> Andrew>   - I've rebased the remaining patches onto HEAD of upstream.
>
> I skimmed through these and the overall direction all seems fine, no
> real comments from me.  Thank you for doing this.  I didn't read the
> patches in *too* much detail but I think you should still go ahead and
> check them in.

I went ahead and pushed these.

Thanks,
Andrew