Re: [C++ PATCH] Sanity check __cxa_* user declarations (PR c++/88482)

[Jakub Jelinek <jakub at redhat dot com>]

On Sat, Dec 15, 2018 at 12:22:44PM +0100, Jakub Jelinek wrote:
> > Fix libitm.
> 
> Ok, will do.

Hre it is in patch form:

2018-12-15  Jakub Jelinek  <jakub@redhat.com>

	PR c++/88482
	* except.c (verify_library_fn): New function.
	(declare_library_fn): Use it.  Initialize TM even if the non-TM
	library function has been user declared.
	(do_end_catch): Don't set TREE_NOTHROW on error_mark_node.
	(expand_start_catch_block): Don't call initialize_handler_parm
	for error_mark_node.
	(build_throw): Use verify_library_fn.  Initialize TM even if the
	non-TM library function has been user declared.  Don't crash if
	any library fn is error_mark_node.

	* g++.dg/eh/builtin5.C: New test.
	* g++.dg/eh/builtin6.C: New test.
	* g++.dg/eh/builtin7.C: New test.
	* g++.dg/eh/builtin8.C: New test.
	* g++.dg/eh/builtin9.C: New test.
	* g++.dg/eh/builtin10.C: New test.
	* g++.dg/eh/builtin11.C: New test.
	* g++.dg/parse/crash55.C: Adjust expected diagnostics.

	* eh_cpp.cc (__cxa_throw): Change DEST argument type from
	void * to void (*) (void *).
	(_ITM_cxa_throw): Likewise.
	* libitm.h (_ITM_cxa_throw): Likewise.
	* libitm.texi (_ITM_cxa_throw): Likewise.

--- gcc/cp/except.c.jj	2018-12-14 00:32:56.042200543 +0100
+++ gcc/cp/except.c	2018-12-15 13:39:29.301459729 +0100
@@ -132,6 +132,49 @@ build_exc_ptr (void)
 		       1, integer_zero_node);
 }
 
