PR middle-end/97631 - bogus "writing one too many bytes" warning for memcpy with...
authorMartin Sebor <msebor@redhat.com>
Mon, 8 Mar 2021 20:28:52 +0000 (13:28 -0700)
committerMartin Sebor <msebor@redhat.com>
Mon, 8 Mar 2021 20:28:52 +0000 (13:28 -0700)
gcc/ChangeLog:

PR middle-end/97631
* tree-ssa-strlen.c (maybe_warn_overflow): Test rawmem.
(handle_builtin_stxncpy_strncat): Rename locals.  Determine
destination size from allocation calls.  Issue a more appropriate
kind of warning.
(handle_builtin_memcpy): Pass true as rawmem to maybe_warn_overflow.
(handle_builtin_memset): Same.

gcc/testsuite/ChangeLog:

PR middle-end/97631
* c-c++-common/Wstringop-overflow.c: Remove unexpected warnings.
Add an xfail.
* c-c++-common/Wstringop-truncation.c: Add expected warnings.
* gcc.dg/Wstringop-overflow-10.c: Also enable -Wstringop-truncation.
* gcc.dg/Wstringop-overflow-66.c: New test.
* gcc.dg/tree-ssa/strncpy-2.c: Adjust expected warning.

gcc/testsuite/c-c++-common/Wstringop-overflow.c
gcc/testsuite/c-c++-common/Wstringop-truncation.c
gcc/testsuite/gcc.dg/Wstringop-overflow-10.c
gcc/testsuite/gcc.dg/Wstringop-overflow-66.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/tree-ssa/strncpy-2.c
gcc/tree-ssa-strlen.c

index 53f5166..5757a23 100644 (file)
@@ -115,28 +115,31 @@ void test_strncpy (char **d, const char* s, int i)
   T (d, "123", sizeof "123");
   T (d, ar, sizeof ar);
 
-  T (d, s, strlen (s));       /* { dg-warning "\\\[-Wstringop-overflow=]" } */
+  /* There is no overflow in the following calls but they are diagnosed
+     by -Wstringop-truncation.  Verify that they aren'y also diagnosed
+     by -Wstringop-overflow.  */
+  T (d, s, strlen (s));
 
   {
-    int n = strlen (s);       /* { dg-message "length computed here" } */
-    T (d, s, n);              /* { dg-warning "\\\[-Wstringop-overflow=]" } */
+    int n = strlen (s);
+    T (d, s, n);
   }
 
   {
-    unsigned n = strlen (s);   /* { dg-message "length computed here" } */
-    T (d, s, n);               /* { dg-warning "\\\[-Wstringop-overflow=]" } */
+    unsigned n = strlen (s);
+    T (d, s, n);
   }
 
   {
     size_t n;
-    n = strlen (s);           /* { dg-message "length computed here" } */
-    T (d, s, n);              /* { dg-warning "\\\[-Wstringop-overflow=]" } */
+    n = strlen (s);
+    T (d, s, n);
   }
 
   {
     size_t n;
-    n = strlen (s) - 1;       /* { dg-message "length computed here" } */
-    T (d, s, n);              /* { dg-warning "\\\[-Wstringop-overflow=]" } */
+    n = strlen (s) - 1;
+    T (d, s, n);
   }
 
   {
@@ -148,11 +151,8 @@ void test_strncpy (char **d, const char* s, int i)
 
   {
     /* This use of strncpy is certainly dubious and it could well be
-       diagnosed by -Wstringop-truncation but it isn't.  That it is
-       diagnosed with -Wstringop-overflow is more by accident than
-       by design.  -Wstringop-overflow considers any dependency of
-       the bound on strlen(s) a potential bug.  */
-    size_t n = i < strlen (s) ? i : strlen (s);   /* { dg-message "length computed here" } */
-    T (d, s, n);                  /* { dg-message ".strncpy\[^\n\r]* specified bound depends on the length of the source argument" } */
+       diagnosed by -Wstringop-truncation but it isn't.  */
+    size_t n = i < strlen (s) ? i : strlen (s);   /* { dg-message "length computed here" "note" { xfail *-*-* } } */
+    T (d, s, n);                  /* { dg-message ".strncpy\[^\n\r]* specified bound depends on the length of the source argument" "pr?????" { xfail *-*-* } } */
   }
 }
