[PATCH] Fix ICE when mixing VLAs and statement expressions [PR91038]

["Uecker, Martin" <Martin.Uecker@med.uni-goettingen.de>]

Here is an attempt to fix some old and annoying bugs related
to VLAs and statement expressions. In particulary, this seems
to fix the issues with variably-modified types which are
returned from statement expressions (which works on clang),
but there are still bugs remaining related to structs
with VLA members (which seems to be a FE bug).

Of course, I might be doing something stupid... 

The patch survives bootstrapping and regresstion testing
on x86_64.


--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. Some of these issues can be addressed by
gimplifying the base expression earlier in gimplify_compound_lval.
This fixes PR91038 and some of the test cases from PR29970
(structs with VLA members need further work).

    
    2021-08-01  Martin Uecker  <muecker@gwdg.de>
    
    gcc/
	PR c/91038
	PR c/29970
	* gimplify.c (gimplify_var_or_parm_decl): Update comment.
	(gimplify_compound_lval): Gimplify base expression first.
    
    gcc/testsuite/
	PR c/91038
	PR c/29970
	* gcc.dg/vla-stexp-01.c: New test.
	* gcc.dg/vla-stexp-02.c: New test.
	* gcc.dg/vla-stexp-03.c: New test.
	* gcc.dg/vla-stexp-04.c: New test.


diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 21ff32ee4aa..885d5f73585 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -2839,7 +2839,10 @@ 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 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)
@@ -2984,9 +2987,23 @@ 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.  */
+     So we do this in three steps.  First we gimplify the base,
+     then we deal with the annotations for any variables in the
+     components, 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 first. */
+
+  /* Step 1 is to gimplify the base expression.  Make sure lvalue is set
+     so as to match the min_lval predicate.  Failure to do so may result
+     in the creation of large aggregate temporaries.  */
+  tret = gimplify_expr (p, pre_p, post_p, is_gimple_min_lval,
+			fallback | fb_lvalue);
+
+  ret = MIN (ret, tret);
+
+
   for (i = expr_stack.length () - 1; i >= 0; i--)
     {
       tree t = expr_stack[i];
@@ -3076,12 +3093,6 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	}
     }
 
-  /* Step 2 is to gimplify the base expression.  Make sure lvalue is set
-     so as to match the min_lval predicate.  Failure to do so may result
-     in the creation of large aggregate temporaries.  */
-  tret = gimplify_expr (p, pre_p, post_p, is_gimple_min_lval,
-			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.  */
diff --git a/gcc/testsuite/gcc.dg/vla-stexpr-01.c b/gcc/testsuite/gcc.dg/vla-stexpr-01.c
new file mode 100644
index 00000000000..27e1817eb63
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vla-stexpr-01.c
@@ -0,0 +1,94 @@
+/* PR29970, PR91038 */
+/* { dg-do run } */
+/* { dg-options "-O0" } */
+
+int foo3b(void)   // should not return 0
+{
+        int n = 0;
+        return sizeof *({ n = 10; int x[n]; x; &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;
+        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-stexpr-02.c b/gcc/testsuite/gcc.dg/vla-stexpr-02.c
new file mode 100644
index 00000000000..99b4f571e52
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vla-stexpr-02.c
@@ -0,0 +1,31 @@
+/* PR29970 */
+/* { dg-do run } */
+/* { dg-options "" } */
+
+
+
+
+int foo2a(void)   // should not ICE
+{
+        return ({ int n = 20; struct { int x[n];} x; x.x[12] = 1; sizeof(x); });
+}
+
+#if 0
+int foo2b(void)   // should not ICE
+{
+        return sizeof *({ int n = 20; struct { int x[n];} x; x.x[12] = 1; &x; });
+}
+#endif
+
+int main()
+{
+	if (sizeof(struct { int x[20]; }) != foo2a())
+		__builtin_abort();
+#if 0
+	if (sizeof(struct { int x[20]; }) != foo2b())
+		__builtin_abort();
+#endif
+	return 0;
+}
+
+
diff --git a/gcc/testsuite/gcc.dg/vla-stexpr-03.c b/gcc/testsuite/gcc.dg/vla-stexpr-03.c
new file mode 100644
index 00000000000..61f8986eaa3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vla-stexpr-03.c
@@ -0,0 +1,94 @@
+/* PR29970, PR91038 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+int foo3b(void)   // should not return 0
+{
+        int n = 0;
+        return sizeof *({ n = 10; int x[n]; x; &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;
+        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-stexpr-04.c b/gcc/testsuite/gcc.dg/vla-stexpr-04.c
new file mode 100644
index 00000000000..ffc8b12fa9b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vla-stexpr-04.c
@@ -0,0 +1,38 @@
+/* PR91038 */
+/* { dg-do run } */
+/* { dg-options "" } */
+
+
+#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();
+}
+
+int main()
+{
+	zouhe_left();
+	return 0;
+}
+
+