[PATCH] handle folded nonconstant array bounds [PR101702]

[PATCH] handle folded nonconstant array bounds [PR101702]

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 594 bytes --]

-Warray-parameter and -Wvla-parameter assume that array bounds
in function parameters are either constant integers or variable,
but not something in between like a cast of a constant that's
not recognized as an INTEGER_CST until we strip the cast from
it.  This leads to an ICE as the the internal checks fail.

The attached patch fixes the problem by stripping the casts
earlier than before, preventing the inconsistency.  In addition,
it also folds the array bound, avoiding a class of false
positives and negatives that not doing so would lead to otherwise.

Tested on x86_64-linux.

Martin

[-- Attachment #2: gcc-101702.diff --]
[-- Type: text/x-patch, Size: 2319 bytes --]

Handle folded nonconstant array bounds [PR101702]

PR c/101702 - ICE: in handle_argspec_attribute, at c-family/c-attribs.c:3623

gcc/c/ChangeLog:

	PR c/101702
	* c-decl.c (get_parm_array_spec): Strip casts earlier and fold array
	bounds before deciding if they're constant.

gcc/testsuite/ChangeLog:

	PR c/101702
	* gcc.dg/Warray-parameter-11.c: New test.

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 186fa1692c1..63d806a84c9 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -5866,6 +5866,12 @@ get_parm_array_spec (const struct c_parm *parm, tree attrs)
       if (pd->u.array.static_p)
 	spec += 's';
 
+      if (!INTEGRAL_TYPE_P (TREE_TYPE (nelts)))
+	/* Avoid invalid NELTS.  */
+	return attrs;
+
+      STRIP_NOPS (nelts);
+      nelts = c_fully_fold (nelts, false, nullptr);
       if (TREE_CODE (nelts) == INTEGER_CST)
 	{
 	  /* Skip all constant bounds except the most significant one.
@@ -5883,13 +5889,9 @@ get_parm_array_spec (const struct c_parm *parm, tree attrs)
 	  spec += buf;
 	  break;
 	}
-      else if (!INTEGRAL_TYPE_P (TREE_TYPE (nelts)))
-	/* Avoid invalid NELTS.  */
-	return attrs;
 
       /* Each variable VLA bound is represented by a dollar sign.  */
       spec += "$";
-      STRIP_NOPS (nelts);
       vbchain = tree_cons (NULL_TREE, nelts, vbchain);
     }
 
diff --git a/gcc/testsuite/gcc.dg/Warray-parameter-11.c b/gcc/testsuite/gcc.dg/Warray-parameter-11.c
new file mode 100644
index 00000000000..8ca1b55bd28
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-parameter-11.c
@@ -0,0 +1,24 @@
+/* PR c/101702 - ICE on invalid function redeclaration
+   { dg-do compile }
+   { dg-options "-Wall" } */
+
+typedef __INTPTR_TYPE__ intptr_t;
+
+#define copysign(x, y) __builtin_copysign (x, y)
+
+void f0 (double[!copysign (~2, 3)]);
+
+void f1 (double[!copysign (~2, 3)]);
+void f1 (double[1]);                    // { dg-warning "-Warray-parameter" }
+
+void f2 (int[(int)+1.0]);
+void f2 (int[(int)+1.1]);
+
+/* Also verify that equivalent expressions don't needlessly cause false
+   positives or negatives.  */
+struct S { int a[1]; };
+extern struct S *sp;
+
+void f3 (int[(intptr_t)((char*)sp->a - (char*)sp)]);
+void f3 (int[(intptr_t)((char*)&sp->a[0] - (char*)sp)]);
+void f3 (int[(intptr_t)((char*)&sp->a[1] - (char*)sp)]);   // { dg-warning "-Warray-parameter" }

Re: [PATCH] handle folded nonconstant array bounds [PR101702]

[Marek Polacek]

On Tue, Nov 16, 2021 at 05:32:00PM -0700, Martin Sebor via Gcc-patches wrote:
> -Warray-parameter and -Wvla-parameter assume that array bounds
> in function parameters are either constant integers or variable,
> but not something in between like a cast of a constant that's
> not recognized as an INTEGER_CST until we strip the cast from
> it.  This leads to an ICE as the the internal checks fail.
> 
> The attached patch fixes the problem by stripping the casts
> earlier than before, preventing the inconsistency.  In addition,
> it also folds the array bound, avoiding a class of false
> positives and negatives that not doing so would lead to otherwise.
> 
> Tested on x86_64-linux.
> 
> Martin

> Handle folded nonconstant array bounds [PR101702]
> 
> PR c/101702 - ICE: in handle_argspec_attribute, at c-family/c-attribs.c:3623
> 
> gcc/c/ChangeLog:
> 
> 	PR c/101702
> 	* c-decl.c (get_parm_array_spec): Strip casts earlier and fold array
> 	bounds before deciding if they're constant.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c/101702
> 	* gcc.dg/Warray-parameter-11.c: New test.
> 
> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index 186fa1692c1..63d806a84c9 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -5866,6 +5866,12 @@ get_parm_array_spec (const struct c_parm *parm, tree attrs)
>        if (pd->u.array.static_p)
>  	spec += 's';
>  
> +      if (!INTEGRAL_TYPE_P (TREE_TYPE (nelts)))
> +	/* Avoid invalid NELTS.  */
> +	return attrs;
> +
> +      STRIP_NOPS (nelts);
> +      nelts = c_fully_fold (nelts, false, nullptr);

STRIP_NOPS before a call to c_fully_fold looks sort of weird, but I see
it's needed to prevent bogus warnings in Wvla-parameter-12.c:

void f2ci_can (const int m, char a[m]);
void f2ci_can (int n,       char a[n])

OK for trunk then.

>        if (TREE_CODE (nelts) == INTEGER_CST)
>  	{
>  	  /* Skip all constant bounds except the most significant one.
> @@ -5883,13 +5889,9 @@ get_parm_array_spec (const struct c_parm *parm, tree attrs)
>  	  spec += buf;
>  	  break;
>  	}
> -      else if (!INTEGRAL_TYPE_P (TREE_TYPE (nelts)))
> -	/* Avoid invalid NELTS.  */
> -	return attrs;
>  
>        /* Each variable VLA bound is represented by a dollar sign.  */
>        spec += "$";
> -      STRIP_NOPS (nelts);
>        vbchain = tree_cons (NULL_TREE, nelts, vbchain);
>      }
>  
> diff --git a/gcc/testsuite/gcc.dg/Warray-parameter-11.c b/gcc/testsuite/gcc.dg/Warray-parameter-11.c
> new file mode 100644
> index 00000000000..8ca1b55bd28
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/Warray-parameter-11.c
> @@ -0,0 +1,24 @@
> +/* PR c/101702 - ICE on invalid function redeclaration
> +   { dg-do compile }
> +   { dg-options "-Wall" } */
> +
> +typedef __INTPTR_TYPE__ intptr_t;
> +
> +#define copysign(x, y) __builtin_copysign (x, y)
> +
> +void f0 (double[!copysign (~2, 3)]);
> +
> +void f1 (double[!copysign (~2, 3)]);
> +void f1 (double[1]);                    // { dg-warning "-Warray-parameter" }
> +
> +void f2 (int[(int)+1.0]);
> +void f2 (int[(int)+1.1]);
> +
> +/* Also verify that equivalent expressions don't needlessly cause false
> +   positives or negatives.  */
> +struct S { int a[1]; };
> +extern struct S *sp;
> +
> +void f3 (int[(intptr_t)((char*)sp->a - (char*)sp)]);
> +void f3 (int[(intptr_t)((char*)&sp->a[0] - (char*)sp)]);
> +void f3 (int[(intptr_t)((char*)&sp->a[1] - (char*)sp)]);   // { dg-warning "-Warray-parameter" }


Marek