Don't assume break/continue inside a TRY block works

Message ID 562FB0E6.2050001@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves Oct. 27, 2015, 5:14 p.m. UTC
  On 10/27/2015 05:11 PM, Pedro Alves wrote:
> In C++, this:
> 
> 	try
> 	  {
> 	    break;
> 	  }
> 	catch ()
> 	  {}
> 
> is invalid.  However, because our TRY/CATCH macros support it in C,
> the C++ version of those macros support it too.  To catch such
> assumptions, this adds a (disabled) hack that maps TRY/CATCH to raw
> C++ try/catch.  Then it goes through all instances that building on
> x86_64 GNU/Linux trips on, fixing them.
> 
> This isn't strictly necessary yet, but I think it's nicer to try to
> keep the tree in a state where it's easier to eliminate the TRY/CATCH
> macros.
> 
> gdb/ChangeLog:
> 2015-10-27  Pedro Alves  <palves@redhat.com>
> 
> 	* dwarf2-frame-tailcall.c (dwarf2_tailcall_sniffer_first): Don't
> 	assume that "break" breaks out of a TRY/CATCH.
> 	* python/py-framefilter.c (py_print_single_arg): Don't assume
> 	"continue" breaks out of a TRY/CATCH.
> 	* python/py-value.c (valpy_binop_throw): New function, factored
> 	out from ...
> 	(valpy_binop): ... this.
> 	(valpy_richcompare_throw): New function, factored
> 	out from ...
> 	(valpy_richcompare): ... this.
> 	* solib.c (solib_read_symbols): Don't assume "break" breaks out
> 	of a TRY/CATCH.
> 	* common/common-exceptions.h [USE_RAW_CXX_TRY]
> 	<TRY/CATCH/END_CATCH>: Define as 1-1 wrappers around try/catch.

Here's a "git diff -w" version, which is easier to read.
  

Patch

diff --git c/gdb/common/common-exceptions.h w/gdb/common/common-exceptions.h
index 2e6a6d9..51795b1 100644
--- c/gdb/common/common-exceptions.h
+++ w/gdb/common/common-exceptions.h
@@ -195,6 +195,14 @@  struct exception_try_scope
   void *saved_state;
 };

+/* Define this 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 USE_RAW_CXX_TRY
+
+#ifndef USE_RAW_CXX_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
@@ -216,6 +224,14 @@  struct exception_try_scope
   {						\
     exception_rethrow ();			\
   }
+#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
diff --git c/gdb/dwarf2-frame-tailcall.c w/gdb/dwarf2-frame-tailcall.c
index 952bc14..613cc34 100644
--- c/gdb/dwarf2-frame-tailcall.c
+++ w/gdb/dwarf2-frame-tailcall.c
@@ -386,14 +386,16 @@  dwarf2_tailcall_sniffer_first (struct frame_info *this_frame,
       /* call_site_find_chain can throw an exception.  */
       chain = call_site_find_chain (prev_gdbarch, prev_pc, this_pc);

