From patchwork Fri Jul 1 20:48:14 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 55646 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3E224385DC01 for ; Fri, 1 Jul 2022 20:48:34 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id B2EE23858C51 for ; Fri, 1 Jul 2022 20:48:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B2EE23858C51 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.cz Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 49F6E22260 for ; Fri, 1 Jul 2022 20:48:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1656708495; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=6EydD8LamOmzkv/i6RtoWf0A+P/JBmhv8wDw5vM31tE=; b=e9/RvluJrqzOhTEAEUGSGqpU2KnAnDvvM6ia3iPViQY5L0rQUEWGpSQgen/U6VZeC76OU2 NZKPrRRD0FrWKqC03a7JFzmJi1BOFu2rNc9ziJLlr8NUyFLN4SYyfKUFB9Frvx2kpozQsZ r+ICYgwlJkZM5KWRgKta05841FrjBoI= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1656708495; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=6EydD8LamOmzkv/i6RtoWf0A+P/JBmhv8wDw5vM31tE=; b=l9TAnPmk4I6OyoR6cTAiRmqLFvaIE74ejZcSGS0bMT/Dk0A82Id986ydhePgE7GU8pBuyP ifaMYMHuiF207XBQ== Received: from suse.cz (virgil.suse.cz [10.100.13.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 32B782C141; Fri, 1 Jul 2022 20:48:15 +0000 (UTC) From: Martin Jambor To: GCC Patches Subject: [PATCH] tree-sra: Fix union handling in build_reconstructed_reference (PR 105860) User-Agent: Notmuch/0.35 (https://notmuchmail.org) Emacs/28.1 (x86_64-suse-linux-gnu) Date: Fri, 01 Jul 2022 22:48:14 +0200 Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Richard Biener Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Hi, As the testcase in PR 105860 shows, the code that tries to re-use the handled_component chains in SRA can be horribly confused by unions, where it thinks it has found a compatible structure under which it can chain the references, but in fact it found the type it was looking for elsewhere in a union and generated a write to a completely wrong part of an aggregate. I don't remember whether the plan was to support unions at all in build_reconstructed_reference but it can work, to an extent, if we make sure that we start the search only outside the outermost union, which is what the patch does (and the extra testcase verifies). Bootstrapped and tested on x86_64-linux. OK for trunk and then for release branches? Thanks, Martin gcc/ChangeLog: 2022-07-01 Martin Jambor PR tree-optimization/105860 * tree-sra.cc (build_reconstructed_reference): Start expr traversal only just below the outermost union. gcc/testsuite/ChangeLog: 2022-07-01 Martin Jambor PR tree-optimization/105860 * gcc.dg/tree-ssa/alias-access-path-13.c: New test. * gcc.dg/tree-ssa/pr105860.c: Likewise. --- .../gcc.dg/tree-ssa/alias-access-path-13.c | 31 +++++++++ gcc/testsuite/gcc.dg/tree-ssa/pr105860.c | 63 +++++++++++++++++++ gcc/tree-sra.cc | 13 +++- 3 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr105860.c diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c b/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c new file mode 100644 index 00000000000..e502a97bc75 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c @@ -0,0 +1,31 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-fre1" } */ + +struct inn +{ + int val; +}; + +union foo +{ + struct inn inn; + long int baz; +} *fooptr; + +struct bar +{ + union foo foo; + int val2; +} *barptr; + +int +test () +{ + union foo foo; + foo.inn.val = 0; + barptr->val2 = 123; + *fooptr = foo; + return barptr->val2; +} + +/* { dg-final { scan-tree-dump-times "return 123" 1 "fre1"} } */ diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c b/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c new file mode 100644 index 00000000000..77bcb4a6739 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c @@ -0,0 +1,63 @@ +/* { dg-do run } */ +/* { dg-options "-O1" } */ + +struct S1 { + unsigned int _0; + unsigned int _1; +} ; +struct S2 { + struct S1 _s1; + unsigned long _x2; +} ; + +struct ufld_type1 { + unsigned int _u1t; + struct S2 _s2; +} ; + +struct ufld_type2 { + unsigned int _u2t; + struct S1 _s1; +} ; +struct parm_type { + union { + struct ufld_type1 var_1; + struct ufld_type2 var_2; + } U; +}; + +struct parm_type bad_function( struct parm_type arg0 ) +{ + struct parm_type rv; + struct S2 var4; + switch( arg0.U.var_2._u2t ) { + case 4294967041: + var4._s1 = arg0.U.var_1._s2._s1; + rv.U.var_1._u1t = 4294967041; + rv.U.var_1._s2 = var4; + break; + case 4294967043: + rv.U.var_2._u2t = 4294967043; + rv.U.var_2._s1 = arg0.U.var_2._s1; + break; + default: + break; + } + return rv; +} + +int main() { + struct parm_type val; + struct parm_type out; + val.U.var_2._u2t = 4294967043; + val.U.var_2._s1._0 = 0x01010101; + val.U.var_2._s1._1 = 0x02020202; + out = bad_function(val); + if (val.U.var_2._u2t != 4294967043) + __builtin_abort (); + if (out.U.var_2._s1._0 != 0x01010101) + __builtin_abort (); + if (val.U.var_2._s1._1 != 0x02020202 ) + __builtin_abort (); + return 0; +} diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc index 081c51b58a4..099e8dbe873 100644 --- a/gcc/tree-sra.cc +++ b/gcc/tree-sra.cc @@ -1667,7 +1667,18 @@ build_ref_for_offset (location_t loc, tree base, poly_int64 offset, static tree build_reconstructed_reference (location_t, tree base, struct access *model) { - tree expr = model->expr, prev_expr = NULL; + tree expr = model->expr; + /* We have to make sure to start just below the outermost union. */ + tree start_expr = expr; + while (handled_component_p (expr)) + { + if (TREE_CODE (TREE_TYPE (TREE_OPERAND (expr, 0))) == UNION_TYPE) + start_expr = expr; + expr = TREE_OPERAND (expr, 0); + } + + expr = start_expr; + tree prev_expr = NULL_TREE; while (!types_compatible_p (TREE_TYPE (expr), TREE_TYPE (base))) { if (!handled_component_p (expr))