From patchwork Sun Oct 17 13:52:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Uecker, Martin" X-Patchwork-Id: 1542238 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HXM0129XKz9s1l for ; Mon, 18 Oct 2021 00:52:47 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 092823858032 for ; Sun, 17 Oct 2021 13:52:45 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail1.med.uni-goettingen.de (mail1.med.uni-goettingen.de [134.76.103.230]) by sourceware.org (Postfix) with ESMTPS id 50E0C3858D28 for ; Sun, 17 Oct 2021 13:52:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 50E0C3858D28 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=med.uni-goettingen.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=med.uni-goettingen.de Received: from umg-exc-03.ads.local.med.uni-goettingen.de ([10.76.100.76]:47242) by mail1.med.uni-goettingen.de with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1mc6aS-0001cY-1a; Sun, 17 Oct 2021 15:52:28 +0200 Received: from umg-exc-01.ads.local.med.uni-goettingen.de (10.76.100.74) by umg-exc-03.ads.local.med.uni-goettingen.de (10.76.100.76) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.14; Sun, 17 Oct 2021 15:52:25 +0200 Received: from umg-exc-01.ads.local.med.uni-goettingen.de ([fe80::2886:b6b:10e3:deea]) by umg-exc-01.ads.local.med.uni-goettingen.de ([fe80::2886:b6b:10e3:deea%6]) with mapi id 15.01.2308.014; Sun, 17 Oct 2021 15:52:25 +0200 From: "Uecker, Martin" To: "gcc-patches@gcc.gnu.org" Subject: [PATCH v4] Fix ICE when mixing VLAs and statement expressions [PR91038] Thread-Topic: [PATCH v4] Fix ICE when mixing VLAs and statement expressions [PR91038] Thread-Index: AQHXw143MftMnbergkWYZPcUCydOGw== Date: Sun, 17 Oct 2021 13:52:25 +0000 Message-ID: <8b3a94a09d35d51da2b0330ecff543d6dbcbe029.camel@med.uni-goettingen.de> Accept-Language: de-DE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.76.100.67] Content-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-11.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Here is the 4th version of the patch. I tried to implement Jason's suggestion and this also fixes the problem. But I am not sure I understand the condition on the TREE_SIDE_EFFECTS ... And there is now another problem: c_finish_omp_for in c-family/c-omp.c does not seem to understand the expressions anymore and I get a test failure in testsuite/c-c++-common/gomp/for-5.c where I now get an "invalid increment expression" instead of the expected error. (bootstrapping and all other tests work fine) Martin Fix ICE when mixing VLAs and statement expressions [PR91038] When returning VM-types from statement expressions, this can lead to an ICE when declarations from the statement expression are referred to later. Most of these issues can be addressed by gimplifying the base expression earlier in gimplify_compound_lval. Another issue is fixed by adding SAVE_EXPRs in pointer_int_sum in the FE to force a correct order of evaluation. This fixes PR91038 and some of the test cases from PR29970 (structs with VLA members need further work). 2021-08-01 Martin Uecker 2021-08-01 Martin Uecker gcc/ PR c/91038 PR c/29970 * gimplify.c (gimplify_var_or_parm_decl): Update comment. (gimplify_compound_lval): Gimplify base expression first. (gimplify_target_expr): Add comment. * c-family/c- common.c (pointer_int_sum): Wrap pointer operand in SAVE_EXPR and also it to the integer argument. gcc/testsuite/ PR c/91038 PR c/29970 * gcc.dg/vla-stexp-3.c: New test. * gcc.dg/vla-stexp-4.c: New test. * gcc.dg/vla-stexp-5.c: New test. * gcc.dg/vla-stexp-6.c: New test. * gcc.dg/vla-stexp-7.c: New test. * gcc.dg/vla-stexp- 8.c: New test. * gcc.dg/vla-stexp-9.c: New test. diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 9d19e352725..522085664f5 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -3348,6 +3348,16 @@ pointer_int_sum (location_t loc, enum tree_code resultcode, intop = convert (c_common_type_for_size (TYPE_PRECISION (sizetype), TYPE_UNSIGNED (sizetype)), intop); + /* Wrap the pointer expression in a SAVE_EXPR to make sure it + * is evaluated first because the size expression may depend on it + * for VM types. + */ + if (TREE_SIDE_EFFECTS (size_exp)) + { + ptrop = build1_loc (loc, SAVE_EXPR, TREE_TYPE (ptrop), ptrop); + intop = build2 (COMPOUND_EXPR, TREE_TYPE (intop), ptrop, intop); + } + /* Replace the integer argument with a suitable product by the object size. Do this multiplication as signed, then convert to the appropriate type for the pointer operation and disregard an overflow that occurred only diff --git a/gcc/gimplify.c b/gcc/gimplify.c index d8e4b139349..be5b00b6716 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -2958,7 +2958,10 @@ gimplify_var_or_parm_decl (tree *expr_p) declaration, for which we've already issued an error. It would be really nice if the front end wouldn't leak these at all. Currently the only known culprit is C++ destructors, as seen - in g++.old-deja/g++.jason/binding.C. */ + in g++.old-deja/g++.jason/binding.C. + Another possible culpit are size expressions for variably modified + types which are lost in the FE or not gimplified correctly. + */ if (VAR_P (decl) && !DECL_SEEN_IN_BIND_EXPR_P (decl) && !TREE_STATIC (decl) && !DECL_EXTERNAL (decl) @@ -3103,16 +3106,22 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, expression until we deal with any variable bounds, sizes, or positions in order to deal with PLACEHOLDER_EXPRs. - So we do this in three steps. First we deal with the annotations - for any variables in the components, then we gimplify the base, - then we gimplify any indices, from left to right. */ + The base expression may contain a statement expression that + has declarations used in size expressions, so has to be + gimplified before gimplifying the size expressions. + + So we do this in three steps. First we deal with variable + bounds, sizes, and positions, then we gimplify the base, + then we deal with the annotations for any variables in the + components and any indices, from left to right. */ + for (i = expr_stack.length () - 1; i >= 0; i--) { tree t = expr_stack[i]; if (TREE_CODE (t) == ARRAY_REF || TREE_CODE (t) == ARRAY_RANGE_REF) { - /* Gimplify the low bound and element type size and put them into + /* Deal with the low bound and element type size and put them into the ARRAY_REF. If these values are set, they have already been gimplified. */ if (TREE_OPERAND (t, 2) == NULL_TREE) @@ -3121,18 +3130,8 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, if (!is_gimple_min_invariant (low)) { TREE_OPERAND (t, 2) = low; - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, - post_p, is_gimple_reg, - fb_rvalue); - ret = MIN (ret, tret); } } - else - { - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, - is_gimple_reg, fb_rvalue); - ret = MIN (ret, tret); - } if (TREE_OPERAND (t, 3) == NULL_TREE) { @@ -3149,18 +3148,8 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, elmt_size, factor); TREE_OPERAND (t, 3) = elmt_size; - tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p, - post_p, is_gimple_reg, - fb_rvalue); - ret = MIN (ret, tret); } } - else - { - tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p, post_p, - is_gimple_reg, fb_rvalue); - ret = MIN (ret, tret); - } } else if (TREE_CODE (t) == COMPONENT_REF) { @@ -3180,18 +3169,8 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, offset, factor); TREE_OPERAND (t, 2) = offset; - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, - post_p, is_gimple_reg, - fb_rvalue); - ret = MIN (ret, tret); } } - else - { - tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, - is_gimple_reg, fb_rvalue); - ret = MIN (ret, tret); - } } } @@ -3202,21 +3181,34 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, fallback | fb_lvalue); ret = MIN (ret, tret); - /* And finally, the indices and operands of ARRAY_REF. During this - loop we also remove any useless conversions. */ + /* Step 3: gimplify size expressions and the indices and operands of + ARRAY_REF. During this loop we also remove any useless conversions. */ + for (; expr_stack.length () > 0; ) { tree t = expr_stack.pop (); if (TREE_CODE (t) == ARRAY_REF || TREE_CODE (t) == ARRAY_RANGE_REF) { + /* Gimplify the low bound and element type size. */ + tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, + is_gimple_reg, fb_rvalue); + ret = MIN (ret, tret); + + tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p, post_p, + is_gimple_reg, fb_rvalue); + ret = MIN (ret, tret); + /* Gimplify the dimension. */ - if (!is_gimple_min_invariant (TREE_OPERAND (t, 1))) - { - tret = gimplify_expr (&TREE_OPERAND (t, 1), pre_p, post_p, - is_gimple_val, fb_rvalue); - ret = MIN (ret, tret); - } + tret = gimplify_expr (&TREE_OPERAND (t, 1), pre_p, post_p, + is_gimple_val, fb_rvalue); + ret = MIN (ret, tret); + } + else if (TREE_CODE (t) == COMPONENT_REF) + { + tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p, post_p, + is_gimple_reg, fb_rvalue); + ret = MIN (ret, tret); } STRIP_USELESS_TYPE_CONVERSION (TREE_OPERAND (t, 0)); @@ -6914,6 +6906,8 @@ gimplify_target_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p) { if (!TYPE_SIZES_GIMPLIFIED (TREE_TYPE (temp))) gimplify_type_sizes (TREE_TYPE (temp), pre_p); + /* FIXME: this is correct only when the size of the type does + not depend on expressions evaluated in init. */ gimplify_vla_decl (temp, pre_p); } else