gimple-ssa-warn-access: Adjust maybe_warn_nonstring_arg for nonstring multidimensional arrays [PR117178]

Message ID Z86V/VZaQ6h5whBy@tucnak
State New
Headers
Series gimple-ssa-warn-access: Adjust maybe_warn_nonstring_arg for nonstring multidimensional arrays [PR117178] |

Commit Message

Jakub Jelinek March 10, 2025, 7:34 a.m. UTC
  Hi!

I've rushed up the PR117178 multi-dimensional nonstring support.  Before
a week off I've just bootstrapped/regtested the actual change, so
handle_nonstring_attribute/get_attr_nonstring_decl adjustments after playing
with simple short tests and left the testcases for the next day, where I've
basically copied the attr-nonstring-{1,...,8}.c tests to
attr-nonstring-{9,...,16}.c and adjusted them so that they test
multi-dimensional nonstring instead of single-dimensional ones.  But I
forgot I haven't finished adjusting the dg-* lines and a few others to make
all the tests pass and posted the patch for review anyway.
When it got approved recently, I've only found out that some of the tests
still FAIL when about to commit it and quickly adjusted them, but I've added
4 xfails in there.  Sorry.

The following patch fixes those 4 xfails (and results in 2 false positive
warnings not being produced either).
The thing is that maybe_warn_nonstring_arg simply assumed that nonstring
arrays must be single-dimensional, so when it sees a nonstring decl with
ARRAY_TYPE, it just used its dimension.  With multi-dimensional arrays
that is not the right dimension to use though, it can be dimension of
some outer dimension, e.g. if we have
char a[5][6][7] __attribute__((nonstring)) if decl is
a[5] it would assume maximum non-NUL terminated string length of 5 rather than
7, if a[5][6] it would assume 6 and only for a[5][6][0] it would assume the
correct 7.  So, the following patch looks through all the outer dimensions
to reach the innermost one (which for attribute nonstring is guaranteed to
have char/unsigned char/signed char element type).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2025-03-10  Jakub Jelinek  <jakub@redhat.com>

	PR c/117178
	* gimple-ssa-warn-access.cc (maybe_warn_nonstring_arg): Look through
	multi-dimensional array types, stop at the innermost ARRAY_TYPE.

	* c-c++-common/attr-nonstring-11.c: Remove xfails.
	* c-c++-common/attr-nonstring-12.c (warn_strcmp_cst_1,
	warn_strcmp_cst_2): Don't expect any warnings here.
	(warn_strcmp_cst_3, warn_strcmp_cst_4): New functions with expected
	warnings.


	Jakub
  

Comments

Richard Biener March 10, 2025, 8:25 a.m. UTC | #1
> Am 10.03.2025 um 08:35 schrieb Jakub Jelinek <jakub@redhat.com>:
> 
> Hi!
> 
> I've rushed up the PR117178 multi-dimensional nonstring support.  Before
> a week off I've just bootstrapped/regtested the actual change, so
> handle_nonstring_attribute/get_attr_nonstring_decl adjustments after playing
> with simple short tests and left the testcases for the next day, where I've
> basically copied the attr-nonstring-{1,...,8}.c tests to
> attr-nonstring-{9,...,16}.c and adjusted them so that they test
> multi-dimensional nonstring instead of single-dimensional ones.  But I
> forgot I haven't finished adjusting the dg-* lines and a few others to make
> all the tests pass and posted the patch for review anyway.
> When it got approved recently, I've only found out that some of the tests
> still FAIL when about to commit it and quickly adjusted them, but I've added
> 4 xfails in there.  Sorry.
> 
> The following patch fixes those 4 xfails (and results in 2 false positive
> warnings not being produced either).
> The thing is that maybe_warn_nonstring_arg simply assumed that nonstring
> arrays must be single-dimensional, so when it sees a nonstring decl with
> ARRAY_TYPE, it just used its dimension.  With multi-dimensional arrays
> that is not the right dimension to use though, it can be dimension of
> some outer dimension, e.g. if we have
> char a[5][6][7] __attribute__((nonstring)) if decl is
> a[5] it would assume maximum non-NUL terminated string length of 5 rather than
> 7, if a[5][6] it would assume 6 and only for a[5][6][0] it would assume the
> correct 7.  So, the following patch looks through all the outer dimensions
> to reach the innermost one (which for attribute nonstring is guaranteed to
> have char/unsigned char/signed char element type).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok

Richard 