index f29eee2..114837b 100644 (file)
@@ -226,19 +226,18 @@ void test_strncpy_ptr (char *d, const char* s, const char *t, int i)
   }
 
   {
-    /* The following is likely buggy but there's no apparent truncation
-       so it's not diagnosed by -Wstringop-truncation.  Instead, it is
-       diagnosed by -Wstringop-overflow (tested elsewhere).  */
+    /* The following truncates the terminating nul.  The warning should
+       say that but doesn't.  */
     int n;
     n = strlen (s) - 1;
-    CPY (d, s, n);
+    CPY (d, s, n);                  /* { dg-warning "\\\[-Wstringop-truncation" } */
   }
 
   {
     /* Same as above.  */
     size_t n;
     n = strlen (s) - 1;
-    CPY (d, s, n);
+    CPY (d, s, n);                  /* { dg-warning "\\\[-Wstringop-truncation" } */
   }
 
   {
index 2e22130..bace08a 100644 (file)
@@ -1,5 +1,7 @@
-/* { dg-do compile } */
-/* { dg-options "-O2 -Wstringop-overflow" } */
+/* PR tree-optimization/89500 - ICE: tree check: expected integer_cst,
+   have ssa_name in get_len
+   { dg-do compile }
+   { dg-options "-O2 -Wstringop-overflow -Wstringop-truncation" } */
 
 void
 foo (char *a)
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-66.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-66.c
new file mode 100644 (file)
index 0000000..0ecf511
--- /dev/null
@@ -0,0 +1,180 @@
+/* PR middle-end/97631 - bogus "writing one too many bytes" warning for
+   memcpy with strlen argument
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+#define NOIPA __attribute__ ((noipa))
+
+typedef __SIZE_TYPE__ size_t;
+
+extern void* malloc (size_t);
+extern void* memcpy (void*, const void*, size_t);
+extern void* memmove (void*, const void*, size_t);
+extern void* memset (void*, int, size_t);
+extern char* strcpy (char*, const char*);
+extern char* strncpy (char*, const char*, size_t);
+extern size_t strlen (const char*);
+
+
+NOIPA char* nowarn_strcpy (char *s)
+{
+  size_t n = strlen (s);
+  char *d = malloc (n + 1);
+  strcpy (d, s);
+  return d;
+}
+
+
+NOIPA char* warn_strcpy (char *s)
+{
+  size_t n = strlen (s);
+  char *d = malloc (n);
+  strcpy (d, s);        // { dg-warning "\\\[-Wstringop-overflow" }
+  return d;
+}
+
+NOIPA char* warn_strcpy_nz (char *s)
+{
+  size_t n = strlen (s);
+  if (n == 0)
+    return 0;
+
+  char *d = malloc (n);
+  strcpy (d, s);        // { dg-warning "\\\[-Wstringop-overflow" }
+  return d;
+}
+
+NOIPA char* warn_strcpy_nn (char *s)
+{
+  size_t n = strlen (s);
+  char *d = malloc (n);
+  if (!d)
+    return 0;
+
+  strcpy (d, s);        // { dg-warning "\\\[-Wstringop-overflow" }
+  return d;
+}
+
+NOIPA char* warn_strcpy_nz_nn (char *s)
+{
+  size_t n = strlen (s);
+  if (n == 0)
+    return 0;
+
+  char *d = malloc (n);
+  if (!d)
+    return 0;
+
+  strcpy (d, s);        // { dg-warning "\\\[-Wstringop-overflow" }
+  return d;
+}
+
+
+NOIPA char* nowarn_strncpy_1 (char *s)
+{
+  /* There's no overflow or truncation below so verify there is no
+     warning either.  */
+  size_t n = strlen (s) + 1;
+  char *d = malloc (n);
+  strncpy (d, s, n);
+  return d;
+}
+
+
+NOIPA char* warn_strncpy (char *s)
+{
+  size_t n = strlen (s);
+  char *d = malloc (n);
+  strncpy (d, s, n);    // { dg-warning "\\\[-Wstringop-truncation" }
+  return d;
+}
+
+NOIPA char* warn_strncpy_p1 (char *s)
+{
+  size_t n = strlen (s);
+  char *d = malloc (n + 1);
+  strncpy (d, s, n);    // { dg-warning "\\\[-Wstringop-truncation" }
+  return d;
+}
+
+NOIPA char* warn_strncpy_nz (char *s)
+{
+  size_t n = strlen (s);
+  if (n == 0)
+    return 0;
+
+  char *d = malloc (n);
+  strncpy (d, s, n);    // { dg-warning "\\\[-Wstringop-truncation" }
+  return d;
+
+}
+
+
+NOIPA char* nowarn_memcpy (char *s)
+{
+  size_t n = strlen (s);
+  char *d = malloc (n);
+  memcpy (d, s, n);     // { dg-bogus "\\\[-Wstringop-overflow" }
+  return d;
+}
+
+NOIPA char* nowarn_memcpy_nz (char *s)
+{
+  size_t n = strlen (s);
+  if (n == 0)
+    return 0;
+
+  char *d = malloc (n);
+  memcpy (d, s, n);     // { dg-bogus "\\\[-Wstringop-overflow" }
+  return d;
+}
+
+NOIPA char* nowarn_memcpy_nn (char *s)
+{
+  size_t n = strlen (s);
+  char *d = malloc (n);
+  if (!d)
+    return 0;
+
+  memcpy (d, s, n);     // { dg-bogus "\\\[-Wstringop-overflow" }
+  return d;
+}
+
+NOIPA char* nowarn_memcpy_nn_nz (char *s)
+{
+  size_t n = strlen (s);
+  if (n == 0)
+    return 0;
+
+  char *d = malloc (n);
+  if (!d)
+    return 0;
+
+  memcpy (d, s, n);     // { dg-bogus "\\\[-Wstringop-overflow" }
+  return d;
+
+}
+
+
+NOIPA char* nowarn_memmove (char *s)
+{
+  size_t n = strlen (s);
+  if (n == 0)
+    return 0;
+
+  char *d = malloc (n);
+  memmove (d, s, n);    // { dg-bogus "\\\[-Wstringop-overflow" }
+  return d;
+}
+
+
+NOIPA char* nowarn_memset (char *s, int c)
+{
+  size_t n = strlen (s);
+  if (n == 0)
+    return 0;
+
+  char *d = malloc (n);
+  memset (d, c, n);     // { dg-bogus "\\\[-Wstringop-overflow" }
+  return d;
+}
index 2ef9cd6..e2216ab 100644 (file)
@@ -1,6 +1,6 @@
 /* PR tree-optimization/83075 - Invalid strncpy optimization */
 /* { dg-do run } */
