* [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
* [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 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
* 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).