> 2025-03-10  Jakub Jelinek  <jakub@redhat.com>
> 
>    PR c/117178
>    * gimple-ssa-warn-access.cc (maybe_warn_nonstring_arg): Look through
>    multi-dimensional array types, stop at the innermost ARRAY_TYPE.
> 
>    * c-c++-common/attr-nonstring-11.c: Remove xfails.
>    * c-c++-common/attr-nonstring-12.c (warn_strcmp_cst_1,
>    warn_strcmp_cst_2): Don't expect any warnings here.
>    (warn_strcmp_cst_3, warn_strcmp_cst_4): New functions with expected
>    warnings.
> 
> --- gcc/gimple-ssa-warn-access.cc.jj    2025-01-02 11:23:17.000000000 +0100
> +++ gcc/gimple-ssa-warn-access.cc    2025-03-08 11:44:09.304064889 +0100
> @@ -602,6 +602,10 @@ maybe_warn_nonstring_arg (tree fndecl, G
>       bool known_size = false;
>       tree type = TREE_TYPE (decl);
> 
> +      while (TREE_CODE (type) == ARRAY_TYPE
> +         && TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
> +    type = TREE_TYPE (type);
> +
>       /* Determine the array size.  For arrays of unknown bound and
>     pointers reset BOUND to trigger the appropriate warning.  */
>       if (TREE_CODE (type) == ARRAY_TYPE)
> --- gcc/testsuite/c-c++-common/attr-nonstring-11.c.jj    2025-03-08 00:49:09.958121143 +0100
> +++ gcc/testsuite/c-c++-common/attr-nonstring-11.c    2025-03-08 11:50:15.513041709 +0100
> @@ -165,8 +165,8 @@ void test_strcmp (struct MemArrays *p)
> void test_strncmp_warn (struct MemArrays *p)
> {
>   enum { N = sizeof arr[2] };
> -  T (strncmp (str[2], arr[2], N));    /* { dg-bogus "argument 2 declared attribute 'nonstring' is smaller than the specified bound 4" "" { xfail *-*-* } } */
> -  T (strncmp (arr[2], str[2], N));    /* { dg-bogus "argument 1 declared attribute 'nonstring' is smaller than the specified bound 4" "" { xfail *-*-* } } */
> +  T (strncmp (str[2], arr[2], N));
> +  T (strncmp (arr[2], str[2], N));
> 
>   T (strncmp (str[2], arr[2], N + 1));   /* { dg-warning "argument 2 declared attribute .nonstring. is smaller than the specified bound 5" } */
>   T (strncmp (arr[2], str[2], N + 1));   /* { dg-warning "argument 1 declared attribute .nonstring. is smaller than the specified bound 5" } */
> @@ -237,7 +237,7 @@ void test_stpncpy_warn (struct MemArrays
>   enum { N = sizeof arr[2] };
> 
>   T (stpncpy (str[2], str[2], N));
> -  T (stpncpy (str[2], arr[2], N));    /* { dg-bogus "argument 2 declared attribute 'nonstring' is smaller than the specified bound 4" "" { xfail *-*-* } } */
> +  T (stpncpy (str[2], arr[2], N));
>   T (stpncpy (arr[2], str[2], N));
> 
>   T (stpncpy (str[2], *ptr, N));
> @@ -370,7 +370,7 @@ void test_stnrdup_warn (struct MemArrays
>   enum { N = sizeof arr[2] };
> 
>   T (strndup (str[2], N));
> -  T (strndup (arr[2], N));    /* { dg-bogus "argument 1 declared attribute 'nonstring' is smaller than the specified bound 4" "" { xfail *-*-* } } */
> +  T (strndup (arr[2], N));
> 
>   T (strndup (*ptr, N));
>   T (strndup (*parr, N));
> --- gcc/testsuite/c-c++-common/attr-nonstring-12.c.jj    2025-03-08 00:07:01.879907901 +0100
> +++ gcc/testsuite/c-c++-common/attr-nonstring-12.c    2025-03-08 11:52:11.850445944 +0100
> @@ -26,12 +26,22 @@ enum { X = sizeof ar5[2] + 1 };
> 
> int warn_strcmp_cst_1 (void)
> {
> -  return strcmp ("bar", arx[3]);       /* { dg-warning "argument 2 declared attribute .nonstring." } */
> +  return strcmp ("bar", arx[3]);
> }
> 
> int warn_strcmp_cst_2 (void)
> {
> -  return strcmp (arx[3], "foo");       /* { dg-warning "argument 1 declared attribute .nonstring." } */
> +  return strcmp (arx[3], "foo");
> +}
> +
> +int warn_strcmp_cst_3 (void)
> +{
> +  return strcmp ("barfoobazquxcorge1", arx[3]);       /* { dg-warning "argument 2 declared attribute .nonstring." } */
> +}
> +
> +int warn_strcmp_cst_4 (void)
> +{
> +  return strcmp (arx[3], "foobarbazquxcorge1");       /* { dg-warning "argument 1 declared attribute .nonstring." } */
> }
> 
> 
> 
>    Jakub
>
  

Patch

--- gcc/gimple-ssa-warn-access.cc.jj	2025-01-02 11:23:17.000000000 +0100
+++ gcc/gimple-ssa-warn-access.cc	2025-03-08 11:44:09.304064889 +0100
@@ -602,6 +602,10 @@  maybe_warn_nonstring_arg (tree fndecl, G
       bool known_size = false;
       tree type = TREE_TYPE (decl);
 
+      while (TREE_CODE (type) == ARRAY_TYPE
+	     && TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
+	type = TREE_TYPE (type);
+
       /* Determine the array size.  For arrays of unknown bound and
 	 pointers reset BOUND to trigger the appropriate warning.  */
       if (TREE_CODE (type) == ARRAY_TYPE)
--- gcc/testsuite/c-c++-common/attr-nonstring-11.c.jj	2025-03-08 00:49:09.958121143 +0100
+++ gcc/testsuite/c-c++-common/attr-nonstring-11.c	2025-03-08 11:50:15.513041709 +0100
@@ -165,8 +165,8 @@  void test_strcmp (struct MemArrays *p)
 void test_strncmp_warn (struct MemArrays *p)
 {
   enum { N = sizeof arr[2] };
-  T (strncmp (str[2], arr[2], N));	/* { dg-bogus "argument 2 declared attribute 'nonstring' is smaller than the specified bound 4" "" { xfail *-*-* } } */
-  T (strncmp (arr[2], str[2], N));	/* { dg-bogus "argument 1 declared attribute 'nonstring' is smaller than the specified bound 4" "" { xfail *-*-* } } */
+  T (strncmp (str[2], arr[2], N));
+  T (strncmp (arr[2], str[2], N));
 
   T (strncmp (str[2], arr[2], N + 1));   /* { dg-warning "argument 2 declared attribute .nonstring. is smaller than the specified bound 5" } */
   T (strncmp (arr[2], str[2], N + 1));   /* { dg-warning "argument 1 declared attribute .nonstring. is smaller than the specified bound 5" } */
@@ -237,7 +237,7 @@  void test_stpncpy_warn (struct MemArrays
   enum { N = sizeof arr[2] };
 
   T (stpncpy (str[2], str[2], N));
-  T (stpncpy (str[2], arr[2], N));	/* { dg-bogus "argument 2 declared attribute 'nonstring' is smaller than the specified bound 4" "" { xfail *-*-* } } */
+  T (stpncpy (str[2], arr[2], N));
   T (stpncpy (arr[2], str[2], N));
 
   T (stpncpy (str[2], *ptr, N));
@@ -370,7 +370,7 @@  void test_stnrdup_warn (struct MemArrays
   enum { N = sizeof arr[2] };
 
   T (strndup (str[2], N));
-  T (strndup (arr[2], N));	/* { dg-bogus "argument 1 declared attribute 'nonstring' is smaller than the specified bound 4" "" { xfail *-*-* } } */
+  T (strndup (arr[2], N));
 
   T (strndup (*ptr, N));
   T (strndup (*parr, N));
--- gcc/testsuite/c-c++-common/attr-nonstring-12.c.jj	2025-03-08 00:07:01.879907901 +0100
+++ gcc/testsuite/c-c++-common/attr-nonstring-12.c	2025-03-08 11:52:11.850445944 +0100
@@ -26,12 +26,22 @@  enum { X = sizeof ar5[2] + 1 };
 
 int warn_strcmp_cst_1 (void)
 {
-  return strcmp ("bar", arx[3]);       /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  return strcmp ("bar", arx[3]);
 }
 
 int warn_strcmp_cst_2 (void)
 {
-  return strcmp (arx[3], "foo");       /* { dg-warning "argument 1 declared attribute .nonstring." } */
+  return strcmp (arx[3], "foo");
+}
+
+int warn_strcmp_cst_3 (void)
+{
+  return strcmp ("barfoobazquxcorge1", arx[3]);       /* { dg-warning "argument 2 declared attribute .nonstring." } */
+}
+
+int warn_strcmp_cst_4 (void)
+{
+  return strcmp (arx[3], "foobarbazquxcorge1");       /* { dg-warning "argument 1 declared attribute .nonstring." } */
 }