* [PATCH 0/2] Fix two TUI crashes @ 2020-06-06 20:12 Tom Tromey 2020-06-06 20:13 ` [PATCH 1/2] Fix C-x 1 from gdb prompt Tom Tromey ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Tom Tromey @ 2020-06-06 20:12 UTC (permalink / raw) To: gdb-patches This short series fixes a couple of crashes involving the TUI. Tom ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] Fix C-x 1 from gdb prompt 2020-06-06 20:12 [PATCH 0/2] Fix two TUI crashes Tom Tromey @ 2020-06-06 20:13 ` Tom Tromey 2020-06-08 17:57 ` Christian Biesinger 2020-06-06 20:13 ` [PATCH 2/2] Fix crash when exiting TUI with gdb -tui Tom Tromey 2020-06-16 15:54 ` [PATCH 0/2] Fix two TUI crashes Pedro Alves 2 siblings, 1 reply; 8+ messages in thread From: Tom Tromey @ 2020-06-06 20:13 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey Pedro pointed out on irc that C-x 1 from the gdb prompt will cause a crash. This happened because of a bug in remove_windows -- it would always remove all the windows from the layout. This patch fixes this bug, and also arranges to have C-x 1 preserve the status window. gdb/ChangeLog 2020-06-06 Tom Tromey <tom@tromey.com> * tui/tui-layout.c (tui_layout_split::remove_windows): Fix logic. Also preserve the status window. --- gdb/ChangeLog | 5 +++++ gdb/tui/tui-layout.c | 11 ++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c index 491ce275acb..b87d21eb6de 100644 --- a/gdb/tui/tui-layout.c +++ b/gdb/tui/tui-layout.c @@ -790,13 +790,14 @@ tui_layout_split::remove_windows (const char *name) const char *this_name = m_splits[i].layout->get_name (); if (this_name == nullptr) m_splits[i].layout->remove_windows (name); + else if (strcmp (this_name, name) == 0 + || strcmp (this_name, "cmd") == 0 + || strcmp (this_name, "status") == 0) + { + /* Keep. */ + } else { - if (strcmp (this_name, name) == 0 - || strcmp (this_name, "cmd") == 0) - { - /* Keep. */ - } m_splits.erase (m_splits.begin () + i); --i; } -- 2.17.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Fix C-x 1 from gdb prompt 2020-06-06 20:13 ` [PATCH 1/2] Fix C-x 1 from gdb prompt Tom Tromey @ 2020-06-08 17:57 ` Christian Biesinger 2020-06-12 0:26 ` Tom Tromey 0 siblings, 1 reply; 8+ messages in thread From: Christian Biesinger @ 2020-06-08 17:57 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On Sat, Jun 6, 2020 at 3:13 PM Tom Tromey <tom@tromey.com> wrote: > + else if (strcmp (this_name, name) == 0 > + || strcmp (this_name, "cmd") == 0 > + || strcmp (this_name, "status") == 0) Maybe those strings should be constants somewhere? Christian ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Fix C-x 1 from gdb prompt 2020-06-08 17:57 ` Christian Biesinger @ 2020-06-12 0:26 ` Tom Tromey 0 siblings, 0 replies; 8+ messages in thread From: Tom Tromey @ 2020-06-12 0:26 UTC (permalink / raw) To: Christian Biesinger; +Cc: Tom Tromey, gdb-patches >>>>> "Christian" == Christian Biesinger <cbiesinger@google.com> writes: Christian> On Sat, Jun 6, 2020 at 3:13 PM Tom Tromey <tom@tromey.com> wrote: >> + else if (strcmp (this_name, name) == 0 >> + || strcmp (this_name, "cmd") == 0 >> + || strcmp (this_name, "status") == 0) Christian> Maybe those strings should be constants somewhere? Most of them were constants in tui-data.h already. I thought I'd tack this patch on to the end of the series. Tom commit 9aaf1b1b02745ee803dcfa2b7bf91add2cf0a6ea Author: Tom Tromey <tom@tromey.com> Date: Thu Jun 11 18:19:12 2020 -0600 Use macros for TUI window names Christian pointed out that tui-layout.c hard-codes various window names. This patch changes the code to use the macros from tui-data.h instead. For each window, I searched for uses of the name; but I only found any in tui-layout.c. This also adds a new macro to account for the "status" window. gdb/ChangeLog 2020-06-11 Tom Tromey <tom@tromey.com> * tui/tui-data.h (STATUS_NAME): New macro. * tui/tui-layout.c (tui_remove_some_windows) (initialize_known_windows, tui_register_window) (tui_layout_split::remove_windows, initialize_layouts) (tui_new_layout_command): Don't use hard-coded window names. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index cd6ff0b8fe8..f674fef7e16 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,11 @@ +2020-06-11 Tom Tromey <tom@tromey.com> + + * tui/tui-data.h (STATUS_NAME): New macro. + * tui/tui-layout.c (tui_remove_some_windows) + (initialize_known_windows, tui_register_window) + (tui_layout_split::remove_windows, initialize_layouts) + (tui_new_layout_command): Don't use hard-coded window names. + 2020-06-06 Tom Tromey <tom@tromey.com> PR tui/25348: diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h index 2dc73b963da..abe77272291 100644 --- a/gdb/tui/tui-data.h +++ b/gdb/tui/tui-data.h @@ -130,6 +130,7 @@ struct tui_gen_win_info #define CMD_NAME "cmd" #define DATA_NAME "regs" #define DISASSEM_NAME "asm" +#define STATUS_NAME "status" #define MIN_WIN_HEIGHT 3 #define MIN_CMD_WIN_HEIGHT 3 diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c index b87d21eb6de..8164b346370 100644 --- a/gdb/tui/tui-layout.c +++ b/gdb/tui/tui-layout.c @@ -255,7 +255,7 @@ tui_remove_some_windows () { tui_win_info *focus = tui_win_with_focus (); - if (strcmp (focus->name (), "cmd") == 0) + if (strcmp (focus->name (), CMD_NAME) == 0) { /* Try leaving the source or disassembly window. If neither exists, just do nothing. */ @@ -373,18 +373,18 @@ initialize_known_windows () { known_window_types = new std::unordered_map<std::string, window_factory>; - known_window_types->emplace ("src", + known_window_types->emplace (SRC_NAME, make_standard_window<SRC_WIN, tui_source_window>); - known_window_types->emplace ("cmd", + known_window_types->emplace (CMD_NAME, make_standard_window<CMD_WIN, tui_cmd_window>); - known_window_types->emplace ("regs", + known_window_types->emplace (DATA_NAME, make_standard_window<DATA_WIN, tui_data_window>); - known_window_types->emplace ("asm", + known_window_types->emplace (DISASSEM_NAME, make_standard_window<DISASSEM_WIN, tui_disasm_window>); - known_window_types->emplace ("status", get_locator_window); + known_window_types->emplace (STATUS_NAME, get_locator_window); } /* See tui-layout.h. */ @@ -394,8 +394,8 @@ tui_register_window (const char *name, window_factory &&factory) { std::string name_copy = name; - if (name_copy == "src" || name_copy == "cmd" || name_copy == "regs" - || name_copy == "asm" || name_copy == "status") + if (name_copy == SRC_NAME || name_copy == CMD_NAME || name_copy == DATA_NAME + || name_copy == DISASSEM_NAME || name_copy == STATUS_NAME) error (_("Window type \"%s\" is built-in"), name); known_window_types->emplace (std::move (name_copy), @@ -791,8 +791,8 @@ tui_layout_split::remove_windows (const char *name) if (this_name == nullptr) m_splits[i].layout->remove_windows (name); else if (strcmp (this_name, name) == 0 - || strcmp (this_name, "cmd") == 0 - || strcmp (this_name, "status") == 0) + || strcmp (this_name, CMD_NAME) == 0 + || strcmp (this_name, STATUS_NAME) == 0) { /* Keep. */ } @@ -888,37 +888,37 @@ initialize_layouts () tui_layout_split *layout; layout = new tui_layout_split (); - layout->add_window ("src", 2); - layout->add_window ("status", 0); - layout->add_window ("cmd", 1); - add_layout_command ("src", layout); + layout->add_window (SRC_NAME, 2); + layout->add_window (STATUS_NAME, 0); + layout->add_window (CMD_NAME, 1); + add_layout_command (SRC_NAME, layout); layout = new tui_layout_split (); - layout->add_window ("asm", 2); - layout->add_window ("status", 0); - layout->add_window ("cmd", 1); - add_layout_command ("asm", layout); + layout->add_window (DISASSEM_NAME, 2); + layout->add_window (STATUS_NAME, 0); + layout->add_window (CMD_NAME, 1); + add_layout_command (DISASSEM_NAME, layout); layout = new tui_layout_split (); - layout->add_window ("src", 1); - layout->add_window ("asm", 1); - layout->add_window ("status", 0); - layout->add_window ("cmd", 1); + layout->add_window (SRC_NAME, 1); + layout->add_window (DISASSEM_NAME, 1); + layout->add_window (STATUS_NAME, 0); + layout->add_window (CMD_NAME, 1); add_layout_command ("split", layout); layout = new tui_layout_split (); - layout->add_window ("regs", 1); - layout->add_window ("src", 1); - layout->add_window ("status", 0); - layout->add_window ("cmd", 1); + layout->add_window (DATA_NAME, 1); + layout->add_window (SRC_NAME, 1); + layout->add_window (STATUS_NAME, 0); + layout->add_window (CMD_NAME, 1); layouts.emplace_back (layout); src_regs_layout = layout; layout = new tui_layout_split (); - layout->add_window ("regs", 1); - layout->add_window ("asm", 1); - layout->add_window ("status", 0); - layout->add_window ("cmd", 1); + layout->add_window (DATA_NAME, 1); + layout->add_window (DISASSEM_NAME, 1); + layout->add_window (STATUS_NAME, 0); + layout->add_window (CMD_NAME, 1); layouts.emplace_back (layout); asm_regs_layout = layout; } @@ -1010,8 +1010,8 @@ tui_new_layout_command (const char *spec, int from_tty) error (_("Missing '}' in layout specification")); if (seen_windows.empty ()) error (_("New layout does not contain any windows")); - if (seen_windows.find ("cmd") == seen_windows.end ()) - error (_("New layout does not contain the \"cmd\" window")); + if (seen_windows.find (CMD_NAME) == seen_windows.end ()) + error (_("New layout does not contain the \"" CMD_NAME "\" window")); gdb::unique_xmalloc_ptr<char> cmd_name = make_unique_xstrdup (new_name.c_str ()); ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] Fix crash when exiting TUI with gdb -tui 2020-06-06 20:12 [PATCH 0/2] Fix two TUI crashes Tom Tromey 2020-06-06 20:13 ` [PATCH 1/2] Fix C-x 1 from gdb prompt Tom Tromey @ 2020-06-06 20:13 ` Tom Tromey 2020-06-08 17:57 ` Christian Biesinger 2020-06-16 15:54 ` [PATCH 0/2] Fix two TUI crashes Pedro Alves 2 siblings, 1 reply; 8+ messages in thread From: Tom Tromey @ 2020-06-06 20:13 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey 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(+) 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>" -- 2.17.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Fix crash when exiting TUI with gdb -tui 2020-06-06 20:13 ` [PATCH 2/2] Fix crash when exiting TUI with gdb -tui Tom Tromey @ 2020-06-08 17:57 ` Christian Biesinger 2020-06-12 3:24 ` Tom Tromey 0 siblings, 1 reply; 8+ messages in thread From: Christian Biesinger @ 2020-06-08 17:57 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Fix crash when exiting TUI with gdb -tui 2020-06-08 17:57 ` Christian Biesinger @ 2020-06-12 3:24 ` Tom Tromey 0 siblings, 0 replies; 8+ messages in thread From: Tom Tromey @ 2020-06-12 3:24 UTC (permalink / raw) To: Christian Biesinger; +Cc: Tom Tromey, gdb-patches >>>>> "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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] Fix two TUI crashes 2020-06-06 20:12 [PATCH 0/2] Fix two TUI crashes Tom Tromey 2020-06-06 20:13 ` [PATCH 1/2] Fix C-x 1 from gdb prompt Tom Tromey 2020-06-06 20:13 ` [PATCH 2/2] Fix crash when exiting TUI with gdb -tui Tom Tromey @ 2020-06-16 15:54 ` Pedro Alves 2 siblings, 0 replies; 8+ messages in thread From: Pedro Alves @ 2020-06-16 15:54 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 6/6/20 9:12 PM, Tom Tromey wrote: > This short series fixes a couple of crashes involving the TUI. LGTM. Thanks for fixing this. Pedro Alves ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-06-16 15:54 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-06 20:12 [PATCH 0/2] Fix two TUI crashes Tom Tromey 2020-06-06 20:13 ` [PATCH 1/2] Fix C-x 1 from gdb prompt Tom Tromey 2020-06-08 17:57 ` Christian Biesinger 2020-06-12 0:26 ` Tom Tromey 2020-06-06 20:13 ` [PATCH 2/2] Fix crash when exiting TUI with gdb -tui Tom Tromey 2020-06-08 17:57 ` Christian Biesinger 2020-06-12 3:24 ` Tom Tromey 2020-06-16 15:54 ` [PATCH 0/2] Fix two TUI crashes Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).