+/* Check that user declared function FN is a function and has return
+   type RTYPE and argument types ARG{1,2,3}TYPE.  */
+
+static bool
+verify_library_fn (tree fn, const char *name, tree rtype,
+		   tree arg1type, tree arg2type, tree arg3type)
+{
+  if (TREE_CODE (fn) != FUNCTION_DECL
+      || TREE_CODE (TREE_TYPE (fn)) != FUNCTION_TYPE)
+    {
+  bad:
+      error_at (DECL_SOURCE_LOCATION (fn), "%qs declared incorrectly", name);
+      return false;
+    }
+  tree fntype = TREE_TYPE (fn);
+  if (!same_type_p (TREE_TYPE (fntype), rtype))
+    goto bad;
+  tree targs = TYPE_ARG_TYPES (fntype);
+  tree args[3] = { arg1type, arg2type, arg3type };
+  for (int i = 0; i < 3 && args[i]; i++)
+    {
+      if (targs == NULL_TREE)
+	goto bad;
+      if (!same_type_p (TREE_VALUE (targs), args[i]))
+	{
+	  if (i == 0)
+	    goto bad;
+	  /* Be less strict for second and following arguments, __cxa_throw
+	     needs to be more permissive.  */
+	  if (TYPE_PTROBV_P (TREE_VALUE (targs)) && TYPE_PTROBV_P (args[i]))
+	    /* Both object pointers.  */;
+	  else if (TYPE_PTRFN_P (TREE_VALUE (targs)) && TYPE_PTRFN_P (args[i]))
+	    /* Both function pointers.  */;
+	  else
+	    goto bad;
+	}
+      targs = TREE_CHAIN (targs);
+    }
+  if (targs != void_list_node)
+    goto bad;
+  return true;
+}
+
 /* Find or declare a function NAME, returning RTYPE, taking a single
    parameter PTYPE, with an empty exception specification. ECF are the
    library fn flags.  If TM_ECF is non-zero, also find or create a
@@ -148,21 +191,39 @@ declare_library_fn (const char *name, tr
 {
   tree ident = get_identifier (name);
   tree res = get_global_binding (ident);
+  tree fntype = NULL_TREE;
+  tree except = NULL_TREE;
   if (!res)
     {
-      tree type = build_function_type_list (rtype, ptype, NULL_TREE);
-      tree except = ecf & ECF_NOTHROW ? empty_except_spec : NULL_TREE;
-      res = push_library_fn (ident, type, except, ecf);
-      if (tm_ecf && flag_tm)
+      fntype = build_function_type_list (rtype, ptype, NULL_TREE);
+      if (ecf & ECF_NOTHROW)
+	except = empty_except_spec;
+      res = push_library_fn (ident, fntype, except, ecf);
+    }
+  else if (!verify_library_fn (res, name, rtype, ptype, NULL_TREE, NULL_TREE))
+    return error_mark_node;
+
+  if (tm_ecf && flag_tm)
+    {
+      char *tm_name = concat ("_ITM_", name + 2, NULL_TREE);
+      tree tm_ident = get_identifier (tm_name);
+      tree tm_fn = get_global_binding (tm_ident);
+      if (!tm_fn)
 	{
-	  char *tm_name = concat ("_ITM_", name + 2, NULL_TREE);
-	  tree tm_ident = get_identifier (tm_name);
-	  free (tm_name);
-	  tree tm_fn = get_global_binding (tm_ident);
-	  if (!tm_fn)
-	    tm_fn = push_library_fn (tm_ident, type, except, ecf | tm_ecf);
-	  record_tm_replacement (res, tm_fn);
+	  if (!fntype)
+	    {
+	      fntype = build_function_type_list (rtype, ptype, NULL_TREE);
+	      if (ecf & ECF_NOTHROW)
+		except = empty_except_spec;
+	    }
+	  tm_fn = push_library_fn (tm_ident, fntype, except, ecf | tm_ecf);
 	}
+      else if (!verify_library_fn (tm_fn, tm_name, rtype, ptype,
+				   NULL_TREE, NULL_TREE))
+	tm_fn = error_mark_node;
+      free (tm_name);
+      if (tm_fn != error_mark_node)
+	record_tm_replacement (res, tm_fn);
     }
   return res;
 }
@@ -236,7 +297,8 @@ do_end_catch (tree type)
 
   tree cleanup = cp_build_function_call_vec (end_catch_fn,
 					     NULL, tf_warning_or_error);
-  TREE_NOTHROW (cleanup) = dtor_nothrow (type);
+  if (cleanup != error_mark_node)
+    TREE_NOTHROW (cleanup) = dtor_nothrow (type);
 
   return cleanup;
 }
@@ -400,7 +462,8 @@ expand_start_catch_block (tree decl)
 	   && TYPE_NEEDS_CONSTRUCTING (TREE_TYPE (decl)))
     {
       exp = do_get_exception_ptr ();
-      initialize_handler_parm (decl, exp);
+      if (exp != error_mark_node)
+	initialize_handler_parm (decl, exp);
       finish_expr_stmt (init);
     }
 
@@ -608,23 +671,44 @@ build_throw (tree exp)
 
       if (!throw_fn)
 	{
-	  tree name = get_identifier ("__cxa_throw");
-	  throw_fn = get_global_binding (name);
+	  const char *name = "__cxa_throw";
+	  tree ident = get_identifier (name);
+	  tree fntype = NULL_TREE;
+	  throw_fn = get_global_binding (ident);
 	  if (!throw_fn)
 	    {
 	      /* Declare void __cxa_throw (void*, void*, void (*)(void*)).  */
 	      /* ??? Second argument is supposed to be "std::type_info*".  */
-	      tmp = build_function_type_list (void_type_node,
-					      ptr_type_node, ptr_type_node,
-					      cleanup_type, NULL_TREE);
-	      throw_fn = push_throw_library_fn (name, tmp);
+	      fntype = build_function_type_list (void_type_node,
+						 ptr_type_node, ptr_type_node,
+						 cleanup_type, NULL_TREE);
+	      throw_fn = push_throw_library_fn (ident, fntype);
+	    }
+	  else if (!verify_library_fn (throw_fn, name, void_type_node,
+				       ptr_type_node, ptr_type_node,
+				       cleanup_type))
+	    throw_fn = error_mark_node;
 
