Don't let TUI exceptions escape to readline (PR tui/9765)

Message ID ed397bcd-0562-d18d-b7e8-365bbf8e7b37@redhat.com
State New
Headers show
Series
  • Don't let TUI exceptions escape to readline (PR tui/9765)
Related show

Commit Message

Pedro Alves Jan. 10, 2020, 1:36 p.m.
On 1/10/20 12:53 PM, Pedro Alves wrote:

> I didn't delve deep into the patch, but, I should point out one

> thing -- as described in the PR, it's a problem to let exceptions

> cross ncurses.  Any kind of C++ exception.  So which ncurses callback/entry

> point in gdb were we at?  We need to look into it and make sure that

> no exceptions are thrown from it back into ncurses.  Above, you're rethrowing

> non-memory exceptions, which is what made me wonder, since it sounds like

> for  example a Ctrl-C at some "wrong" time may bring down GDB.

> For readline, we ended up with TRY_SJLJ/CATCH_SJLJ.



There's actually a backtrace in the PR.  And I can still (*) reproduce it.
Here's the current backtrace I get:

(top-gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007ffff4633d31 in __GI_abort () at abort.c:79
#2  0x00007ffff4c5d2a5 in __gnu_cxx::__verbose_terminate_handler () at ../../../../libstdc++-v3/libsupc++/vterminate.cc:95
#3  0x00007ffff4c5ae96 in __cxxabiv1::__terminate (handler=<optimized out>) at ../../../../libstdc++-v3/libsupc++/eh_terminate.cc:47
#4  0x00007ffff4c59d99 in __cxa_call_terminate (During symbol reading, Child DIE 0x2cee6 and its abstract origin 0x5a17 have different parents.
ue_header=ue_header@entry=0x1a011a0) at ../../../../libstdc++-v3/libsupc++/eh_call.cc:54
#5  0x00007ffff4c5a788 in __cxxabiv1::__gxx_personality_v0 (version=<optimized out>, actions=<optimized out>, exception_class=5138137972254386944, ue_header=0x1a011a0, context=<optimized out>) at ../../../../libstdc++-v3/libsupc++/eh_personality.cc:676
#6  0x00007ffff49c3f33 in _Unwind_RaiseException_Phase2 (exc=exc@entry=0x1a011a0, context=context@entry=0x7fffffffcfa0) at ../../../libgcc/unwind.inc:62
#7  0x00007ffff49c475e in _Unwind_Resume (exc=0x1a011a0) at ../../../libgcc/unwind.inc:230
#8  0x000000000093313e in tui_disasm_window::set_contents (this=0x1c00f10, arch=0x1a3f2a0, sal=...) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-disasm.c:227
#9  0x000000000094c8fd in tui_source_window_base::update_source_window_as_is (this=0x1c00f10, gdbarch=0x1a3f2a0, sal=...) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-winsource.c:184
#10 0x0000000000933428 in tui_disasm_window::do_scroll_vertical (this=0x1c00f10, num_to_scroll=13) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-disasm.c:337
#11 0x000000000094a322 in tui_win_info::forward_scroll (this=0x1c00f10, num_to_scroll=12) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-win.c:476
#12 0x000000000093deac in tui_dispatch_ctrl_char (ch=338) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-io.c:921
#13 0x000000000093e05c in tui_getc (fp=0x7ffff49ae9e0 <_IO_2_1_stdin_>) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-io.c:1005
#14 0x00000000009dbedd in rl_read_key () at /home/pedro/gdb/binutils-gdb/src/readline/readline/input.c:495
#15 0x00000000009be41c in readline_internal_char () at /home/pedro/gdb/binutils-gdb/src/readline/readline/readline.c:573
#16 0x00000000009dca7b in rl_callback_read_char () at /home/pedro/gdb/binutils-gdb/src/readline/readline/callback.c:262
...

The issue is actually crossing readline here, tui_getc -> rl_read_key, not ncurses.

* - eh, I was the one who filed this, I forgot, lol.

I think we should add this patch, in addition to your fix, or something
like it.

From abb77826ee2ea565282d675d5c82a98e55601c41 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>

Date: Fri, 10 Jan 2020 13:32:07 +0000
Subject: [PATCH] Don't let TUI exceptions escape to readline (PR tui/9765)

PR tui/9765 shows a use case where GDB dies from an uncaught exception,
here:

(top-gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007ffff4633d31 in __GI_abort () at abort.c:79
#2  0x00007ffff4c5d2a5 in __gnu_cxx::__verbose_terminate_handler () at ../../../../libstdc++-v3/libsupc++/vterminate.cc:95
#3  0x00007ffff4c5ae96 in __cxxabiv1::__terminate (handler=<optimized out>) at ../../../../libstdc++-v3/libsupc++/eh_terminate.cc:47
#4  0x00007ffff4c59d99 in __cxa_call_terminate (During symbol reading, Child DIE 0x2cee6 and its abstract origin 0x5a17 have different parents.
ue_header=ue_header@entry=0x1a011a0) at ../../../../libstdc++-v3/libsupc++/eh_call.cc:54
#5  0x00007ffff4c5a788 in __cxxabiv1::__gxx_personality_v0 (version=<optimized out>, actions=<optimized out>, exception_class=5138137972254386944, ue_header=0x1a011a0, context=<optimized out>) at ../../../../libstdc++-v3/libsupc++/eh_personality.cc:676
#6  0x00007ffff49c3f33 in _Unwind_RaiseException_Phase2 (exc=exc@entry=0x1a011a0, context=context@entry=0x7fffffffcfa0) at ../../../libgcc/unwind.inc:62
#7  0x00007ffff49c475e in _Unwind_Resume (exc=0x1a011a0) at ../../../libgcc/unwind.inc:230
#8  0x000000000093313e in tui_disasm_window::set_contents (this=0x1c00f10, arch=0x1a3f2a0, sal=...) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-disasm.c:227
#9  0x000000000094c8fd in tui_source_window_base::update_source_window_as_is (this=0x1c00f10, gdbarch=0x1a3f2a0, sal=...) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-winsource.c:184
#10 0x0000000000933428 in tui_disasm_window::do_scroll_vertical (this=0x1c00f10, num_to_scroll=13) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-disasm.c:337
#11 0x000000000094a322 in tui_win_info::forward_scroll (this=0x1c00f10, num_to_scroll=12) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-win.c:476
#12 0x000000000093deac in tui_dispatch_ctrl_char (ch=338) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-io.c:921
#13 0x000000000093e05c in tui_getc (fp=0x7ffff49ae9e0 <_IO_2_1_stdin_>) at /home/pedro/gdb/binutils-gdb/src/gdb/tui/tui-io.c:1005
#14 0x00000000009dbedd in rl_read_key () at /home/pedro/gdb/binutils-gdb/src/readline/readline/input.c:495
#15 0x00000000009be41c in readline_internal_char () at /home/pedro/gdb/binutils-gdb/src/readline/readline/readline.c:573
#16 0x00000000009dca7b in rl_callback_read_char () at /home/pedro/gdb/binutils-gdb/src/readline/readline/callback.c:262
...

This is triggered by simply scrolling off the end of the dissasembly
window.  This commit doesn't fix the actual exception that is being
thrown, which will still need to be fixed, but makes sure that we
don't ever throw an exception out to readline.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	PR tui/9765
	* tui/tui-io.c (tui_getc): Rename to ...
	(tui_getc_1): ... this.
	(tui_get): New, reimplent as try/catch wrapper around tui_getc_1.
---
 gdb/tui/tui-io.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)


base-commit: ffebb0bbde7deae978ab3e4d3d3d90acf52b7d69
-- 
2.14.5

Comments

Shahab Vahedi Jan. 10, 2020, 2:30 p.m. | #1
This patch must be in as well! It takes care of the "call stack 2"
from the other e-mail.

Cheers,
--
Shahab
Tom Tromey Jan. 10, 2020, 2:40 p.m. | #2
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


Pedro> gdb/ChangeLog:
Pedro> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

Pedro> 	PR tui/9765
Pedro> 	* tui/tui-io.c (tui_getc): Rename to ...
Pedro> 	(tui_getc_1): ... this.
Pedro> 	(tui_get): New, reimplent as try/catch wrapper around tui_getc_1.
Pedro> ---
Pedro>  gdb/tui/tui-io.c | 31 ++++++++++++++++++++++++++++---
Pedro>  1 file changed, 28 insertions(+), 3 deletions(-)

This seems like a good idea measure to me.
Ideally I suppose these TUI functions shouldn't throw.  However, in gdb
it can be difficult to ensure that, so this provides some defense.

thanks,
Tom

Patch

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 9cb41104fe9..d9f23334f57 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -950,10 +950,12 @@  tui_dispatch_ctrl_char (unsigned int ch)
   return 0;
 }
 
-/* Get a character from the command window.  This is called from the
-   readline package.  */
+/* Main worker for tui_getc.  Get a character from the command window.
+   This is called from the readline package, but wrapped in a
+   try/catch by tui_getc.  */
+
 static int
-tui_getc (FILE *fp)
+tui_getc_1 (FILE *fp)
 {
   int ch;
   WINDOW *w;
@@ -1036,6 +1038,29 @@  tui_getc (FILE *fp)
   return ch;
 }
 
+/* Get a character from the command window.  This is called from the
+   readline package.  */
+
+static int
+tui_getc (FILE *fp)
+{
+  try
+    {
+      return tui_getc_1 (fp);
+    }
+  catch (const gdb_exception &ex)
+    {
+      /* Just in case, don't ever let an exception escape to readline.
+	 This shouldn't ever happen, but if it does, print the
+	 exception instead of just crashing GDB.  */
+      exception_print (gdb_stderr, ex);
+
+      /* If we threw an exception, it's because we recognized the
+	 character.  */
+      return 0;
+    }
+}
+
 /* See tui-io.h.  */
 
 gdb::unique_xmalloc_ptr<char>