gdb: use tui_set_layout not show_layout to fix window focus

Message ID 20191223014804.23240-1-andrew.burgess@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess Dec. 23, 2019, 1:48 a.m. UTC
  When calling tui_add_win_to_layout, use tui_set_layout not show_layout
so that window focus is correctly updated.  If the focus is not
correctly maintained then GDB can be crashed like this:

  start
  tui enable
  layout asm
  list SOME_FUNCTION

At this point GDB will have "popped up" the source window to
display SOME_FUNCTION.  Previously no window would have focus at this
point, and so if the user now does 'focus next' or 'focus prev', then
GDB would crash.

Calling tui_set_layout ensures that focus is correctly calculated as
the source window is "popped up", and this fixes the issue.

gdb/ChangeLog:

	* tui/tui-layout.c (tui_add_win_to_layout): Use tui_set_layout not
	show_layout.

gdb/testsuite/ChangeLog:

	* gdb.tui/list.exp: Test 'focus next' after 'list main'.

Change-Id: Id0b13f99b0e889261efedfd0adabe82020202f44
---
 gdb/ChangeLog                  |  5 +++++
 gdb/testsuite/ChangeLog        |  4 ++++
 gdb/testsuite/gdb.tui/list.exp |  3 +++
 gdb/tui/tui-layout.c           | 12 ++++++------
 4 files changed, 18 insertions(+), 6 deletions(-)
  

Comments

Andrew Burgess Jan. 5, 2020, 10:05 p.m. UTC | #1
* Andrew Burgess <andrew.burgess@embecosm.com> [2019-12-23 01:48:04 +0000]:

> When calling tui_add_win_to_layout, use tui_set_layout not show_layout
> so that window focus is correctly updated.  If the focus is not
> correctly maintained then GDB can be crashed like this:
> 
>   start
>   tui enable
>   layout asm
>   list SOME_FUNCTION
> 
> At this point GDB will have "popped up" the source window to
> display SOME_FUNCTION.  Previously no window would have focus at this
> point, and so if the user now does 'focus next' or 'focus prev', then
> GDB would crash.
> 
> Calling tui_set_layout ensures that focus is correctly calculated as
> the source window is "popped up", and this fixes the issue.
> 
> gdb/ChangeLog:
> 
> 	* tui/tui-layout.c (tui_add_win_to_layout): Use tui_set_layout not
> 	show_layout.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.tui/list.exp: Test 'focus next' after 'list main'.

I've gone ahead and pushed this patch now.

Thanks,
Andrew

