avoid duplicate warning for strcmp with a nonstring (PR 85359)

Message ID fda59513-b7ed-a572-ad34-30c513194ea9@gmail.com
State New
Headers show
Series
  • avoid duplicate warning for strcmp with a nonstring (PR 85359)
Related show

Commit Message

Martin Sebor April 12, 2018, 8:52 p.m.
The attached patch makes a small tweak to avoid issuing a duplicate
warning for calls to strcmp with a nonstring argument.  The most
onerous part of this was figuring out how to test for the absence
of duplicate warnings.  The "hack" I used (dg-regexp) is in place
until a more straightforward solution becomes available.  (David
Malcolm has something planned for GCC 9.)

Martin

Comments

Martin Sebor April 13, 2018, 12:02 a.m. | #1
Attached is a minor update that avoids additional duplicate
warnings exposed by more extensive testing (for PR 85369).

On 04/12/2018 02:52 PM, Martin Sebor wrote:
> The attached patch makes a small tweak to avoid issuing a duplicate
> warning for calls to strcmp with a nonstring argument.  The most
> onerous part of this was figuring out how to test for the absence
> of duplicate warnings.  The "hack" I used (dg-regexp) is in place
> until a more straightforward solution becomes available.  (David
> Malcolm has something planned for GCC 9.)
>
> Martin
PR middle-end/85359 - duplicate -Wstringop-overflow for a strcmp call with a nonstring pointer

gcc/ChangeLog:

	PR middle-end/85359
	* builtins.c (expand_builtin_strcpy): Call maybe_warn_nonstring_arg
	only when expasion succeeds.
	(expand_builtin_strcmp): Same.
	(expand_builtin_strncmp): Same.

gcc/testsuite/ChangeLog:

	PR middle-end/85359
	* gcc.dg/attr-nonstring.c: New test.
Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c	(revision 259298)
+++ gcc/builtins.c	(working copy)
@@ -3777,7 +3777,17 @@ expand_builtin_strcpy (tree exp, rtx target)
 		    src, destsize);
     }
 
-  return expand_builtin_strcpy_args (dest, src, target);
+  if (rtx ret = expand_builtin_strcpy_args (dest, src, target))
+    {
+      /* Check to see if the argument was declared attribute nonstring
+	 and if so, issue a warning since at this point it's not known
+	 to be nul-terminated.  */
+      tree fndecl = get_callee_fndecl (exp);
+      maybe_warn_nonstring_arg (fndecl, exp);
+      return ret;
+    }
+
+  return NULL_RTX;
 }
 
 /* Helper function to do the actual work for expand_builtin_strcpy.  The
@@ -4570,14 +4580,14 @@ expand_builtin_strcmp (tree exp, ATTRIBUTE_UNUSED
 	}
     }
 
-  /* Check to see if the argument was declared attribute nonstring
-     and if so, issue a warning since at this point it's not known
-     to be nul-terminated.  */
   tree fndecl = get_callee_fndecl (exp);
