On 12/07/2017 09:55 AM, Jakub Jelinek wrote:
On Wed, Dec 06, 2017 at 05:30:53PM +0100, Jakub Jelinek wrote:
On Wed, Dec 06, 2017 at 09:20:15AM -0700, Martin Sebor wrote:
Attached is a patch with the comment updated/simplified.
The tests do the job they need to do today so I just removed
the useless attribute but otherwise left them unchanged. If
you would like to enhance them in some way please feel free.
Ok for trunk, with a minor nit. I'll tweak the tests incrementally
when it is in.
So here is the fix for those testcases.
They didn't test what they meant to test, because they didn't FAIL
without the patch. That is because the bug was that the -W* option
affected code generation, so with -O2 -Wno-stringop-overflow it didn't
trigger it.
Doh! I suppose that underscores that the right way to write test
cases for optimization bugs is to prune warnings out of their output
rather than suppressing them via -Wno-foo. I've done the former
for this very reason a number of times but clearly fell into the
trap of suppressing the warnings in this instance. Thanks for
pointing it out!
I've changed the tests to test both in a separate noipa function where
it doesn't know about the aliasing and string lengths from the caller,
in that case it does more verifications, including the content of the
whole buffer, and the individual values of the lengths,
and what you did before.
I don't have an opinion on the rest of these changes. I do want
to make one comment about runtime tests. I fairly regularly run
tests with cross-compilers on the build machine. This lets me
verify that compile-only tests pass but it doesn't do anything
for tests that need to run. In fact, with the current mixture
of all kinds of tests in the same directory, it pretty much rules
out drawing any conclusions from test results in this setup. So
while I appreciate the additional testing done by the runtime
tests, I think ideally, having compile time only tests would be
the baseline requirement and runtime tests would be a separate
layer that would provide additional validation when possible.
Martin
Regtested on x86_64-linux and i686-linux, verified that with the
r255446 tree-ssa-strlen.c change reverted it FAILs.
Ok for trunk?
2017-12-07 Jakub Jelinek <ja...@redhat.com>
PR tree-optimization/83075
* gcc.dg/tree-ssa/strncpy-2.c: Use size_t instead of unsigned, add
separate function with noipa attribute to also verify behavior when
optimizers don't know the sizes and aliasing, verify resulting sizes
and array content. Add -Wstringop-overflow to dg-options.
* gcc.dg/tree-ssa/strncat.c: Likewise.
--- gcc/testsuite/gcc.dg/tree-ssa/strncpy-2.c.jj 2017-12-06
20:11:54.000000000 +0100
+++ gcc/testsuite/gcc.dg/tree-ssa/strncpy-2.c 2017-12-07 13:31:32.719722416
+0100
@@ -1,19 +1,35 @@
-/* PR tree-optimization/83075 - Invalid strncpy optimization
- { dg-do run }
- { dg-options "-O2 -Wno-stringop-overflow" } */
+/* PR tree-optimization/83075 - Invalid strncpy optimization */
+/* { dg-do run } */
+/* { dg-options "-O2 -Wstringop-overflow" } */
-int main (void)
+typedef __SIZE_TYPE__ size_t;
+
+__attribute__((noipa)) size_t
+foo (char *p, char *q, size_t *r)
{
- char a[8] = "";
+ size_t n0 = __builtin_strlen (p);
+ __builtin_strncpy (q, p, n0); /* { dg-warning "specified bound
depends on the length" } */
+ size_t n1 = __builtin_strlen (p);
+ *r = n0;
+ return n1;
+}
+int
+main ()
+{
+ char a[8] = "";
__builtin_strcpy (a, "123");
-
- unsigned n0 = __builtin_strlen (a);
-
- __builtin_strncpy (a + 3, a, n0);
-
- unsigned n1 = __builtin_strlen (a);
-
+ size_t n0 = __builtin_strlen (a);
+ __builtin_strncpy (a + 3, a, n0); /* { dg-warning "specified bound depends on
the length" } */
+ size_t n1 = __builtin_strlen (a);
if (n1 == n0)
__builtin_abort ();
+ a[6] = '7';
+ __builtin_strcpy (a, "456");
+ size_t n2;
+ if (foo (a, a + 3, &n2) != 7 || n2 != 3)
+ __builtin_abort ();
+ if (__builtin_memcmp (a, "4564567", sizeof "4564567"))
+ __builtin_abort ();
+ return 0;
}
--- gcc/testsuite/gcc.dg/tree-ssa/strncat.c.jj 2017-12-06 20:11:54.000000000
+0100
+++ gcc/testsuite/gcc.dg/tree-ssa/strncat.c 2017-12-07 13:31:09.568008365
+0100
@@ -1,19 +1,35 @@
-/* PR tree-optimization/83075 - Invalid strncpy optimization
- { dg-do run }
- { dg-options "-O2 -Wno-stringop-overflow" } */
+/* PR tree-optimization/83075 - Invalid strncpy optimization */
+/* { dg-do run } */
+/* { dg-options "-O2 -Wstringop-overflow" } */
-int main (void)
+typedef __SIZE_TYPE__ size_t;
+
+__attribute__((noipa)) size_t
+foo (char *p, char *q, size_t *r)
{
- char a[8] = "";
+ size_t n0 = __builtin_strlen (p);
+ __builtin_strncat (q, p, n0); /* { dg-warning "specified bound
depends on the length" } */
+ size_t n1 = __builtin_strlen (p);
+ *r = n0;
+ return n1;
+}
+int
+main ()
+{
+ char a[8] = "";
__builtin_strcpy (a, "123");
-
- unsigned n0 = __builtin_strlen (a);
-
- __builtin_strncat (a + 3, a, n0);
-
- unsigned n1 = __builtin_strlen (a);
-
+ size_t n0 = __builtin_strlen (a);
+ __builtin_strncat (a + 3, a, n0); /* { dg-warning "specified bound depends on
the length" } */
+ size_t n1 = __builtin_strlen (a);
if (n1 == n0)
__builtin_abort ();
+ a[6] = '7';
+ __builtin_strcpy (a, "456");
+ size_t n2;
+ if (foo (a, a + 3, &n2) != 6 || n2 != 3)
+ __builtin_abort ();
+ if (__builtin_memcmp (a, "456456\0", sizeof "456456\0"))
+ __builtin_abort ();
+ return 0;
}
Jakub
