From patchwork Sat Feb 16 10:56:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 1043415 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-496372-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="YPIeCouB"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 441nBK4KfCz9s7h for ; Sat, 16 Feb 2019 21:56:36 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:mime-version:content-type; q=dns; s=default; b=CPgQW3jI5jFb//2p4oRwGd5BO0/y2GM6dSXzXg5wXBd8kcNtAo SnONL6S0BUzijW8UGtqWtk0W/l6v+8zMWYUPc5rUgcGxSlyg6NXgo0RW727hIl7I SawntEvyN5nEW8JkYNfVumi21DDewDhh45kkbjqlonMiRsjf1ymXodRyA= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:mime-version:content-type; s= default; bh=/T9Sq2ET/2+5wQEzklkpnUDUpUw=; b=YPIeCouBPcTIgcGd4xG2 QqSLTBeSg3nyYPRGxxDYSJmN5cIZw5akEQXOm9fHxUt2WNmwZS0yIs0M/jQBDdKc Ho1BFDAhovsKD0guDXf3OApNfoggFEkmPN3ajWv1XeG2m+utj82nNsMASqMw7m4N AcghzpxylOYyBXWWxf9yNK0= Received: (qmail 78104 invoked by alias); 16 Feb 2019 10:56:27 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 78090 invoked by uid 89); 16 Feb 2019 10:56:27 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.2 spammy=facilitate, quickly, peculiar, tc 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; Sat, 16 Feb 2019 10:56:25 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 6768CACB1 for ; Sat, 16 Feb 2019 10:56:23 +0000 (UTC) From: Martin Jambor To: GCC Patches Cc: Richard Biener Subject: [PR 89209] Avoid segfault in a peculiar corner case in SRA User-Agent: Notmuch/0.26 (https://notmuchmail.org) Emacs/26.1 (x86_64-suse-linux-gnu) Date: Sat, 16 Feb 2019 11:56:13 +0100 Message-ID: MIME-Version: 1.0 X-IsSubscribed: yes Hi, PR 89209 takes place because SRA on trunk encounters an assignment into an SSA_NAME from a V_C_E of a structure load which however cannot contain any useful data because (it is not addressable and) there is no store to that portion of the aggregate in the entire function. In such circumstances, SRA conjures up a default-definition SSA name and replaces the RHS of the load with it so that an uninitialized warning is generated. Unfortunately, the code digging through V_C_Es badly interacts with this and what happens is that first we create an aggregate type SSA name which the code avoiding creation of additional V_C_Es then tries to store "into" the SSA name on the LHS, which of course fails. BTW, I was surprised no verifier caught the aggregate SSA name if I just avoided the segfaulting path. Fixed with the patch below which gives the code creating the default-definition SSA_NAME an alternative type which is used if the access type is not a gimple_register_typoe. I have also added an additional test that lacc is not NULL to sra_modify_assign because the code path could trigger if the created default-def SSA_NAME happens to be loaded as two different types. However, I have not managed to quickly create a testcase that would lead to it.. Bootstrapped and tested on x86_64-linux. OK for trunk? Thanks, Martin 2019-02-15 Martin Jambor PR tree-optimization/89209 * tree-sra.c (create_access_replacement): New optional parameter reg_tree. Use it as a type if non-NULL and access type is not of a register type. (get_repl_default_def_ssa_name): New parameter REG_TYPE, pass it to create_access_replacement. (sra_modify_assign): Pass LHS type to get_repl_default_def_ssa_name. Check lacc is non-NULL before attempting to re-create it on the RHS. testsuite/ * gcc.dg/tree-ssa/pr89209.c: New test. --- gcc/testsuite/gcc.dg/tree-ssa/pr89209.c | 16 ++++++++++++ gcc/tree-sra.c | 34 +++++++++++++++---------- 2 files changed, 37 insertions(+), 13 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr89209.c diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr89209.c b/gcc/testsuite/gcc.dg/tree-ssa/pr89209.c new file mode 100644 index 00000000000..f01bda9ae5c --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr89209.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +struct S { + short a, b; +}; +struct T { + int c; + struct S s; +}; +int f () +{ + struct T t; + t.c = t.s.a || t.s.b; + return t.c; +} diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c index e4851daaa3f..eeef31ba496 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -2195,13 +2195,20 @@ sort_and_splice_var_accesses (tree var) /* Create a variable for the given ACCESS which determines the type, name and a few other properties. Return the variable declaration and store it also to - ACCESS->replacement. */ + ACCESS->replacement. REG_TREE is used when creating a declaration to base a + default-definition SSA name on on in order to facilitate an uninitialized + warning. It is used instead of the actual ACCESS type if that is not of a + gimple register type. */ static tree -create_access_replacement (struct access *access) +create_access_replacement (struct access *access, tree reg_type = NULL_TREE) { tree repl; + tree type = access->type; + if (reg_type && !is_gimple_reg_type (type)) + type = reg_type; + if (access->grp_to_be_debug_replaced) { repl = create_tmp_var_raw (access->type); @@ -2210,17 +2217,16 @@ create_access_replacement (struct access *access) else /* Drop any special alignment on the type if it's not on the main variant. This avoids issues with weirdo ABIs like AAPCS. */ - repl = create_tmp_var (build_qualified_type - (TYPE_MAIN_VARIANT (access->type), - TYPE_QUALS (access->type)), "SR"); - if (TREE_CODE (access->type) == COMPLEX_TYPE - || TREE_CODE (access->type) == VECTOR_TYPE) + repl = create_tmp_var (build_qualified_type (TYPE_MAIN_VARIANT (type), + TYPE_QUALS (type)), "SR"); + if (TREE_CODE (type) == COMPLEX_TYPE + || TREE_CODE (type) == VECTOR_TYPE) { if (!access->grp_partial_lhs) DECL_GIMPLE_REG_P (repl) = 1; } else if (access->grp_partial_lhs - && is_gimple_reg_type (access->type)) + && is_gimple_reg_type (type)) TREE_ADDRESSABLE (repl) = 1; DECL_SOURCE_LOCATION (repl) = DECL_SOURCE_LOCATION (access->base); @@ -3450,15 +3456,16 @@ sra_modify_constructor_assign (gimple *stmt, gimple_stmt_iterator *gsi) /* Create and return a new suitable default definition SSA_NAME for RACC which is an access describing an uninitialized part of an aggregate that is being - loaded. */ + loaded. REG_TREE is used instead of the actual RACC type if that is not of + a gimple register type. */ static tree -get_repl_default_def_ssa_name (struct access *racc) +get_repl_default_def_ssa_name (struct access *racc, tree reg_type) { gcc_checking_assert (!racc->grp_to_be_replaced && !racc->grp_to_be_debug_replaced); if (!racc->replacement_decl) - racc->replacement_decl = create_access_replacement (racc); + racc->replacement_decl = create_access_replacement (racc, reg_type); return get_or_create_ssa_default_def (cfun, racc->replacement_decl); } @@ -3530,7 +3537,7 @@ sra_modify_assign (gimple *stmt, gimple_stmt_iterator *gsi) && TREE_CODE (lhs) == SSA_NAME && !access_has_replacements_p (racc)) { - rhs = get_repl_default_def_ssa_name (racc); + rhs = get_repl_default_def_ssa_name (racc, TREE_TYPE (lhs)); modify_this_stmt = true; sra_stats.exprs++; } @@ -3548,7 +3555,8 @@ sra_modify_assign (gimple *stmt, gimple_stmt_iterator *gsi) lhs = build_ref_for_model (loc, lhs, 0, racc, gsi, false); gimple_assign_set_lhs (stmt, lhs); } - else if (AGGREGATE_TYPE_P (TREE_TYPE (rhs)) + else if (lacc + && AGGREGATE_TYPE_P (TREE_TYPE (rhs)) && !contains_vce_or_bfcref_p (rhs)) rhs = build_ref_for_model (loc, rhs, 0, lacc, gsi, false);