-	      if (flag_tm)
+	  if (flag_tm && throw_fn != error_mark_node)
+	    {
+	      const char *itm_name = "_ITM_cxa_throw";
+	      tree itm_ident = get_identifier (itm_name);
+	      tree itm_fn = get_global_binding (itm_ident);
+	      if (!itm_fn)
+		{
+		  if (!fntype)
+		    fntype
+		      = build_function_type_list (void_type_node,
+						  ptr_type_node, ptr_type_node,
+						  cleanup_type, NULL_TREE);
+		  itm_fn = push_throw_library_fn (itm_ident, fntype);
+		}
+	      else if (!verify_library_fn (itm_fn, itm_name, void_type_node,
+					   ptr_type_node, ptr_type_node,
+					   cleanup_type))
+		itm_fn = error_mark_node;
+	      if (itm_fn != error_mark_node)
 		{
-		  tree itm_name = get_identifier ("_ITM_cxa_throw");
-		  tree itm_fn = get_global_binding (itm_name);
-		  if (!itm_fn)
-		    itm_fn = push_throw_library_fn (itm_name, tmp);
 		  apply_tm_attr (itm_fn, get_identifier ("transaction_pure"));
 		  record_tm_replacement (throw_fn, itm_fn);
 		}
@@ -658,6 +742,8 @@ build_throw (tree exp)
 
       /* Allocate the space for the exception.  */
       allocate_expr = do_allocate_exception (temp_type);
+      if (allocate_expr == error_mark_node)
+	return error_mark_node;
       allocate_expr = get_target_expr (allocate_expr);
       ptr = TARGET_EXPR_SLOT (allocate_expr);
       TARGET_EXPR_CLEANUP (allocate_expr) = do_free_exception (ptr);
@@ -760,14 +846,21 @@ build_throw (tree exp)
       /* Rethrow current exception.  */
       if (!rethrow_fn)
 	{
-	  tree name = get_identifier ("__cxa_rethrow");
-	  rethrow_fn = get_global_binding (name);
+	  const char *name = "__cxa_rethrow";
+	  tree ident = get_identifier (name);
+	  rethrow_fn = get_global_binding (ident);
 	  if (!rethrow_fn)
-	    /* Declare void __cxa_rethrow (void).  */
-	    rethrow_fn = push_throw_library_fn
-	      (name, build_function_type_list (void_type_node, NULL_TREE));
+	    {
+	      /* Declare void __cxa_rethrow (void).  */
+	      tree fntype
+		= build_function_type_list (void_type_node, NULL_TREE);
+	      rethrow_fn = push_throw_library_fn (ident, fntype);
+	    }
+	  else if (!verify_library_fn (rethrow_fn, name, void_type_node,
+				       NULL_TREE, NULL_TREE, NULL_TREE))
+	    rethrow_fn = error_mark_node;
 
-	  if (flag_tm)
+	  if (flag_tm && rethrow_fn != error_mark_node)
 	    apply_tm_attr (rethrow_fn, get_identifier ("transaction_pure"));
 	}
 
--- gcc/testsuite/g++.dg/eh/builtin5.C.jj	2018-12-15 13:00:41.134490069 +0100
+++ gcc/testsuite/g++.dg/eh/builtin5.C	2018-12-15 13:00:41.134490069 +0100
@@ -0,0 +1,29 @@
+// PR c++/88482
+// { dg-do compile }
+
+extern "C" void __cxa_throw (void *, void *, void (*) (void *));
+extern "C" void *__cxa_get_exception_ptr (void *);
+extern "C" void *__cxa_begin_catch (void *);
+extern "C" void __cxa_end_catch ();
+extern "C" void __cxa_rethrow ();
+extern "C" void *__cxa_allocate_exception (__SIZE_TYPE__);
+extern "C" void __cxa_free_exception (void *);
+
+struct S { S (); S (const S &); ~S (); };
+
+int
+foo (int x)
+{
+  if (x > 27)
+    throw 19;
+  try
+    {
+      if (x > 15)
+	throw S ();
+    }
+  catch (S s)
+    {
+      throw;
+    }
+  return x + 3;
+}
--- gcc/testsuite/g++.dg/eh/builtin6.C.jj	2018-12-15 13:00:41.134490069 +0100
+++ gcc/testsuite/g++.dg/eh/builtin6.C	2018-12-15 13:00:41.134490069 +0100
@@ -0,0 +1,29 @@
+// PR c++/88482
+// { dg-do compile }
+
+float __cxa_throw;		// { dg-error "declared incorrectly" }
+extern "C" void *__cxa_get_exception_ptr (void *);
+float __cxa_begin_catch;	// { dg-error "declared incorrectly" }
+float __cxa_end_catch;		// { dg-error "declared incorrectly" }
+float __cxa_rethrow;		// { dg-error "declared incorrectly" }
+float __cxa_allocate_exception;	// { dg-error "declared incorrectly" }
+extern "C" void __cxa_free_exception (void *);
+
+struct S { S (); S (const S &); ~S (); };
+
+int
+foo (int x)
+{
+  if (x > 27)
+    throw 19;
+  try
+    {
+      if (x > 15)
+	throw S ();
+    }
+  catch (S s)
+    {
+      throw;
+    }
+  return x + 3;
+}
--- gcc/testsuite/g++.dg/eh/builtin7.C.jj	2018-12-15 13:00:41.134490069 +0100
+++ gcc/testsuite/g++.dg/eh/builtin7.C	2018-12-15 13:00:41.134490069 +0100
@@ -0,0 +1,29 @@
+// PR c++/88482
+// { dg-do compile }
+
+extern "C" void __cxa_throw (void *, void *, void (*) (void *));
+int __cxa_get_exception_ptr;		// { dg-error "declared incorrectly" }
+extern "C" void *__cxa_begin_catch (void *);
+extern "C" void __cxa_end_catch ();
+extern "C" void __cxa_rethrow ();
+extern "C" void *__cxa_allocate_exception (__SIZE_TYPE__);
+int __cxa_free_exception;		// { dg-error "declared incorrectly" }
+
+struct S { S (); S (const S &); ~S (); };
+
+int
+foo (int x)
+{
+  if (x > 27)
+    throw 19;
+  try
+    {
+      if (x > 15)
+	throw S ();
+    }
+  catch (S s)
+    {
+      throw;
+    }
+  return x + 3;
+}
--- gcc/testsuite/g++.dg/eh/builtin8.C.jj	2018-12-15 13:00:41.134490069 +0100
+++ gcc/testsuite/g++.dg/eh/builtin8.C	2018-12-15 13:00:41.134490069 +0100
@@ -0,0 +1,23 @@
+// PR c++/88482
+// { dg-do compile }
+
+#include <cxxabi.h>
+
+struct S { S (); S (const S &); ~S (); };
+
+int
+foo (int x)
+{
+  if (x > 27)
+    throw 19;
+  try
+    {
+      if (x > 15)
+	throw S ();
+    }
+  catch (S s)
+    {
+      throw;
+    }
+  return x + 3;
+}
--- gcc/testsuite/g++.dg/eh/builtin9.C.jj	2018-12-15 13:00:41.135490052 +0100
+++ gcc/testsuite/g++.dg/eh/builtin9.C	2018-12-15 13:00:41.134490069 +0100
@@ -0,0 +1,29 @@
+// PR c++/88482
+// { dg-do compile }
+
+extern "C" int __cxa_throw (void *, void *, void (*) (void *));	// { dg-error "declared incorrectly" }
+extern "C" void *__cxa_get_exception_ptr (void *);
+extern "C" double __cxa_begin_catch (void *);			// { dg-error "declared incorrectly" }
+extern "C" long *__cxa_end_catch ();				// { dg-error "declared incorrectly" }
+extern "C" char __cxa_rethrow ();				// { dg-error "declared incorrectly" }
+extern "C" void __cxa_allocate_exception (__SIZE_TYPE__);	// { dg-error "declared incorrectly" }
+extern "C" void __cxa_free_exception (void *);
+
+struct S { S (); S (const S &); ~S (); };
+
+int
+foo (int x)
+{
+  if (x > 27)
+    throw 19;
+  try
+    {
+      if (x > 15)
+	throw S ();
+    }
+  catch (S s)
+    {
+      throw;
+    }
+  return x + 3;
+}
--- gcc/testsuite/g++.dg/eh/builtin10.C.jj	2018-12-15 13:00:41.135490052 +0100
+++ gcc/testsuite/g++.dg/eh/builtin10.C	2018-12-15 13:00:41.135490052 +0100
@@ -0,0 +1,29 @@
+// PR c++/88482
+// { dg-do compile }
+
+extern "C" void __cxa_throw (void *, void *, void (*) (void *));
+extern "C" float __cxa_get_exception_ptr (void *);		// { dg-error "declared incorrectly" }
+extern "C" void *__cxa_begin_catch (void *);
+extern "C" void __cxa_end_catch ();
+extern "C" void __cxa_rethrow ();
+extern "C" void *__cxa_allocate_exception (__SIZE_TYPE__);
+extern "C" int __cxa_free_exception (void *);			// { dg-error "declared incorrectly" }
+
+struct S { S (); S (const S &); ~S (); };
+
+int
+foo (int x)
+{
+  if (x > 27)
+    throw 19;
+  try
+    {
+      if (x > 15)
+	throw S ();
+    }
+  catch (S s)
+    {
+      throw;
+    }
+  return x + 3;
+}
--- gcc/testsuite/g++.dg/eh/builtin11.C.jj	2018-12-15 13:00:41.135490052 +0100
+++ gcc/testsuite/g++.dg/eh/builtin11.C	2018-12-15 13:00:41.135490052 +0100
@@ -0,0 +1,29 @@
+// PR c++/88482
+// { dg-do compile }
+
+extern "C" void __cxa_throw (float, void *, void (*) (void *));	// { dg-error "declared incorrectly" }
+extern "C" void *__cxa_get_exception_ptr (void *);
+extern "C" void *__cxa_begin_catch (int);			// { dg-error "declared incorrectly" }
+extern "C" void __cxa_end_catch (long long);			// { dg-error "declared incorrectly" }
+extern "C" void __cxa_rethrow (int);				// { dg-error "declared incorrectly" }
+extern "C" void *__cxa_allocate_exception (void *);		// { dg-error "declared incorrectly" }
+extern "C" void __cxa_free_exception (void *);
+
+struct S { S (); S (const S &); ~S (); };
+
+int
+foo (int x)
+{
+  if (x > 27)
+    throw 19;
+  try
+    {
+      if (x > 15)
+	throw S ();
+    }
+  catch (S s)
+    {
+      throw;
+    }
+  return x + 3;
+}
--- gcc/testsuite/g++.dg/parse/crash55.C.jj	2018-12-14 00:32:55.998201251 +0100
+++ gcc/testsuite/g++.dg/parse/crash55.C	2018-12-15 13:00:41.156489709 +0100
@@ -1,8 +1,8 @@
 // PR c++/42038
 
-extern int __cxa_begin_catch;
+extern int __cxa_begin_catch;	// { dg-error "declared incorrectly" }
 
 void f(void)
 {
-  try { } catch (int) { } // { dg-error "cannot be used" }
+  try { } catch (int) { }
 }
--- libitm/eh_cpp.cc.jj	2018-01-03 10:42:59.250764052 +0100
+++ libitm/eh_cpp.cc	2018-12-15 13:04:26.787804572 +0100
@@ -89,7 +89,7 @@ struct __cxa_eh_globals
 
 extern void *__cxa_allocate_exception (size_t) WEAK;
 extern void __cxa_free_exception (void *) WEAK;
-extern void __cxa_throw (void *, void *, void *) WEAK;
+extern void __cxa_throw (void *, void *, void (*) (void *)) WEAK;
 extern void *__cxa_begin_catch (void *) WEAK;
 extern void __cxa_end_catch (void) WEAK;
 extern void __cxa_tm_cleanup (void *, void *, unsigned int) WEAK;
@@ -98,7 +98,7 @@ extern __cxa_eh_globals *__cxa_get_globa
 #if !defined (HAVE_ELF_STYLE_WEAKREF) 
 void *__cxa_allocate_exception (size_t) { return NULL; }
 void __cxa_free_exception (void *) { return; }
-void __cxa_throw (void *, void *, void *) { return; }
+void __cxa_throw (void *, void *, void (*) (void *)) { return; }
 void *__cxa_begin_catch (void *) { return NULL; }
 void __cxa_end_catch (void) { return; }
 void __cxa_tm_cleanup (void *, void *, unsigned int) { return; }
@@ -136,7 +136,7 @@ _ITM_cxa_free_exception (void *exc_ptr)
 }
 
 void
