public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] avoid bogus -Wstringop-truncation when inlining (PR 84480)
@ 2018-02-21 21:19 Martin Sebor
  2018-02-22  2:53 ` Jeff Law
  0 siblings, 1 reply; 3+ messages in thread
From: Martin Sebor @ 2018-02-21 21:19 UTC (permalink / raw)
  To: Gcc Patch List

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

The attached patch eliminates -Wstringop-truncation false
positives reported in bug 84480 - bogus -Wstringop-truncation
despite assignment with an inlined string literal.  It does
that by delegating early strncpy checks during folding to
the same machinery in tree-ssa-strlen that looks for a NUL
assignment to the destination the next statement.

The patch also adds inlining context to the warnings via
the %G directive.

Tested on x86_64-linux with no regressions.

Martin

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

PR tree-optimization/84480 - bogus -Wstringop-truncation despite assignment with an inlined string literal

gcc/ChangeLog:

	PR tree-optimization/84480
	* gimple-fold.c (gimple_fold_builtin_strcpy): Move warnings
	to maybe_diag_stxncpy_trunc.  Call it.
	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Integrate warnings
	from gimple_fold_builtin_strcpy.  Print inlining stack.
	(handle_builtin_stxncpy): Print inlining stack.
	* tree-ssa-strlen.h (maybe_diag_stxncpy_trunc): Declare.

gcc/testsuite/ChangeLog:

	PR tree-optimization/84480
	* c-c++-common/Wstringop-truncation.c: Adjust text of expected warnings.
	* g++.dg/warn/Wstringop-truncation-1.C: New test.
Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c	(revision 257875)
+++ gcc/gimple-fold.c	(working copy)
@@ -65,6 +65,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "intl.h"
 #include "calls.h"
 #include "tree-vector-builder.h"
+#include "tree-ssa-strlen.h"
 
 /* Return true when DECL can be referenced from current unit.
    FROM_DECL (if non-null) specify constructor of variable DECL was taken from.
@@ -1710,38 +1711,9 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator
   if (tree_int_cst_lt (ssize, len))
     return false;
 
-  if (!nonstring)
-    {
-      if (tree_int_cst_lt (len, slen))
-	{
-	  tree fndecl = gimple_call_fndecl (stmt);
-	  gcall *call = as_a <gcall *> (stmt);
+  /* Diagnose truncation that leaves the copy unterminated.  */
+  maybe_diag_stxncpy_trunc (*gsi, src, len);
 
-	  warning_at (loc, OPT_Wstringop_truncation,
-		      (tree_int_cst_equal (size_one_node, len)
-		       ? G_("%G%qD output truncated copying %E byte "
-			    "from a string of length %E")
-		       : G_("%G%qD output truncated copying %E bytes "
-			    "from a string of length %E")),
-		      call, fndecl, len, slen);
-	}
-      else if (tree_int_cst_equal (len, slen))
-	{
-	  tree fndecl = gimple_call_fndecl (stmt);
-	  gcall *call = as_a <gcall *> (stmt);
-
-	  warning_at (loc, OPT_Wstringop_truncation,
-		      (tree_int_cst_equal (size_one_node, len)
-		       ? G_("%G%qD output truncated before terminating nul "
-			    "copying %E byte from a string of the same "
-			    "length")
-		       : G_("%G%qD output truncated before terminating nul "
-			    "copying %E bytes from a string of the same "
-			    "length")),
-		      call, fndecl, len);
-	}
-    }
-
   /* OK transform into builtin memcpy.  */
   tree fn = builtin_decl_implicit (BUILT_IN_MEMCPY);
   if (!fn)
Index: gcc/testsuite/c-c++-common/Wstringop-truncation.c
===================================================================
--- gcc/testsuite/c-c++-common/Wstringop-truncation.c	(revision 257875)
+++ gcc/testsuite/c-c++-common/Wstringop-truncation.c	(working copy)
@@ -188,7 +188,7 @@ void test_strncpy_ptr (char *d, const char* s, con
   CPY (d, CHOOSE (s, t), 2);
 
   CPY (d, CHOOSE ("", "123"), 1);   /* { dg-warning ".strncpy\[^\n\r\]* output may be truncated copying 1 byte from a string of length 3" } */
-  CPY (d, CHOOSE ("1", "123"), 1);  /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 1" } */
+  CPY (d, CHOOSE ("1", "123"), 1);  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying 1 byte from a string of the same length" } */
   CPY (d, CHOOSE ("12", "123"), 1); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 2" } */
   CPY (d, CHOOSE ("123", "12"), 1); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 2" } */
 
@@ -331,7 +331,7 @@ void test_strncpy_array (Dest *pd, int i, const ch
     /* This might be better written using memcpy() but it's safe so
        it probably shouldn't be diagnosed.  It currently triggers
        a warning because of bug 81704.  */
-    strncpy (dst7, "0123456", sizeof dst7);   /* { dg-bogus "truncated" "bug 81704" { xfail *-*-* } } */
+    strncpy (dst7, "0123456", sizeof dst7);   /* { dg-bogus "\\\[-Wstringop-truncation]" "bug 81704" { xfail *-*-* } } */
     dst7[sizeof dst7 - 1] = '\0';
     sink (dst7);
   }
@@ -350,7 +350,7 @@ void test_strncpy_array (Dest *pd, int i, const ch
   }
 
   {
-    strncpy (pd->a5, "01234", sizeof pd->a5);   /* { dg-bogus "truncated" "bug 81704" { xfail *-*-* } } */
+    strncpy (pd->a5, "01234", sizeof pd->a5);   /* { dg-bogus "\\\[-Wstringop-truncation]" "bug 81704" { xfail *-*-* } } */
     pd->a5[sizeof pd->a5 - 1] = '\0';
     sink (pd);
   }
Index: gcc/testsuite/g++.dg/warn/Wstringop-truncation-1.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wstringop-truncation-1.C	(nonexistent)
+++ gcc/testsuite/g++.dg/warn/Wstringop-truncation-1.C	(working copy)
@@ -0,0 +1,126 @@
+/* PR/tree-optimization/84480 - bogus -Wstringop-truncation despite
+   assignment with an inlined string literal
+   { dg-do compile }
+   { dg-options "-O2 -Wstringop-truncation" }  */
+
+#include <string.h>
+
+template <size_t N>
+class GoodString
+{
+public:
+  GoodString (const char *s, size_t slen = N)
+  {
+    if (slen > N)
+      slen = N;
+
+    strncpy (str, s, slen);
+
+    str[slen] = '\0';
+  }
+
+private:
+  char str[N + 1];
+};
+
+void sink (void*);
+
+void good_nowarn_size_m2 ()
+{
+  GoodString<3> str ("12");
+  sink (&str);
+}
+
+void good_nowarn_size_m1 ()
+{
+  GoodString<3> str ("123");    // { dg-bogus "\\\[-Wstringop-truncation]" }
+  sink (&str);
+}
+
+void good_nowarn_size_m1_var (const char* s)
+{
+  GoodString<3> str (s);        // { dg-bogus "\\\[-Wstringop-truncation]" }
+  sink (&str);
+}
+
+void call_good_nowarn_size_m1_var ()
+{
+  good_nowarn_size_m1_var ("456");
+}
+
+
+template <size_t N>
+class BadString1
+{
+public:
+  BadString1 (const char *s, size_t slen = N)
+  {
+    if (slen > N)
+      slen = N;
+
+    strncpy (str, s, slen);
+  }
+
+private:
+  char str[N + 1];
+};
+
+void bad1_nowarn_size_m2 ()
+{
+  BadString1<3> str ("12");
+  sink (&str);
+}
+
+
+template <size_t N>
+class BadString2
+{
+public:
+  BadString2 (const char *s, size_t slen = N)
+  {
+    if (slen > N)
+      slen = N;
+
+    strncpy (str, s, slen);     // { dg-warning "\\\[-Wstringop-truncation]" }
+  }
+
+private:
+  char str[N + 1];
+};
+
+void bad2_warn_size_m1 ()
+{
+  BadString2<3> str ("123");
+  sink (&str);
+}
+
+// { dg-message "inlined from .void bad2_warn_size_m1." "" { target *-*-* } 0 }
+
+template <size_t N>
+class BadString3
+{
+public:
+  BadString3 (const char *s, size_t slen = N)
+  {
+    if (slen > N)
+      slen = N;
+
+    strncpy (str, s, slen);     // { dg-warning "\\\[-Wstringop-truncation]" }
+  }
+
+private:
+  char str[N + 1];
+};
+
+void bad3_warn_size_m1_var (const char *s)
+{
+  BadString3<3> str (s);
+  sink (&str);
+}
+
+void call_bad3_warn_size_m1_var ()
+{
+  bad3_warn_size_m1_var ("456");
+}
+
+// { dg-message "inlined from .void call_bad3_warn_size_m1_var." "" { target *-*-* } 0 }
Index: gcc/tree-ssa-strlen.c
===================================================================
--- gcc/tree-ssa-strlen.c	(revision 257875)
+++ gcc/tree-ssa-strlen.c	(working copy)
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "domwalk.h"
 #include "tree-ssa-alias.h"
 #include "tree-ssa-propagate.h"
+#include "tree-ssa-strlen.h"
 #include "params.h"
 #include "ipa-chkp.h"
 #include "tree-hash-traits.h"
@@ -1780,11 +1781,12 @@ is_strlen_related_p (tree src, tree len)
   return false;
 }
 
-/* A helper of handle_builtin_stxncpy.  Check to see if the specified
-   bound is a) equal to the size of the destination DST and if so, b)
-   if it's immediately followed by DST[CNT - 1] = '\0'.  If a) holds
-   and b) does not, warn.  Otherwise, do nothing.  Return true if
-   diagnostic has been issued.
+/* Called by handle_builtin_stxncpy and by gimple_fold_builtin_strncpy
+   in gimple-fold.c.
+   Check to see if the specified bound is a) equal to the size of
+   the destination DST and if so, b) if it's immediately followed by
+   DST[CNT - 1] = '\0'.  If a) holds and b) does not, warn.  Otherwise,
+   do nothing.  Return true if diagnostic has been issued.
 
    The purpose is to diagnose calls to strncpy and stpncpy that do
    not nul-terminate the copy while allowing for the idiom where
@@ -1795,7 +1797,7 @@ is_strlen_related_p (tree src, tree len)
      a[sizeof a - 1] = '\0';
 */
 
-static bool
+bool
 maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
 {
   gimple *stmt = gsi_stmt (gsi);
@@ -1831,8 +1833,11 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi
     return false;
 
   /* Negative value is the constant string length.  If it's less than
-     the lower bound there is no truncation.  */
-  int sidx = get_stridx (src);
+     the lower bound there is no truncation.  Avoid calling get_stridx()
+     when ssa_ver_to_stridx is empty.  That implies the caller isn't
+     running under the control of this pass and ssa_ver_to_stridx hasn't
+     been created yet.  */
+  int sidx = ssa_ver_to_stridx.length () ? get_stridx (src) : 0;
   if (sidx < 0 && wi::gtu_p (cntrange[0], ~sidx))
     return false;
 
@@ -1935,7 +1940,19 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi
 	  lenrange[0] = wi::shwi (0, prec);
 	}
 
-      if (wi::geu_p (lenrange[0], cntrange[1]))
+      gcall *call = as_a <gcall *> (stmt);
+
+      if (lenrange[0] == cntrange[1] && cntrange[0] == cntrange[1])
+	return warning_at (callloc, OPT_Wstringop_truncation,
+			   (integer_onep (cnt)
+			    ? G_("%G%qD output truncated before terminating "
+				 "nul copying %E byte from a string of the "
+				 "same length")
+			    : G_("%G%qD output truncated before terminating nul "
+				 "copying %E bytes from a string of the same "
+				 "length")),
+			   call, func, cnt);
+      else if (wi::geu_p (lenrange[0], cntrange[1]))
 	{
 	  /* The shortest string is longer than the upper bound of
 	     the count so the truncation is certain.  */
@@ -1942,16 +1959,16 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi
 	  if (cntrange[0] == cntrange[1])
 	    return warning_at (callloc, OPT_Wstringop_truncation,
 			       integer_onep (cnt)
-			       ? G_("%qD output truncated copying %E byte "
+			       ? G_("%G%qD output truncated copying %E byte "
 				    "from a string of length %wu")
-			       : G_("%qD output truncated copying %E bytes "
+			       : G_("%G%qD output truncated copying %E bytes "
 				    "from a string of length %wu"),
-			       func, cnt, lenrange[0].to_uhwi ());
+			       call, func, cnt, lenrange[0].to_uhwi ());
 
 	  return warning_at (callloc, OPT_Wstringop_truncation,
-			     "%qD output truncated copying between %wu "
+			     "%G%qD output truncated copying between %wu "
 			     "and %wu bytes from a string of length %wu",
-			     func, cntrange[0].to_uhwi (),
+			     call, func, cntrange[0].to_uhwi (),
 			     cntrange[1].to_uhwi (), lenrange[0].to_uhwi ());
 	}
       else if (wi::geu_p (lenrange[1], cntrange[1]))
@@ -1961,16 +1978,16 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi
 	  if (cntrange[0] == cntrange[1])
 	    return warning_at (callloc, OPT_Wstringop_truncation,
 			       integer_onep (cnt)
-			       ? G_("%qD output may be truncated copying %E "
+			       ? G_("%G%qD output may be truncated copying %E "
 				    "byte from a string of length %wu")
-			       : G_("%qD output may be truncated copying %E "
+			       : G_("%G%qD output may be truncated copying %E "
 				    "bytes from a string of length %wu"),
-			       func, cnt, lenrange[1].to_uhwi ());
+			       call, func, cnt, lenrange[1].to_uhwi ());
 
 	  return warning_at (callloc, OPT_Wstringop_truncation,
-			     "%qD output may be truncated copying between %wu "
+			     "%G%qD output may be truncated copying between %wu "
 			     "and %wu bytes from a string of length %wu",
-			     func, cntrange[0].to_uhwi (),
+			     call, func, cntrange[0].to_uhwi (),
 			     cntrange[1].to_uhwi (), lenrange[1].to_uhwi ());
 	}
 
@@ -1982,9 +1999,9 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi
 	     the lower bound of the specified count but shorter than the
 	     upper bound the copy may (but need not) be truncated.  */
 	  return warning_at (callloc, OPT_Wstringop_truncation,
-			     "%qD output may be truncated copying between %wu "
-			     "and %wu bytes from a string of length %wu",
-			     func, cntrange[0].to_uhwi (),
+			     "%G%qD output may be truncated copying between "
+			     "%wu and %wu bytes from a string of length %wu",
+			     call, func, cntrange[0].to_uhwi (),
 			     cntrange[1].to_uhwi (), lenrange[0].to_uhwi ());
 	}
     }
@@ -2003,8 +2020,8 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi
 
       if (cntrange[0] == cntrange[1])
 	return warning_at (callloc, OPT_Wstringop_truncation,
-			   "%qD specified bound %E equals destination size",
-			   func, cnt);
+			   "%G%qD specified bound %E equals destination size",
+			   as_a <gcall *> (stmt), func, cnt);
     }
 
   return false;
@@ -2103,14 +2120,15 @@ handle_builtin_stxncpy (built_in_function, gimple_
   if (sisrc == silen
       && is_strlen_related_p (src, len)
       && warning_at (callloc, OPT_Wstringop_truncation,
-		     "%qD output truncated before terminating nul "
+		     "%G%qD output truncated before terminating nul "
 		     "copying as many bytes from a string as its length",
-		     func))
+		     as_a <gcall *>(stmt), func))
     warned = true;
   else if (silen && is_strlen_related_p (src, silen->ptr))
     warned = warning_at (callloc, OPT_Wstringop_overflow_,
-			 "%qD specified bound depends on the length "
-			 "of the source argument", func);
+			 "%G%qD specified bound depends on the length "
+			 "of the source argument",
+			 as_a <gcall *>(stmt), func);
   if (warned)
     {
       location_t strlenloc = pss->second;
Index: gcc/tree-ssa-strlen.h
===================================================================
--- gcc/tree-ssa-strlen.h	(nonexistent)
+++ gcc/tree-ssa-strlen.h	(working copy)
@@ -0,0 +1,29 @@
+/* Declarations of tree-ssa-strlen API.
+
+   Copyright (C) 2018 Free Software Foundation, Inc.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it under
+   the terms of the GNU General Public License as published by the Free
+   Software Foundation; either version 3, or (at your option) any later
+   version.
+
+   GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or
+   FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+   for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_TREE_SSA_STRLEN_H
+#define GCC_TREE_SSA_STRLEN_H
+
+#include "gimple-iterator.h"
+#include "tree.h"
+
+extern bool maybe_diag_stxncpy_trunc (gimple_stmt_iterator, tree, tree);
+
+#endif   // GCC_TREE_SSA_STRLEN_H

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-02-22 17:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-21 21:19 [PATCH] avoid bogus -Wstringop-truncation when inlining (PR 84480) Martin Sebor
2018-02-22  2:53 ` Jeff Law
2018-02-22 17:49   ` Martin Sebor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).