From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9181 invoked by alias); 19 Jun 2017 14:56:50 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 9163 invoked by uid 89); 19 Jun 2017 14:56:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 19 Jun 2017 14:56:48 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 1A010ABF2; Mon, 19 Jun 2017 14:56:51 +0000 (UTC) Date: Mon, 19 Jun 2017 14:56:00 -0000 From: Richard Biener To: Jakub Jelinek cc: =?ISO-8859-15?Q?Martin_Li=A8ka?= , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix yet another -fsanitize=undefined ubsan_encode_value ICE (PR sanitizer/81125) In-Reply-To: <20170619144417.GS2123@tucnak> Message-ID: References: <20170619144417.GS2123@tucnak> User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-SW-Source: 2017-06/txt/msg01322.txt.bz2 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 > > 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 SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)