This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR82991
On Thu, 23 Nov 2017, Jakub Jelinek wrote:
> On Thu, Nov 23, 2017 at 11:30:22AM +0100, Richard Biener wrote:
> > FAIL: gcc.c-torture/execute/builtins/memcpy-chk.c execution, -O2
> > ...
> > FAIL: gcc.c-torture/execute/builtins/memmove-chk.c execution, -O2
> > ...
> > FAIL: gcc.c-torture/execute/builtins/strncat-chk.c execution, -O2
> > ...
> > FAIL: gcc.dg/builtin-object-size-4.c execution test
> >
> > where I looked into the object-size case. For
> >
> > r = &a.a[4];
> > r = memset (r, 'a', 2);
> > if (__builtin_object_size (r, 3) != sizeof (a.a) - 4)
> > abort ();
> > r = memset (r + 2, 'b', 2) + 2;
> > if (__builtin_object_size (r, 3) != sizeof (a.a) - 8)
> > abort ();
> >
> > FRE now propagates &a.a[4] + CST adjustments through the memsets
> > and constant folds the last __builtin_object_size check to false.
>
> That is because it tests a [13] mode which as you know is fragile and
> doesn't like any kind of folding into MEM_REFs where the original access
> path (&a.a[4] + 4) is lost. So, if it is match.pd that does this, whatever is
> doing it there should be also guarded with cfun->after_inlining.
> While mode 3 is probably never used in real-world, mode 1 is used a lot and
> so we shouldn't regress that.
Hum. But that pessimizes a _lot_ of early folding. Like we'd no
longer optimize
r = &a.a[4];
r = r + 1;
if (r != &a.a[0])
and similar stuff exposed a lot by C++ abstraction. We really only
want to avoid elimination in certain places.
Even the current stop-gap (that isn't really one) is bad for
optimization.
> BTW, seems pass_through_call could now use ERF_RETURNS_ARG stuff
> except for __builtin_assume_aligned which intentionally doesn't have
> ret1 because we really don't want to "optimize" it by propagating the
> first argument to its uses too early. I'll handle it.
Thanks (double-check all those builtins have ret1).
> Though, with your changes perhaps there won't be any pass through calls
> anymore except for __builtin_assume_aligned.
For the object-size pass before FRE there certainly would be.
So I have the following testsuite adjustment for the builtins.exp
FAILs. Two of them quite obvious - just hide the conditional equivalence
we exposed. The strncat-chk.c is a little less obvious. We have
const char *const s1 = "hello world";
char dst[64], *d2;
...
/* These __strncat_chk calls should be optimized into __strcat_chk,
as strlen (src) <= len. */
strcpy (dst, s1);
if (strncat (dst, "foo", 3) != dst || strcmp (dst, "hello worldfoo"))
abort ();
strcpy (dst, s1);
if (strncat (dst, "foo", 100) != dst || strcmp (dst, "hello worldfoo"))
abort ();
strcpy (dst, s1);
if (strncat (dst, s1, 100) != dst || strcmp (dst, "hello worldhello
world"))
abort ();
if (chk_calls != 3)
abort ();
so it's obvious we can optimize away all checking given the strlen
pass ought to be able to track the length of dst. Somehow we
currently don't but with the patch we do (but not at -O[1sg]).
Looks like a missed optimization to me (currently). I couldn't
figure any way to remove that optimization possibility while
preserving strlen (src) <= len knowledge. Like hiding 's1'
because that makes the strcpy do a chk and thus we get 6 checks
(that I could have consistently at all opt levels).
Any preference for strncat-chk.c?
Thanks,
Richard.
2017-11-23 Richard Biener <rguenther@suse.de>
* gcc.c-torture/execute/builtins/memcpy-chk.c: Hide established
conditional equivalence of buf3 and buf1.
* gcc.c-torture/execute/builtins/memmove-chk.c: Likewise.
* gcc.c-torture/execute/builtins/strncat-chk.c: Allow for optimizing
of chk calls.
Index: gcc/testsuite/gcc.c-torture/execute/builtins/memcpy-chk.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/builtins/memcpy-chk.c (revision 255093)
+++ gcc/testsuite/gcc.c-torture/execute/builtins/memcpy-chk.c (working copy)
@@ -146,6 +146,7 @@ test2_sub (long *buf3, char *buf4, char
call. */
/* buf3 points to an unknown object, so __memcpy_chk should not be done. */
+ __asm ("" : "=r" (buf3) : "0" (buf3));
if (memcpy ((char *) buf3 + 4, buf5, n + 6) != (char *) buf1 + 4
|| memcmp (buf1, "aBcdRSTUVWklmnopq\0", 19))
abort ();
Index: gcc/testsuite/gcc.c-torture/execute/builtins/memmove-chk.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/builtins/memmove-chk.c (revision 255093)
+++ gcc/testsuite/gcc.c-torture/execute/builtins/memmove-chk.c (working copy)
@@ -149,6 +149,7 @@ test2_sub (long *buf3, char *buf4, char
call. */
/* buf3 points to an unknown object, so __memmove_chk should not be done. */
+ __asm ("" : "=r" (buf3) : "0" (buf3));
if (memmove ((char *) buf3 + 4, buf5, n + 6) != (char *) buf1 + 4
|| memcmp (buf1, "aBcdRSTUVWklmnopq\0", 19))
abort ();
Index: gcc/testsuite/gcc.c-torture/execute/builtins/strncat-chk.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/builtins/strncat-chk.c (revision 255093)
+++ gcc/testsuite/gcc.c-torture/execute/builtins/strncat-chk.c (working copy)
@@ -78,7 +78,9 @@ test1 (void)
strcpy (dst, s1);
if (strncat (dst, s1, 100) != dst || strcmp (dst, "hello worldhello world"))
abort ();
- if (chk_calls != 3)
+ /* With enough optimization we can track the string length in dst,
+ currently at -O2+ but not -O[1sg]. */
+ if (chk_calls != 0 && chk_calls != 3)
abort ();
chk_calls = 0;
Index: gcc/testsuite/gcc.c-torture/execute/builtins/strncat-chk.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/builtins/strncat-chk.c (revision 255093)
+++ gcc/testsuite/gcc.c-torture/execute/builtins/strncat-chk.c (working copy)
@@ -69,16 +69,18 @@ test1 (void)
/* These __strncat_chk calls should be optimized into __strcat_chk,
as strlen (src) <= len. */
- strcpy (dst, s1);
+ const char *s1p;
+ __asm ("" : "=r" (s1p) : "0" (s1));
+ strcpy (dst, s1p);
if (strncat (dst, "foo", 3) != dst || strcmp (dst, "hello worldfoo"))
abort ();
- strcpy (dst, s1);
+ strcpy (dst, s1p);
if (strncat (dst, "foo", 100) != dst || strcmp (dst, "hello worldfoo"))
abort ();
- strcpy (dst, s1);
+ strcpy (dst, s1p);
if (strncat (dst, s1, 100) != dst || strcmp (dst, "hello worldhello world"))
abort ();
- if (chk_calls != 3)
+ if (chk_calls != 6)
abort ();
chk_calls = 0;