On Fri, Feb 01, 2019 at 11:52:04PM +0100, Eric Botcazou wrote:
> > So, can we e.g. keep emitting the epilogue where it is now for
> > naked_return_label != NULL_RTX and move it otherwise?
> > For __builtin_return the setter and use of the hard register won't be
> > adjacent in any case.
>
> See my comment in the audit trail of the PR; I'd suspend it and go to bed. ;-)
While the set of -fno- and -f options in some PRs are unlikely to be used by
people in the wild, often those PRs uncover latent bugs that could cause
serious wrong-code or ICEs, of course not always. So IMHO we shouldn't
ignore those PRs, especially if they are regressions.
In the meantime, I've bootstrapped/regtested successfully following version
of the patch that should fix the builtin return case.
In addition to normal {x86_64,i686}-linux bootstrap/regtest I've done a
distro build where we 1) build the compiler itself with
-fstack-protector-strong 2) run testsuite with
--tool-test=\{,-fstack-protector-strong\}, so far on
{x86_64,i686,powerpc64le}-linux, other targets still pending.
The only "regression" was gcc.target/i386/call-1.c with
-fstack-protector-strong, but that is because the test is invalid:
void set_eax(int val)
{
__asm__ __volatile__ ("mov %0, %%eax" : : "m" (val));
}
- missing "eax" clobber or "=a" (dummy) output and when set_eax is inlined
that can break optimizations badly. Of course, in addition to fixing that,
I'd expect if the tests wants to test what it originally wanted to test, it
needs to disable inlining or perhaps all IPA opts, not sure if just for
set_eax or also for foo/bar. Perhaps we can keep the testcase as is with
the "eax" clobber and add another one with __attribute__((noipa)) on
set_eax/foo/bar.
2019-02-01 Jakub Jelinek <ja...@redhat.com>
PR rtl-optimization/87485
* function.c (expand_function_end): Move stack_protect_epilogue
before loading of return value into hard register(s).
* gcc.dg/pr87485.c: New test.
--- gcc/function.c.jj 2019-01-29 16:47:02.000000000 +0100
+++ gcc/function.c 2019-02-01 16:23:07.471877843 +0100
@@ -5330,6 +5330,12 @@ expand_function_end (void)
communicate between __builtin_eh_return and the epilogue. */
expand_eh_return ();
+ /* If stack protection is enabled for this function, check the guard. */
+ if (crtl->stack_protect_guard
+ && targetm.stack_protect_runtime_enabled_p ()
+ && naked_return_label == NULL_RTX)
+ stack_protect_epilogue ();
+
/* If scalar return value was computed in a pseudo-reg, or was a named
return value that got dumped to the stack, copy that to the hard
return register. */
@@ -5476,7 +5482,9 @@ expand_function_end (void)
emit_insn (gen_blockage ());
/* If stack protection is enabled for this function, check the guard. */
- if (crtl->stack_protect_guard && targetm.stack_protect_runtime_enabled_p ())
+ if (crtl->stack_protect_guard
+ && targetm.stack_protect_runtime_enabled_p ()
+ && naked_return_label)
stack_protect_epilogue ();
/* If we had calls to alloca, and this machine needs
--- gcc/testsuite/gcc.dg/pr87485.c.jj 2019-02-01 16:30:51.101211900 +0100
+++ gcc/testsuite/gcc.dg/pr87485.c 2019-02-01 16:31:48.660260183 +0100
@@ -0,0 +1,29 @@
+/* PR rtl-optimization/87485 */
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-O2 -fschedule-insns -fno-guess-branch-probability
-fno-isolate-erroneous-paths-dereference -fno-omit-frame-pointer
-fno-split-wide-types -fno-tree-ccp -fno-tree-sra" } */
+/* { dg-additional-options "-fstack-protector-strong" { target
fstack_protector } } */
+
+int *a;
+
+int
+foo (__int128 x, int y, int z)
+{
+ __int128 b;
+ *a = ((!!y ? y : x) * y | x) * 2;
+ if (z == 0)
+ {
+ unsigned int c = 1;
+ __int128 *d = &b;
+ for (*a = 0; *a < 1; *a += y)
+ ;
+ *a += b < (c / 0); /* { dg-warning "division by zero" } */
+ goto l;
+ m:
+ while (b < 1)
+ ;
+ ++*a;
+ }
+ goto m;
+ l:
+ return 0;
+}
Jakub
