Patchwork [pushed] Add missing-END_CATCH detection/protection (to gdb's TRY/CATCH/END_CATCH)

login
register
mail settings
Submitter Pedro Alves
Date Oct. 4, 2017, 10:04 a.m.
Message ID <1507111485-6319-1-git-send-email-palves@redhat.com>
Download mbox | patch
Permalink /patch/23321/
State New
Headers show

Comments

Pedro Alves - Oct. 4, 2017, 10:04 a.m.
While we still have cleanups (i.e., make_cleanup & co), we must be
sure to add END_CATCH at the end of a TRY/CATCH/END_CATCH construct.
However, it's currently too easy to miss adding the END_CATCH, because
the code compiles anyway without it.  I realized this when I noticed
that another patch I was working on missed several adding END_CATCH in
several new TRY/CATCH uses.

This commit fixes that by making TRY open a new scope that is only
closed by END_CATCH.  This way, if you forget to add the END_CATCH,
then compilation fails due to the unbalanced curly braces.

This caught a couple places where we were missing END_CATCH in current
master, also fixed by the patch.

gdb/ChangeLog:
2017-10-04  Pedro Alves  <palves@redhat.com>

	* cli/cli-cmds.c (complete_command): Add missing END_CATCH.
	* common/common-exceptions.h (TRY): Open an outermost scope.
	Expand intro comment.
	(CATCH): Reindent.
	(END_CATCH): Close the outermost scope.
	* completer.c (complete_line_internal): Add missing END_CATCH.
---
 gdb/cli/cli-cmds.c             |  1 +
 gdb/common/common-exceptions.h | 41 +++++++++++++++++++++++++++--------------
 gdb/completer.c                |  1 +
 3 files changed, 29 insertions(+), 14 deletions(-)

Patch

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index dad5ffa..51271ad 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -296,6 +296,7 @@  complete_command (char *arg_entry, int from_tty)
     {
       return;
     }
+  END_CATCH
 
   std::string arg_prefix (arg, word - arg);
 
diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h
index 49f309f..b39abc0 100644
--- a/gdb/common/common-exceptions.h
+++ b/gdb/common/common-exceptions.h
@@ -231,25 +231,38 @@  struct exception_try_scope
 #if GDB_XCPT == GDB_XCPT_TRY
 
 /* We still need to wrap TRY/CATCH in C++ so that cleanups and C++
-   exceptions can coexist.  The TRY blocked is wrapped in a
-   do/while(0) so that break/continue within the block works the same
-   as in C.  */
+   exceptions can coexist.
+
+   The TRY blocked is wrapped in a do/while(0) so that break/continue
+   within the block works the same as in C.
+
+   END_CATCH makes sure that even if the CATCH block doesn't want to
+   catch the exception, we stop at every frame in the unwind chain to
+   run its cleanups, which may e.g., have pointers to stack variables
+   that are going to be destroyed.
+
+   There's an outer scope around the whole TRY/END_CATCH in order to
+   cause a compilation error if you forget to add the END_CATCH at the
+   end a TRY/CATCH construct.  */
+
 #define TRY								\
-  try									\
-    {									\
-      exception_try_scope exception_try_scope_instance;			\
-      do								\
-	{
+  {									\
+    try									\
+      {									\
+	exception_try_scope exception_try_scope_instance;		\
+	do								\
+	  {
 
 #define CATCH(EXCEPTION, MASK)						\
-	} while (0);							\
-    }								        \
-  catch (struct gdb_exception ## _ ## MASK &EXCEPTION)
+	  } while (0);							\
+	}								\
+    catch (struct gdb_exception ## _ ## MASK &EXCEPTION)
 
 #define END_CATCH				\
-  catch (...)					\
-  {						\
-    exception_rethrow ();			\
+    catch (...)					\
+      {						\
+	exception_rethrow ();			\
+      }						\
   }
 
 #else
diff --git a/gdb/completer.c b/gdb/completer.c
index 3022333..cd0ecc3 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -1447,6 +1447,7 @@  complete_line_internal (completion_tracker &tracker,
       if (except.error != MAX_COMPLETIONS_REACHED_ERROR)
 	throw_exception (except);
     }
+  END_CATCH
 }
 
 /* See completer.h.  */