public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Fix C++ exceptions across readline result in std::terminate() -> SIGABRT
@ 2016-04-21 23:50 Pedro Alves
  2016-04-21 23:50 ` [PATCH 1/3] Rename rl_callback_read_char_wrapper -> gdb_rl_callback_read_char Pedro Alves
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Pedro Alves @ 2016-04-21 23:50 UTC (permalink / raw)
  To: gdb-patches

We saw that if we map GDB'S TRY/CATCH macros to C++ try/catch, GDB
breaks on systems where readline isn't built with exceptions support.

Yao tripped on this on ARM/Fedora 19, and I saw the same thing on AIX.

The problem is that readline calls into GDB through the callback
interface, and if GDB's callback throws a C++ exception/error, the
system unwinder won't manage to unwind past the readline frame, and
ends up calling std::terminate(), which aborts the process:

 (gdb) whatever-command-that-causes-an-error
 terminate called after throwing an instance of 'gdb_exception_RETURN_MASK_ERROR'
 Aborted
 $

This series:

 - stops GDB from throwing C++ exceptions across readline.

 - then flips back TRY/CATCH to C++ try/catch (which is necessary for
   automatically calling local variable destructors when
   throw_exception is called).

Tested on x86_64 Fedora, and confirmed that it fixes the issue on AIX.

Pedro Alves (3):
  Rename rl_callback_read_char_wrapper -> gdb_rl_callback_read_char
  Propagate GDB/C++ exceptions across readline using sj/lj-based
    TRY/CATCH
  Switch gdb's TRY/CATCH to C++ try/catch

 gdb/common/common-exceptions.h |  69 ++++++++++++++-----------
 gdb/common/common-exceptions.c |  38 +++++++++++---
 gdb/event-top.c                | 112 +++++++++++++++++++++++++++++++++++------
 3 files changed, 169 insertions(+), 50 deletions(-)

-- 
2.5.5

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] Rename rl_callback_read_char_wrapper -> gdb_rl_callback_read_char
  2016-04-21 23:50 [PATCH 0/3] Fix C++ exceptions across readline result in std::terminate() -> SIGABRT Pedro Alves
@ 2016-04-21 23:50 ` Pedro Alves
  2016-04-21 23:50 ` [PATCH 3/3] Switch gdb's TRY/CATCH to C++ try/catch Pedro Alves
  2016-04-21 23:50 ` [PATCH 2/3] Propagate GDB/C++ exceptions across readline using sj/lj-based TRY/CATCH Pedro Alves
  2 siblings, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2016-04-21 23:50 UTC (permalink / raw)
  To: gdb-patches

Use the "gdb_rl_" prefix like other gdb readline function wrappers to
make it clear this is a gdb function, not a readline function.

gdb/ChangeLog:
2016-04-21  Pedro Alves  <palves@redhat.com>

	* event-top.c (rl_callback_read_char_wrapper): Rename to ...
	(gdb_rl_callback_read_char_wrapper): ... this.
	(change_line_handler, gdb_setup_readline): Adjust.
---
 gdb/event-top.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index ee8684b..b9947c4 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -48,7 +48,6 @@
 /* readline defines this.  */
 #undef savestring
 
-static void rl_callback_read_char_wrapper (gdb_client_data client_data);
 static void command_line_handler (char *rl);
 static void change_line_handler (void);
 static char *top_level_prompt (void);
@@ -141,7 +140,7 @@ static struct async_signal_handler *sigtstp_token;
 #endif
 static struct async_signal_handler *async_sigterm_token;
 
-/* This hook is called by rl_callback_read_char_wrapper after each
+/* This hook is called by gdb_rl_callback_read_char_wrapper after each
    character is processed.  */
 void (*after_char_processing_hook) (void);
 \f
@@ -150,7 +149,7 @@ void (*after_char_processing_hook) (void);
    loop expects the callback function to have a paramter, while
    readline expects none.  */
 static void