-/* { dg-options "-O2 -Wstringop-overflow" } */
+/* { dg-options "-O2 -Wstringop-truncation" } */
 
 typedef __SIZE_TYPE__ size_t;
 
@@ -8,7 +8,7 @@ __attribute__((noipa)) size_t
 foo (char *p, char *q, size_t *r)
 {
   size_t n0 = __builtin_strlen (p);
-  __builtin_strncpy (q, p, n0);                /* { dg-warning "specified bound depends on the length" } */
+  __builtin_strncpy (q, p, n0);                /* { dg-warning "\\\[-Wstringop-truncation" } */
   size_t n1 = __builtin_strlen (p);
   *r = n0;
   return n1;
index 8912a11..cccd4a0 100644 (file)
@@ -1992,17 +1992,12 @@ maybe_warn_overflow (gimple *stmt, tree len, pointer_query &ptr_qry,
       && wi::leu_p (lenrng[1], spcrng[1]))
     return;
 
-  if (lenrng[0] == spcrng[1]
-      && (len != destsize
-         || !si || !is_strlen_related_p (si->ptr, len)))
-    return;
-
   location_t loc = gimple_or_expr_nonartificial_location (stmt, dest);
   bool warned = false;
   if (wi::leu_p (lenrng[0], spcrng[1]))
     {
       if (len != destsize
-         && (!si || !is_strlen_related_p (si->ptr, len)))
+         && (!si || rawmem || !is_strlen_related_p (si->ptr, len)))
        return;
 
       warned = (writefn
@@ -3083,7 +3078,12 @@ handle_builtin_stxncpy_strncat (bool append_p, gimple_stmt_iterator *gsi)
   tree dst = gimple_call_arg (stmt, 0);
   tree src = gimple_call_arg (stmt, 1);
   tree len = gimple_call_arg (stmt, 2);
-  tree dstsize = NULL_TREE, srcsize = NULL_TREE;
+  /* An upper bound of the size of the destination.  */
+  tree dstsize = NULL_TREE;
+  /* The length of the destination and source strings (plus 1 for those
+     whose FULL_STRING_P is set, i.e., whose length is exact rather than
+     a lower bound).  */
+  tree dstlenp1 = NULL_TREE, srclenp1 = NULL_TREE;;
 
   int didx = get_stridx (dst);
   if (strinfo *sidst = didx > 0 ? get_strinfo (didx) : NULL)
@@ -3096,11 +3096,16 @@ handle_builtin_stxncpy_strncat (bool append_p, gimple_stmt_iterator *gsi)
            {
              /* String is known to be nul-terminated.  */
              tree type = TREE_TYPE (sidst->nonzero_chars);
-             dstsize = fold_build2 (PLUS_EXPR, type, sidst->nonzero_chars,
+             dstlenp1 = fold_build2 (PLUS_EXPR, type, sidst->nonzero_chars,
                                     build_int_cst (type, 1));
            }
          else
-           dstsize = sidst->nonzero_chars;
+           dstlenp1 = sidst->nonzero_chars;
+       }
+      else if (TREE_CODE (sidst->ptr) == SSA_NAME)
+       {
+         gimple *def_stmt = SSA_NAME_DEF_STMT (sidst->ptr);
+         dstsize = gimple_call_alloc_size (def_stmt);
        }
 
       dst = sidst->ptr;
@@ -3121,19 +3126,19 @@ handle_builtin_stxncpy_strncat (bool append_p, gimple_stmt_iterator *gsi)
          if (sisrc->full_string_p)
            {
              tree type = TREE_TYPE (sisrc->nonzero_chars);
-             srcsize = fold_build2 (PLUS_EXPR, type, sisrc->nonzero_chars,
+             srclenp1 = fold_build2 (PLUS_EXPR, type, sisrc->nonzero_chars,
                                     build_int_cst (type, 1));
            }
          else
-           srcsize = sisrc->nonzero_chars;
+           srclenp1 = sisrc->nonzero_chars;
        }
 
        src = sisrc->ptr;
     }
   else
-    srcsize = NULL_TREE;
+    srclenp1 = NULL_TREE;
 
-  if (check_bounds_or_overlap (stmt, dst, src, dstsize, srcsize))
+  if (check_bounds_or_overlap (stmt, dst, src, dstlenp1, srclenp1))
     {
       gimple_set_no_warning (stmt, true);
       return;
@@ -3165,9 +3170,10 @@ handle_builtin_stxncpy_strncat (bool append_p, gimple_stmt_iterator *gsi)
   /* When -Wstringop-truncation is set, try to determine truncation
      before diagnosing possible overflow.  Truncation is implied by
      the LEN argument being equal to strlen(SRC), regardless of
-     whether its value is known.  Otherwise, issue the more generic
-     -Wstringop-overflow which triggers for LEN arguments that in
-     any meaningful way depend on strlen(SRC).  */
+     whether its value is known.  Otherwise, when appending, or
+     when copying into a destination of known size, issue the more
+     generic -Wstringop-overflow which triggers for LEN arguments
+     that in any meaningful way depend on strlen(SRC).  */
   if (!append_p
       && sisrc == silen
       && is_strlen_related_p (src, len)
@@ -3176,11 +3182,19 @@ handle_builtin_stxncpy_strncat (bool append_p, gimple_stmt_iterator *gsi)
                     "copying as many bytes from a string as its length",
                     stmt, func))
     warned = true;
-  else if (silen && is_strlen_related_p (src, silen->ptr))
-    warned = warning_at (callloc, OPT_Wstringop_overflow_,
-                        "%G%qD specified bound depends on the length "
-                        "of the source argument",
-                        stmt, func);
+  else if ((append_p || !dstsize || len == dstlenp1)
+          && silen && is_strlen_related_p (src, silen->ptr))
+    {
+      /* Issue -Wstringop-overflow when appending or when writing into
+        a destination of a known size.  Otherwise, when copying into
+        a destination of an unknown size, it's truncation.  */
+      int opt = (append_p || dstsize
+                ? OPT_Wstringop_overflow_ : OPT_Wstringop_truncation);
+      warned = warning_at (callloc, opt,
+                          "%G%qD specified bound depends on the length "
+                          "of the source argument",
+                          stmt, func);
+    }
   if (warned)
     {
       location_t strlenloc = pss->second;
@@ -3216,7 +3230,7 @@ handle_builtin_memcpy (enum built_in_function bcode, gimple_stmt_iterator *gsi,
   if (olddsi != NULL
       && !integer_zerop (len))
     {
-      maybe_warn_overflow (stmt, len, ptr_qry, olddsi, false, false);
+      maybe_warn_overflow (stmt, len, ptr_qry, olddsi, false, true);
       adjust_last_stmt (olddsi, stmt, false, ptr_qry);
     }
 
@@ -3684,7 +3698,7 @@ handle_builtin_memset (gimple_stmt_iterator *gsi, bool *zero_write,
   tree memset_size = gimple_call_arg (memset_stmt, 2);
 
   /* Check for overflow.  */
-  maybe_warn_overflow (memset_stmt, memset_size, ptr_qry, NULL, false, false);
+  maybe_warn_overflow (memset_stmt, memset_size, ptr_qry, NULL, false, true);
 
   /* Bail when there is no statement associated with the destination
      (the statement may be null even when SI1->ALLOC is not).  */