From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailrelay.tugraz.at (mailrelay.tugraz.at [129.27.2.202]) by sourceware.org (Postfix) with ESMTPS id 2247D3858D35 for ; Mon, 22 May 2023 17:24:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2247D3858D35 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=tugraz.at Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tugraz.at Received: from vra-174-248.tugraz.at (vra-174-248.tugraz.at [129.27.174.248]) by mailrelay.tugraz.at (Postfix) with ESMTPSA id 4QQ4732ZHlz3wNt; Mon, 22 May 2023 19:23:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tugraz.at; s=mailrelay; t=1684776239; bh=Y4MKe9i4YDak4TCJghZ4YnBouLwHMwbOkHQdmjgsFMU=; h=Subject:From:To:Cc:Date; b=OOe1R2dS1c1vodTEPxQb2n/ml9baaLG6caxkDoJGpXjojGwHP7X4P3SAGpKAg5LkB 9TvuYQ0KlF59V1Zf6FAauQTvXBwoATIE1ZcXsfSjsr86BwXv41JIPp5NWQVD0/kzWW t+e0zqN6xQ2/2ZbDTtTqr7IrOdGZ06dAc+3PWM1Q= Message-ID: Subject: [C PATCH v3] Fix ICEs related to VM types in C 2/2 From: Martin Uecker To: gcc-patches@gcc.gnu.org Cc: Joseph Myers , Richard Biener Date: Mon, 22 May 2023 19:23:58 +0200 Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.3-1+deb11u1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-TUG-Backscatter-control: G/VXY7/6zeyuAY/PU2/0qw X-Spam-Scanner: SpamAssassin 3.003001 X-Spam-Score-relay: -1.9 X-Scanned-By: MIMEDefang 2.74 on 129.27.10.116 X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_NUMSUBJECT,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,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 List-Id: This version contains the middle-end changes for PR109450 and test cases as before. The main middle-end change is that we use gimplify_type_sizes also for parameters and remove the special code that also walked into pointers (which is incorrect).   In addition, in the C FE this patch now also adds DECL_EXPR for vm-types which are pointed-to by parameters declared as arrays. The new function created contains the exact code previously used only for regular pointers, and is now also called for parameters declared as arrays. Martin Fix ICEs related to VM types in C 2/2 [PR109450] Size expressions were sometimes lost and not gimplified correctly, leading to ICEs and incorrect evaluation order. Fix this by 1) not recursing pointers when gimplifying parameters in the middle-end (the code is merged with gimplify_type_sizes), which is incorrect because it might access variables declared later for incomplete structs, and 2) adding a decl expr for variably-modified arrays that are pointed to by parameters declared as arrays. PR c/109450 gcc/ * c/c-decl.cc (add_decl_expr): New function. (grokdeclarator): Add decl expr for size expression in types pointed to by parameters declared as arrays. * function.cc (gimplify_parm_type): Remove function. (gimplify_parameters): Call gimplify_parm_sizes. * gimplify.cc (gimplify_type_sizes): Make function static. (gimplify_parm_sizes): New function. gcc/testsuite/ * gcc.dg/pr109450-1.c: New test. * gcc.dg/pr109450-2.c: New test. * gcc.dg/vla-26.c: New test. diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc index 494d3cf1747..c35347734b2 100644 --- a/gcc/c/c-decl.cc +++ b/gcc/c/c-decl.cc @@ -6490,6 +6490,55 @@ smallest_type_quals_location (const location_t *locations, return loc; } + +/* We attach an artificial TYPE_DECL to pointed-to type + and arrange for it to be included in a DECL_EXPR. This + forces the sizes evaluation at a safe point and ensures it + is not deferred until e.g. within a deeper conditional context. + + PARM contexts have no enclosing statement list that + can hold the DECL_EXPR, so we need to use a BIND_EXPR + instead, and add it to the list of expressions that + need to be evaluated. + + TYPENAME contexts do have an enclosing statement list, + but it would be incorrect to use it, as the size should + only be evaluated if the containing expression is + evaluated. We might also be in the middle of an + expression with side effects on the pointed-to type size + "arguments" prior to the pointer declaration point and + the fake TYPE_DECL in the enclosing context would force + the size evaluation prior to the side effects. We therefore + use BIND_EXPRs in TYPENAME contexts too. */ +static void +add_decl_expr(location_t loc, enum decl_context decl_context, tree type, tree *expr) +{ + tree bind = NULL_TREE; + if (decl_context == TYPENAME || decl_context == PARM || decl_context == FIELD) + { + bind = build3 (BIND_EXPR, void_type_node, NULL_TREE, NULL_TREE, NULL_TREE); + TREE_SIDE_EFFECTS (bind) = 1; + BIND_EXPR_BODY (bind) = push_stmt_list (); + push_scope (); + } + + tree decl = build_decl (loc, TYPE_DECL, NULL_TREE, type); + pushdecl (decl); + DECL_ARTIFICIAL (decl) = 1; + add_stmt (build_stmt (DECL_SOURCE_LOCATION (decl), DECL_EXPR, decl)); + TYPE_NAME (type) = decl; + + if (bind) + { + pop_scope (); + BIND_EXPR_BODY (bind) = pop_stmt_list (BIND_EXPR_BODY (bind)); + if (*expr) + *expr = build2 (COMPOUND_EXPR, void_type_node, *expr, bind); + else + *expr = bind; + } +} + /* Given declspecs and a declarator, determine the name and type of the object declared and construct a ..._DECL node for it. @@ -7474,58 +7523,9 @@ grokdeclarator (const struct c_declarator *declarator, This is expected to happen automatically when the pointed-to type has a name/declaration of it's own, but special attention - is required if the type is anonymous. - - We attach an artificial TYPE_DECL to such pointed-to type - and arrange for it to be included in a DECL_EXPR. This - forces the sizes evaluation at a safe point and ensures it - is not deferred until e.g. within a deeper conditional context. - - PARM contexts have no enclosing statement list that - can hold the DECL_EXPR, so we need to use a BIND_EXPR - instead, and add it to the list of expressions that - need to be evaluated. - - TYPENAME contexts do have an enclosing statement list, - but it would be incorrect to use it, as the size should - only be evaluated if the containing expression is - evaluated. We might also be in the middle of an - expression with side effects on the pointed-to type size - "arguments" prior to the pointer declaration point and - the fake TYPE_DECL in the enclosing context would force - the size evaluation prior to the side effects. We therefore - use BIND_EXPRs in TYPENAME contexts too. */ - if (!TYPE_NAME (type) - && c_type_variably_modified_p (type)) - { - tree bind = NULL_TREE; - if (decl_context == TYPENAME || decl_context == PARM - || decl_context == FIELD) - { - bind = build3 (BIND_EXPR, void_type_node, NULL_TREE, - NULL_TREE, NULL_TREE); - TREE_SIDE_EFFECTS (bind) = 1; - BIND_EXPR_BODY (bind) = push_stmt_list (); - push_scope (); - } - tree decl = build_decl (loc, TYPE_DECL, NULL_TREE, type); - pushdecl (decl); - DECL_ARTIFICIAL (decl) = 1; - add_stmt (build_stmt (DECL_SOURCE_LOCATION (decl), DECL_EXPR, decl)); - TYPE_NAME (type) = decl; - - if (bind) - { - pop_scope (); - BIND_EXPR_BODY (bind) - = pop_stmt_list (BIND_EXPR_BODY (bind)); - if (*expr) - *expr = build2 (COMPOUND_EXPR, void_type_node, *expr, - bind); - else - *expr = bind; - } - } + is required if the type is anonymous. */ + if (!TYPE_NAME (type) && c_type_variably_modified_p (type)) + add_decl_expr (loc, decl_context, type, expr); type = c_build_pointer_type (type); @@ -7787,6 +7787,11 @@ grokdeclarator (const struct c_declarator *declarator, if (type_quals) type = c_build_qualified_type (type, type_quals, orig_qual_type, orig_qual_indirect); + + /* The pointed-to type may need a decl expr (see above). */ + if (!TYPE_NAME (type) && c_type_variably_modified_p (type)) + add_decl_expr (loc, decl_context, type, expr); + type = c_build_pointer_type (type); type_quals = array_ptr_quals; if (type_quals) diff --git a/gcc/function.cc b/gcc/function.cc index f0ae641512d..5699b9d495d 100644 --- a/gcc/function.cc +++ b/gcc/function.cc @@ -3872,30 +3872,6 @@ assign_parms (tree fndecl) } } -/* A subroutine of gimplify_parameters, invoked via walk_tree. - For all seen types, gimplify their sizes. */ - -static tree -gimplify_parm_type (tree *tp, int *walk_subtrees, void *data) -{ - tree t = *tp; - - *walk_subtrees = 0; - if (TYPE_P (t)) - { - if (POINTER_TYPE_P (t)) - *walk_subtrees = 1; - else if (TYPE_SIZE (t) && !TREE_CONSTANT (TYPE_SIZE (t)) - && !TYPE_SIZES_GIMPLIFIED (t)) - { - gimplify_type_sizes (t, (gimple_seq *) data); - *walk_subtrees = 1; - } - } - - return NULL; -} - /* Gimplify the parameter list for current_function_decl. This involves evaluating SAVE_EXPRs of variable sized parameters and generating code to implement callee-copies reference parameters. Returns a sequence of @@ -3931,14 +3907,7 @@ gimplify_parameters (gimple_seq *cleanup) SAVE_EXPRs (amongst others) onto a pending sizes list. This turned out to be less than manageable in the gimple world. Now we have to hunt them down ourselves. */ - walk_tree_without_duplicates (&data.arg.type, - gimplify_parm_type, &stmts); - - if (TREE_CODE (DECL_SIZE_UNIT (parm)) != INTEGER_CST) - { - gimplify_one_sizepos (&DECL_SIZE (parm), &stmts); - gimplify_one_sizepos (&DECL_SIZE_UNIT (parm), &stmts); - } + gimplify_parm_sizes (parm, &stmts); if (data.arg.pass_by_reference) { diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc index d0d16a24820..fd990c328eb 100644 --- a/gcc/gimplify.cc +++ b/gcc/gimplify.cc @@ -242,6 +242,7 @@ static struct gimplify_omp_ctx *gimplify_omp_ctxp; static bool in_omp_construct; /* Forward declaration. */ +static void gimplify_type_sizes (tree type, gimple_seq *list_p); static enum gimplify_status gimplify_compound_expr (tree *, gimple_seq *, bool); static hash_map *oacc_declare_returns; static enum gimplify_status gimplify_expr (tree *, gimple_seq *, gimple_seq *, @@ -17425,7 +17426,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, /* Look through TYPE for variable-sized objects and gimplify each such size that we find. Add to LIST_P any statements generated. */ -void +static void gimplify_type_sizes (tree type, gimple_seq *list_p) { if (type == NULL || type == error_mark_node) @@ -17533,6 +17534,21 @@ gimplify_type_sizes (tree type, gimple_seq *list_p) } } +/* Gimplify sizes in parameter declarations. */ + +void +gimplify_parm_sizes (tree parm, gimple_seq *list_p) +{ + gimplify_type_sizes (TREE_TYPE (parm), list_p); + + if (TREE_CODE (DECL_SIZE_UNIT (parm)) != INTEGER_CST) + { + gimplify_one_sizepos (&DECL_SIZE (parm), list_p); + gimplify_one_sizepos (&DECL_SIZE_UNIT (parm), list_p); + } +} + + /* A subroutine of gimplify_type_sizes to make sure that *EXPR_P, a size or position, has had all of its SAVE_EXPRs evaluated. We add any required statements to *STMT_P. */ diff --git a/gcc/gimplify.h b/gcc/gimplify.h index f4a3eea2606..17ea0580647 100644 --- a/gcc/gimplify.h +++ b/gcc/gimplify.h @@ -78,7 +78,7 @@ extern enum gimplify_status gimplify_expr (tree *, gimple_seq *, gimple_seq *, int omp_construct_selector_matches (enum tree_code *, int, int *); -extern void gimplify_type_sizes (tree, gimple_seq *); +extern void gimplify_parm_sizes (tree, gimple_seq *); extern void gimplify_one_sizepos (tree *, gimple_seq *); extern gbind *gimplify_body (tree, bool); extern enum gimplify_status gimplify_arg (tree *, gimple_seq *, location_t, diff --git a/gcc/testsuite/gcc.dg/pr109450-1.c b/gcc/testsuite/gcc.dg/pr109450-1.c new file mode 100644 index 00000000000..aec127f2afc --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr109450-1.c @@ -0,0 +1,21 @@ +/* PR c/109450 + * { dg-do run } + * { dg-options "-std=gnu99" } + * */ + +int bar(int n, struct foo* x) /* { dg-warning "not be visible" } */ +{ + int a = n; + struct foo { char buf[n++]; }* p = x; + return a; +} + +int main() +{ + if (1 != bar(1, 0)) + __builtin_abort(); +} + + + + diff --git a/gcc/testsuite/gcc.dg/pr109450-2.c b/gcc/testsuite/gcc.dg/pr109450-2.c new file mode 100644 index 00000000000..06799f6df23 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr109450-2.c @@ -0,0 +1,18 @@ +/* PR c/109450 + * { dg-do run } + * { dg-options "-std=gnu99" } + * */ + +int bar(int n, struct foo *x) /* { dg-warning "not be visible" } */ +{ + int a = n; + struct foo { char buf[a++]; }* p = x; + return n == a; +} + +int main() +{ + if (bar(1, 0)) + __builtin_abort(); +} + diff --git a/gcc/testsuite/gcc.dg/vla-26.c b/gcc/testsuite/gcc.dg/vla-26.c new file mode 100644 index 00000000000..5d2fa3e280a --- /dev/null +++ b/gcc/testsuite/gcc.dg/vla-26.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-std=c99 -O2" } */ + +void ed(int n, float s[3][n]) +{ + for (int i = 0; i < n; i++) + s[1][i]; +} + +void e(int n, float s[3][n]) +{ + ed(n, s); +} + +