> 
> Change-Id: Id0b13f99b0e889261efedfd0adabe82020202f44
> ---
>  gdb/ChangeLog                  |  5 +++++
>  gdb/testsuite/ChangeLog        |  4 ++++
>  gdb/testsuite/gdb.tui/list.exp |  3 +++
>  gdb/tui/tui-layout.c           | 12 ++++++------
>  4 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.tui/list.exp b/gdb/testsuite/gdb.tui/list.exp
> index 08153c695d7..7f241706f63 100644
> --- a/gdb/testsuite/gdb.tui/list.exp
> +++ b/gdb/testsuite/gdb.tui/list.exp
> @@ -35,3 +35,6 @@ Term::check_contents "asm window shows main" "$hex <main>"
>  
>  Term::command "list main"
>  Term::check_contents "list main" "21 *return 0"
> +# The following 'focus next' must be immediately after 'list main' to
> +# ensure that GDB has a valid idea of what is currently focused.
> +Term::command "focus next"
> diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
> index 9ab89a87fa4..62d400a97c4 100644
> --- a/gdb/tui/tui-layout.c
> +++ b/gdb/tui/tui-layout.c
> @@ -201,9 +201,9 @@ tui_add_win_to_layout (enum tui_win_type type)
>  	  && cur_layout != SRC_DATA_COMMAND)
>  	{
>  	  if (cur_layout == DISASSEM_DATA_COMMAND)
> -	    show_layout (SRC_DATA_COMMAND);
> +	    tui_set_layout (SRC_DATA_COMMAND);
>  	  else
> -	    show_layout (SRC_COMMAND);
> +	    tui_set_layout (SRC_COMMAND);
>  	}
>        break;
>      case DISASSEM_WIN:
> @@ -212,9 +212,9 @@ tui_add_win_to_layout (enum tui_win_type type)
>  	  && cur_layout != DISASSEM_DATA_COMMAND)
>  	{
>  	  if (cur_layout == SRC_DATA_COMMAND)
> -	    show_layout (DISASSEM_DATA_COMMAND);
> +	    tui_set_layout (DISASSEM_DATA_COMMAND);
>  	  else
> -	    show_layout (DISASSEM_COMMAND);
> +	    tui_set_layout (DISASSEM_COMMAND);
>  	}
>        break;
>      case DATA_WIN:
> @@ -222,9 +222,9 @@ tui_add_win_to_layout (enum tui_win_type type)
>  	  && cur_layout != DISASSEM_DATA_COMMAND)
>  	{
>  	  if (cur_layout == DISASSEM_COMMAND)
> -	    show_layout (DISASSEM_DATA_COMMAND);
> +	    tui_set_layout (DISASSEM_DATA_COMMAND);
>  	  else
> -	    show_layout (SRC_DATA_COMMAND);
> +	    tui_set_layout (SRC_DATA_COMMAND);
>  	}
>        break;
>      default:
> -- 
> 2.14.5
>
  

Patch

diff --git a/gdb/testsuite/gdb.tui/list.exp b/gdb/testsuite/gdb.tui/list.exp
index 08153c695d7..7f241706f63 100644
--- a/gdb/testsuite/gdb.tui/list.exp
+++ b/gdb/testsuite/gdb.tui/list.exp
@@ -35,3 +35,6 @@  Term::check_contents "asm window shows main" "$hex <main>"
 
 Term::command "list main"
 Term::check_contents "list main" "21 *return 0"
+# The following 'focus next' must be immediately after 'list main' to
+# ensure that GDB has a valid idea of what is currently focused.
+Term::command "focus next"
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 9ab89a87fa4..62d400a97c4 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -201,9 +201,9 @@  tui_add_win_to_layout (enum tui_win_type type)
 	  && cur_layout != SRC_DATA_COMMAND)
 	{
 	  if (cur_layout == DISASSEM_DATA_COMMAND)
-	    show_layout (SRC_DATA_COMMAND);
+	    tui_set_layout (SRC_DATA_COMMAND);
 	  else
-	    show_layout (SRC_COMMAND);
+	    tui_set_layout (SRC_COMMAND);
 	}
       break;
     case DISASSEM_WIN:
@@ -212,9 +212,9 @@  tui_add_win_to_layout (enum tui_win_type type)
 	  && cur_layout != DISASSEM_DATA_COMMAND)
 	{
 	  if (cur_layout == SRC_DATA_COMMAND)
-	    show_layout (DISASSEM_DATA_COMMAND);
+	    tui_set_layout (DISASSEM_DATA_COMMAND);
 	  else
-	    show_layout (DISASSEM_COMMAND);
+	    tui_set_layout (DISASSEM_COMMAND);
 	}
       break;
     case DATA_WIN:
@@ -222,9 +222,9 @@  tui_add_win_to_layout (enum tui_win_type type)
 	  && cur_layout != DISASSEM_DATA_COMMAND)
 	{
 	  if (cur_layout == DISASSEM_COMMAND)
-	    show_layout (DISASSEM_DATA_COMMAND);
+	    tui_set_layout (DISASSEM_DATA_COMMAND);
 	  else
-	    show_layout (SRC_DATA_COMMAND);
+	    tui_set_layout (SRC_DATA_COMMAND);
 	}
       break;
     default: