[RFC master/7.12.1] Don't propagate C++ exceptions across readline using SjLj on SjLj-based exception unwinding

[RFC master/7.12.1] Don't propagate C++ exceptions across readline using SjLj on SjLj-based exception unwinding

[Yao Qi]

Nowadays, we propagate C++ exceptions across readline using
setjmp/longjmp 89525768cd086a0798a504c81fdf7ebcd4c904e1
(Propagate GDB/C++ exceptions across readline using sj/lj-based TRY/CATCH)
because DWARF-based unwinding can't cross C function (see
details from the commit above).  However, SjLj-based exception
unwinding doesn't have such issue.

What is more, looks longjmp may break the SjLj-based exception
handling, because _Unwind_SjLj_Unregister, which is put the exit
of function, is not executed due to longjmp.

 (gdb) [New Thread 2936.0xb80]
 kill

 Thread 1 received signal SIGSEGV, Segmentation fault.
 0x03ff662b in ?? ()
 top?bt 15
 #0  0x03ff662b in ?? ()
 #1  0x00526b92 in stdin_event_handler (error=0, client_data=0x172ed8)
    at ../../binutils-gdb/gdb/event-top.c:555
 #2  0x00525a94 in handle_file_event (ready_mask=<optimized out>,
    file_ptr=0x3ff5cb8) at ../../binutils-gdb/gdb/event-loop.c:733
 #3  gdb_wait_for_event (block=block@entry=1)
    at ../../binutils-gdb/gdb/event-loop.c:884
 #4  0x00525bfb in gdb_do_one_event ()
    at ../../binutils-gdb/gdb/event-loop.c:347
 #5  0x00525ce5 in start_event_loop ()
    at ../../binutils-gdb/gdb/event-loop.c:371
 #6  0x0051fada in captured_command_loop (data=0x0)
    at ../../binutils-gdb/gdb/main.c:324
 #7  0x0051cf5d in catch_errors (
    func=func@entry=0x51fab0 <captured_command_loop(void*)>,
    func_args=func_args@entry=0x0,
    errstring=errstring@entry=0x7922bf <VEC_interp_factory_p_quick_push(VEC_inte rp_factory_p*, interp_factory*, char const*, unsigned int)::__PRETTY_FUNCTION__+351> "", mask=mask@entry=RETURN_MASK_ALL)
    at ../../binutils-gdb/gdb/exceptions.c:236
 #8  0x00520f0c in captured_main (data=0x328feb4)
    at ../../binutils-gdb/gdb/main.c:1149
 #9  gdb_main (args=args@entry=0x328feb4) at ../../binutils-gdb/gdb/main.c:1159
 #10 0x0071e400 in main (argc=1, argv=0x171220)
    at ../../binutils-gdb/gdb/gdb.c:32

I dig into libcc/unwind-sjlj.c and gcc/except.c, but I still
don't find much clue.  This patch fixes this issue by not propagating
the exception via setjmp/longjmp if __USING_SJLJ_EXCEPTIONS__.

gdb:

2016-12-19  Yao Qi  <yao.qi@linaro.org>

	PR gdb/20977
	* event-top.c (gdb_rl_callback_read_char_wrapper): New function.
	(gdb_rl_callback_read_char_wrapper): Call
	gdb_rl_callback_read_char_wrapper.
	(gdb_rl_callback_handler_1): New function.
	(gdb_rl_callback_handler): Call gdb_rl_callback_handler_1.
---
 gdb/event-top.c | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index acf8474..db3a18f 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -121,6 +121,14 @@ static struct async_signal_handler *async_sigterm_token;
 void (*after_char_processing_hook) (void);
 \f
 
+static void
+gdb_rl_callback_read_char_wrapper (void)
+{
+  rl_callback_read_char ();
+  if (after_char_processing_hook)
+    (*after_char_processing_hook) ();
+}
+
 /* Wrapper function for calling into the readline library.  This takes
    care of a couple things:
 
@@ -136,7 +144,8 @@ void (*after_char_processing_hook) (void);
    Any exception that tries to propagate through such code will fail
    and the result is a call to std::terminate.  While some ABIs, such
    as x86-64, require all code to be built with exception tables,
-   others don't.
+   others don't.  However SJLJ-based unwinding doesn't have such
+   problem.
 
    This is a problem when GDB calls some non-EH-aware C library code,
    that calls into GDB again through a callback, and that GDB callback
@@ -162,6 +171,9 @@ void (*after_char_processing_hook) (void);
 static void
 gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
 {
+#ifdef __USING_SJLJ_EXCEPTIONS__
+  gdb_rl_callback_read_char_wrapper();
+#else
   struct gdb_exception gdb_expt = exception_none;
 
   /* C++ exceptions can't normally be thrown across readline (unless
@@ -170,9 +182,7 @@ gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
      TRY/CATCH, and rethrow the GDB exception once back in GDB.  */
   TRY_SJLJ
     {
-      rl_callback_read_char ();
-      if (after_char_processing_hook)
-	(*after_char_processing_hook) ();
+      gdb_rl_callback_read_char_wrapper();
     }
   CATCH_SJLJ (ex, RETURN_MASK_ALL)
     {
@@ -183,6 +193,15 @@ gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
   /* Rethrow using the normal EH mechanism.  */
   if (gdb_expt.reason < 0)
     throw_exception (gdb_expt);
+#endif
+}
+
+static void
+gdb_rl_callback_handler_1 (char *rl)
+{
+  struct ui *ui = current_ui;
+
+  ui->input_handler (rl);
 }
 
 /* GDB's readline callback handler.  Calls the current INPUT_HANDLER,
@@ -192,12 +211,14 @@ gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
 static void
 gdb_rl_callback_handler (char *rl)
 {
+#ifdef __USING_SJLJ_EXCEPTIONS__
+  gdb_rl_callback_handler_1 (rl);
+#else
   struct gdb_exception gdb_rl_expt = exception_none;
-  struct ui *ui = current_ui;
 
   TRY
     {
-      ui->input_handler (rl);
+      gdb_rl_callback_handler_1 (rl);
     }
   CATCH (ex, RETURN_MASK_ALL)
     {
@@ -214,6 +235,7 @@ gdb_rl_callback_handler (char *rl)
      dtors are NOT run automatically.  */
   if (gdb_rl_expt.reason < 0)
     throw_exception_sjlj (gdb_rl_expt);
+#endif
 }
 
 /* Change the function to be invoked every time there is a character
-- 
1.9.1

Re: [RFC master/7.12.1] Don't propagate C++ exceptions across readline using SjLj on SjLj-based exception unwinding

[Pedro Alves]

On 12/19/2016 02:42 PM, Yao Qi wrote:
> Nowadays, we propagate C++ exceptions across readline using
> setjmp/longjmp 89525768cd086a0798a504c81fdf7ebcd4c904e1
> (Propagate GDB/C++ exceptions across readline using sj/lj-based TRY/CATCH)
> because DWARF-based unwinding can't cross C function (see
> details from the commit above).  However, SjLj-based exception
> unwinding doesn't have such issue.

Nice find.

I built a sjlj-based GCC trunk here, and building GDB with that
indeed triggers this.

> 
> What is more, looks longjmp may break the SjLj-based exception
> handling, because _Unwind_SjLj_Unregister, which is put the exit
> of function, is not executed due to longjmp.
> 
>  (gdb) [New Thread 2936.0xb80]
>  kill
> 
>  Thread 1 received signal SIGSEGV, Segmentation fault.
>  0x03ff662b in ?? ()
>  top?bt 15
>  #0  0x03ff662b in ?? ()
>  #1  0x00526b92 in stdin_event_handler (error=0, client_data=0x172ed8)
>     at ../../binutils-gdb/gdb/event-top.c:555
>  #2  0x00525a94 in handle_file_event (ready_mask=<optimized out>,
>     file_ptr=0x3ff5cb8) at ../../binutils-gdb/gdb/event-loop.c:733
...

> 
> I dig into libcc/unwind-sjlj.c and gcc/except.c, but I still
> don't find much clue.  This patch fixes this issue by not propagating
> the exception via setjmp/longjmp if __USING_SJLJ_EXCEPTIONS__.

I think I have a working approach that isn't specific to sjlj
exceptions, which I'd prefer.  I.e., use "noexcept", to make
the compiler understand that the function doesn't throw, and hence
doesn't need to issue _Unwind_SjLj_Register / _Unwind_SjLj_Unregister
calls.  That seems to do the trick.  I tried both -O0 and -O2 (with
GCC trunk).  Does this work for you too?

From fd8544c25b0f383fcc9517687a38c538d91e53c1 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 20 Dec 2016 11:26:36 +0000
Subject: [PATCH] noexcept

---
 gdb/event-top.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index acf8474..fa58def 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -157,10 +157,12 @@ void (*after_char_processing_hook) (void);
    sjlj-based TRY/CATCH mechanism, which knows to handle multiple
    levels of active setjmp/longjmp frames, needed in order to handle
    the readline callback recursing, as happens with e.g., secondary
-   prompts / queries, through gdb_readline_wrapper.  */
+   prompts / queries, through gdb_readline_wrapper.  This must be
+   noexcept in order to avoid problems with mixing sjlj and
+   (sjlj-based) C++ exceptions.  */
 
-static void
-gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
+static struct gdb_exception
+gdb_rl_callback_read_char_wrapper_noexcept () noexcept
 {
   struct gdb_exception gdb_expt = exception_none;
 
@@ -180,6 +182,15 @@ gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
     }
   END_CATCH_SJLJ
 
+  return gdb_expt;
+}
+
+static void
+gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
+{
+  struct gdb_exception gdb_expt
+    = gdb_rl_callback_read_char_wrapper_noexcept ();
+
   /* Rethrow using the normal EH mechanism.  */
   if (gdb_expt.reason < 0)
     throw_exception (gdb_expt);
@@ -187,10 +198,12 @@ gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
 
 /* GDB's readline callback handler.  Calls the current INPUT_HANDLER,
    and propagates GDB exceptions/errors thrown from INPUT_HANDLER back
-   across readline.  See gdb_rl_callback_read_char_wrapper.  */
+   across readline.  See gdb_rl_callback_read_char_wrapper.  This must
+   be noexcept in order to avoid problems with mixing sjlj and
+   (sjlj-based) C++ exceptions.  */
 
 static void
-gdb_rl_callback_handler (char *rl)
+gdb_rl_callback_handler (char *rl) noexcept
 {
   struct gdb_exception gdb_rl_expt = exception_none;
   struct ui *ui = current_ui;
-- 
2.5.5

Re: [RFC master/7.12.1] Don't propagate C++ exceptions across readline using SjLj on SjLj-based exception unwinding

[Yao Qi]

On 16-12-20 11:49:58, Pedro Alves wrote:
> > 
> > I dig into libcc/unwind-sjlj.c and gcc/except.c, but I still
> > don't find much clue.  This patch fixes this issue by not propagating
> > the exception via setjmp/longjmp if __USING_SJLJ_EXCEPTIONS__.
> 
> I think I have a working approach that isn't specific to sjlj
> exceptions, which I'd prefer.  I.e., use "noexcept", to make
> the compiler understand that the function doesn't throw, and hence
> doesn't need to issue _Unwind_SjLj_Register / _Unwind_SjLj_Unregister
> calls.  That seems to do the trick.  I tried both -O0 and -O2 (with
> GCC trunk).  Does this work for you too?

Yes, it is better.  I wanted to let gcc not to generate
_Unwind_SjLj_Unregister calls, but I failed to find "noexcept" does
the trick.

The patch works for me.

-- 
Yao (齐尧)

Re: [RFC master/7.12.1] Don't propagate C++ exceptions across readline using SjLj on SjLj-based exception unwinding

[Pedro Alves]

On 12/20/2016 02:33 PM, Yao Qi wrote:
> On 16-12-20 11:49:58, Pedro Alves wrote:
>>>
>>> I dig into libcc/unwind-sjlj.c and gcc/except.c, but I still
>>> don't find much clue.  This patch fixes this issue by not propagating
>>> the exception via setjmp/longjmp if __USING_SJLJ_EXCEPTIONS__.
>>
>> I think I have a working approach that isn't specific to sjlj
>> exceptions, which I'd prefer.  I.e., use "noexcept", to make
>> the compiler understand that the function doesn't throw, and hence
>> doesn't need to issue _Unwind_SjLj_Register / _Unwind_SjLj_Unregister
>> calls.  That seems to do the trick.  I tried both -O0 and -O2 (with
>> GCC trunk).  Does this work for you too?
> 
> Yes, it is better.  I wanted to let gcc not to generate
> _Unwind_SjLj_Unregister calls, but I failed to find "noexcept" does
> the trick.
> 
> The patch works for me.


Great, I've applied it to master now, like below.  Will apply to
the 7.12 branch in a moment.

From 2693a26216c329bd7ec2aae7743409f572de4fa5 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 20 Dec 2016 15:46:44 +0000
Subject: [PATCH] Fix longjmp across readline w/ --enable-sjlj-exceptions
 toolchains

Nowadays, GDB propagates C++ exceptions across readline using
setjmp/longjmp 89525768cd08 ("Propagate GDB/C++ exceptions across
readline using sj/lj-based TRY/CATCH") because DWARF-based unwinding
can't cross C functions compiled without -fexceptions (see details
from the commit above).

Unfortunately, toolchains that use SjLj-based C++ exceptions got
broken with that fix, because _Unwind_SjLj_Unregister, which is put at
the exit of a function, is not executed due to the longjmp added by
that commit.

 (gdb) [New Thread 2936.0xb80]
 kill

 Thread 1 received signal SIGSEGV, Segmentation fault.
 0x03ff662b in ?? ()
 top?bt 15
 #0  0x03ff662b in ?? ()
 #1  0x00526b92 in stdin_event_handler (error=0, client_data=0x172ed8)
    at ../../binutils-gdb/gdb/event-top.c:555
 #2  0x00525a94 in handle_file_event (ready_mask=<optimized out>,
    file_ptr=0x3ff5cb8) at ../../binutils-gdb/gdb/event-loop.c:733
 #3  gdb_wait_for_event (block=block@entry=1)
    at ../../binutils-gdb/gdb/event-loop.c:884
 #4  0x00525bfb in gdb_do_one_event ()
    at ../../binutils-gdb/gdb/event-loop.c:347
 #5  0x00525ce5 in start_event_loop ()
    at ../../binutils-gdb/gdb/event-loop.c:371
 #6  0x0051fada in captured_command_loop (data=0x0)
    at ../../binutils-gdb/gdb/main.c:324
 #7  0x0051cf5d in catch_errors (
    func=func@entry=0x51fab0 <captured_command_loop(void*)>,
    func_args=func_args@entry=0x0,
    errstring=errstring@entry=0x7922bf <VEC_interp_factory_p_quick_push(VEC_inte rp_factory_p*, interp_factory*, char const*, unsigned int)::__PRETTY_FUNCTION__+351> "", mask=mask@entry=RETURN_MASK_ALL)
    at ../../binutils-gdb/gdb/exceptions.c:236
 #8  0x00520f0c in captured_main (data=0x328feb4)
    at ../../binutils-gdb/gdb/main.c:1149
 #9  gdb_main (args=args@entry=0x328feb4) at ../../binutils-gdb/gdb/main.c:1159
 #10 0x0071e400 in main (argc=1, argv=0x171220)
    at ../../binutils-gdb/gdb/gdb.c:32

Fix this by making the functions involved in setjmp/longjmp as
noexcept, so that the compiler knows it doesn't need to emit the
_Unwind_SjLj_Register / _Unwind_SjLj_Unregister calls for C++
exceptions.

Tested on x86_64 Fedora 23 with:
 - GCC 5.3.1 w/ DWARF-based exceptions.
 - GCC 7 built with --enable-sjlj-exceptions.

gdb/ChangeLog:
2016-12-20  Pedro Alves  <palves@redhat.com>
	    Yao Qi  <yao.qi@linaro.org>

	PR gdb/20977
	* event-top.c (gdb_rl_callback_read_char_wrapper_noexcept): New
	noexcept function, factored out from ...
	(gdb_rl_callback_read_char_wrapper): ... this.
	(gdb_rl_callback_handler): Mark noexcept.
---
 gdb/ChangeLog   |  9 +++++++++
 gdb/event-top.c | 23 ++++++++++++++++++-----
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 233a43c..5f0e6fe 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2016-12-20  Pedro Alves  <palves@redhat.com>
+	    Yao Qi  <yao.qi@linaro.org>
+
+	PR gdb/20977
+	* event-top.c (gdb_rl_callback_read_char_wrapper_noexcept): New
+	noexcept function, factored out from ...
+	(gdb_rl_callback_read_char_wrapper): ... this.
+	(gdb_rl_callback_handler): Mark noexcept.
+
 2016-12-20  Antoine Tremblay  <antoine.tremblay@ericsson.com>
 
 	* .dir-locals.el: Set c++ mode for the directory and set indent
diff --git a/gdb/event-top.c b/gdb/event-top.c
index acf8474..fa58def 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -157,10 +157,12 @@ void (*after_char_processing_hook) (void);
    sjlj-based TRY/CATCH mechanism, which knows to handle multiple
    levels of active setjmp/longjmp frames, needed in order to handle
    the readline callback recursing, as happens with e.g., secondary
-   prompts / queries, through gdb_readline_wrapper.  */
+   prompts / queries, through gdb_readline_wrapper.  This must be
+   noexcept in order to avoid problems with mixing sjlj and
+   (sjlj-based) C++ exceptions.  */
 
-static void
-gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
+static struct gdb_exception
+gdb_rl_callback_read_char_wrapper_noexcept () noexcept
 {
   struct gdb_exception gdb_expt = exception_none;
 
@@ -180,6 +182,15 @@ gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
     }
   END_CATCH_SJLJ
 
+  return gdb_expt;
+}
+
+static void
+gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
+{
+  struct gdb_exception gdb_expt
+    = gdb_rl_callback_read_char_wrapper_noexcept ();
+
   /* Rethrow using the normal EH mechanism.  */
   if (gdb_expt.reason < 0)
     throw_exception (gdb_expt);
@@ -187,10 +198,12 @@ gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
 
 /* GDB's readline callback handler.  Calls the current INPUT_HANDLER,
    and propagates GDB exceptions/errors thrown from INPUT_HANDLER back
-   across readline.  See gdb_rl_callback_read_char_wrapper.  */
+   across readline.  See gdb_rl_callback_read_char_wrapper.  This must
+   be noexcept in order to avoid problems with mixing sjlj and
+   (sjlj-based) C++ exceptions.  */
 
 static void
-gdb_rl_callback_handler (char *rl)
+gdb_rl_callback_handler (char *rl) noexcept
 {
   struct gdb_exception gdb_rl_expt = exception_none;
   struct ui *ui = current_ui;
-- 
2.5.5

Re: [RFC master/7.12.1] Don't propagate C++ exceptions across readline using SjLj on SjLj-based exception unwinding

[Pedro Alves]

On 12/20/2016 04:25 PM, Pedro Alves wrote:

> Great, I've applied it to master now, like below.  Will apply to
> the 7.12 branch in a moment.

... and the buildbot let me know that this broke the branch ...

> -static void
> -gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
> +static struct gdb_exception
> +gdb_rl_callback_read_char_wrapper_noexcept () noexcept

... because I completely forgot that 7.12 is supposed to
build in C and C++03 as well...  I've applied this fix on the
branch now.  master doesn't need it, since it requires C++11.

From 9bfe0298332782a9c082fb475bdf8eeeef8cf45e Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 20 Dec 2016 19:18:15 +0000
Subject: [PATCH] gdb: Fix C and C++03 builds

The readline/sjlj-exceptions fix added an unconditional use of
noexcept, but that's only valid C++11, and 7.12 must build with C and
C++03 too.  Fix this by adding a GDB_EXCEPT macro that compiles away
to nothing in C, and to throw() in C++03, which I've confirmed fixes
the original issue just the same as noexcept, with GCC 7 + -std=gnu+03
+ sjlj-exceptions.

gdb/ChangeLog:
2016-12-20  Pedro Alves  <palves@redhat.com>

	PR gdb/20977
	* event-top.c (GDB_NOEXCEPT): Define.
	(gdb_rl_callback_read_char_wrapper_noexcept): Use GDB_NOEXCEPT
	instead of noexcept and use (void) instead of ().
	(gdb_rl_callback_handler): Use GDB_NOEXCEPT instead of noexcept.
---
 gdb/ChangeLog   |  8 ++++++++
 gdb/event-top.c | 12 ++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e99025e..0aaae46 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,4 +1,12 @@
 2016-12-20  Pedro Alves  <palves@redhat.com>
+
+	PR gdb/20977
+	* event-top.c (GDB_NOEXCEPT): Define.
+	(gdb_rl_callback_read_char_wrapper_noexcept): Use GDB_NOEXCEPT
+	instead of noexcept and use (void) instead of ().
+	(gdb_rl_callback_handler): Use GDB_NOEXCEPT instead of noexcept.
+
+2016-12-20  Pedro Alves  <palves@redhat.com>
 	    Yao Qi  <yao.qi@linaro.org>
 
 	PR gdb/20977
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 3e218ff..7f590a9 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -73,6 +73,14 @@ static void async_stop_sig (gdb_client_data);
 #endif
 static void async_sigterm_handler (gdb_client_data arg);
 
+#ifndef __cplusplus
+# define GDB_NOEXCEPT
+#elif __cplusplus < 201103L
+# define GDB_NOEXCEPT throw ()
+#else
+# define GDB_NOEXCEPT noexcept
+#endif
+
 /* Instead of invoking (and waiting for) readline to read the command
    line and pass it back for processing, we use readline's alternate
    interface, via callback functions, so that the event loop can react
@@ -162,7 +170,7 @@ void (*after_char_processing_hook) (void);
    (sjlj-based) C++ exceptions.  */
 
 static struct gdb_exception
-gdb_rl_callback_read_char_wrapper_noexcept () noexcept
+gdb_rl_callback_read_char_wrapper_noexcept (void) GDB_NOEXCEPT
 {
   struct gdb_exception gdb_expt = exception_none;
 
@@ -203,7 +211,7 @@ gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
    (sjlj-based) C++ exceptions.  */
 
 static void
-gdb_rl_callback_handler (char *rl) noexcept
+gdb_rl_callback_handler (char *rl) GDB_NOEXCEPT
 {
   struct gdb_exception gdb_rl_expt = exception_none;
   struct ui *ui = current_ui;
-- 
2.5.5