-  maybe_warn_nonstring_arg (fndecl, exp);
-
   if (result)
     {
+      /* Check to see if the argument was declared attribute nonstring
+	 and if so, issue a warning since at this point it's not known
+	 to be nul-terminated.  */
+      maybe_warn_nonstring_arg (fndecl, exp);
+
       /* Return the value in the proper mode for this function.  */
       machine_mode mode = TYPE_MODE (TREE_TYPE (exp));
       if (GET_MODE (result) == mode)
@@ -4674,14 +4684,14 @@ expand_builtin_strncmp (tree exp, ATTRIBUTE_UNUSED
 					 arg2_rtx, TREE_TYPE (len), arg3_rtx,
 					 MIN (arg1_align, arg2_align));
 
-  /* Check to see if the argument was declared attribute nonstring
-     and if so, issue a warning since at this point it's not known
-     to be nul-terminated.  */
   tree fndecl = get_callee_fndecl (exp);
-  maybe_warn_nonstring_arg (fndecl, exp);
-
   if (result)
     {
+      /* Check to see if the argument was declared attribute nonstring
+	 and if so, issue a warning since at this point it's not known
+	 to be nul-terminated.  */
+      maybe_warn_nonstring_arg (fndecl, exp);
+
       /* Return the value in the proper mode for this function.  */
       mode = TYPE_MODE (TREE_TYPE (exp));
       if (GET_MODE (result) == mode)

===================================================================
--- gcc/testsuite/gcc.dg/attr-nonstring.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/attr-nonstring.c	(working copy)
@@ -0,0 +1,123 @@
+/* PR middle-end/85359 - duplicate -Wstringop-overflow for a strcmp call
+   with a nonstring pointer
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+typedef __SIZE_TYPE__       size_t;
+typedef __builtin_va_list   va_list;
+
+int printf (const char*, ...);
+int puts (const char*);
+int puts_unlocked (const char*);
+int sprintf (char*, const char*, ...);
+int snprintf (char*, size_t, const char*, ...);
+int vsprintf (char*, const char*, va_list);
+int vsnprintf (char*, size_t, const char*, va_list);
+
+int strcmp (const char*, const char*);
+int strncmp (const char*, const char*, size_t);
+
+char* stpcpy (char*, const char*);
+char* stpncpy (char*, const char*, size_t);
+
+char* strcat (char*, const char*);
+char* strncat (char*, const char*, size_t);
+
+char* strcpy (char*, const char*);
+char* strncpy (char*, const char*, size_t);
+
+char* strchr (const char*, int);
+char* strrchr (const char*, int);
+char* strstr (const char*, const char*);
+char* strdup (const char*);
+size_t strlen (const char*);
+size_t strnlen (const char*, size_t);
+char* strndup (const char*, size_t);
+
+#define NONSTRING __attribute__ ((nonstring))
+
+extern char ns5[5] NONSTRING;
+
+int strcmp_nonstring_1 (NONSTRING const char *a, const char *b)
+{
+  /* dg-warning matches one or more instances of the warning so it's
+     no good on its own.  Use dg-regexp instead to verify that just
+     one instance of the warning is issued.  See gcc.dg/pr64223-1
+     for a different approach.  */
+  return strcmp (a, b);  /* { dg-regexp "\[^\n\r\]+: warning: .strcmp. argument 1 declared attribute .nonstring. \\\[-Wstringop-overflow=]" "strcmp" } */
+}
+
+int strcmp_nonstring_2 (const char *a, NONSTRING const char *b)
+{
+  return strcmp (a, b);  /* { dg-regexp "\[^\n\r\]+: warning: .strcmp. argument 2 declared attribute .nonstring. \\\[-Wstringop-overflow=]" "strcmp" } */
+}
+
+int strncmp_nonstring_1 (const char *s)
+{
+  return strncmp (s, ns5, sizeof ns5 + 1);  /* { dg-regexp "\[^\n\r\]+: warning: .strncmp. argument 2 declared attribute .nonstring. \[^\n\r\]+ \\\[-Wstringop-overflow=]" "strncmp" } */
+}
+
+int strncmp_nonstring_2 (const char *s)
+{
+  return strncmp (ns5, s, sizeof ns5 + 1);  /* { dg-regexp "\[^\n\r\]+: warning: .strncmp. argument 1 declared attribute .nonstring. \[^\n\r\]+ \\\[-Wstringop-overflow=]" "strncmp" } */
+}
+
+char* stpcpy_nonstring (char *d, NONSTRING const char *s)
+{
+  return stpcpy (d, s);  /* { dg-regexp "\[^\n\r\]+: warning: .stpcpy. argument 2 declared attribute .nonstring. \\\[-Wstringop-overflow=]" "stpcpy" } */
+}
+
+char* stpncpy_nonstring (char *d)
+{
+  return stpncpy (d, ns5, sizeof ns5 + 1);  /* { dg-regexp "\[^\n\r\]+: warning: .stpncpy. argument 2 declared attribute .nonstring. \[^\n\r\]+ \\\[-Wstringop-overflow=]" "stpncpy" } */
+}
+
+char* strchr_nonstring (NONSTRING const char *s, int c)
+{
+  return strchr (s, c);  /* { dg-regexp "\[^\n\r\]+: warning: .strchr. argument 1 declared attribute .nonstring. \\\[-Wstringop-overflow=]" "strchr" } */
+}
+
+char* strrchr_nonstring (NONSTRING const char *s, int c)
+{
+  return strrchr (s, c);  /* { dg-regexp "\[^\n\r\]+: warning: .strrchr. argument 1 declared attribute .nonstring. \\\[-Wstringop-overflow=]" "strrchr" } */
+}
+
+char* strcpy_nonstring (char *d, NONSTRING const char *s)
+{
+  return strcpy (d, s);  /* { dg-regexp "\[^\n\r\]+: warning: .strcpy. argument 2 declared attribute .nonstring. \\\[-Wstringop-overflow=]" "strcpy" } */
+}
+
+char* strncpy_nonstring (char *d)
+{
+  return strncpy (d, ns5, sizeof ns5 + 1);  /* { dg-regexp "\[^\n\r\]+: warning: .strncpy. argument 2 declared attribute .nonstring. \[^\n\r\]+ \\\[-Wstringop-overflow=]" "strncpy" } */
+}
+
+char* strstr_nonstring_1 (NONSTRING const char *a, const char *b)
+{
+  return strstr (a, b);  /* { dg-regexp "\[^\n\r\]+: warning: .strstr. argument 1 declared attribute .nonstring. \\\[-Wstringop-overflow=]" "strstr" } */
+}
+
+char* strstr_nonstring_2 (const char *a, NONSTRING const char *b)
+{
+  return strstr (a, b);  /* { dg-regexp "\[^\n\r\]+: warning: .strstr. argument 2 declared attribute .nonstring. \\\[-Wstringop-overflow=]" "strstr" } */
+}
+
+char* stdup_nonstring (NONSTRING const char *s)
+{
+  return strdup (s);  /* { dg-regexp "\[^\n\r\]+: warning: .strdup. argument 1 declared attribute .nonstring. \\\[-Wstringop-overflow=]" "strdup" } */
+}
+
+size_t strlen_nonstring (NONSTRING const char *s)
+{
+  return strlen (s);  /* { dg-regexp "\[^\n\r\]+: warning: .strlen. argument 1 declared attribute .nonstring. \\\[-Wstringop-overflow=]" "strlen" } */
+}
+
+int printf_nonstring (NONSTRING const char *s)
+{
+  return printf (s);  /* { dg-regexp "\[^\n\r\]+: warning: .printf. argument 1 declared attribute .nonstring. \\\[-Wstringop-overflow=]" "printf" } */
+}
+
+int sprintf_nonstring_2 (char *d, NONSTRING const char *s)
+{
+  return sprintf (d, s);  /* { dg-regexp "\[^\n\r\]+: warning: .sprintf. argument 2 declared attribute .nonstring. \\\[-Wstringop-overflow=]" "sprintf" } */
+}
Martin Sebor April 19, 2018, 4:03 p.m. | #2
Ping: https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00650.html

This just suppresses a duplicate warning.  Please let me know
if it's preferable to defer it until GCC 9.  Otherwise, I'll
be traveling the next two weeks with only limited availability
(none the first week in May).

On 04/12/2018 02:52 PM, Martin Sebor wrote:
> The attached patch makes a small tweak to avoid issuing a duplicate
> warning for calls to strcmp with a nonstring argument.  The most
> onerous part of this was figuring out how to test for the absence
> of duplicate warnings.  The "hack" I used (dg-regexp) is in place
> until a more straightforward solution becomes available.  (David
> Malcolm has something planned for GCC 9.)
>
> Martin

Patch

PR middle-end/85359 - duplicate -Wstringop-overflow for a strcmp call with a nonstring pointer

gcc/ChangeLog:

	PR middle-end/85359
	* builtins.c (expand_builtin_strcmp): Take care to avoid issuing
	a duplicate warning.

gcc/testsuite/ChangeLog:

	PR middle-end/85359
	* gcc.dg/attr-nonstring.c: New test.
Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c	(revision 259298)
+++ gcc/builtins.c	(working copy)
@@ -4570,14 +4570,15 @@  expand_builtin_strcmp (tree exp, ATTRIBUTE_UNUSED
 	}
     }
 
