gdb: use tui_set_layout not show_layout to fix window focus

Message ID 20191223014804.23240-1-andrew.burgess@embecosm.com
State New
Headers show
Series
  • gdb: use tui_set_layout not show_layout to fix window focus
Related show

Commit Message

Andrew Burgess Dec. 23, 2019, 1:48 a.m.
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(-)

-- 
2.14.5

Comments

Andrew Burgess Jan. 5, 2020, 10:05 p.m. | #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: