[2/2] Fix crash when exiting TUI with gdb -tui

Message ID 20200606201301.19145-3-tom@tromey.com
State New
Headers show
Series
  • Fix two TUI crashes
Related show

Commit Message

Tom Tromey June 6, 2020, 8:13 p.m.
PR tui/25348 points out that, when "gdb -tui" is used, then exiting
the TUI will cause a crash.

This happens because tui_setup_io stashes some readline variables --
but because this happens before readline is initialized, some of these
are NULL.  Then, when exiting the TUI, the NULL values are "restored",
causing a crash in readline.

This patch fixes the problem by ensuring that readline is initialized
first.  Back in commit 11061048d ("Give a name to the TUI SingleKey
keymap"), a call to rl_initialize was removed from
tui_initialize_readline; this patch resurrects the call, but moves it
to the end of the function, so as not to remove the ability to modify
the SingleKey map from .inputrc.

gdb/ChangeLog
2020-06-06  Tom Tromey  <tom@tromey.com>

	PR tui/25348:
	* tui/tui.c (tui_initialize_readline): Only run once.  Call
	rl_initialize.
	* tui/tui-io.c (tui_setup_io): Call tui_initialize_readline.
---
 gdb/ChangeLog    | 7 +++++++
 gdb/tui/tui-io.c | 4 ++++
 gdb/tui/tui.c    | 9 +++++++++
 3 files changed, 20 insertions(+)

-- 
2.17.2

Comments

Simon Marchi via Gdb-patches June 8, 2020, 5:57 p.m. | #1
On Sat, Jun 6, 2020 at 3:13 PM Tom Tromey <tom@tromey.com> wrote:
> --- a/gdb/tui/tui.c

> +++ b/gdb/tui/tui.c

> @@ -270,6 +270,12 @@ tui_set_key_mode (enum tui_key_mode mode)

>  void

>  tui_initialize_readline (void)

>  {

> +  static bool initialized;

> +

> +  if (initialized)

> +    return;

> +  initialized = true;


Would it make sense to rename this function to
tui_ensure_readline_initialized() or something, to make it clear that
it can be called more than once?

Christian
Tom Tromey June 12, 2020, 3:24 a.m. | #2
>>>>> "Christian" == Christian Biesinger <cbiesinger@google.com> writes:


Christian> On Sat, Jun 6, 2020 at 3:13 PM Tom Tromey <tom@tromey.com> wrote:
>> --- a/gdb/tui/tui.c

>> +++ b/gdb/tui/tui.c

>> @@ -270,6 +270,12 @@ tui_set_key_mode (enum tui_key_mode mode)

>> void

>> tui_initialize_readline (void)

>> {

>> +  static bool initialized;

>> +

>> +  if (initialized)

>> +    return;

>> +  initialized = true;


Christian> Would it make sense to rename this function to
Christian> tui_ensure_readline_initialized() or something, to make it clear that
Christian> it can be called more than once?

Makes sense.  I made this change.

Tom

Patch

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index e7a8ac77bce..75fab1f8e91 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -746,6 +746,10 @@  tui_setup_io (int mode)
 
   if (mode)
     {
+      /* Ensure that readline has been initialized before saving any
+	 of its variables.  */
+      tui_initialize_readline ();
+
       /* Redirect readline to TUI.  */
       tui_old_rl_redisplay_function = rl_redisplay_function;
       tui_old_rl_deprep_terminal = rl_deprep_term_function;
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index ac435d16d6c..50f846d4090 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -270,6 +270,12 @@  tui_set_key_mode (enum tui_key_mode mode)
 void
 tui_initialize_readline (void)
 {
+  static bool initialized;
+
+  if (initialized)
+    return;
+  initialized = true;
+
   int i;
   Keymap tui_ctlx_keymap;
 
@@ -325,6 +331,9 @@  tui_initialize_readline (void)
   rl_bind_key_in_map ('q', tui_rl_next_keymap, tui_keymap);
   rl_bind_key_in_map ('s', tui_rl_next_keymap, emacs_ctlx_keymap);
   rl_bind_key_in_map ('s', tui_rl_next_keymap, tui_ctlx_keymap);
+
+  /* Initialize readline after the above.  */
+  rl_initialize ();
 }
 
 /* Return the TERM variable from the environment, or "<unset>"