-  /* Check to see if the argument was declared attribute nonstring
-     and if so, issue a warning since at this point it's not known
-     to be nul-terminated.  */
   tree fndecl = get_callee_fndecl (exp);
-  maybe_warn_nonstring_arg (fndecl, exp);
-
   if (result)
     {
+      /* Check to see if the argument was declared attribute nonstring
+	 and if so, issue a warning since at this point it's not known
+	 to be nul-terminated.  Avoid doing this when RESULT is false
+	 and let expand_call() do it.  */
+      maybe_warn_nonstring_arg (fndecl, exp);
+
       /* Return the value in the proper mode for this function.  */
       machine_mode mode = TYPE_MODE (TREE_TYPE (exp));
       if (GET_MODE (result) == mode)
Index: gcc/testsuite/gcc.dg/attr-nonstring.c
===================================================================
--- gcc/testsuite/gcc.dg/attr-nonstring.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/attr-nonstring.c	(working copy)
@@ -0,0 +1,58 @@ 
+/* PR middle-end/85359 - duplicate -Wstringop-overflow for a strcmp call
+   with a nonstring pointer
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+extern char* strchr (const char*, int);
+extern char* strrchr (const char*, int);
+extern char* stpcpy (char*, const char*);
+extern char* strcpy (char*, const char*);
+extern int strcmp (const char*, const char*);
+extern char* strstr (const char*, const char*);
+
+#define NONSTRING __attribute__ ((nonstring))
+
+int strcmp_nonstring_1 (NONSTRING const char *a, const char *b)
+{
+  /* dg-warning matches one or more instances of the warning so it's
+     no good on its own.  Use dg-regexp instead to verify that just
+     one instance of the warning is issued.  See gcc.dg/pr64223-1
+     for a different approach.  */
+  return strcmp (a, b);  /* { dg-regexp "\[^\n\r\]+: warning: .strcmp. argument 1 declared attribute .nonstring. \\\[-Wstringop-overflow=]" } */
+}
+
+int strcmp_nonstring_2 (const char *a, NONSTRING const char *b)
+{
+  return strcmp (a, b);  /* { dg-regexp "\[^\n\r\]+: warning: .strcmp. argument 2 declared attribute .nonstring. \\\[-Wstringop-overflow=]" } */
+}
+
+
+char* stpcpy_nonstring (char *a, NONSTRING const char *b)
+{
+  return stpcpy (a, b);  /* { dg-regexp "\[^\n\r\]+: warning: .stpcpy. argument 2 declared attribute .nonstring. \\\[-Wstringop-overflow=]" } */
+}
+
+char* strchr_nonstring (NONSTRING const char *s, int c)
+{
+  return strchr (s, c);  /* { dg-regexp "\[^\n\r\]+: warning: .strchr. argument 1 declared attribute .nonstring. \\\[-Wstringop-overflow=]" } */
+}
+
+char* strrchr_nonstring (NONSTRING const char *s, int c)
+{
+  return strrchr (s, c);  /* { dg-regexp "\[^\n\r\]+: warning: .strrchr. argument 1 declared attribute .nonstring. \\\[-Wstringop-overflow=]" } */
+}
+
+char* strcpy_nonstring (char *a, NONSTRING const char *b)
+{
+  return strcpy (a, b);  /* { dg-regexp "\[^\n\r\]+: warning: .strcpy. argument 2 declared attribute .nonstring. \\\[-Wstringop-overflow=]" } */
+}
+
+char* strstr_nonstring_1 (NONSTRING const char *a, const char *b)
+{
+  return strstr (a, b);  /* { dg-regexp "\[^\n\r\]+: warning: .strstr. argument 1 declared attribute .nonstring. \\\[-Wstringop-overflow=]" } */
+}
+
+char* strstr_nonstring_2 (const char *a, NONSTRING const char *b)
+{
+  return strstr (a, b);  /* { dg-regexp "\[^\n\r\]+: warning: .strstr. argument 2 declared attribute .nonstring. \\\[-Wstringop-overflow=]" } */
+}