This time with the patch.
-------- Forwarded Message --------
Subject: [PATCH] [PR target/83641] Fix incorrect CFI for stack clash
protected noreturn function on x86/x86_64
Date: Tue, 2 Jan 2018 11:58:04 -0700
From: Jeff Law <l...@redhat.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>
Oh how I sometimes wish we'd never opened the can of worms WRT stack
clash protection of noreturn functions.
In this BZ we have a noreturn function. So we trigger the special bits
to emit a push/pop sequence to explicitly probe *sp. For ia32 we
push/pop %esi.
The pop %esi tells the generic CFI machinery that %esi's value is
returned to its state in the caller. But that's not entirely correct as
the value will be over written in the body of the function.
This situation shows up in some of the nptl code within glibc
(pthread_unwind). This in turn is believed to cause giac to behave
improperly.
--
It's fairly obvious that the probe of *sp isn't actually necessary here
because the register saves in the prologue act as probe points for *sp.
In fact, the only way this can ever cause problems is if %esi is used in
the body in which case it would have been callee saved in the prologue.
So if we detect that %esi is already callee saved in the prologue then
we could eliminate the explicit probe of *sp.
But we can do even better. If any register is saved in the prologue,
then that callee register save functions as an implicit probe of *sp and
we do not need to explicitly probe *sp.
While this was reported with -m32, I'm pretty sure we can trigger a
similar issue on x86_64.
Bootstrapped and regression tested on x86_64. Also verified the
testcase behavior on -m32. The test uses flags to hopefully ensure
expected behavior on x86/Solaris, but I did not explicitly test that
configuration.
OK for the trunk?
Jeff
* config/i386/i386.c (ix86_adjut_stack_and_probe_stack_clash): Do not
explicitly probe *sp in a noreturn function if there were any callee
register saves.
* gcc.target/i386/stack-check-17.c: New test
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 9ff9ca4e37f..5bd37d9cda5 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -12222,7 +12222,8 @@ ix86_adjust_stack_and_probe_stack_clash (const
HOST_WIDE_INT size)
offset would be zero. For a noreturn function we would reset
it to PROBE_INTERVAL - (STACK_BOUNDARY / BITS_PER_UNIT). Then
we just probe when we cross PROBE_INTERVAL. */
- if (TREE_THIS_VOLATILE (cfun->decl))
+ if (TREE_THIS_VOLATILE (cfun->decl)
+ && !(m->frame.nregs || m->frame.nsseregs || frame_pointer_needed))
{
/* We can safely use any register here since we're just going to push
its value and immediately pop it back. But we do try and avoid
diff --git a/gcc/testsuite/gcc.target/i386/stack-check-17.c
b/gcc/testsuite/gcc.target/i386/stack-check-17.c
new file mode 100644
index 00000000000..d2ef83b348a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/stack-check-17.c
@@ -0,0 +1,37 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fstack-clash-protection -mtune=generic
-fomit-frame-pointer" } */
+/* { dg-require-effective-target supports_stack_clash_protection } */
+
+
+int x0, x1;
+void f1 (void);
+void f2 (int, int);
+
+__attribute__ ((noreturn))
+void
+f3 (void)
+{
+ int y0 = x0;
+ int y1 = x1;
+ f1 ();
+ f2 (y0, y1);
+ while (1);
+}
+
+/* Verify no explicit probes. */
+/* { dg-final { scan-assembler-not "or\[ql\]" } } */
+
+/* We also want to verify we did not use a push/pop sequence
+ to probe *sp as the callee register saves are sufficient
+ to probe *sp.
+
+ y0/y1 are live across the call and thus must be allocated
+ into either a stack slot or callee saved register. The former
+ would be rather dumb. So assume it does not happen.
+
+ So search for two/four pushes for the callee register saves/argument
+ pushes and no pops (since the function has no reachable epilogue). */
+/* { dg-final { scan-assembler-times "push\[ql\]" 2 { target { ! ia32 } } } }
*/
+/* { dg-final { scan-assembler-times "push\[ql\]" 4 { target { ia32 } } } } */
+/* { dg-final { scan-assembler-not "pop" } } */
+
