On 12/13/18 6:29 PM, Jakub Jelinek wrote:
Hi!
If the user provides his own __cxa_* prototypes and does so incorrectly
(or even worse declares them as variables etc.), we can get various ICEs.
The following patch adds some sanity checking, mainly that they are actually
functions and with a compatible return type and argument type(s).
For __cxa_throw it gives some freedom for the second and third arguments,
but apparently not enough for libitm where it causes
+FAIL: libitm.c++/throwdown.C (test for excess errors)
This is because cxxabi.h has:
void
__cxa_throw(void*, std::type_info*, void (_GLIBCXX_CDTOR_CALLABI *) (void *))
__attribute__((__noreturn__));
and the patch checks that the second argument is a pointer (any kind) and
third parameter is a function pointer, but libitm.h has:
extern void _ITM_cxa_throw (void *obj, void *tinfo, void *dest);
and eh_cpp.cc has:
extern void __cxa_throw (void *, void *, void *) WEAK;
void
_ITM_cxa_throw (void *obj, void *tinfo, void *dest)
{
// This used to be instrumented, but does not need to be anymore.
__cxa_throw (obj, tinfo, dest);
}
Shall we fix libitm to use void (*dest) (void *) instead of void *dest,
or shall I make the verify_library_fn handle both i == 1 and i == 2
the same?
Fix libitm. Do function pointers and object pointers have the same
representation on all the targets we support?
Bootstrapped/regtested on x86_64-linux and i686-linux (with the above
mentioned FAIL), ok for trunk (and with what solution for libitm)?
2018-12-13 Jakub Jelinek <ja...@redhat.com>
PR c++/88482
* except.c (verify_library_fn): New function.
(declare_library_fn): Use it.
(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. 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.
--- gcc/cp/except.c.jj 2018-12-07 00:23:15.008998854 +0100
+++ gcc/cp/except.c 2018-12-13 20:05:23.053122023 +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)
+{
Do we want to skip all of this if DECL_ARTIFICIAL?
+ 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 __cxa_throw last 2 arguments. */
+ if (i == 1 && TREE_CODE (TREE_VALUE (targs)) != POINTER_TYPE)
+ goto bad;
+ if (i == 2
+ && (TREE_CODE (TREE_VALUE (targs)) != POINTER_TYPE
+ || (TREE_CODE (TREE_TYPE (TREE_VALUE (targs)))
+ != FUNCTION_TYPE)))
These seem to assume that any library function with more than one
parameter will have pointers for the second and third parameters.
TYPE_PTROBV_P might be useful.
+ 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
@@ -161,9 +204,16 @@ declare_library_fn (const char *name, tr
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);
+ else if (!verify_library_fn (tm_fn, tm_name, rtype, ptype,
+ NULL_TREE, NULL_TREE))
+ tm_fn = error_mark_node;
+ if (tm_fn != error_mark_node)
+ record_tm_replacement (res, tm_fn);
}
}
+ else if (!verify_library_fn (res, name, rtype, ptype, NULL_TREE, NULL_TREE))
+ return error_mark_node;
+
return res;
}
@@ -236,7 +286,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 +451,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);
}
@@ -625,10 +677,22 @@ build_throw (tree exp)
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);
+ else if (!verify_library_fn (itm_fn, "_ITM_cxa_throw",
+ void_type_node, ptr_type_node,
+ ptr_type_node, cleanup_type))
+ itm_fn = error_mark_node;
+ if (itm_fn != error_mark_node)
+ {
+ apply_tm_attr (itm_fn,
+ get_identifier ("transaction_pure"));
+ record_tm_replacement (throw_fn, itm_fn);
+ }
}
}
+ else if (!verify_library_fn (throw_fn, "__cxa_throw", void_type_node,
+ ptr_type_node, ptr_type_node,
+ cleanup_type))
+ throw_fn = error_mark_node;
This should happen whether or not flag_tm.
Let's avoid repeating the strings "__cxa_throw" and "_ITM_cxa_throw".
Jason
