[PATCH] make -Wrestrict for strcat more meaningful (PR 83698)

[Martin Sebor <msebor at gmail dot com>]

PR 83698 - bogus offset in -Wrestrict messages for strcat of
unknown strings, points out that the offsets printed by
-Wrestrict for possibly overlapping strcat calls with unknown
strings don't look meaningful in some cases.  The root cause
of the bogus values is wrapping during the conversion from
offset_int in which the pass tracks numerical values to
HOST_WIDE_INT for printing.  (The problem will go away once
GCC's pretty-printer supports wide int formatting.)  For
instance, the following:

  extern char *d;
  strcat (d + 3, d + 5);

results in

  warning: ‘strcat’ accessing 0 or more bytes at offsets 3 and 5 may 
overlap 1 byte at offset [3, -9223372036854775806]

which, besides printing the bogus negative offset on LP64
targets, isn't correct because strcat always accesses at least
one byte (the nul) and there can be no overlap at offset 3.
To be more accurate, the warning should say something like:

  warning: ‘strcat’ accessing 3 or more bytes at offsets 3 and 5 may 
overlap 1 byte at offset 5 [-Wrestrict]

because the function must access at least 3 bytes in order to
cause an overlap, and when it does, the overlap starts at the
higher of the two offsets, i.e., 5.  (Though it's virtually
impossible to have a single sentence and a singled set of
numbers cover all the cases with perfect accuracy.)

The attached patch fixes these issues to make the printed values
make more sense.  (It doesn't affect when diagnostics are printed.)

Although this isn't strictly a regression, it has an impact on
the readability of the warnings.  If left unchanged, the original
messages are likely to confuse users and lead to bug reports.

Martin

PR tree-optimization/83698 - bogus offset in -Wrestrict messages for strcat of unknown strings

gcc/ChangeLog:

	PR tree-optimization/83698
	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref): For
	arrays constrain the offset range to their bounds.
	(builtin_access::strcat_overlap): Adjust the bounds of overlap offset.
	(builtin_access::overlap): Avoid setting the size of overlap if it's
	already been set.
	(maybe_diag_overlap): Also consider arrays when deciding what values
	of offsets to include in diagnostics.

gcc/testsuite/ChangeLog:

	PR tree-optimization/83698
	* gcc.dg/Wrestrict-7.c: New test.
	* c-c++-common/Wrestrict.c: Adjust expected values for strcat.
	* gcc.target/i386/chkp-stropt-17.c: Same.

Index: gcc/gimple-ssa-warn-restrict.c
===================================================================
--- gcc/gimple-ssa-warn-restrict.c	(revision 256752)
+++ gcc/gimple-ssa-warn-restrict.c	(working copy)
@@ -384,6 +384,12 @@ builtin_memref::builtin_memref (tree expr, tree si
 	  base = SSA_NAME_VAR (base);
       }
 
+  if (DECL_P (base) && TREE_CODE (TREE_TYPE (base)) == ARRAY_TYPE)
+    {
+      if (offrange[0] < 0 && offrange[1] > 0)
+	offrange[0] = 0;
+    }
+
   if (size)
     {
       tree range[2];
@@ -1079,14 +1085,35 @@ builtin_access::strcat_overlap ()
     return false;
 
   /* When strcat overlap is certain it is always a single byte:
-     the terminatinn NUL, regardless of offsets and sizes.  When
+     the terminating NUL, regardless of offsets and sizes.  When
      overlap is only possible its range is [0, 1].  */
   acs.ovlsiz[0] = dstref->sizrange[0] == dstref->sizrange[1] ? 1 : 0;
   acs.ovlsiz[1] = 1;
-  acs.ovloff[0] = (dstref->sizrange[0] + dstref->offrange[0]).to_shwi ();
-  acs.ovloff[1] = (dstref->sizrange[1] + dstref->offrange[1]).to_shwi ();
 
-  acs.sizrange[0] = wi::smax (acs.dstsiz[0], srcref->sizrange[0]).to_shwi ();
+  offset_int endoff = dstref->offrange[0] + dstref->sizrange[0];
+  if (endoff <= srcref->offrange[0])
+    acs.ovloff[0] = wi::smin (maxobjsize, srcref->offrange[0]).to_shwi ();
+  else
+    acs.ovloff[0] = wi::smin (maxobjsize, endoff).to_shwi ();
+
+  acs.sizrange[0] = wi::smax (wi::abs (endoff - srcref->offrange[0]) + 1,
+			      srcref->sizrange[0]).to_shwi ();
+  if (dstref->offrange[0] == dstref->offrange[1])
+    {
+      if (srcref->offrange[0] == srcref->offrange[1])
+	acs.ovloff[1] = acs.ovloff[0];
+      else
+	acs.ovloff[1]
+	  = wi::smin (maxobjsize,
+		      srcref->offrange[1] + srcref->sizrange[1]).to_shwi ();
+    }
+  else
+    acs.ovloff[1]
+      = wi::smin (maxobjsize,
+		  dstref->offrange[1] + dstref->sizrange[1]).to_shwi ();
+
+  if (acs.sizrange[0] == 0)
+    acs.sizrange[0] = 1;
   acs.sizrange[1] = wi::smax (acs.dstsiz[1], srcref->sizrange[1]).to_shwi ();
   return true;
 }
@@ -1224,8 +1251,12 @@ builtin_access::overlap ()
   /* Call the appropriate function to determine the overlap.  */
   if ((this->*detect_overlap) ())
     {
-      sizrange[0] = wi::smax (acs.dstsiz[0], srcref->sizrange[0]).to_shwi ();
-      sizrange[1] = wi::smax (acs.dstsiz[1], srcref->sizrange[1]).to_shwi ();
+      if (!sizrange[1])
+	{
+	  /* Unless the access size range has already been set, do so here.  */
+	  sizrange[0] = wi::smax (acs.dstsiz[0], srcref->sizrange[0]).to_shwi ();
+	  sizrange[1] = wi::smax (acs.dstsiz[1], srcref->sizrange[1]).to_shwi ();
+	}
       return true;
     }
 
@@ -1401,10 +1432,17 @@ maybe_diag_overlap (location_t loc, gcall *call, b
   /* Use more concise wording when one of the offsets is unbounded
      to avoid confusing the user with large and mostly meaningless
      numbers.  */
-  bool open_range = ((dstref.offrange[0] == -maxobjsize - 1
-		      && dstref.offrange[1] == maxobjsize)
-		     || (srcref.offrange[0] == -maxobjsize - 1
-			 && srcref.offrange[1] == maxobjsize));
+  bool open_range;
+  if (DECL_P (dstref.base) && TREE_CODE (TREE_TYPE (dstref.base)) == ARRAY_TYPE)
+    open_range = ((dstref.offrange[0] == 0
+		   && dstref.offrange[1] == maxobjsize)
+		  || (srcref.offrange[0] == 0
+		      && srcref.offrange[1] == maxobjsize));
+  else
+    open_range = ((dstref.offrange[0] == -maxobjsize - 1
+		   && dstref.offrange[1] == maxobjsize)
+		  || (srcref.offrange[0] == -maxobjsize - 1
+		      && srcref.offrange[1] == maxobjsize));
 
   if (sizrange[0] == sizrange[1] || sizrange[1] == 1)
     {
Index: gcc/testsuite/c-c++-common/Wrestrict.c
===================================================================
--- gcc/testsuite/c-c++-common/Wrestrict.c	(revision 256752)
+++ gcc/testsuite/c-c++-common/Wrestrict.c	(working copy)
@@ -616,11 +616,14 @@ void test_strcat_var (char *d, const char *s)
   } while (0)
 
   T (d, d);                       /* { dg-warning "source argument is the same as destination" "strcat" } */
-  T (d, d + 1);                   /* { dg-warning "accessing 0 or more bytes at offsets 0 and 1 may overlap 1 byte" "strcat" } */
-  T (d, d + 2);                   /* { dg-warning "accessing 0 or more bytes at offsets 0 and 2 may overlap 1 byte" "strcat" } */
-  T (d, d + 999);                 /* { dg-warning "accessing 0 or more bytes at offsets 0 and 999 may overlap 1 byte" "strcat" } */
-  T (d, d + -99);                 /* { dg-warning "accessing 0 or more bytes at offsets 0 and -99 may overlap 1 byte" "strcat" } */
+  T (d, d + 1);                   /* { dg-warning "accessing 2 or more bytes at offsets 0 and 1 may overlap 1 byte" "strcat" } */
+  T (d, d + 2);                   /* { dg-warning "accessing 3 or more bytes at offsets 0 and 2 may overlap 1 byte at offset 2" "strcat" } */
+  T (d, d + 999);                 /* { dg-warning "accessing 1000 or more bytes at offsets 0 and 999 may overlap 1 byte at offset 999" "strcat" } */
 
+  /* The source string must be at least 100 bytes in length for the copy
+     below to overlap.  */
+  T (d, d + -99);                 /* { dg-warning "accessing 100 or more bytes at offsets 0 and -99 may overlap 1 byte" "strcat" } */
+
   size_t n = unsigned_value ();
 
   T (d + n, d + n);                       /* { dg-warning "\\\[-Wrestrict" "strcat" } */
Index: gcc/testsuite/gcc.dg/Wrestrict-7.c
===================================================================
--- gcc/testsuite/gcc.dg/Wrestrict-7.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/Wrestrict-7.c	(working copy)
@@ -0,0 +1,51 @@
+/* PR tree-optimization/83698 - bogus offset in -Wrestrict messages for
+   strcat of unknown strings
+   { dg-do compile }
+   { dg-options "-O2 -Wrestrict -ftrack-macro-expansion=0" } */
+
+extern char* strcat (char*, const char*);
+
+void sink (char*);
+
+#define T(d, s) sink (strcat (d, s))
+
+extern char arr[];
+
+
+void test_strcat_array_cst_offset (void)
+{
+  T (arr, arr + 1);           /* { dg-warning "accessing 2 or more bytes at offsets 0 and 1 may overlap 1 byte at offset 1" } */
+  T (arr, arr + 2);           /* { dg-warning "accessing 3 or more bytes at offsets 0 and 2 may overlap 1 byte at offset 2" } */
+  T (arr, arr + 13);          /* { dg-warning "accessing 14 or more bytes at offsets 0 and 13 may overlap 1 byte at offset 13" } */
+
+  T (arr + 1, arr);           /* { dg-warning "accessing 2 or more bytes at offsets 1 and 0 may overlap 1 byte at offset 1" } */
+  T (arr + 17, arr + 11);     /* { dg-warning "accessing 7 or more bytes at offsets 17 and 11 may overlap 1 byte at offset 17" } */
+  T (arr + 36, arr + 20);     /* { dg-warning "accessing 17 or more bytes at offsets 36 and 20 may overlap 1 byte at offset 36" } */
+}
+
+void test_strcat_ptr_cst_offset (char *d)
+{
+  T (d - 12, d + 34);         /* { dg-warning "accessing 47 or more bytes at offsets -12 and 34 may overlap 1 byte at offset 34" } */
+  T (d + 12, d + 34);         /* { dg-warning "accessing 23 or more bytes at offsets 12 and 34 may overlap 1 byte at offset 34" } */
+  T (d + 20, d + 36);         /* { dg-warning "accessing 17 or more bytes at offsets 20 and 36 may overlap 1 byte at offset 36" } */
+}
+
+void test_strcat_array_var_offset (int i, int j)
+{
+  T (arr + i, arr);           /* { dg-warning "accessing 1 or more bytes at offsets \\\[0, \[0-9\]+] and 0 may overlap 1 byte at offset \\\[0, \[0-9\]+]" } */
+  T (arr, arr + j);           /* { dg-warning "accessing 1 or more bytes at offsets 0 and \\\[0, \[0-9\]+] may overlap 1 byte at offset \\\[0, \[0-9\]+]" } */
+  T (arr + i, arr + j);       /* { dg-warning "accessing 1 or more bytes at offsets \\\[0, \[0-9\]+] and \\\[0, \[0-9\]+] may overlap 1 byte at offset \\\[0, \[0-9\]+]" } */
+
+  T (arr + i, arr + 5);       /* { dg-warning "accessing 6 or more bytes at offsets \\\[0, \[0-9\]+] and 5 may overlap 1 byte at offset \\\[5, \[0-9\]+]" } */
+  T (arr + 7, arr + j);       /* { dg-warning "accessing 8 or more bytes at offsets 7 and \\\[0, \[0-9\]+] may overlap 1 byte at offset \\\[7, \[0-9\]+]" } */
+}
+
+void test_strcat_ptr_var_offset (char *d, int i, int j)
+{
+  T (d + i, d);               /* { dg-warning "accessing \[0-9\]+ or more bytes at offsets \\\[-\[0-9\]+, \[0-9\]+] and 0 may overlap 1 byte at offset \\\[0, \[0-9\]+]" } */
+  T (d, d + j);               /* { dg-warning "accessing \[0-9\]+ or more bytes at offsets 0 and \\\[-\[0-9\]+, \[0-9\]+] may overlap 1 byte at offset \\\[0, \[0-9\]+]" } */
+  T (d + i, d + j);           /* { dg-warning "accessing 1 or more bytes at offsets \\\[-\[0-9\]+, \[0-9\]+] and \\\[-\[0-9\]+, \[0-9\]+] may overlap 1 byte at offset \\\[-\[0-9\]+, \[0-9\]+]" } */
+
+  T (d + i, d + 3);           /* { dg-warning "accessing \[0-9\]+ or more bytes at offsets \\\[-\[0-9\]+, \[0-9\]+] and 3 may overlap 1 byte at offset \\\[3, \[0-9\]+]" } */
+  T (d + 9, d + j);           /* { dg-warning "accessing \[0-9\]+ or more bytes at offsets 9 and \\\[-\[0-9\]+, \[0-9\]+] may overlap 1 byte at offset \\\[9, \[0-9\]+]" } */
+}
Index: gcc/testsuite/gcc.target/i386/chkp-stropt-17.c
===================================================================
--- gcc/testsuite/gcc.target/i386/chkp-stropt-17.c	(revision 256752)
+++ gcc/testsuite/gcc.target/i386/chkp-stropt-17.c	(working copy)
@@ -51,7 +51,7 @@ void test_strcpy (void)
 
 void test_strcat (int n)
 {
-  strcat (a, a + 3);   /* { dg-warning ".strcat\.chkp. accessing 0 or more bytes at offsets 0 and 3 may overlap 1 byte" } */
+  strcat (a, a + 3);   /* { dg-warning ".strcat\.chkp. accessing 4 or more bytes at offsets 0 and 3 may overlap 1 byte at offset 3" } */
 }
 
 void test_strncat (int n)