From patchwork Mon May 2 18:37:54 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 93702 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 08053B6F12 for ; Tue, 3 May 2011 04:38:18 +1000 (EST) Received: (qmail 5991 invoked by alias); 2 May 2011 18:38:16 -0000 Received: (qmail 5977 invoked by uid 22791); 2 May 2011 18:38:15 -0000 X-SWARE-Spam-Status: No, hits=-6.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, TW_EG, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 02 May 2011 18:37:56 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p42IbtfI030891 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 2 May 2011 14:37:56 -0400 Received: from [127.0.0.1] (ovpn-113-54.phx2.redhat.com [10.3.113.54]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p42IbtG4011642 for ; Mon, 2 May 2011 14:37:55 -0400 Message-ID: <4DBEFA02.7060301@redhat.com> Date: Mon, 02 May 2011 14:37:54 -0400 From: Jason Merrill User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.15) Gecko/20110307 Fedora/3.1.9-0.39.b3pre.fc14 Lightning/1.0b2 Thunderbird/3.1.9 MIME-Version: 1.0 To: gcc-patches List Subject: Re: C++ PATCH for c++/48446 (ICE with VLA) References: <4DA70A16.8050303@redhat.com> In-Reply-To: <4DA70A16.8050303@redhat.com> 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 On 04/14/2011 10:52 AM, Jason Merrill wrote: > This patch avoids this issue by saving bounds values which have side > effects into a local automatic variable (since VLAs can only appear in > automatic variables). We stick with the SAVE_EXPR approach for bounds > without side-effects to avoid breaking vla9.C. I finally found the code in grokdeclarator that I thought I remembered while working on this bug but couldn't find, and the comment there led me to put together a testcase broken by this change. So I'm reverting the change to compute_array_index_type in favor of a more aggressive version of the precalculation that grokdeclarator already does. Tested x86_64-pc-linux-gnu, applying to trunk. commit dc6c2d45b04636be3430ca7722a5f648001659d2 Author: Jason Merrill Date: Fri Apr 29 11:56:33 2011 -0400 PR c++/48446 * decl.c (stabilize_save_expr_r, stabilize_vla_size): New. (compute_array_index_type): Revert earlier 48446 changes. (grokdeclarator): Use stabilize_vla_size. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 0a2e1dd..f9dd6de 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -7576,6 +7576,38 @@ check_static_variable_definition (tree decl, tree type) return 0; } +/* *expr_p is part of the TYPE_SIZE of a variably-sized array. If any + SAVE_EXPRs in *expr_p wrap expressions with side-effects, break those + expressions out into temporary variables so that walk_tree doesn't + step into them (c++/15764). */ + +static tree +stabilize_save_expr_r (tree *expr_p, int *walk_subtrees, void *data) +{ + struct pointer_set_t *pset = (struct pointer_set_t *)data; + tree expr = *expr_p; + if (TREE_CODE (expr) == SAVE_EXPR) + { + tree op = TREE_OPERAND (expr, 0); + cp_walk_tree (&op, stabilize_save_expr_r, data, pset); + if (TREE_SIDE_EFFECTS (op)) + TREE_OPERAND (expr, 0) = get_temp_regvar (TREE_TYPE (op), op); + } + else if (!EXPR_P (expr)) + *walk_subtrees = 0; + return NULL; +} + +/* Entry point for the above. */ + +static void +stabilize_vla_size (tree size) +{ + struct pointer_set_t *pset = pointer_set_create (); + /* Break out any function calls into temporary variables. */ + cp_walk_tree (&size, stabilize_save_expr_r, pset, pset); +} + /* Given the SIZE (i.e., number of elements) in an array, compute an appropriate index type for the array. If non-NULL, NAME is the name of the thing being declared. */ @@ -7769,16 +7801,8 @@ compute_array_index_type (tree name, tree size, tsubst_flags_t complain) processing_template_decl = saved_processing_template_decl; if (!TREE_CONSTANT (itype)) - { - /* A variable sized array. */ - if (TREE_SIDE_EFFECTS (itype)) - /* Use get_temp_regvar rather than variable_size here so that - people walking expressions that use a variable of this type - don't walk into this expression. */ - itype = get_temp_regvar (TREE_TYPE (itype), itype); - else - itype = variable_size (itype); - } + /* A variable sized array. */ + itype = variable_size (itype); /* Make sure that there was no overflow when creating to a signed index type. (For example, on a 32-bit machine, an array with size 2^32 - 1 is too big.) */ @@ -9051,7 +9075,12 @@ grokdeclarator (const cp_declarator *declarator, && (decl_context == NORMAL || decl_context == FIELD) && at_function_scope_p () && variably_modified_type_p (type, NULL_TREE)) - finish_expr_stmt (TYPE_SIZE (type)); + { + /* First break out any side-effects. */ + stabilize_vla_size (TYPE_SIZE (type)); + /* And then force evaluation of the SAVE_EXPR. */ + finish_expr_stmt (TYPE_SIZE (type)); + } if (declarator->kind == cdk_reference) { @@ -9126,6 +9155,14 @@ grokdeclarator (const cp_declarator *declarator, } } + /* We need to stabilize side-effects in VLA sizes for regular array + declarations too, not just pointers to arrays. */ + if (type != error_mark_node && !TYPE_NAME (type) + && (decl_context == NORMAL || decl_context == FIELD) + && at_function_scope_p () + && variably_modified_type_p (type, NULL_TREE)) + stabilize_vla_size (TYPE_SIZE (type)); + /* A `constexpr' specifier used in an object declaration declares the object as `const'. */ if (constexpr_p && innermost_code != cdk_function) diff --git a/gcc/testsuite/c-c++-common/vla-1.c b/gcc/testsuite/c-c++-common/vla-1.c new file mode 100644 index 0000000..401c4e0 --- /dev/null +++ b/gcc/testsuite/c-c++-common/vla-1.c @@ -0,0 +1,21 @@ +/* Test that changes to a variable are reflected in a VLA later in the + expression. */ +/* { dg-options "" } */ + +#ifdef __cplusplus +extern "C" +#endif +void abort(); + +int i = 4; +int f() +{ + return i; +} + +int main() +{ + if (i+=2, sizeof(*(int(*)[f()])0) != 6*sizeof(int)) + abort(); + return 0; +}