-rl_callback_read_char_wrapper (gdb_client_data client_data)
+gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
 {
   rl_callback_read_char ();
   if (after_char_processing_hook)
@@ -188,7 +187,7 @@ change_line_handler (void)
   if (async_command_editing_p)
     {
       /* Turn on editing by using readline.  */
-      call_readline = rl_callback_read_char_wrapper;
+      call_readline = gdb_rl_callback_read_char_wrapper;
       input_handler = command_line_handler;
     }
   else
@@ -1109,8 +1108,10 @@ set_async_editing_command (char *args, int from_tty,
 }
 
 /* Set things up for readline to be invoked via the alternate
-   interface, i.e. via a callback function (rl_callback_read_char),
-   and hook up instream to the event loop.  */
+   interface, i.e. via a callback function
+   (gdb_rl_callback_read_char), and hook up instream to the event
+   loop.  */
+
 void
 gdb_setup_readline (void)
 {
@@ -1136,7 +1137,7 @@ gdb_setup_readline (void)
 	  
       /* When a character is detected on instream by select or poll,
 	 readline will be invoked via this callback function.  */
-      call_readline = rl_callback_read_char_wrapper;
+      call_readline = gdb_rl_callback_read_char_wrapper;
     }
   else
     {
-- 
2.5.5

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] Switch gdb's TRY/CATCH to C++ try/catch
  2016-04-21 23:50 [PATCH 0/3] Fix C++ exceptions across readline result in std::terminate() -> SIGABRT Pedro Alves
  2016-04-21 23:50 ` [PATCH 1/3] Rename rl_callback_read_char_wrapper -> gdb_rl_callback_read_char Pedro Alves
@ 2016-04-21 23:50 ` Pedro Alves
  2016-04-21 23:50 ` [PATCH 2/3] Propagate GDB/C++ exceptions across readline using sj/lj-based TRY/CATCH Pedro Alves
  2 siblings, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2016-04-21 23:50 UTC (permalink / raw)
  To: gdb-patches

The exceptions-across-readline issue was fixed by the previous commit.
Let's try this again.

gdb/ChangeLog:
2016-04-21  Pedro Alves  <palves@redhat.com>

	* common/common-exceptions.h (GDB_XCPT_TRY): Remove mention of
	the foreign frames issue.
	[__cplusplus] (GDB_XCPT): Define as GDB_XCPT_TRY.
---
 gdb/common/common-exceptions.h | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h
index 4a9f675..ed1cf5e 100644
--- a/gdb/common/common-exceptions.h
+++ b/gdb/common/common-exceptions.h
@@ -122,9 +122,7 @@ struct gdb_exception
    the only mode supported when GDB is built as a C program.  */
 #define GDB_XCPT_SJMP 1
 
-/* Make GDB exceptions use try/catch behind the scenes.  Can't be made
-   the default until we handle exceptions crossing foreign frames
-   (gdb -> readline callback -> gdb -> error).  */
+/* Make GDB exceptions use try/catch behind the scenes.  */
 #define GDB_XCPT_TRY 2
 
 /* Specify this mode to build with TRY/CATCH mapped directly to raw
@@ -133,8 +131,11 @@ struct gdb_exception
    spurious code between the TRY and the CATCH block.  */
 #define GDB_XCPT_RAW_TRY 3
 
-/* Always use setjmp/longmp, even in C++ mode.  */
-#define GDB_XCPT GDB_XCPT_SJMP
+#ifdef __cplusplus
+# define GDB_XCPT GDB_XCPT_TRY
+#else
+# define GDB_XCPT GDB_XCPT_SJMP
+#endif
 
 /* Functions to drive the sjlj-based exceptions state machine.  Though
    declared here by necessity, these functions should be considered
-- 
2.5.5

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] Propagate GDB/C++ exceptions across readline using sj/lj-based TRY/CATCH
  2016-04-21 23:50 [PATCH 0/3] Fix C++ exceptions across readline result in std::terminate() -> SIGABRT Pedro Alves
  2016-04-21 23:50 ` [PATCH 1/3] Rename rl_callback_read_char_wrapper -> gdb_rl_callback_read_char Pedro Alves
  2016-04-21 23:50 ` [PATCH 3/3] Switch gdb's TRY/CATCH to C++ try/catch Pedro Alves
@ 2016-04-21 23:50 ` Pedro Alves
  2016-04-22  8:19   ` Yao Qi
  2 siblings, 1 reply; 7+ messages in thread
From: Pedro Alves @ 2016-04-21 23:50 UTC (permalink / raw)
  To: gdb-patches

If we map GDB'S TRY/CATCH macros to C++ try/catch, GDB breaks on
systems where readline isn't built with exceptions support.  The
problem is that readline calls into GDB through the callback
interface, and if GDB's callback throws a C++ exception/error, the
system unwinder won't manage to unwind past the readline frame, and
ends up calling std::terminate(), which aborts the process:

 (gdb) whatever-command-that-causes-an-error
 terminate called after throwing an instance of 'gdb_exception_RETURN_MASK_ERROR'
 Aborted
 $

This went unnoticed for so long because:

- the x86-64 ABI requires fasynchronous-unwind-tables, making it
  possible for exceptions to cross readline with no special handling.
  But e.g., on ARM or AIX, unless you build readline with
  -fexceptions, you trip on the problem.

- TRY/CATCH was mapped to setjmp/longjmp, even in C++ mode, until
  quite recently.

The fix is to catch and save any GDB exception that is thrown inside
the GDB readline callback, and then once the callback returns back to
the GDB code that called into readline in the first place, rethrow the
saved GDB exception.

This is similar in spirit to how we catch/map GDB exceptions at the
GDB/Python and GDB/Guile API boundaries.

The next question is then: if we intercept all exceptions within GDB's
readline callback, should we simply return normally to readline?  The
callback prototype has no way to signal an error back to readline (*).
The answer is no -- if we return normally, we'll be returning to a
loop inside rl_callback_read_char that continues processing pending
input, calling into GDB again, redisplaying the prompt, etc.  Thus if
we want to error out of rl_callback_read_char, we need to long jump
across it, just like we always did before TRY/CATCH were ever mapped
to C++ exceptions.

My first approach built a specialized API to handle this, with a
couple macros to hide the setjmp/longjmp and the struct gdb_exception
saving/rethrowing.

However, I realized that we need to:

 - Handle multiple active rl_callback_read_char invocations.  If,
   while processing input something triggers a secondary prompt, we
   end up in a nested rl_callback_read_char call, through
   gdb_readline_wrapper.

 - Propagate a struct gdb_exception along with the longjmp.

... and that this is exactly what the setjmp/longjmp-based TRY/CATCH
does.

So the fix makes the setjmp/longjmp TRY/CATCH always available under
new TRY_SJLJ/CATCH_SJLJ aliases, even when TRY/CATCH is mapped to C++
try/catch, and then uses TRY_SJLJ/CATCH_SJLJ to propagate GDB
exceptions across the readline callback.

This turns out to be a much better looking fix than my bespoke API
attempt, even.  We'll probably be able to simplify TRY_SJLJ/CATCH_SJLJ
when we finally get rid of TRY/CATCH all over the tree, but until
then, this reuse seems quite nice for avoiding a second parallel
setjmp/longjmp mechanism.

(*) - maybe we could propose a readline API change, but we still need
      to handle current readline, anyway.

gdb/ChangeLog:
2016-04-21  Pedro Alves  <palves@redhat.com>

	* common/common-exceptions.c (enum catcher_state, struct catcher)
	(current_catcher): Define in C++ mode too.
	(exceptions_state_mc_catch): Call throw_exception_sjlj instead of
	throw_exception.
	(throw_exception_sjlj, throw_exception_cxx): New functions,
	factored out from throw_exception.
	(throw_exception): Reimplement.
	* common/common-exceptions.h (exceptions_state_mc_init)
	(exceptions_state_mc_action_iter)
	(exceptions_state_mc_action_iter_1, exceptions_state_mc_catch):
	Declare in C++ mode too.
	(TRY): Rename to ...
	(TRY_SJLJ): ... this.
	(CATCH): Rename to ...
	(CATCH_SJLJ): ... this.
	(END_CATCH): Rename to ...
	(END_CATCH_SJLJ): ... this.
	[GDB_XCPT == GDB_XCPT_SJMP] (TRY, CATCH, END_CATCH): Map to SJLJ
	equivalents.
	(throw_exception): Update comments.
	(throw_exception_sjlj): Declare.
	* event-top.c (gdb_rl_callback_read_char_wrapper): Extend intro
	comment.  Wrap body in TRY_SJLJ/CATCH_SJLJ and rethrow any
	intercepted exception.
	(gdb_rl_callback_handler): New function.
	(gdb_rl_callback_handler_install): Always install
	gdb_rl_callback_handler as readline callback.
---
 gdb/common/common-exceptions.h | 58 +++++++++++++++----------
 gdb/common/common-exceptions.c | 38 +++++++++++++----
 gdb/event-top.c                | 97 +++++++++++++++++++++++++++++++++++++++---
 3 files changed, 155 insertions(+), 38 deletions(-)

diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h
index 1ef3db3..4a9f675 100644
--- a/gdb/common/common-exceptions.h
+++ b/gdb/common/common-exceptions.h
@@ -136,17 +136,17 @@ struct gdb_exception
 /* Always use setjmp/longmp, even in C++ mode.  */
 #define GDB_XCPT GDB_XCPT_SJMP
 
-/* Functions to drive the exceptions state machine.  Though declared
-   here by necessity, these functions should be considered internal to
-   the exceptions subsystem and not used other than via the TRY/CATCH
-   macros defined below.  */
+/* Functions to drive the sjlj-based exceptions state machine.  Though
+   declared here by necessity, these functions should be considered
+   internal to the exceptions subsystem and not used other than via
+   the TRY/CATCH (or TRY_SJLJ/CATCH_SJLJ) macros defined below.  */
 
-#if GDB_XCPT == GDB_XCPT_SJMP
 extern jmp_buf *exceptions_state_mc_init (void);
 extern int exceptions_state_mc_action_iter (void);
 extern int exceptions_state_mc_action_iter_1 (void);
 extern int exceptions_state_mc_catch (struct gdb_exception *, int);
-#else
+
+#if GDB_XCPT != GDB_XCPT_SJMP
 extern void *exception_try_scope_entry (void);
 extern void exception_try_scope_exit (void *saved_state);
 extern void exception_rethrow (void);
@@ -175,11 +175,13 @@ extern void exception_rethrow (void);
      }
    END_CATCH
 
-  */
+  Note the macros are actually called TRY_SJLJ/CATCH_SJLJ in order to
+  make it possible to call them even when TRY/CATCH are mapped to C++
+  try/catch.  The SJLJ variants are needed in some cases where gdb
+  exceptions need to cross third-party library code compiled without
+  exceptions support (e.g., readline).  */
 
-#if GDB_XCPT == GDB_XCPT_SJMP
-
-#define TRY \
+#define TRY_SJLJ \
      { \
        jmp_buf *buf = \
 	 exceptions_state_mc_init (); \
@@ -188,14 +190,20 @@ extern void exception_rethrow (void);
      while (exceptions_state_mc_action_iter ()) \
        while (exceptions_state_mc_action_iter_1 ())
 
-#define CATCH(EXCEPTION, MASK)				\
+#define CATCH_SJLJ(EXCEPTION, MASK)				\
   {							\
     struct gdb_exception EXCEPTION;				\
     if (exceptions_state_mc_catch (&(EXCEPTION), MASK))
 
-#define END_CATCH				\
+#define END_CATCH_SJLJ				\
   }
 
+#if GDB_XCPT == GDB_XCPT_SJMP
+
+#define TRY TRY_SJLJ
+#define CATCH CATCH_SJLJ
+#define END_CATCH END_CATCH_SJLJ
+
 #endif /* GDB_XCPT_SJMP */
 
 #if GDB_XCPT == GDB_XCPT_TRY || GDB_XCPT == GDB_XCPT_RAW_TRY
@@ -270,19 +278,23 @@ struct gdb_exception_RETURN_MASK_QUIT : public gdb_exception_RETURN_MASK_ALL
 
 /* *INDENT-ON* */
 
-/* Throw an exception (as described by "struct gdb_exception").  Will
-   execute a LONG JUMP to the inner most containing exception handler
-   established using catch_exceptions() (or similar).
-
-   Code normally throws an exception using error() et.al.  For various
-   reaons, GDB also contains code that throws an exception directly.
-   For instance, the remote*.c targets contain CNTRL-C signal handlers
-   that propogate the QUIT event up the exception chain.  ``This could
-   be a good thing or a dangerous thing.'' -- the Existential
-   Wombat.  */
-
+/* Throw an exception (as described by "struct gdb_exception").  When
+   GDB is built as a C program, executes a LONG JUMP to the inner most
+   containing exception handler established using TRY/CATCH.  When
+   built as a C++ program, throws a C++ exception, using "throw".  */
 extern void throw_exception (struct gdb_exception exception)
      ATTRIBUTE_NORETURN;
+
+/* Throw an exception by executing a LONG JUMP to the inner most
+   containing exception handler established using TRY_SJLJ.  Works the
+   same regardless of whether GDB is built as a C program or a C++
+   program.  Necessary in some cases where we need to throw GDB
+   exceptions across third-party library code (e.g., readline).  */
+extern void throw_exception_sjlj (struct gdb_exception exception)
+     ATTRIBUTE_NORETURN;
+
+/* Convenience wrappers around throw_exception that throw GDB
+   errors.  */
 extern void throw_verror (enum errors, const char *fmt, va_list ap)
      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (2, 0);
 extern void throw_vquit (const char *fmt, va_list ap)
diff --git a/gdb/common/common-exceptions.c b/gdb/common/common-exceptions.c
index 2e63862..33fff21 100644
--- a/gdb/common/common-exceptions.c
+++ b/gdb/common/common-exceptions.c
@@ -22,8 +22,6 @@
 
 const struct gdb_exception exception_none = { (enum return_reason) 0, GDB_NO_ERROR, NULL };
 
-#if GDB_XCPT == GDB_XCPT_SJMP
-
 /* Possible catcher states.  */
 enum catcher_state {
   /* Initial state, a new catcher has just been created.  */
@@ -57,6 +55,8 @@ struct catcher
 /* Where to go for throw_exception().  */
 static struct catcher *current_catcher;
 
+#if GDB_XCPT == GDB_XCPT_SJMP
+
 /* Return length of current_catcher list.  */
 
 static int
@@ -73,6 +73,8 @@ catcher_list_size (void)
   return size;
 }
 
+#endif
+
 jmp_buf *
 exceptions_state_mc_init (void)
 {
@@ -193,8 +195,8 @@ exceptions_state_mc_catch (struct gdb_exception *exception,
 	}
 
       /* The caller didn't request that the event be caught, relay the
-	 event to the next exception_catch/CATCH.  */
-      throw_exception (*exception);
+	 event to the next exception_catch/CATCH_SJLJ.  */
+      throw_exception_sjlj (*exception);
     }
 
   /* No exception was thrown.  */
@@ -213,7 +215,7 @@ exceptions_state_mc_action_iter_1 (void)
   return exceptions_state_mc (CATCH_ITER_1);
 }
 
-#else /* !GDB_XCPT_SJMP */
+#if GDB_XCPT != GDB_XCPT_SJMP
 
 /* How many nested TRY blocks we have.  See exception_messages and
    throw_it.  */
@@ -265,18 +267,27 @@ gdb_exception_sliced_copy (struct gdb_exception *to, const struct gdb_exception
 /* Return EXCEPTION to the nearest containing catch_errors().  */
 
 void
-throw_exception (struct gdb_exception exception)
+throw_exception_sjlj (struct gdb_exception exception)
 {
   do_cleanups (all_cleanups ());
 
-#if GDB_XCPT == GDB_XCPT_SJMP
   /* Jump to the containing catch_errors() call, communicating REASON
      to that call via setjmp's return value.  Note that REASON can't
      be zero, by definition in defs.h.  */
   exceptions_state_mc (CATCH_THROWING);
   current_catcher->exception = exception;
   longjmp (current_catcher->buf, exception.reason);
-#else
+}
+
+#if GDB_XCPT != GDB_XCPT_SJMP
+
+/* Implementation of throw_exception that uses C++ try/catch.  */
+
+static ATTRIBUTE_NORETURN void
+throw_exception_cxx (struct gdb_exception exception)
+{
+  do_cleanups (all_cleanups ());
+
   if (exception.reason == RETURN_QUIT)
     {
       gdb_exception_RETURN_MASK_QUIT ex;
@@ -293,6 +304,17 @@ throw_exception (struct gdb_exception exception)
     }
   else
     gdb_assert_not_reached ("invalid return reason");
+}
+
+#endif
+
+void
+throw_exception (struct gdb_exception exception)
+{
+#if GDB_XCPT == GDB_XCPT_SJMP
+  throw_exception_sjlj (exception);
+#else
+  throw_exception_cxx (exception);
 #endif
 }
 
diff --git a/gdb/event-top.c b/gdb/event-top.c
index b9947c4..f43fd0a 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -145,15 +145,98 @@ static struct async_signal_handler *async_sigterm_token;
 void (*after_char_processing_hook) (void);
 \f
 
-/* Wrapper function for calling into the readline library.  The event
-   loop expects the callback function to have a paramter, while
-   readline expects none.  */
+/* Wrapper function for calling into the readline library.  This takes
+   care of a couple things:
+
+   - The event loop expects the callback function to have a parameter,
+     while readline expects none.
+
+   - Propagation of GDB exceptions/errors thrown from INPUT_HANDLER
+     across readline requires special handling.
+
+   On the exceptions issue:
+
+   DWARF-based unwinding cannot cross code built without -fexceptions.
+   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.
+
+   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
+   code throws a C++ exception.  Turns out this is exactly what
+   happens with GDB's readline callback.
+
+   In such cases, we must catch and save any C++ exception that might
+   be thrown from the GDB callback before returning to the
+   non-EH-aware code.  When the non-EH-aware function itself returns
+   back to GDB, we then rethrow the original C++ exception.
+
+   In the readline case however, the right thing to do is to longjmp
+   out of the callback, rather than do a normal return -- there's no
+   way for the callback to return to readline an indication that an
+   error happened, so a normal return would have rl_callback_read_char
+   potentially continue processing further input, redisplay the
+   prompt, etc.  Instead of raw setjmp/longjmp however, we use our
+   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.  */
+
 static void
 gdb_rl_callback_read_char_wrapper (gdb_client_data client_data)
 {
-  rl_callback_read_char ();
-  if (after_char_processing_hook)
-    (*after_char_processing_hook) ();
+  struct gdb_exception gdb_expt = exception_none;
+
+  /* C++ exceptions can't normally be thrown across readline (unless
+     it is built with -fexceptions, but it won't by default on many
+     ABIs).  So we instead wrap the readline call with a sjlj-based
+     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) ();
+    }
+  CATCH_SJLJ (ex, RETURN_MASK_ALL)
+    {
+      gdb_expt = ex;
+    }
+  END_CATCH_SJLJ
+
+  /* Rethrow using the normal EH mechanism.  */
+  if (gdb_expt.reason < 0)
+    throw_exception (gdb_expt);
+}
+
+/* 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.  */
+
+static void
+gdb_rl_callback_handler (char *rl)
+{
+  struct gdb_exception gdb_rl_expt = exception_none;
+
+  TRY
+    {
+      input_handler (rl);
+    }
+  CATCH (ex, RETURN_MASK_ALL)
+    {
+      gdb_rl_expt = ex;
+    }
+  END_CATCH
+
+  /* If we caught a GDB exception, longjmp out of the readline
+     callback.  There's no other way for the callback to signal to
+     readline that an error happened.  A normal return would have
+     readline potentially continue processing further input, redisplay
+     the prompt, etc.  (This is what GDB historically did when it was
+     a C program.)  Note that since we're long jumping, local variable
+     dtors are NOT run automatically.  */
+  if (gdb_rl_expt.reason < 0)
+    throw_exception_sjlj (gdb_rl_expt);
 }
 
 /* Initialize all the necessary variables, start the event loop,
@@ -236,7 +319,7 @@ gdb_rl_callback_handler_install (const char *prompt)
      therefore loses input.  */
   gdb_assert (!callback_handler_installed);
 
-  rl_callback_handler_install (prompt, input_handler);
+  rl_callback_handler_install (prompt, gdb_rl_callback_handler);
   callback_handler_installed = 1;
 }
 
-- 
2.5.5

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] Propagate GDB/C++ exceptions across readline using sj/lj-based TRY/CATCH
  2016-04-21 23:50 ` [PATCH 2/3] Propagate GDB/C++ exceptions across readline using sj/lj-based TRY/CATCH Pedro Alves
@ 2016-04-22  8:19   ` Yao Qi
  2016-04-22  9:05     ` Pedro Alves
  0 siblings, 1 reply; 7+ messages in thread
From: Yao Qi @ 2016-04-22  8:19 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves <palves@redhat.com> writes:

Hi Pedro,
this series is fine with me.  I tested them on arm/fedora19, and the
exception can be correctly handled.

> This turns out to be a much better looking fix than my bespoke API
> attempt, even.  We'll probably be able to simplify TRY_SJLJ/CATCH_SJLJ
> when we finally get rid of TRY/CATCH all over the tree, but until

I don't see anything we can simplify in TRY_SJLJ/CATCH_SJLJ.  Can you
elaborate please?

> then, this reuse seems quite nice for avoiding a second parallel
> setjmp/longjmp mechanism.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] Propagate GDB/C++ exceptions across readline using sj/lj-based TRY/CATCH
  2016-04-22  8:19   ` Yao Qi
@ 2016-04-22  9:05     ` Pedro Alves
  2016-04-22 15:25       ` Pedro Alves
  0 siblings, 1 reply; 7+ messages in thread
From: Pedro Alves @ 2016-04-22  9:05 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 04/22/2016 09:17 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
> Hi Pedro,
> this series is fine with me.  I tested them on arm/fedora19, and the
> exception can be correctly handled.

Great, thanks.

>> This turns out to be a much better looking fix than my bespoke API
>> attempt, even.  We'll probably be able to simplify TRY_SJLJ/CATCH_SJLJ
>> when we finally get rid of TRY/CATCH all over the tree, but until
> 
> I don't see anything we can simplify in TRY_SJLJ/CATCH_SJLJ.  Can you
> elaborate please?

When we get to the point when we longer need TRY/CATCH for regular 
exception handling it means we'll no longer need TRY/CATCH to
manage and run cleanups, as all cleaning up will be done by
RAII / destructors.

So the bits in TRY_SLJLJ/CATCH_SJLJ that manage the cleanup chain and 
the bit in throw_exception_sjlj that runs cleanups can all disappear.

Also, since we always want RETURN_MASK_ALL, the second parameter
of CATCH_SJLJ can be eliminated, along with all the return_mask
support code.  E.g., since CATCH_SJLJ will always catch all
exceptions, exceptions_state_mc_catch becomes simply
something like:

void
exceptions_state_mc_catch (struct gdb_exception *exception)
{
  *exception = current_catcher->exception;
  catcher_pop ();
}

... which may itself expose further possible simplifications,
like e.g., moving catch_pop to the caller directly.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] Propagate GDB/C++ exceptions across readline using sj/lj-based TRY/CATCH
  2016-04-22  9:05     ` Pedro Alves
@ 2016-04-22 15:25       ` Pedro Alves
  0 siblings, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2016-04-22 15:25 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 04/22/2016 10:05 AM, Pedro Alves wrote:
> On 04/22/2016 09:17 AM, Yao Qi wrote:
>> Pedro Alves <palves@redhat.com> writes:
>>
>> Hi Pedro,
>> this series is fine with me.  I tested them on arm/fedora19, and the
>> exception can be correctly handled.
> 
> Great, thanks.

I've applied this now.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-04-22 15:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-21 23:50 [PATCH 0/3] Fix C++ exceptions across readline result in std::terminate() -> SIGABRT Pedro Alves
2016-04-21 23:50 ` [PATCH 1/3] Rename rl_callback_read_char_wrapper -> gdb_rl_callback_read_char Pedro Alves
2016-04-21 23:50 ` [PATCH 3/3] Switch gdb's TRY/CATCH to C++ try/catch Pedro Alves
2016-04-21 23:50 ` [PATCH 2/3] Propagate GDB/C++ exceptions across readline using sj/lj-based TRY/CATCH Pedro Alves
2016-04-22  8:19   ` Yao Qi
2016-04-22  9:05     ` Pedro Alves
2016-04-22 15:25       ` 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).