Am Mittwoch, den 03.11.2021, 10:18 -0400 schrieb Jason Merrill:
> On 10/31/21 05:22, Uecker, Martin wrote:
> > Hi Jason,
> >
> > here is the fourth version of the patch.
> >
> > I followed your suggestion and now make this
> > transformation sooner in pointer_int_sum.
> > I also added a check to only do this
> > transformation when the pointer is not a
> > VAR_DECL, which avoids it in the most
> > common cases where it is not necessary.
> >
> > Looking for BIND_EXPR seems complicated
> > and I could not convince myself that it is
> > worth it. I also see the risk that this
> > makes potential failure cases even more
> > subtle. What do you think?
>
> That makes sense. Though see some minor comments below.
Thank you! I made these changes and ran
bootstrap and tests again.
Ok for trunk?
Any idea how to fix returning structs with
VLA member from statement expressions?
Otherwise, I will add an error message to
the FE in another patch.
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 wrapping the pointer expression in
> > pointer_int_sum. This fixes PR91038 and some of the test cases
> > from PR29970 (structs with VLA members need further work).
> >
> >
> > 2021-10-30 Martin Uecker <muec...@gwdg.de>
> >
> > gcc/
> > PR c/91038
> > PR c/29970
> > * c-family/c-common.c (pointer_int_sum): Make sure
> > pointer expressions are evaluated first when the size
> > expression depends on for variably-modified types.
> > * gimplify.c (gimplify_var_or_parm_decl): Update comment.
> > (gimplify_compound_lval): Gimplify base expression first.
> > (gimplify_target_expr): Add comment.
> >
> > 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 436df45df68..668a2a129c6 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -3306,7 +3306,19 @@ pointer_int_sum (location_t loc, enum tree_code
resultcode,
TREE_TYPE (result_type)))
size_exp = integer_one_node;
else
- size_exp = size_in_bytes_loc (loc, TREE_TYPE (result_type));
+ {
+ size_exp = size_in_bytes_loc (loc, TREE_TYPE (result_type));
+ /* Wrap the pointer expression in a SAVE_EXPR to make sure it
+ is evaluated first when the size expression may depend
+ on it for VM types. */
+ if (TREE_SIDE_EFFECTS (size_exp)
+ && TREE_SIDE_EFFECTS (ptrop)
+ && variably_modified_type_p (TREE_TYPE (ptrop), NULL))
+ {
+ ptrop = build1_loc (loc, SAVE_EXPR, TREE_TYPE (ptrop), ptrop);
+ size_exp = build2 (COMPOUND_EXPR, TREE_TYPE (intop), ptrop, size_exp);
+ }
+ }
/* We are manipulating pointer values, so we don't need to warn
about relying on undefined signed overflow. We disable the
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index c2ab96e7e18..84f7dc3c248 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -2964,7 +2964,9 @@ 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)
@@ -3109,16 +3111,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)
@@ -3127,18 +3135,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)
{
@@ -3155,18 +3153,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)
{
@@ -3186,18 +3174,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);
- }
}
}
@@ -3208,21 +3186,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));
@@ -6927,6 +6918,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
