[PATCH] Fix yet another -fsanitize=undefined ubsan_encode_value ICE (PR sanitizer/81125)
Richard Biener
rguenther@suse.de
Mon Jun 19 14:56:00 GMT 2017
On Mon, 19 Jun 2017, Jakub Jelinek wrote:
> Hi!
>
> And here is another ICE. While we have a current_function_decl
> in this case, still create_tmp_var's called gimple_add_tmp_var
> and mark_addressable don't work too well when the current function
> is a C++ ctor or dtor that the FE then duplicates.
>
> Fixed by telling ubsan_encode_value whether it is called from the
> FE (when it shouldn't use gimple_add_tmp_var nor mark_addressable
> and should use TARGET_EXPR), or from GIMPLE passes (when it should
> do what it did before with in_expand_p == false) or from RTL expansion
> (when it should do what it did with in_expand_p == true).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ugh.
Ok.
Thanks,
Richard.
> 2017-06-19 Jakub Jelinek <jakub@redhat.com>
>
> PR sanitizer/81125
> * ubsan.h (enum ubsan_encode_value_phase): New.
> (ubsan_encode_value): Change second argument to
> enum ubsan_encode_value_phase with default value of
> UBSAN_ENCODE_VALUE_GENERIC.
> * ubsan.c (ubsan_encode_value): Change second argument to
> enum ubsan_encode_value_phase PHASE from bool IN_EXPAND_P,
> adjust uses, for UBSAN_ENCODE_VALUE_GENERIC use just
> create_tmp_var_raw instead of create_tmp_var and use a
> TARGET_EXPR.
> (ubsan_expand_bounds_ifn, ubsan_build_overflow_builtin,
> instrument_bool_enum_load, ubsan_instrument_float_cast): Adjust
> ubsan_encode_value callers.
>
> * g++.dg/ubsan/pr81125.C: New test.
>
> --- gcc/ubsan.h.jj 2017-06-19 08:26:17.000000000 +0200
> +++ gcc/ubsan.h 2017-06-19 10:06:52.723664974 +0200
> @@ -42,6 +42,13 @@ enum ubsan_print_style {
> UBSAN_PRINT_ARRAY
> };
>
> +/* This controls ubsan_encode_value behavior. */
> +enum ubsan_encode_value_phase {
> + UBSAN_ENCODE_VALUE_GENERIC,
> + UBSAN_ENCODE_VALUE_GIMPLE,
> + UBSAN_ENCODE_VALUE_RTL
> +};
> +
> extern bool ubsan_expand_bounds_ifn (gimple_stmt_iterator *);
> extern bool ubsan_expand_null_ifn (gimple_stmt_iterator *);
> extern bool ubsan_expand_objsize_ifn (gimple_stmt_iterator *);
> @@ -49,7 +56,8 @@ extern bool ubsan_expand_vptr_ifn (gimpl
> extern bool ubsan_instrument_unreachable (gimple_stmt_iterator *);
> extern tree ubsan_create_data (const char *, int, const location_t *, ...);
> extern tree ubsan_type_descriptor (tree, enum ubsan_print_style = UBSAN_PRINT_NORMAL);
> -extern tree ubsan_encode_value (tree, bool = false);
> +extern tree ubsan_encode_value (tree, enum ubsan_encode_value_phase
> + = UBSAN_ENCODE_VALUE_GENERIC);
> extern bool is_ubsan_builtin_p (tree);
> extern tree ubsan_build_overflow_builtin (tree_code, location_t, tree, tree,
> tree, tree *);
> --- gcc/ubsan.c.jj 2017-06-19 10:27:33.000000000 +0200
> +++ gcc/ubsan.c 2017-06-19 10:13:53.434541556 +0200
> @@ -114,10 +114,10 @@ decl_for_type_insert (tree type, tree de
> /* Helper routine, which encodes a value in the pointer_sized_int_node.
> Arguments with precision <= POINTER_SIZE are passed directly,
> the rest is passed by reference. T is a value we are to encode.
> - IN_EXPAND_P is true if this function is called during expansion. */
> + PHASE determines when this function is called. */
>
> tree
> -ubsan_encode_value (tree t, bool in_expand_p)
> +ubsan_encode_value (tree t, enum ubsan_encode_value_phase phase)
> {
> tree type = TREE_TYPE (t);
> const unsigned int bitsize = GET_MODE_BITSIZE (TYPE_MODE (type));
> @@ -144,7 +144,7 @@ ubsan_encode_value (tree t, bool in_expa
> /* The reason for this is that we don't want to pessimize
> code by making vars unnecessarily addressable. */
> tree var;
> - if (current_function_decl)
> + if (phase != UBSAN_ENCODE_VALUE_GENERIC)
> {
> var = create_tmp_var (type);
> mark_addressable (var);
> @@ -154,7 +154,7 @@ ubsan_encode_value (tree t, bool in_expa
> var = create_tmp_var_raw (type);
> TREE_ADDRESSABLE (var) = 1;
> }
> - if (in_expand_p)
> + if (phase == UBSAN_ENCODE_VALUE_RTL)
> {
> rtx mem
> = assign_stack_temp_for_type (TYPE_MODE (type),
> @@ -164,7 +164,7 @@ ubsan_encode_value (tree t, bool in_expa
> expand_assignment (var, t, false);
> return build_fold_addr_expr (var);
> }
> - if (current_function_decl)
> + if (phase != UBSAN_ENCODE_VALUE_GENERIC)
> {
> tree tem = build2 (MODIFY_EXPR, void_type_node, var, t);
> t = build_fold_addr_expr (var);
> @@ -725,9 +725,9 @@ ubsan_expand_bounds_ifn (gimple_stmt_ite
> ? BUILT_IN_UBSAN_HANDLE_OUT_OF_BOUNDS
> : BUILT_IN_UBSAN_HANDLE_OUT_OF_BOUNDS_ABORT;
> tree fn = builtin_decl_explicit (bcode);
> - tree val
> - = force_gimple_operand_gsi (gsi, ubsan_encode_value (orig_index), true,
> - NULL_TREE, true, GSI_SAME_STMT);
> + tree val = ubsan_encode_value (orig_index, UBSAN_ENCODE_VALUE_GIMPLE);
> + val = force_gimple_operand_gsi (gsi, val, true, NULL_TREE, true,
> + GSI_SAME_STMT);
> g = gimple_build_call (fn, 2, data, val);
> }
> gimple_set_location (g, loc);
> @@ -1283,9 +1283,11 @@ ubsan_build_overflow_builtin (tree_code
> tree fn = builtin_decl_explicit (fn_code);
> return build_call_expr_loc (loc, fn, 2 + (code != NEGATE_EXPR),
> build_fold_addr_expr_loc (loc, data),
> - ubsan_encode_value (op0, true),
> - op1 ? ubsan_encode_value (op1, true)
> - : NULL_TREE);
> + ubsan_encode_value (op0, UBSAN_ENCODE_VALUE_RTL),
> + op1
> + ? ubsan_encode_value (op1,
> + UBSAN_ENCODE_VALUE_RTL)
> + : NULL_TREE);
> }
>
> /* Perform the signed integer instrumentation. GSI is the iterator
> @@ -1476,9 +1478,9 @@ instrument_bool_enum_load (gimple_stmt_i
> : BUILT_IN_UBSAN_HANDLE_LOAD_INVALID_VALUE_ABORT;
> tree fn = builtin_decl_explicit (bcode);
>
> - tree val = force_gimple_operand_gsi (&gsi2, ubsan_encode_value (urhs),
> - true, NULL_TREE, true,
> - GSI_SAME_STMT);
> + tree val = ubsan_encode_value (urhs, UBSAN_ENCODE_VALUE_GIMPLE);
> + val = force_gimple_operand_gsi (&gsi2, val, true, NULL_TREE, true,
> + GSI_SAME_STMT);
> g = gimple_build_call (fn, 2, data, val);
> }
> gimple_set_location (g, loc);
> @@ -1642,7 +1644,7 @@ ubsan_instrument_float_cast (location_t
> fn = builtin_decl_explicit (bcode);
> fn = build_call_expr_loc (loc, fn, 2,
> build_fold_addr_expr_loc (loc, data),
> - ubsan_encode_value (expr, false));
> + ubsan_encode_value (expr));
> }
>
> return fold_build3 (COND_EXPR, void_type_node, t, fn, integer_zero_node);
> --- gcc/testsuite/g++.dg/ubsan/pr81125.C.jj 2017-06-19 10:21:58.607642573 +0200
> +++ gcc/testsuite/g++.dg/ubsan/pr81125.C 2017-06-19 10:21:28.000000000 +0200
> @@ -0,0 +1,20 @@
> +// PR sanitizer/81125
> +// { dg-do compile }
> +// { dg-options "-fsanitize=undefined" }
> +
> +#ifdef __SIZEOF_INT128__
> +typedef __int128 T;
> +#else
> +typedef long long int T;
> +#endif
> +
> +struct A
> +{
> + A (long);
> + T a;
> +};
> +
> +A::A (long c)
> +{
> + long b = a % c;
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
More information about the Gcc-patches
mailing list