-_ITM_cxa_throw (void *obj, void *tinfo, void *dest)
+_ITM_cxa_throw (void *obj, void *tinfo, void (*dest) (void *))
 {
   // This used to be instrumented, but does not need to be anymore.
   __cxa_throw (obj, tinfo, dest);
--- libitm/libitm.h.jj	2018-01-03 10:42:59.249764052 +0100
+++ libitm/libitm.h	2018-12-15 13:01:38.585551744 +0100
@@ -284,7 +284,7 @@ extern void _ITM_deregisterTMCloneTable
 
 extern void *_ITM_cxa_allocate_exception (size_t);
 extern void _ITM_cxa_free_exception (void *exc_ptr);
-extern void _ITM_cxa_throw (void *obj, void *tinfo, void *dest);
+extern void _ITM_cxa_throw (void *obj, void *tinfo, void (*dest) (void *));
 extern void *_ITM_cxa_begin_catch (void *exc_ptr);
 extern void _ITM_cxa_end_catch (void);
 extern void _ITM_commitTransactionEH(void *exc_ptr) ITM_REGPARM;
--- libitm/libitm.texi.jj	2018-01-03 10:10:52.879434767 +0100
+++ libitm/libitm.texi	2018-12-15 13:02:05.535111585 +0100
@@ -269,7 +269,7 @@ transactions.
 void _ITM_commitTransactionEH(void *exc_ptr) ITM_REGPARM;
 void *_ITM_cxa_allocate_exception (size_t);
 void _ITM_cxa_free_exception (void *exc_ptr);
-void _ITM_cxa_throw (void *obj, void *tinfo, void *dest);
+void _ITM_cxa_throw (void *obj, void *tinfo, void (*dest) (void *));
 void *_ITM_cxa_begin_catch (void *exc_ptr);
 void _ITM_cxa_end_catch (void);
 @end example

	Jakub