Re: [PATCH] avoid bogus -Wstringop-truncation when inlining (PR 84480)

2018-02-22 Thread Martin Sebor

On 02/21/2018 07:53 PM, Jeff Law wrote:

On 02/21/2018 02:19 PM, Martin Sebor wrote:

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

gcc-84480.diff


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.

In general our guidelines are that the users of a .h file should include
any dependencies rather than having the .h file itself include other .h
files (Now that we've detangled the header files we may want to revisit
that guideline, but that's not a gcc-8 item).

It looks like gimple-fold.c and tree-ssa-strlen.c already have the
prereqs.  So in theory you should be able to just remove the bogus
#includes from tree-ssa-strlen.h.


Done.



In general we want to avoid adding more warnings to folding code.  But I
think the argument here is that we're already trying to warn within the
folder and just doing a poor job -- so we're removing that
implementation and delegating the warning to a better implementation.
Right?


Exactly.



So I think you just need to remove the bogus #includes from
tree-ssa-strlen and this is OK.


Thanks.  Committed in r257910.

Martin


Re: [PATCH] avoid bogus -Wstringop-truncation when inlining (PR 84480)

2018-02-21 Thread Jeff Law
On 02/21/2018 02:19 PM, Martin Sebor wrote:
> 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
> 
> gcc-84480.diff
> 
> 
> 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.
In general our guidelines are that the users of a .h file should include
any dependencies rather than having the .h file itself include other .h
files (Now that we've detangled the header files we may want to revisit
that guideline, but that's not a gcc-8 item).

It looks like gimple-fold.c and tree-ssa-strlen.c already have the
prereqs.  So in theory you should be able to just remove the bogus
#includes from tree-ssa-strlen.h.

In general we want to avoid adding more warnings to folding code.  But I
think the argument here is that we're already trying to warn within the
folder and just doing a poor job -- so we're removing that
implementation and delegating the warning to a better implementation.
Right?

So I think you just need to remove the bogus #includes from
tree-ssa-strlen and this is OK.

jeff



[PATCH] avoid bogus -Wstringop-truncation when inlining (PR 84480)

2018-02-21 Thread Martin Sebor

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
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  (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  (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