Am Montag, den 08.11.2021, 19:13 +0100 schrieb Martin Uecker:
> Am Montag, den 08.11.2021, 12:13 -0500 schrieb Jason Merrill:
> > On 11/7/21 01:40, Uecker, Martin wrote:
> > > Am Mittwoch, den 03.11.2021, 10:18 -0400 schrieb Jason Merrill:
>
> ...
>
> > > Thank you! I made these changes and ran
> > > bootstrap and tests again.
> >
> > Hmm, it doesn't look like you made the change to use the save_expr
> > function instead of build1?
>
> Oh, sorry. I wanted to change it and then forgot.
> Now also with this change (changelog as before).
Ok, with is this change?
Best,
Martin
> > > Ok for trunk?
> > >
> > >
> > > Any idea how to fix returning structs with
> > > VLA member from statement expressions?
> >
> > Testcase?
>
> void foo(void)
> {
> ({ int N = 3; struct { char x[N]; } x; x; });
> }
>
> The difference to the tests in this patch (which
> also forgot to include in the last version) is that
> the object of variable size is returned from the
> statement expression and not a pointer to it.
> This can not happen with arrays because they decay
> to pointers.
>
>
> Martin
>
>
> > > Otherwise, I will add an error message to
> > > the FE in another patch.
> > >
> > > Martin
> > >
>
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index 436df45df68..95083f95442 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 = save_expr (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
> diff --git a/gcc/testsuite/gcc.dg/vla-stexp-3.c
> b/gcc/testsuite/gcc.dg/vla-stexp-3.c
> new file mode 100644
> index 00000000000..e663de1cd72
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vla-stexp-3.c
> @@ -0,0 +1,11 @@
> +/* PR91038 */
> +/* { dg-do compile } */
> +/* { dg-options "" } */
> +
> +
> +void bar(void)
> +{
> + ({ int N = 2; int (*x)[9][N] = 0; x; })[1];
> + ({ int N = 2; int (*x)[9][N] = 0; x; })[0]; // should not ice
> +}
> +
> diff --git a/gcc/testsuite/gcc.dg/vla-stexp-4.c
> b/gcc/testsuite/gcc.dg/vla-stexp-4.c
> new file mode 100644
> index 00000000000..612b5a802fc
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vla-stexp-4.c
> @@ -0,0 +1,94 @@
> +/* PR29970, PR91038 */
> +/* { dg-do run } */
> +/* { dg-options "-O0 -Wunused-variable" } */
> +
> +int foo3b(void) // should not return 0
> +{
> + int n = 0;
> + return sizeof *({ n = 10; int x[n]; &x; });
> +}
> +
> +int foo4(void) // should not ICE
> +{
> + return (*({
> + int n = 20;
> + char (*x)[n][n] = __builtin_malloc(n * n);
> + (*x)[12][1] = 1;
> + x;
> + }))[12][1];
> +}
> +
> +int foo5(void) // should return 1, returns 0
> +{
> + int n = 0;
> + return (*({
> + n = 20;
> + char (*x)[n][n] = __builtin_malloc(n * n);
> + (*x)[12][1] = 1;
> + (*x)[0][1] = 0;
> + x;
> + }))[12][1];
> +}
> +
> +int foo5c(void) // should return 400
> +{
> + int n = 0;
> + return sizeof(*({
> + n = 20;
> + char (*x)[n][n] = __builtin_malloc(n * n);
> + (*x)[12][1] = 1;
> + (*x)[0][1] = 0;
> + x;
> + }));
> +}
> +
> +int foo5b(void) // should return 1, returns 0
> +{
> + int n = 0; /* { dg-warning "unused variable" } */
> + return (*({
> + int n = 20;
> + char (*x)[n][n] = __builtin_malloc(n * n);
> + (*x)[12][1] = 1;
> + (*x)[0][1] = 0;
> + x;
> + }))[12][1];
> +}
> +
> +int foo5a(void) // should return 1, returns 0
> +{
> + return (*({
> + int n = 20;
> + char (*x)[n][n] = __builtin_malloc(n * n);
> + (*x)[12][1] = 1;
> + (*x)[0][1] = 0;
> + x;
> + }))[12][1];
> +}
> +
> +
> +
> +
> +int main()
> +{
> + if (sizeof(int[10]) != foo3b())
> + __builtin_abort();
> +
> + if (1 != foo4())
> + __builtin_abort();
> +
> + if (400 != foo5c())
> + __builtin_abort();
> +
> + if (1 != foo5a())
> + __builtin_abort();
> +
> + if (1 != foo5b()) // -O0
> + __builtin_abort();
> +
> + if (1 != foo5())
> + __builtin_abort();
> +
> + return 0;
> +}
> +
> +
> diff --git a/gcc/testsuite/gcc.dg/vla-stexp-5.c
> b/gcc/testsuite/gcc.dg/vla-stexp-5.c
> new file mode 100644
> index 00000000000..d6a7f2b34b8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vla-stexp-5.c
> @@ -0,0 +1,30 @@
> +/* PR29970 */
> +/* { dg-do run } */
> +/* { dg-options "-Wunused-variable" } */
> +
> +
> +
> +
> +int foo2a(void) // should not ICE
> +{
> + return ({ int n = 20; struct { int x[n];} x; x.x[12] = 1; sizeof(x);
> });
> +}
> +
> +
> +int foo2b(void) // should not ICE
> +{
> + return sizeof *({ int n = 20; struct { int x[n];} x; x.x[12] = 1;
> &x; });
> +}
> +
> +int main()
> +{
> + if (sizeof(struct { int x[20]; }) != foo2a())
> + __builtin_abort();
> +
> + if (sizeof(struct { int x[20]; }) != foo2b())
> + __builtin_abort();
> +
> + return 0;
> +}
> +
> +
> diff --git a/gcc/testsuite/gcc.dg/vla-stexp-6.c
> b/gcc/testsuite/gcc.dg/vla-stexp-6.c
> new file mode 100644
> index 00000000000..3d96d38898b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vla-stexp-6.c
> @@ -0,0 +1,94 @@
> +/* PR29970, PR91038 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -Wunused-variable" } */
> +
> +int foo3b(void) // should not return 0
> +{
> + int n = 0;
> + return sizeof *({ n = 10; int x[n]; &x; });
> +}
> +
> +int foo4(void) // should not ICE
> +{
> + return (*({
> + int n = 20;
> + char (*x)[n][n] = __builtin_malloc(n * n);
> + (*x)[12][1] = 1;
> + x;
> + }))[12][1];
> +}
> +
> +int foo5(void) // should return 1, returns 0
> +{
> + int n = 0;
> + return (*({
> + n = 20;
> + char (*x)[n][n] = __builtin_malloc(n * n);
> + (*x)[12][1] = 1;
> + (*x)[0][1] = 0;
> + x;
> + }))[12][1];
> +}
> +
> +int foo5c(void) // should return 400
> +{
> + int n = 0;
> + return sizeof(*({
> + n = 20;
> + char (*x)[n][n] = __builtin_malloc(n * n);
> + (*x)[12][1] = 1;
> + (*x)[0][1] = 0;
> + x;
> + }));
> +}
> +
> +int foo5b(void) // should return 1, returns 0
> +{
> + int n = 0; /* { dg-warning "unused variable" } */
> + return (*({
> + int n = 20;
> + char (*x)[n][n] = __builtin_malloc(n * n);
> + (*x)[12][1] = 1;
> + (*x)[0][1] = 0;
> + x;
> + }))[12][1];
> +}
> +
> +int foo5a(void) // should return 1, returns 0
> +{
> + return (*({
> + int n = 20;
> + char (*x)[n][n] = __builtin_malloc(n * n);
> + (*x)[12][1] = 1;
> + (*x)[0][1] = 0;
> + x;
> + }))[12][1];
> +}
> +
> +
> +
> +
> +int main()
> +{
> + if (sizeof(int[10]) != foo3b())
> + __builtin_abort();
> +
> + if (1 != foo4())
> + __builtin_abort();
> +
> + if (400 != foo5c())
> + __builtin_abort();
> +
> + if (1 != foo5a())
> + __builtin_abort();
> +
> + if (1 != foo5b()) // -O0
> + __builtin_abort();
> +
> + if (1 != foo5())
> + __builtin_abort();
> +
> + return 0;
> +}
> +
> +
> diff --git a/gcc/testsuite/gcc.dg/vla-stexp-7.c
> b/gcc/testsuite/gcc.dg/vla-stexp-7.c
> new file mode 100644
> index 00000000000..3091b9184c2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vla-stexp-7.c
> @@ -0,0 +1,44 @@
> +/* PR91038 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -Wunused-variable" } */
> +
> +
> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> +
> +struct lbm {
> +
> + int D;
> + const int* DQ;
> +
> +} D2Q9 = { 2,
> + (const int*)&(const int[9][2]){
> + { 0, 0 },
> + { 1, 0 }, { 0, 1 }, { -1, 0 }, { 0, -1 },
> + { 1, 1 }, { -1, 1 }, { -1, -1 }, { 1, -1 },
> + }
> +};
> +
> +void zouhe_left(void)
> +{
> + __auto_type xx = (*({ int N = 2; struct lbm __x = D2Q9; ((const
> int(*)[9][N])__x.DQ); }));
> +
> + if (1 != xx[1][0])
> + __builtin_abort();
> +
> + if (2 != ARRAY_SIZE(xx[1]))
> + __builtin_abort();
> +
> + if (1 != (*({ int N = 2; struct lbm __x = D2Q9; ((const
> int(*)[9][N])__x.DQ); }))[1][0])
> + __builtin_abort();
> +
> + if (2 != ARRAY_SIZE(*({ int N = 2; struct lbm __x = D2Q9; ((const
> int(*)[9][N])__x.DQ);
> })[1]))
> + __builtin_abort();
> +}
> +
> +int main()
> +{
> + zouhe_left();
> + return 0;
> +}
> +
> +
> diff --git a/gcc/testsuite/gcc.dg/vla-stexp-8.c
> b/gcc/testsuite/gcc.dg/vla-stexp-8.c
> new file mode 100644
> index 00000000000..5b475eb6cf2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vla-stexp-8.c
> @@ -0,0 +1,47 @@
> +/* PR29970, PR91038 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wunused-variable" } */
> +
> +
> +int foo0(void)
> +{
> + int c = *(*(*({ int n = 10; int (*x)[n][n] = __builtin_malloc(sizeof
> *x); x; }) + 5) + 5);
> + return c;
> +}
> +
> +int foo1(void)
> +{
> + int c = *(5 + *(5 + *({ int n = 10; int (*x)[n][n] =
> __builtin_malloc(sizeof *x); x; })));
> + return c;
> +}
> +
> +int bar2(void)
> +{
> + int c = (*({ int n = 10; struct { int y[n]; int z; }* x =
> __builtin_malloc(sizeof *x); x;
> })).z;
> + return c;
> +}
> +
> +int bar3(void)
> +{
> + int n = 2; /* { dg-warning "unused variable" } */
> + int c = (*({ int n = 3; /* { dg-warning "unused variable" } */
> + ({ int n = 10; int (*x)[n][n] = __builtin_malloc(sizeof *x); x;
> }); }))[5][5];
> + return c;
> +}
> +
> +int bar3b(void)
> +{
> + int n = 2; /* { dg-warning "unused variable" } */
> + int c = (*({ int n = 3; /* { dg-warning "unused variable" } */
> + ({ int n = 10; int (*x)[n][n] = __builtin_malloc(sizeof *x); x;
> }); }))[0][0];
> + return c;
> +}
> +
> +int bar4(void)
> +{
> + int n = 2; /* { dg-warning "unused variable" } */
> + int c = *(5 + *( 5 + *({ int n = 3; /* { dg-warning "unused
> variable" } */
> + ({ int n = 10; int (*x)[n][n] = __builtin_malloc(sizeof *x); x;
> }); })));
> + return c;
> +}
> +
> diff --git a/gcc/testsuite/gcc.dg/vla-stexp-9.c
> b/gcc/testsuite/gcc.dg/vla-stexp-9.c
> new file mode 100644
> index 00000000000..3593a790785
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vla-stexp-9.c
> @@ -0,0 +1,53 @@
> +/* PR91038 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -Wunused-variable" } */
> +
> +
> +
> +void foo(void)
> +{
> + if (2 * sizeof(int) != sizeof((*({ int N = 2; int (*x)[9][N] = 0; x;
> })[1])))
> + __builtin_abort();
> +}
> +
> +void bar(void)
> +{
> + if (2 * sizeof(int) != sizeof((*({ int N = 2; int (*x)[9][N] = 0; x;
> })[0])))
> + __builtin_abort();
> +}
> +
> +void bar0(void)
> +{
> + if (2 * 9 * sizeof(int) != sizeof((*({ int N = 2; int (*x)[9][N] = 0;
> x; }))))
> + __builtin_abort();
> +}
> +
> +void bar11(void)
> +{
> + sizeof(*((*({ int N = 2; int (*x)[9][N] = 0; x; }) + 0)));
> +}
> +
> +void bar12(void)
> +{
> + if (2 * sizeof(int) != sizeof(*((*({ int N = 2; int (*x)[9][N] = 0; x;
> }) ))))
> + __builtin_abort();
> +}
> +
> +void bar1(void)
> +{
> + if (2 * sizeof(int) != sizeof(*((*({ int N = 2; int (*x)[9][N] = 0; x;
> }) + 0))))
> + __builtin_abort();
> +}
> +
> +
> +
> +
> +int main()
> +{
> + foo();
> + bar0();
> + bar12();
> + bar1();
> + bar();
> +}
> +
>
