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