-      if (entry_cfa_sp_offsetp == NULL)
-	break;
+      if (entry_cfa_sp_offsetp != NULL)
+	{
 	  sp_regnum = gdbarch_sp_regnum (prev_gdbarch);
-      if (sp_regnum == -1)
-	break;
+	  if (sp_regnum != -1)
+	    {
 	      prev_sp = frame_unwind_register_unsigned (this_frame, sp_regnum);
 	      prev_sp_p = 1;
 	    }
+	}
+    }
   CATCH (except, RETURN_MASK_ERROR)
     {
       if (entry_values_debug)
diff --git c/gdb/python/py-framefilter.c w/gdb/python/py-framefilter.c
index ac97723..0f8dc62 100644
--- c/gdb/python/py-framefilter.c
+++ w/gdb/python/py-framefilter.c
@@ -448,10 +448,11 @@  py_print_single_arg (struct ui_out *out,
 	    {
 	      retval = EXT_LANG_BT_ERROR;
 	      do_cleanups (cleanups);
-	      continue;
 	    }
 	}

+      if (retval != EXT_LANG_BT_ERROR)
+	{
 	  if (val != NULL)
 	    annotate_arg_value (value_type (val));

@@ -480,6 +481,7 @@  py_print_single_arg (struct ui_out *out,

 	  do_cleanups (cleanups);
 	}
+    }
   CATCH (except, RETURN_MASK_ERROR)
     {
       gdbpy_convert_exception (except);
diff --git c/gdb/python/py-value.c w/gdb/python/py-value.c
index ac6b224..140aaf5 100644
--- c/gdb/python/py-value.c
+++ w/gdb/python/py-value.c
@@ -1018,38 +1018,38 @@  enum valpy_opcode
 #define STRIP_REFERENCE(TYPE) \
   ((TYPE_CODE (TYPE) == TYPE_CODE_REF) ? (TYPE_TARGET_TYPE (TYPE)) : (TYPE))

-/* Returns a value object which is the result of applying the operation
-   specified by OPCODE to the given arguments.  Returns NULL on error, with
-   a python exception set.  */
+/* Helper for valpy_binop.  Returns a value object which is the result
+   of applying the operation specified by OPCODE to the given
+   arguments.  Throws a GDB exception on error.  */
+
 static PyObject *
-valpy_binop (enum valpy_opcode opcode, PyObject *self, PyObject *other)
+valpy_binop_throw (enum valpy_opcode opcode, PyObject *self, PyObject *other)
 {
   PyObject *result = NULL;

-  TRY
-    {
   struct value *arg1, *arg2;
   struct cleanup *cleanup = make_cleanup_value_free_to_mark (value_mark ());
   struct value *res_val = NULL;
   enum exp_opcode op = OP_NULL;
   int handled = 0;

-      /* If the gdb.Value object is the second operand, then it will be passed
-	 to us as the OTHER argument, and SELF will be an entirely different
-	 kind of object, altogether.  Because of this, we can't assume self is
-	 a gdb.Value object and need to convert it from python as well.  */
+  /* If the gdb.Value object is the second operand, then it will be
+     passed to us as the OTHER argument, and SELF will be an entirely
+     different kind of object, altogether.  Because of this, we can't
+     assume self is a gdb.Value object and need to convert it from
+     python as well.  */
   arg1 = convert_value_from_python (self);
   if (arg1 == NULL)
     {
       do_cleanups (cleanup);
-	  break;
+      return NULL;
     }

   arg2 = convert_value_from_python (other);
   if (arg2 == NULL)
     {
       do_cleanups (cleanup);
-	  break;
+      return NULL;
     }

   switch (opcode)
@@ -1145,6 +1145,20 @@  valpy_binop (enum valpy_opcode opcode, PyObject *self, PyObject *other)
     result = value_to_value_object (res_val);

   do_cleanups (cleanup);
+  return result;
+}
+
+/* Returns a value object which is the result of applying the operation
+   specified by OPCODE to the given arguments.  Returns NULL on error, with
+   a python exception set.  */
+static PyObject *
+valpy_binop (enum valpy_opcode opcode, PyObject *self, PyObject *other)
+{
+  PyObject *result = NULL;
+
+  TRY
+    {
+      result = valpy_binop_throw (opcode, self, other);
     }
   CATCH (except, RETURN_MASK_ALL)
     {
@@ -1351,66 +1365,49 @@  valpy_xor (PyObject *self, PyObject *other)
   return valpy_binop (VALPY_BITXOR, self, other);
 }

-/* Implements comparison operations for value objects.  Returns NULL on error,
-   with a python exception set.  */
-static PyObject *
-valpy_richcompare (PyObject *self, PyObject *other, int op)
-{
-  int result = 0;
-
-  if (other == Py_None)
-    /* Comparing with None is special.  From what I can tell, in Python
-       None is smaller than anything else.  */
-    switch (op) {
-      case Py_LT:
-      case Py_LE:
-      case Py_EQ:
-	Py_RETURN_FALSE;
-      case Py_NE:
-      case Py_GT:
-      case Py_GE:
-	Py_RETURN_TRUE;
-      default:
-	/* Can't happen.  */
-	PyErr_SetString (PyExc_NotImplementedError,
-			 _("Invalid operation on gdb.Value."));
-	return NULL;
-    }
+/* Helper for valpy_richcompare.  Implements comparison operations for
+   value objects.  Returns true/false on success.  Returns -1 with a
+   Python exception set if a Python error is detected.  Throws a GDB
+   exception on other errors (memory error, etc.).  */

-  TRY
+static int
+valpy_richcompare_throw (PyObject *self, PyObject *other, int op)
 {
-      struct value *value_other, *mark = value_mark ();
+  int result;
+  struct value *value_other;
+  struct value *value_self;
+  struct value *mark = value_mark ();
   struct cleanup *cleanup;

   value_other = convert_value_from_python (other);
   if (value_other == NULL)
-	{
-	  result = -1;
-	  break;
-	}
+    return -1;

   cleanup = make_cleanup_value_free_to_mark (mark);

-      switch (op) {
+  value_self = ((value_object *) self)->value;
+
+  switch (op)
+    {
     case Py_LT:
-	  result = value_less (((value_object *) self)->value, value_other);
+      result = value_less (value_self, value_other);
       break;
     case Py_LE:
-	  result = value_less (((value_object *) self)->value, value_other)
-	    || value_equal (((value_object *) self)->value, value_other);
+      result = value_less (value_self, value_other)
+	|| value_equal (value_self, value_other);
       break;
     case Py_EQ:
-	  result = value_equal (((value_object *) self)->value, value_other);
+      result = value_equal (value_self, value_other);
       break;
     case Py_NE:
-	  result = !value_equal (((value_object *) self)->value, value_other);
+      result = !value_equal (value_self, value_other);
       break;
     case Py_GT:
-	  result = value_less (value_other, ((value_object *) self)->value);
+      result = value_less (value_other, value_self);
       break;
     case Py_GE:
-	  result = value_less (value_other, ((value_object *) self)->value)
-	    || value_equal (((value_object *) self)->value, value_other);
+      result = (value_less (value_other, value_self)
+		|| value_equal (value_self, value_other));
       break;
     default:
       /* Can't happen.  */
@@ -1421,6 +1418,39 @@  valpy_richcompare (PyObject *self, PyObject *other, int op)
     }

   do_cleanups (cleanup);
+  return result;
+}
+
+
+/* Implements comparison operations for value objects.  Returns NULL on error,
+   with a python exception set.  */
+static PyObject *
+valpy_richcompare (PyObject *self, PyObject *other, int op)
+{
+  int result = 0;
+
+  if (other == Py_None)
+    /* Comparing with None is special.  From what I can tell, in Python
+       None is smaller than anything else.  */
+    switch (op) {
+      case Py_LT:
+      case Py_LE:
+      case Py_EQ:
+	Py_RETURN_FALSE;
+      case Py_NE:
+      case Py_GT:
+      case Py_GE:
+	Py_RETURN_TRUE;
+      default:
+	/* Can't happen.  */
+	PyErr_SetString (PyExc_NotImplementedError,
+			 _("Invalid operation on gdb.Value."));
+	return NULL;
+    }
+
+  TRY
+    {
+      result = valpy_richcompare_throw (self, other, op);
     }
   CATCH (except, RETURN_MASK_ALL)
     {
diff --git c/gdb/solib.c w/gdb/solib.c
index ca2c9ab..d715897 100644
--- c/gdb/solib.c
+++ w/gdb/solib.c
@@ -696,9 +696,8 @@  solib_read_symbols (struct so_list *so, int flags)
 		  && so->objfile->addr_low == so->addr_low)
 		break;
 	    }
-	  if (so->objfile != NULL)
-	    break;
-
+	  if (so->objfile == NULL)
+	    {
 	      sap = build_section_addr_info_from_section_table (so->sections,
 								so->sections_end);
 	      so->objfile = symbol_file_add_from_bfd (so->abfd, so->so_name,
@@ -706,6 +705,7 @@  solib_read_symbols (struct so_list *so, int flags)
 						      NULL);
 	      so->objfile->addr_low = so->addr_low;
 	      free_section_addr_info (sap);
+	    }

 	  so->symbols_loaded = 1;
 	}