[v2,14/22] Simplify exception handling

Message ID 20190227201849.32210-15-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Feb. 27, 2019, 8:18 p.m. UTC
  Now that cleanups have been removed, TRY/CATCH can't be SJLJ-based any
more.  This patch simplifies the exception handling code, by removing
the non-working variants.

Note that the "pure" C++ exception handling code is removed as well; I
think the route forward must be to change exceptions to be
self-destructing, so that try_scope_depth can simply be removed.

Some longjmp-based code remains, as it is needed to throw an exception
through readline.

gdb/ChangeLog
2019-02-27  Tom Tromey  <tom@tromey.com>

	* common/common-exceptions.h (GDB_XCPT_SJMP, GDB_XCPT_TRY)
	(GDB_XCPT_RAW_TRY, GDB_XCPT): Remove.
	(TRY, CATCH, END_CATCH): Remove some definitions.
	* common/common-exceptions.c: Don't use GDB_XCPT.
	(catcher_list_size): Remove.
	(throw_exception, throw_it): Simplify.
---
 gdb/ChangeLog                  |  9 +++++++
 gdb/common/common-exceptions.c | 36 --------------------------
 gdb/common/common-exceptions.h | 46 +---------------------------------
 3 files changed, 10 insertions(+), 81 deletions(-)
  

Comments

Pedro Alves April 2, 2019, 10:07 p.m. UTC | #1
On 02/27/2019 08:18 PM, Tom Tromey wrote:
> Now that cleanups have been removed, TRY/CATCH can't be SJLJ-based any
> more.  This patch simplifies the exception handling code, by removing
> the non-working variants.
> 
> Note that the "pure" C++ exception handling code is removed as well; 

That's fine, if we're getting rid of TRY/CATCH.

That mode is/was useful to catch issues like these:
 https://sourceware.org/ml/gdb-patches/2017-10/msg00080.html
but if we're getting rid of TRY/CATCH, then it won't be
needed anymore, of course.

> I
> think the route forward must be to change exceptions to be
> self-destructing, so that try_scope_depth can simply be removed.

Agreed.

> 
> Some longjmp-based code remains, as it is needed to throw an exception
> through readline.
> 
> gdb/ChangeLog
> 2019-02-27  Tom Tromey  <tom@tromey.com>
> 
> 	* common/common-exceptions.h (GDB_XCPT_SJMP, GDB_XCPT_TRY)
> 	(GDB_XCPT_RAW_TRY, GDB_XCPT): Remove.
> 	(TRY, CATCH, END_CATCH): Remove some definitions.
> 	* common/common-exceptions.c: Don't use GDB_XCPT.
> 	(catcher_list_size): Remove.
> 	(throw_exception, throw_it): Simplify.

Looks fine.  I spotted a few comments that need updating,
even after the whole series is in, but no need to stress about it.
It's fine to tackle it with a final pass after the series.

I have comments for the other patches, will send them as soon
as possible.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/common/common-exceptions.c b/gdb/common/common-exceptions.c
index 4e67e898bda..c3529c989fd 100644
--- a/gdb/common/common-exceptions.c
+++ b/gdb/common/common-exceptions.c
@@ -54,26 +54,6 @@  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
-catcher_list_size (void)
-{
-  int size;
-  struct catcher *catcher;
-
-  for (size = 0, catcher = current_catcher;
-       catcher != NULL;
-       catcher = catcher->prev)
-    ++size;
-
-  return size;
-}
-
-#endif
-
 jmp_buf *
 exceptions_state_mc_init (void)
 {
@@ -205,8 +185,6 @@  exceptions_state_mc_action_iter_1 (void)
   return exceptions_state_mc (CATCH_ITER_1);
 }
 
-#if GDB_XCPT != GDB_XCPT_SJMP
-
 /* How many nested TRY blocks we have.  See exception_messages and
    throw_it.  */
 
@@ -248,8 +226,6 @@  gdb_exception_sliced_copy (struct gdb_exception *to, const struct gdb_exception
   *to = *from;
 }
 
