This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] avoid assuming strncat/strncpy don't change length of source string (PR 83075)


On Mon, Nov 20, 2017 at 12:15:09PM -0700, Martin Sebor wrote:
> 	PR tree-optimization/83075
> 	* tree-ssa-strlen.c (handle_builtin_stxncpy): Avoid assuming
> 	strncat/strncpy don't change length of source string.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/83075
> 	* gcc.dg/tree-ssa/strncat.c: New test.
> 	* gcc.dg/tree-ssa/strncpy-2.c: Same.
> 
> Index: gcc/testsuite/gcc.dg/tree-ssa/strncat.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/strncat.c	(nonexistent)
> +++ gcc/testsuite/gcc.dg/tree-ssa/strncat.c	(working copy)
> @@ -0,0 +1,22 @@
> +/* PR tree-optimization/83075 - Invalid strncpy optimization
> +   { dg-do compile }
> +   { dg-options "-O2 -Wno-stringop-overflow -fdump-tree-optimized" } */
> +
> +void test_overlapping_strncpy (void)
> +{
> +  char a[8] = "";
> +
> +  __builtin_strcpy (a, "123");
> +
> +  unsigned n0 = __builtin_strlen (a);
> +
> +  __builtin_strncat (a + 3, a, n0);
> +
> +  unsigned n1 = __builtin_strlen (a);
> +
> +  if (n1 == n0)
> +    __builtin_abort ();
> +}
> +
> +/* Verify the call to abort hasn't been eliminated.
> +   { dg-final { scan-tree-dump-times "abort" 1 "optimized" } } */
> Index: gcc/testsuite/gcc.dg/tree-ssa/strncpy-2.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/strncpy-2.c	(nonexistent)
> +++ gcc/testsuite/gcc.dg/tree-ssa/strncpy-2.c	(working copy)
> @@ -0,0 +1,22 @@
> +/* PR tree-optimization/83075 - Invalid strncpy optimization
> +   { dg-do compile }
> +   { dg-options "-O2 -Wno-stringop-overflow -fdump-tree-optimized" } */
> +
> +void test_overlapping_strncpy (void)
> +{
> +  char a[8] = "";
> +
> +  __builtin_strcpy (a, "123");
> +
> +  unsigned n0 = __builtin_strlen (a);
> +
> +  __builtin_strncpy (a + 3, a, n0);
> +
> +  unsigned n1 = __builtin_strlen (a);
> +
> +  if (n1 == n0)
> +    __builtin_abort ();
> +}
> +
> +/* Verify the call to abort hasn't been eliminated.
> +   { dg-final { scan-tree-dump-times "abort" 1 "optimized" } } */

I think it is better to have a dg-do run testcase and not scan for abort.
We can be able to optimize at some point n0 to 3 and n1 to 6 and optimize
away the abort.  In the testcase I've added to the PR there was a separate
function, you could add __attribute__((noipa)) to it to make sure that
it isn't IPA optimized.
Similarly for the strncat test.

> Index: gcc/tree-ssa-strlen.c
> ===================================================================
> --- gcc/tree-ssa-strlen.c	(revision 254961)
> +++ gcc/tree-ssa-strlen.c	(working copy)
> @@ -1931,10 +1931,9 @@ handle_builtin_stxncpy (built_in_function, gimple_
>    int sidx = get_stridx (src);
>    strinfo *sisrc = sidx > 0 ? get_strinfo (sidx) : NULL;
>  
> -  /* Strncpy() et al. cannot modify the source string.  Prevent the rest
> -     of the pass from invalidating the strinfo data.  */
> -  if (sisrc)
> -    sisrc->dont_invalidate = true;
> +  /* Strncat() and strncpy() can modify the source string by specifying
> +     as a destination SRC + strlen(SRC) and a bound <= strlen(SRC) so
> +     SISRC->DONT_INVALIDATE must be left clear.  */

The destination could be SRC + N and as long as LEN <= N, you shouldn't
invalidate SRC.  Though, if LEN < strlen(SRC) I think you return early.

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]