| Message ID | de4e36b6-918f-c02b-7050-9afbf9b020ff@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] clear VLA bounds in attribute access (PR 97172) | expand |
On Thu, Jan 28, 2021 at 12:08 AM Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Attached is another attempt to fix the problem caused by allowing > front-end trees representing nontrivial VLA bound expressions to > stay in attribute access attached to functions. Since removing > these trees seems to be everyone's preference this patch does that > by extending the free_lang_data pass to look for and zero out these > trees. > > Because free_lang_data only frees anything when LTO is enabled and > we want these trees cleared regardless to keep them from getting > clobbered during gimplification, this change also modifies the pass > to do the clearing even when the pass is otherwise inactive. if (TREE_CODE (bound) == NOP_EXPR) + bound = TREE_OPERAND (bound, 0); + + if (TREE_CODE (bound) == CONVERT_EXPR) + { + tree op0 = TREE_OPERAND (bound, 0); + tree bndtyp = TREE_TYPE (bound); + tree op0typ = TREE_TYPE (op0); + if (TYPE_PRECISION (bndtyp) == TYPE_PRECISION (op0typ)) + bound = op0; + } + + if (TREE_CODE (bound) == NON_LVALUE_EXPR) + bound = TREE_OPERAND (bound, 0); all of the above can be just STRIP_NOPS (bound); which also handles nesting of the above in any order. + if (TREE_CODE (bound) == PLUS_EXPR + && integer_all_onesp (TREE_OPERAND (bound, 1))) + { + bound = TREE_OPERAND (bound, 0); + if (TREE_CODE (bound) == NOP_EXPR) + bound = TREE_OPERAND (bound, 0); + } so it either does or does not strip a -1 but has no indication on whether it did that? That looks fragile and broken. Anyway, the split out of this function seems unrelated to the original problem and should be submitted separately. + for (vblist = TREE_VALUE (vblist); vblist; vblist = TREE_CHAIN (vblist)) + { + tree *pvbnd = &TREE_VALUE (vblist); + if (!*pvbnd || DECL_P (*pvbnd)) + continue; so this doesn't let constant bounds prevail but only symbolical ones? Not that I care but I'd have expected || CONSTANT_CLASS_P (*pvbnd) The free-lang-data parts are OK. Richard. > Tested on x86_64-linux. > > Martin
On Thu, Jan 28, 2021 at 09:31:46AM +0100, Richard Biener via Gcc-patches wrote: > + if (TREE_CODE (bound) == PLUS_EXPR > + && integer_all_onesp (TREE_OPERAND (bound, 1))) > + { > + bound = TREE_OPERAND (bound, 0); > + if (TREE_CODE (bound) == NOP_EXPR) > + bound = TREE_OPERAND (bound, 0); > + } > > so it either does or does not strip a -1 but has no indication on > whether it did that? That looks fragile and broken. Yeah. Plus again STRIP_NOPs in there should be used. But certainly it needs to remember whether there was + -1 or not and compensate for it. > The free-lang-data parts are OK. But is fld the right spot to do it? If it is only the FE that emits the warnings, shouldn't it be stripped already before gimplification so that it isn't actually corrupted? I mean in c_parse_final_cleanups or c_common_parse_file depending on if it is just C or C++ too? Plus, if the expressions in the attribute don't contain SAVE_EXPRs, why there isn't unshare_expr called on them (and if they do, I don't see how it would be guaranteed, can't one e.g. do int bar (void); void foo (int n, int a[n + 1][bar () + 2], int b[sizeof (a[0]) + 32]) { } ?) the unsharing variant I've pasted into the PR. Jakub
On 1/28/21 1:31 AM, Richard Biener wrote: > On Thu, Jan 28, 2021 at 12:08 AM Martin Sebor via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> Attached is another attempt to fix the problem caused by allowing >> front-end trees representing nontrivial VLA bound expressions to >> stay in attribute access attached to functions. Since removing >> these trees seems to be everyone's preference this patch does that >> by extending the free_lang_data pass to look for and zero out these >> trees. >> >> Because free_lang_data only frees anything when LTO is enabled and >> we want these trees cleared regardless to keep them from getting >> clobbered during gimplification, this change also modifies the pass >> to do the clearing even when the pass is otherwise inactive. > > if (TREE_CODE (bound) == NOP_EXPR) > + bound = TREE_OPERAND (bound, 0); > + > + if (TREE_CODE (bound) == CONVERT_EXPR) > + { > + tree op0 = TREE_OPERAND (bound, 0); > + tree bndtyp = TREE_TYPE (bound); > + tree op0typ = TREE_TYPE (op0); > + if (TYPE_PRECISION (bndtyp) == TYPE_PRECISION (op0typ)) > + bound = op0; > + } > + > + if (TREE_CODE (bound) == NON_LVALUE_EXPR) > + bound = TREE_OPERAND (bound, 0); > > all of the above can be just > > STRIP_NOPS (bound); > > which also handles nesting of the above in any order. No, it can't be just STRIP_NOPS. The goal is to strip the meaningless (to the user) cast to sizetype from the array type. For example: void f (int n, int[n]); void f (int n, int[n + 1]); I want the type in the warning to reflect the source: warning: argument 2 of type ‘int[n + 1]’ declared with mismatched bound ‘n + 1’ [-Wvla-parameter] and not: warning: ... ‘int[(sizetype)(n + 1)]’ ... > > + if (TREE_CODE (bound) == PLUS_EXPR > + && integer_all_onesp (TREE_OPERAND (bound, 1))) > + { > + bound = TREE_OPERAND (bound, 0); > + if (TREE_CODE (bound) == NOP_EXPR) > + bound = TREE_OPERAND (bound, 0); > + } > > so it either does or does not strip a -1 but has no indication on > whether it did that? That looks fragile and broken. Indication to what? The caller? The function is only used to recover a meaningful VLA bound for warnings and its callers aren't interested in whether the -1 was stripped or not. > > Anyway, the split out of this function seems unrelated to the > original problem and should be submitted separately. It was a remnant of the previous patch where it was used to format the string representation of the VLA bounds and called from three sites. I've removed the function from this revision (and restored the one site in the pretty printer that open-codes the same thing). > > + for (vblist = TREE_VALUE (vblist); vblist; vblist = TREE_CHAIN (vblist)) > + { > + tree *pvbnd = &TREE_VALUE (vblist); > + if (!*pvbnd || DECL_P (*pvbnd)) > + continue; > > so this doesn't let constant bounds prevail but only symbolical ones? Not > that I care but I'd have expected || CONSTANT_CLASS_P (*pvbnd) There must be some confusion here. There are no constant VLA bounds. The essential purpose of this patch is to remove expressions from the attributes, such as PLUS_EXPR, that denote nontrivial VLA bounds. The test above retains decls that might refer to function parameters or global variables so that they can be mentioned in middle end warnings. Attached is yet another revision of this fix that moves the call to attr_access:free_lang_data() to c_parse_final_cleanups() as Jakub suggested. Martin
On 1/28/21 2:22 AM, Jakub Jelinek wrote: > On Thu, Jan 28, 2021 at 09:31:46AM +0100, Richard Biener via Gcc-patches wrote: >> + if (TREE_CODE (bound) == PLUS_EXPR >> + && integer_all_onesp (TREE_OPERAND (bound, 1))) >> + { >> + bound = TREE_OPERAND (bound, 0); >> + if (TREE_CODE (bound) == NOP_EXPR) >> + bound = TREE_OPERAND (bound, 0); >> + } >> >> so it either does or does not strip a -1 but has no indication on >> whether it did that? That looks fragile and broken. > > Yeah. Plus again STRIP_NOPs in there should be used. But certainly it > needs to remember whether there was + -1 or not and compensate for it. > >> The free-lang-data parts are OK. > > But is fld the right spot to do it? > If it is only the FE that emits the warnings, shouldn't it be stripped > already before gimplification so that it isn't actually corrupted? > > I mean in c_parse_final_cleanups or c_common_parse_file depending on > if it is just C or C++ too? I've been asking for a good place to do it since December and free_lang_data was the only suggestion. I even mentioned it to you this Tuesday. c_parse_final_cleanups looks like a better place but I don't understand why you've waited until now to point me to it. > > Plus, if the expressions in the attribute don't contain SAVE_EXPRs, why > there isn't unshare_expr called on them (and if they do, I don't see how > it would be guaranteed, can't one e.g. do > int bar (void); > void foo (int n, int a[n + 1][bar () + 2], int b[sizeof (a[0]) + 32]) > { > } > ?) the unsharing variant I've pasted into the PR. I'm not sure what you're asking here or if you're just repeating the same question we went over in November when I posted the first patch to unshare the expressions. In that thread, after I explained that the expressions in the attribute aren't evaluated, you ultimately agreed it wasn't necessary: https://gcc.gnu.org/pipermail/gcc-patches/2020-November/559953.html The front end wraps each VLA bound that needs to be evaluated in a SAVE_EXPR (and, as Joseph points out in the same thread, if there are more of them, they're wrapped in a COMPOUND_EXPR). But the attribute doesn't use these SAVE_EXPRs -- instead, it uses the expressions before they're wrapped. This wasn't a choice I made deliberately. I just didn't know about the COMPOUND_EXPR wrapped bounds. Martin
On 1/28/21 1:59 PM, Martin Sebor wrote: > On 1/28/21 1:31 AM, Richard Biener wrote: >> On Thu, Jan 28, 2021 at 12:08 AM Martin Sebor via Gcc-patches >> <gcc-patches@gcc.gnu.org> wrote: >>> >>> Attached is another attempt to fix the problem caused by allowing >>> front-end trees representing nontrivial VLA bound expressions to >>> stay in attribute access attached to functions. Since removing >>> these trees seems to be everyone's preference this patch does that >>> by extending the free_lang_data pass to look for and zero out these >>> trees. >>> >>> Because free_lang_data only frees anything when LTO is enabled and >>> we want these trees cleared regardless to keep them from getting >>> clobbered during gimplification, this change also modifies the pass >>> to do the clearing even when the pass is otherwise inactive. >> >> if (TREE_CODE (bound) == NOP_EXPR) >> + bound = TREE_OPERAND (bound, 0); >> + >> + if (TREE_CODE (bound) == CONVERT_EXPR) >> + { >> + tree op0 = TREE_OPERAND (bound, 0); >> + tree bndtyp = TREE_TYPE (bound); >> + tree op0typ = TREE_TYPE (op0); >> + if (TYPE_PRECISION (bndtyp) == TYPE_PRECISION (op0typ)) >> + bound = op0; >> + } >> + >> + if (TREE_CODE (bound) == NON_LVALUE_EXPR) >> + bound = TREE_OPERAND (bound, 0); >> >> all of the above can be just >> >> STRIP_NOPS (bound); >> >> which also handles nesting of the above in any order. > > No, it can't be just STRIP_NOPS. > > The goal is to strip the meaningless (to the user) cast to sizetype > from the array type. For example: > > void f (int n, int[n]); > void f (int n, int[n + 1]); > > I want the type in the warning to reflect the source: > > warning: argument 2 of type ‘int[n + 1]’ declared with mismatched > bound ‘n + 1’ [-Wvla-parameter] > > and not: > > warning: ... ‘int[(sizetype)(n + 1)]’ ... > >> >> + if (TREE_CODE (bound) == PLUS_EXPR >> + && integer_all_onesp (TREE_OPERAND (bound, 1))) >> + { >> + bound = TREE_OPERAND (bound, 0); >> + if (TREE_CODE (bound) == NOP_EXPR) >> + bound = TREE_OPERAND (bound, 0); >> + } >> >> so it either does or does not strip a -1 but has no indication on >> whether it did that? That looks fragile and broken. > > Indication to what? The caller? The function is only used to recover > a meaningful VLA bound for warnings and its callers aren't interested > in whether the -1 was stripped or not. > >> >> Anyway, the split out of this function seems unrelated to the >> original problem and should be submitted separately. > > It was a remnant of the previous patch where it was used to format > the string representation of the VLA bounds and called from three > sites. I've removed the function from this revision (and restored > the one site in the pretty printer that open-codes the same thing). > >> >> + for (vblist = TREE_VALUE (vblist); vblist; vblist = TREE_CHAIN >> (vblist)) >> + { >> + tree *pvbnd = &TREE_VALUE (vblist); >> + if (!*pvbnd || DECL_P (*pvbnd)) >> + continue; >> >> so this doesn't let constant bounds prevail but only symbolical ones? >> Not >> that I care but I'd have expected || CONSTANT_CLASS_P (*pvbnd) > > There must be some confusion here. There are no constant VLA bounds. > The essential purpose of this patch is to remove expressions from > the attributes, such as PLUS_EXPR, that denote nontrivial VLA bounds. > The test above retains decls that might refer to function parameters > or global variables so that they can be mentioned in middle end > warnings. > > Attached is yet another revision of this fix that moves the call > to attr_access:free_lang_data() to c_parse_final_cleanups() as > Jakub suggested. With no further comments I have committed the final patch in g:0718336a528. Martin
On Mon, Feb 01, 2021 at 09:11:20AM -0700, Martin Sebor via Gcc-patches wrote: > > > > Because free_lang_data only frees anything when LTO is enabled and > > > > we want these trees cleared regardless to keep them from getting > > > > clobbered during gimplification, this change also modifies the pass > > > > to do the clearing even when the pass is otherwise inactive. > > > > > > if (TREE_CODE (bound) == NOP_EXPR) > > > + bound = TREE_OPERAND (bound, 0); > > > + > > > + if (TREE_CODE (bound) == CONVERT_EXPR) > > > + { > > > + tree op0 = TREE_OPERAND (bound, 0); > > > + tree bndtyp = TREE_TYPE (bound); > > > + tree op0typ = TREE_TYPE (op0); > > > + if (TYPE_PRECISION (bndtyp) == TYPE_PRECISION (op0typ)) > > > + bound = op0; > > > + } > > > + > > > + if (TREE_CODE (bound) == NON_LVALUE_EXPR) > > > + bound = TREE_OPERAND (bound, 0); > > > > > > all of the above can be just > > > > > > STRIP_NOPS (bound); > > > > > > which also handles nesting of the above in any order. > > > > No, it can't be just STRIP_NOPS. > > > > The goal is to strip the meaningless (to the user) cast to sizetype > > from the array type. For example: > > > > void f (int n, int[n]); > > void f (int n, int[n + 1]); > > > > I want the type in the warning to reflect the source: > > > > warning: argument 2 of type ‘int[n + 1]’ declared with mismatched > > bound ‘n + 1’ [-Wvla-parameter] > > > > and not: > > > > warning: ... ‘int[(sizetype)(n + 1)]’ ... > > > > > > > > + if (TREE_CODE (bound) == PLUS_EXPR > > > + && integer_all_onesp (TREE_OPERAND (bound, 1))) > > > + { > > > + bound = TREE_OPERAND (bound, 0); > > > + if (TREE_CODE (bound) == NOP_EXPR) > > > + bound = TREE_OPERAND (bound, 0); > > > + } > > > > > > so it either does or does not strip a -1 but has no indication on > > > whether it did that? That looks fragile and broken. > > > > Indication to what? The caller? The function is only used to recover > > a meaningful VLA bound for warnings and its callers aren't interested > > in whether the -1 was stripped or not. > > > > > > > > Anyway, the split out of this function seems unrelated to the > > > original problem and should be submitted separately. > > > > It was a remnant of the previous patch where it was used to format > > the string representation of the VLA bounds and called from three > > sites. I've removed the function from this revision (and restored > > the one site in the pretty printer that open-codes the same thing). > > > > > > > > + for (vblist = TREE_VALUE (vblist); vblist; vblist = > > > TREE_CHAIN (vblist)) > > > + { > > > + tree *pvbnd = &TREE_VALUE (vblist); > > > + if (!*pvbnd || DECL_P (*pvbnd)) > > > + continue; > > > > > > so this doesn't let constant bounds prevail but only symbolical > > > ones? Not > > > that I care but I'd have expected || CONSTANT_CLASS_P (*pvbnd) > > > > There must be some confusion here. There are no constant VLA bounds. > > The essential purpose of this patch is to remove expressions from > > the attributes, such as PLUS_EXPR, that denote nontrivial VLA bounds. > > The test above retains decls that might refer to function parameters > > or global variables so that they can be mentioned in middle end > > warnings. > > > > Attached is yet another revision of this fix that moves the call > > to attr_access:free_lang_data() to c_parse_final_cleanups() as > > Jakub suggested. > > With no further comments I have committed the final patch in > g:0718336a528. This is unacceptable, you chose to ignore Richard's comments, nobody has approved the patch and you've committed it anyway. The code of course should be using STRIP_NOPS, and if the callers don't care if you sometimes strip away + -1 from it or not, they are just broken. Either the expression stands for the largest valid index into the array, or it stands for the number of array elements. If the former, you don't want to strip away + -1 when it appears, if the latter, you do want to strip it away but if you don't find it, you need to add + 1 yourself, the + -1 could disappear from earlier folding. Jakub
On 2/1/21 9:27 AM, Jakub Jelinek wrote: > On Mon, Feb 01, 2021 at 09:11:20AM -0700, Martin Sebor via Gcc-patches wrote: >>>>> Because free_lang_data only frees anything when LTO is enabled and >>>>> we want these trees cleared regardless to keep them from getting >>>>> clobbered during gimplification, this change also modifies the pass >>>>> to do the clearing even when the pass is otherwise inactive. >>>> >>>> if (TREE_CODE (bound) == NOP_EXPR) >>>> + bound = TREE_OPERAND (bound, 0); >>>> + >>>> + if (TREE_CODE (bound) == CONVERT_EXPR) >>>> + { >>>> + tree op0 = TREE_OPERAND (bound, 0); >>>> + tree bndtyp = TREE_TYPE (bound); >>>> + tree op0typ = TREE_TYPE (op0); >>>> + if (TYPE_PRECISION (bndtyp) == TYPE_PRECISION (op0typ)) >>>> + bound = op0; >>>> + } >>>> + >>>> + if (TREE_CODE (bound) == NON_LVALUE_EXPR) >>>> + bound = TREE_OPERAND (bound, 0); >>>> >>>> all of the above can be just >>>> >>>> STRIP_NOPS (bound); >>>> >>>> which also handles nesting of the above in any order. >>> >>> No, it can't be just STRIP_NOPS. >>> >>> The goal is to strip the meaningless (to the user) cast to sizetype >>> from the array type. For example: >>> >>> void f (int n, int[n]); >>> void f (int n, int[n + 1]); >>> >>> I want the type in the warning to reflect the source: >>> >>> warning: argument 2 of type ‘int[n + 1]’ declared with mismatched >>> bound ‘n + 1’ [-Wvla-parameter] >>> >>> and not: >>> >>> warning: ... ‘int[(sizetype)(n + 1)]’ ... >>> >>>> >>>> + if (TREE_CODE (bound) == PLUS_EXPR >>>> + && integer_all_onesp (TREE_OPERAND (bound, 1))) >>>> + { >>>> + bound = TREE_OPERAND (bound, 0); >>>> + if (TREE_CODE (bound) == NOP_EXPR) >>>> + bound = TREE_OPERAND (bound, 0); >>>> + } >>>> >>>> so it either does or does not strip a -1 but has no indication on >>>> whether it did that? That looks fragile and broken. >>> >>> Indication to what? The caller? The function is only used to recover >>> a meaningful VLA bound for warnings and its callers aren't interested >>> in whether the -1 was stripped or not. >>> >>>> >>>> Anyway, the split out of this function seems unrelated to the >>>> original problem and should be submitted separately. >>> >>> It was a remnant of the previous patch where it was used to format >>> the string representation of the VLA bounds and called from three >>> sites. I've removed the function from this revision (and restored >>> the one site in the pretty printer that open-codes the same thing). >>> >>>> >>>> + for (vblist = TREE_VALUE (vblist); vblist; vblist = >>>> TREE_CHAIN (vblist)) >>>> + { >>>> + tree *pvbnd = &TREE_VALUE (vblist); >>>> + if (!*pvbnd || DECL_P (*pvbnd)) >>>> + continue; >>>> >>>> so this doesn't let constant bounds prevail but only symbolical >>>> ones? Not >>>> that I care but I'd have expected || CONSTANT_CLASS_P (*pvbnd) >>> >>> There must be some confusion here. There are no constant VLA bounds. >>> The essential purpose of this patch is to remove expressions from >>> the attributes, such as PLUS_EXPR, that denote nontrivial VLA bounds. >>> The test above retains decls that might refer to function parameters >>> or global variables so that they can be mentioned in middle end >>> warnings. >>> >>> Attached is yet another revision of this fix that moves the call >>> to attr_access:free_lang_data() to c_parse_final_cleanups() as >>> Jakub suggested. >> >> With no further comments I have committed the final patch in >> g:0718336a528. > > This is unacceptable, you chose to ignore Richard's comments, > nobody has approved the patch and you've committed it anyway. You might want to look at the commit first before making accusations. I committed the subset that Richard approved in the place you suggested. That's also what I posted in my last reply: https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564475.html > > The code of course should be using STRIP_NOPS, and if the callers > don't care if you sometimes strip away + -1 from it or not, they are just > broken. Either the expression stands for the largest valid index into the > array, or it stands for the number of array elements. If the former, you > don't want to strip away + -1 when it appears, if the latter, you do want to > strip it away but if you don't find it, you need to add + 1 yourself, the + > -1 could disappear from earlier folding. None of this was committed. Martin
PR middle-end/97172 - ICE: tree code 'ssa_name' is not supported in LTO streams gcc/ChangeLog: PR middle-end/97172 * attribs.c (attr_access::free_lang_data): Define new function. * attribs.h (attr_access::free_lang_data): Declare new function. * tree.c (free_lang_data_in_type): Call attr_access::free_lang_data. (array_bound_from_maxval): Define new function. * tree.h (array_bound_from_maxval): Declare new function. gcc/c-family/ChangeLog: PR middle-end/97172 * c-pretty-print.c (c_pretty_printer::direct_abstract_declarator): Call array_bound_from_maxval. gcc/c/ChangeLog: PR middle-end/97172 * c-decl.c (get_parm_array_spec): Call array_bound_from_maxval. gcc/testsuite/ChangeLog: PR middle-end/97172 * gcc.dg/pr97172.c: New test. diff --git a/gcc/attribs.c b/gcc/attribs.c index 94991fbbeab..81322d40f1d 100644 --- a/gcc/attribs.c +++ b/gcc/attribs.c @@ -2238,6 +2238,38 @@ attr_access::vla_bounds (unsigned *nunspec) const return list_length (size); } +/* Reset front end-specific attribute access data from ATTRS. + Called from the free_lang_data pass. */ + +/* static */ void +attr_access::free_lang_data (tree attrs) +{ + for (tree acs = attrs; (acs = lookup_attribute ("access", acs)); + acs = TREE_CHAIN (acs)) + { + tree vblist = TREE_VALUE (acs); + vblist = TREE_CHAIN (vblist); + if (!vblist) + continue; + + vblist = TREE_VALUE (vblist); + if (!vblist) + continue; + + for (vblist = TREE_VALUE (vblist); vblist; vblist = TREE_CHAIN (vblist)) + { + tree *pvbnd = &TREE_VALUE (vblist); + if (!*pvbnd || DECL_P (*pvbnd)) + continue; + + /* VLA bounds that are expressions as opposed to DECLs are + only used in the front end. Reset them to keep front end + trees leaking into the middle end (see pr97172) and to + free up memory. */ + *pvbnd = NULL_TREE; + } + } +} /* Defined in attr_access. */ constexpr char attr_access::mode_chars[]; diff --git a/gcc/attribs.h b/gcc/attribs.h index 21d28a47f39..898e73db3e4 100644 --- a/gcc/attribs.h +++ b/gcc/attribs.h @@ -274,6 +274,9 @@ struct attr_access /* Return the access mode corresponding to the character code. */ static access_mode from_mode_char (char); + /* Reset front end-specific attribute access data from attributes. */ + static void free_lang_data (tree); + /* The character codes corresponding to all the access modes. */ static constexpr char mode_chars[5] = { '-', 'r', 'w', 'x', '^' }; diff --git a/gcc/c-family/c-pretty-print.c b/gcc/c-family/c-pretty-print.c index 2095d4badf7..c6e8a45afd5 100644 --- a/gcc/c-family/c-pretty-print.c +++ b/gcc/c-family/c-pretty-print.c @@ -635,22 +635,7 @@ c_pretty_printer::direct_abstract_declarator (tree t) /* Strip the expressions from around a VLA bound added internally to make it fit the domain mold, including any casts. */ - if (TREE_CODE (maxval) == NOP_EXPR) - maxval = TREE_OPERAND (maxval, 0); - if (TREE_CODE (maxval) == PLUS_EXPR - && integer_all_onesp (TREE_OPERAND (maxval, 1))) - { - maxval = TREE_OPERAND (maxval, 0); - if (TREE_CODE (maxval) == NOP_EXPR) - maxval = TREE_OPERAND (maxval, 0); - } - if (TREE_CODE (maxval) == SAVE_EXPR) - { - maxval = TREE_OPERAND (maxval, 0); - if (TREE_CODE (maxval) == NOP_EXPR) - maxval = TREE_OPERAND (maxval, 0); - } - + maxval = array_bound_from_maxval (maxval); expression (maxval); } } diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c index 4ba9477f5d1..9dcad5e362d 100644 --- a/gcc/c/c-decl.c +++ b/gcc/c/c-decl.c @@ -5781,7 +5781,8 @@ get_parm_array_spec (const struct c_parm *parm, tree attrs) { /* Each variable VLA bound is represented by the dollar sign. */ - spec += "$"; + spec += '$'; + nelts = array_bound_from_maxval (nelts); tpbnds = tree_cons (NULL_TREE, nelts, tpbnds); } } @@ -5835,7 +5836,8 @@ get_parm_array_spec (const struct c_parm *parm, tree attrs) } /* Each variable VLA bound is represented by a dollar sign. */ - spec += "$"; + spec += '$'; + nelts = array_bound_from_maxval (nelts); vbchain = tree_cons (NULL_TREE, nelts, vbchain); } diff --git a/gcc/testsuite/gcc.dg/pr97172.c b/gcc/testsuite/gcc.dg/pr97172.c new file mode 100644 index 00000000000..ab5b2e9e7e9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr97172.c @@ -0,0 +1,50 @@ +/* PR middle-end/97172 - ICE: tree code ‘ssa_name’ is not supported in LTO + streams + { dg-do compile } + { dg-options "-Wall -flto" } + { dg-require-effective-target lto } */ + +int n; + +void fn (int a[n]); +void fnp1 (int a[n + 1]); + +void fx_n (int a[][n]); +void fx_np1 (int a[][n + 1]); + +void f2_n (int a[2][n]); +void f2_np1 (int a[2][n + 1]); + +void fn_3 (int a[n][3]); +void fnp1_3 (int a[n + 1][3]); + +void fn_n (int a[n][n]); +void fn_np1 (int a[n][n + 1]); +void fnp1_np1 (int a[n + 1][n + 1]); + +void fn_n_n (int a[n][n][n]); +void fn_n_np1 (int a[n][n][n + 1]); +void fn_np1_np1 (int a[n][n + 1][n + 1]); +void fnp1_np1_np1 (int a[n + 1][n + 1][n + 1]); + + +void gn (int a[n]) { fn (a); } +void gnp1 (int a[n + 1]) { fnp1 (a); } + +void gx_n (int a[][n]) { fx_n (a); } +void gx_np1 (int a[][n + 1]) { fx_np1 (a); } + +void g2_n (int a[2][n]) { f2_n (a); } +void g2_np1 (int a[2][n + 1]) { f2_np1 (a); } + +void gn_3 (int a[n][3]) { fn_3 (a); } +void gnp1_3 (int a[n + 1][3]) { fnp1_3 (a); } + +void gn_n (int a[n][n]) { fn_n (a); } +void gn_np1 (int a[n][n + 1]) { fn_np1 (a); } +void gnp1_np1 (int a[n + 1][n + 1]) { fnp1_np1 (a); } + +void gn_n_n (int a[n][n][n]) { fn_n_n (a); } +void gn_n_np1 (int a[n][n][n + 1]) { fn_n_np1 (a); } +void gn_np1_np1 (int a[n][n + 1][n + 1]) { fn_np1_np1 (a); } +void gnp1_np1_np1 (int a[n + 1][n + 1][n + 1]) { fnp1_np1_np1 (a); } diff --git a/gcc/tree.c b/gcc/tree.c index f9d57e6d409..2d9003bfcae 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -5622,6 +5622,10 @@ free_lang_data_in_type (tree type, class free_lang_data_d *fld) if (TREE_CODE (type) == FUNCTION_TYPE) { TREE_TYPE (type) = fld_simplified_type (TREE_TYPE (type), fld); + + /* Free front end-specific attribute access data. */ + attr_access::free_lang_data (TYPE_ATTRIBUTES (type)); + /* Remove the const and volatile qualifiers from arguments. The C++ front end removes them, but the C front end does not, leading to false ODR violation errors when merging two @@ -6374,6 +6378,26 @@ free_lang_data_in_cgraph (class free_lang_data_d *fld) free_lang_data_in_type (t, fld); } +/* Free attribute access data that are not needed by the middle end. */ + +static void +free_attr_access_data () +{ + struct cgraph_node *n; + + /* Iterate over all functions declared in the translation unit. */ + FOR_EACH_FUNCTION (n) + { + tree fntype = TREE_TYPE (n->decl); + if (!fntype) + continue; + tree attrs = TYPE_ATTRIBUTES (fntype); + if (!attrs) + continue; + + attr_access::free_lang_data (attrs); + } +} /* Free resources that are used by FE but are not needed once they are done. */ @@ -6387,6 +6411,9 @@ free_lang_data (void) if (in_lto_p || (!flag_generate_lto && !flag_generate_offload)) { + if (!in_lto_p) + free_attr_access_data (); + /* Rebuild type inheritance graph even when not doing LTO to get consistent profile data. */ rebuild_type_inheritance_graph (); @@ -9356,6 +9383,48 @@ variably_modified_type_p (tree type, tree fn) #undef RETURN_TRUE_IF_VAR } +/* Strip any expressions from around the array bound MAXVAL, including + any casts, added internally to VLAs to make them fit the array domain + mold. Return the result. */ + +tree +array_bound_from_maxval (tree maxval) +{ + tree bound = maxval; + + if (TREE_CODE (bound) == NOP_EXPR) + bound = TREE_OPERAND (bound, 0); + + if (TREE_CODE (bound) == CONVERT_EXPR) + { + tree op0 = TREE_OPERAND (bound, 0); + tree bndtyp = TREE_TYPE (bound); + tree op0typ = TREE_TYPE (op0); + if (TYPE_PRECISION (bndtyp) == TYPE_PRECISION (op0typ)) + bound = op0; + } + + if (TREE_CODE (bound) == NON_LVALUE_EXPR) + bound = TREE_OPERAND (bound, 0); + + if (TREE_CODE (bound) == PLUS_EXPR + && integer_all_onesp (TREE_OPERAND (bound, 1))) + { + bound = TREE_OPERAND (bound, 0); + if (TREE_CODE (bound) == NOP_EXPR) + bound = TREE_OPERAND (bound, 0); + } + + if (TREE_CODE (bound) == SAVE_EXPR) + { + bound = TREE_OPERAND (bound, 0); + if (TREE_CODE (bound) == NOP_EXPR) + bound = TREE_OPERAND (bound, 0); + } + + return bound; +} + /* Given a DECL or TYPE, return the scope in which it was declared, or NULL_TREE if there is no containing scope. */ diff --git a/gcc/tree.h b/gcc/tree.h index 17a811c02e8..0c4ec76ad21 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -5234,6 +5234,7 @@ extern bool int_fits_type_p (const_tree, const_tree) extern void get_type_static_bounds (const_tree, mpz_t, mpz_t); #endif extern bool variably_modified_type_p (tree, tree); +extern tree array_bound_from_maxval (tree) ATTRIBUTE_NONNULL (1); extern int tree_log2 (const_tree); extern int tree_floor_log2 (const_tree); extern unsigned int tree_ctz (const_tree);