-#endif /* !GDB_XCPT_SJMP */
-
 /* Return EXCEPTION to the nearest containing CATCH_SJLJ block.  */
 
 void
@@ -263,8 +239,6 @@  throw_exception_sjlj (struct gdb_exception exception)
   longjmp (current_catcher->buf, exception.reason);
 }
 
-#if GDB_XCPT != GDB_XCPT_SJMP
-
 /* Implementation of throw_exception that uses C++ try/catch.  */
 
 static ATTRIBUTE_NORETURN void
@@ -288,16 +262,10 @@  throw_exception_cxx (struct gdb_exception exception)
     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
 }
 
 /* A stack of exception messages.
@@ -321,11 +289,7 @@  throw_it (enum return_reason reason, enum errors error, const char *fmt,
 {
   struct gdb_exception e;
   char *new_message;
-#if GDB_XCPT == GDB_XCPT_SJMP
-  int depth = catcher_list_size ();
-#else
   int depth = try_scope_depth;
-#endif
 
   gdb_assert (depth > 0);
 
diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h
index 471e7c5cf81..6cc09eab938 100644
--- a/gdb/common/common-exceptions.h
+++ b/gdb/common/common-exceptions.h
@@ -115,22 +115,6 @@  struct gdb_exception
   const char *message;
 };
 
-/* The different exception mechanisms that TRY/CATCH can map to.  */
-
-/* Make GDB exceptions use setjmp/longjmp behind the scenes.  */
-#define GDB_XCPT_SJMP 1
-
-/* 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
-   try/catch.  GDB won't work correctly, but building that way catches
-   code tryin to break/continue out of the try block, along with
-   spurious code between the TRY and the CATCH block.  */
-#define GDB_XCPT_RAW_TRY 3
-
-#define GDB_XCPT GDB_XCPT_TRY
-
 /* 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
@@ -141,13 +125,11 @@  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);
 
-/* Same, but for the C++ try/catch-based TRY/CATCH mechanism.  */
+/* For the C++ try/catch-based TRY/CATCH mechanism.  */
 
-#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) ATTRIBUTE_NORETURN;
-#endif
 
 /* Macro to wrap up standard try/catch behavior.
 
@@ -196,19 +178,6 @@  extern void exception_rethrow (void) ATTRIBUTE_NORETURN;
 #define END_CATCH_SJLJ				\
   }
 
-#if GDB_XCPT == GDB_XCPT_SJMP
-
-/* If using SJLJ-based exceptions for all exceptions, then provide
-   standard aliases.  */
-
-#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
-
 /* Prevent error/quit during TRY from calling cleanups established
    prior to here.  This pops out the scope in either case of normal
    exit or exception exit.  */
@@ -226,8 +195,6 @@  struct exception_try_scope
   void *saved_state;
 };
 
-#if GDB_XCPT == GDB_XCPT_TRY
-
 /* We still need to wrap TRY/CATCH in C++ so that cleanups and C++
    exceptions can coexist.
 
@@ -263,15 +230,6 @@  struct exception_try_scope
       }						\
   }
 
-#else
-
-#define TRY try
-#define CATCH(EXCEPTION, MASK) \
-  catch (struct gdb_exception ## _ ## MASK &EXCEPTION)
-#define END_CATCH
-
-#endif
-
 /* The exception types client code may catch.  They're just shims
    around gdb_exception that add nothing but type info.  Which is used
    is selected depending on the MASK argument passed to CATCH.  */
@@ -288,8 +246,6 @@  struct gdb_exception_RETURN_MASK_QUIT : public gdb_exception_RETURN_MASK_ALL
 {
 };
 
-#endif /* GDB_XCPT_TRY || GDB_XCPT_RAW_TRY */
-
 /* An exception type that inherits from both std::bad_alloc and a gdb
    exception.  This is necessary because operator new can only throw
    std::bad_alloc, and OTOH, we want exceptions thrown due to memory