This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] avoid assuming strncat/strncpy don't change length of source string (PR 83075)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Martin Sebor <msebor at gmail dot com>
- Cc: Gcc Patch List <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 20 Nov 2017 20:39:25 +0100
- Subject: Re: [PATCH] avoid assuming strncat/strncpy don't change length of source string (PR 83075)
- Authentication-results: sourceware.org; auth=none
- References: <38594ab4-39c8-5018-28de-05927cb3bb4b@gmail.com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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